Course pinning, search and filter feature in user's course list#1084
Course pinning, search and filter feature in user's course list#1084AlexandreDoneux wants to merge 18 commits intoINGInious:mainfrom
Conversation
ca80065 to
3629b1f
Compare
… by using tabs in UI
4acd1cc to
d2963cc
Compare
|
BTW take a look at the Codacy report, there are many JS issues reported such as the usage of |
34ced92 to
9bc8b71
Compare
There was a problem hiding this comment.
I would not include the descriptions in the pinned cards, and rather use the internal link for the course as this is the case for the full list in this view.
Otherwise just cosmetic and more idiomatic suggestions.
| $(document).find("#no_pin_message").addClass("d-none"); | ||
| } | ||
|
|
||
| } else { // unpinning |
There was a problem hiding this comment.
Maybe we can provide an upin icon from the pinned card too. This would allow to remove a pinned course without going to the possibly long list of courses.
| if (filter_value === "true" && !$item.hasClass(filter_id)) { | ||
| visible = false; | ||
| } | ||
| if (filter_value === "false" && $item.hasClass(filter_id)) { | ||
| visible = false; | ||
| } |
There was a problem hiding this comment.
Would this be equivalent ?
| if (filter_value === "true" && !$item.hasClass(filter_id)) { | |
| visible = false; | |
| } | |
| if (filter_value === "false" && $item.hasClass(filter_id)) { | |
| visible = false; | |
| } | |
| if (!$item.hasClass(filter_id)) { | |
| visible = !(filter_value === "true" || filter_value === "false"); | |
| } |
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
You can remove those spaces :-)
| data = User.objects(username=username).first() | ||
| if data: | ||
| pinned_courses = data.pinned_courses if data else [] | ||
| if courseid not in pinned_courses: | ||
| pinned_courses.append(courseid) | ||
| User.objects(username=username).update(pinned_courses=pinned_courses) | ||
| return True | ||
| return False |
There was a problem hiding this comment.
I think you can do this more effenciently using the $addToSet MongoDB operator, as you did for the unpin_course method and the $pull operator.
| data = User.objects(username=username).first() | |
| if data: | |
| pinned_courses = data.pinned_courses if data else [] | |
| if courseid not in pinned_courses: | |
| pinned_courses.append(courseid) | |
| User.objects(username=username).update(pinned_courses=pinned_courses) | |
| return True | |
| return False | |
| modified = User.objects(username=username).update(add_to_set__pinned_courses=courseid) | |
| return modified is not None |
| '../../frontend/static/js/groups.js', | ||
| '../../frontend/static/js/checked-list-group.js', | ||
| '../../frontend/static/js/task_dispensers.js', | ||
| '../../frontend/static/js/admin.js', |
There was a problem hiding this comment.
| '../../frontend/static/js/admin.js', | |
| '../../frontend/static/js/mycourses.js' |
This PR aims to improve the user experience regarding the user's course list. Currently displayed as a basic list, it can get difficult to find a specific course if you manage a lot of them. This PR brings multiple ways to improve this list :
This PR is based on the #1077 PR. Therefore it will stay as draft until those changes are accepted.