Reset selection after applying filter#13880
Reset selection after applying filter#13880nucleogenesis merged 5 commits intolearningequality:release-v0.19.xfrom
Conversation
github-actions
Bot
added
APP: Facility
Build Artifacts
|
nucleogenesis
left a comment
nucleogenesis
left a comment
There was a problem hiding this comment.
The logic works but the behavior change needs to be applied to all three users table variants. Left a suggestion for your consideration. Thanks @AllanOXDi !
| watch([numAppliedFilters, searchTerm], (newValues, oldValues) => { | ||
| if (newValues[0] !== oldValues[0] || newValues[1] !== oldValues[1]) { | ||
| clearSelectedUsers(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The logic here looks good to me. However the behavior needs to also apply to the NewUserPage and UsersTrashPage components.
You could look at putting this logic into the useUserManagement.js composable - looks like we have the search and numAppliedFilters and it also houses the list of selectedUsers - so might be able to feed 3 birds with 1 stone since the other user table pages use useUserManagement for these values.
|
|
||
| const clearSelectedUsers = () => { | ||
| selectedUsers.value.clear(); | ||
| selectedUsers.value = new Set(selectedUsers.value); |
There was a problem hiding this comment.
This feels odd - does calling clear() not update the value reactively?
This seems like it's doing the same thing as selectedUsers.value = new Set().
There was a problem hiding this comment.
Doing this help us to clear the selection on the user page
There was a problem hiding this comment.
Did just setting selectedUsers.value = new Set() not work properly in this location?
There was a problem hiding this comment.
I misunderstood you earlier btw 🤦🏾♂️
nucleogenesis
left a comment
nucleogenesis
left a comment
There was a problem hiding this comment.
Just a question re: how you're clearing the selection.
The other thing that'd be great to get in is to use your new method in all three of the user table variants (+Trash page, +New users page)
nucleogenesis
left a comment
nucleogenesis
left a comment
There was a problem hiding this comment.
Code changes look good to me & tested across New, Trash and normal Users pages successfully! Thanks for the quick turnaround @AllanOXDi
nucleogenesis
left a comment
nucleogenesis
left a comment
There was a problem hiding this comment.
Adding a blocking review as we are a bit uncertain on the desired behavior - perhaps this will be a good item to get user testing feedback on as well?
Also just want to avoid merging this in case we would rather target the release branch. (cc @marcellamaki @radinamatic )
nucleogenesis
added
the
TODO: needs decisions
nucleogenesis
modified the milestones:
Kolibri 0.19: Planned Patch 1,
Kolibri 0.19: Planned Patch 2
|
Noting here that we've come to a decision to merge this experience. @tomiwaoLE has some early work on a revision to the table that would improve the table experience which we'll come back to when time/priorities align. So - this needs to be rebased and probably a quick pass at some manual QA would be worthwhile to check for rebase-related regressions. |
AllanOXDi
force-pushed
the
reset-selection-after-applying-filter
branch
from
7a32fde to
849325a
Compare
…lection-after-applying-filter
pcenov
left a comment
pcenov
left a comment
There was a problem hiding this comment.
Thanks @AllanOXDi - I confirm that after applying a filter or searching for a user the previously selected users are no longer selected.
nucleogenesis
dismissed
their stale review
Decision made, executed, and ready to merge!
nucleogenesis
merged commit ac480c8
into
learningequality:release-v0.19.x
4 participants
Summary
This PR fixes any selected users remain selected even after having applied additional filters
Closes #13868
References
#13868
Before
selected.users.and.filters.mp4
After
Screen.Recording.2025-11-06.at.18.32.56.mov
Reviewer guidance