Add search bar to metrics feature watchlist #1353
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| } | ||
| } | ||
|
|
||
| function add_instance_to_dict( |
There was a problem hiding this comment.
the params are long, could convert it into a javascript object
victorhuangwq
requested changes
Sep 18, 2021
There was a problem hiding this comment.
In general the search feature is intuitive to use, and the categories chosen are great.
There are some repetition in the code, which hopefully could be reduced.
Lastly, we would strongly prefer to modify the on-click behavior of results: https://fomantic-ui.com/modules/search.html#/settings
| } else { | ||
| $('#archived_tab').html(archived_html); | ||
| } | ||
| pending_empty_message = "There are no pending students in need of attention"; |
There was a problem hiding this comment.
NIT: could reduce the duplication in the following chunk of code
app/assets/javascripts/watchlist.js
Outdated
| @@ -319,9 +364,55 @@ function get_watchlist_function(){ | |||
| } | |||
| }); | |||
|
|
|||
| $("#pending_search").keypress(function (e) { | |||
There was a problem hiding this comment.
NIT: reduce the repetition here
app/assets/javascripts/watchlist.js
Outdated
| $('.ui.icon').popup(); | ||
| $('.ui.circular.label.condition').popup(); | ||
|
|
||
| $('#pending_search').search({ |
There was a problem hiding this comment.
NIT: similar situation here
app/assets/javascripts/watchlist.js
Outdated
| } | ||
| }); | ||
| $("#resolved_search").keypress(function (e) { | ||
| var code = (e.keyCode ? e.keyCode : e.which); |
There was a problem hiding this comment.
victorhuangwq
approved these changes
Oct 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds a search bar to each tab of the watchlist in the metrics feature so that instructors can search students by name, email, or violated metrics condition.
Motivation and Context
Especially when a tab in the watchlist has a large number of students, instructors may want to search for particular students.
How Has This Been Tested?
Ensure that there are students in the watchlist by adjusting the metrics conditions.
Types of changes
Checklist:
Other issues / help required
In the future, we can iterate on this to implement a more comprehensive search feature that allows instructors to search by multiple categories (i.e. multiple violated conditions or a violated condition and a name). Also, please let me know if you have any design suggestions!
If unsure, feel free to submit first and we'll help you along.