-
Notifications
You must be signed in to change notification settings - Fork 812
Fixes selected users clearing before an action is completed #13701
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
Fixes selected users clearing before an action is completed #13701
Conversation
Build Artifacts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AllanOXDi! Code changes look good. Just found a minor regression, and an oversight in the AssignCoachesSidePanel.
<component | ||
:is="canAssignCoaches ? 'router-link' : 'span'" | ||
:to="overrideRoute($route, { name: PageNames.ASSIGN_COACHES_SIDE_PANEL })" | ||
:to="overrideRoute($route, { name: PageNames.SSAIGN_COACHES_SIDE_PANEL })" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like an unnoticed change here is breaking the open assign coaches side panel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Thanks
}); | ||
function closeSidePanel() { | ||
emit('clearSelection'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This closeSidePanel
is the general method we use to close the side panel, adding the clearSelection here is incorrect because it will always clear the selection when closing the side panel even if the operation was not completed. This should be moved to the _handleAssign
method after closing the side panel.
c48c90f
to
51ee1c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AllanOXDi - I confirm that this is working fine for the Users table, but it's not fixed at all for the New users table:
newusers.mp4
Thanks for pointing that out @pcenov . Let me push the fix momentarily |
Hi @pcenov It should be fixed now. |
Thanks @AllanOXDi - I have just one additional question: In the "New users" table after I complete an action such as enrolling a user in a class or removing them, then the selected user(s) remain selected in the table, while in this case the users should no longer be selected. Is this something that can be addressed as part of this PR or I should report it separately? completed.action.mp4 |
f33489c
to
1022d8b
Compare
@pcenov @AllanOXDi let's keep the experience consistent across the two pages where the user's selection is cleared after they've taken an action on their selection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes lgtm - good to merge once @pcenov 's last bit of feedback has been addressed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AllanOXDi on the New Users page when the selection is cleared, if I'd selected all (and thus the "Select all" checkbox is checked) - then I complete a bulk action.
In this case, all selected users are deselected, but for some reason the "Select all" is still checked.
by reassigning it with a new Set instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AllanOXDi - LGTM!
Seems like there is nothing left . I will proceed with the merge here
Summary
This PR fixes the issue where selected users in the Users table were being cleared whenever a side panel (Assign Coach, Enroll in Class, Remove from Class) was opened and then closed.
fixes #13691
References
#13691
Reviewer guidance