Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add segments filter in assignment view #6689

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Conversation

acelaya
Copy link
Contributor

@acelaya acelaya commented Sep 17, 2024

depends on #6687 and #6688

Add a fourth filtering dropdown for groups or sections (AKA segments), which is only displayed when the assignment view is loaded.

For now, we only display this filter if the assignment has auto-grading enabled, to reduce potentially affected users in case of an issue, but we'll eventually enable it for any assignment, as it's generally useful.

Its selected values are used to filter the students list and the students metrics.

segments-filter-2024-09-18_10.31.19.mp4

Note

This PR is easier to review ignoring whitespaces

Testing steps

  1. Check out this branch
  2. Go to https://hypothesis.instructure.com/courses/319/assignments/7296 and open the dashboard. It should show a fourth filtering dropdown, right before students one.
  3. Changing values in that dropdown should affect the list of users in the table and the users dropdown.
  4. Go to another assignment without auto-grading enabled. You should not see the segments dropdown.
  5. Go to any other view. You should not see the segments dropdown.

Base automatically changed from paginated-multi-select to main September 18, 2024 07:49
@acelaya acelaya force-pushed the segment-filter branch 2 times, most recently from 4662139 to e0e26c1 Compare September 18, 2024 07:56
@acelaya acelaya changed the base branch from main to assignment-segments September 18, 2024 07:56
@acelaya acelaya changed the base branch from assignment-segments to main September 18, 2024 07:57
@acelaya acelaya changed the base branch from main to assignment-segments September 18, 2024 07:57
@acelaya acelaya force-pushed the segment-filter branch 6 times, most recently from f617c75 to 2d28af2 Compare September 18, 2024 10:16
@acelaya acelaya marked this pull request as ready for review September 18, 2024 10:19
@acelaya
Copy link
Contributor Author

acelaya commented Sep 18, 2024

We'll merge this PR into #6688, and then merge both together into main.

Copy link
Member

@robertknight robertknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good with a few minor comments. One general thing I would be careful about is reusing string literals used as type discriminators to construct UI text, and especially constructing derivatives to handle pluralization etc. Usually a developer would expect that changing such a literal would not break anything as long as they do a simple update (search + replace) for references in the code.

@acelaya acelaya merged commit 33c6153 into assignment-segments Sep 19, 2024
9 checks passed
@acelaya acelaya deleted the segment-filter branch September 19, 2024 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants