-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Clear list selections on pagination/search/sort #10329
Clear list selections on pagination/search/sort #10329
Conversation
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
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.
I think this is a really easy and clean way of solving this problem. We probably want to write some tests for the unselecting of list items on search, pagination etc, at the very least, inside useSelected.test.jsx
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.
👍
98c344d
to
c2a57aa
Compare
Build succeeded.
|
@keithjgrant this looks good! |
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
@keithjgrant can you please rebase and fix the conflict? once you are able to do that, we can add the mergeit label |
c2a57aa
to
21ff1d7
Compare
Build succeeded.
|
Build succeeded (gate pipeline).
|
SUMMARY
Updates nearly every list* so that URL param changes (pagination, search, or sort) clear the selection. This prevents the list of selected items in state from including items that may no longer appear on screen — preventing the user from accidentally deleting or otherwise altering an item they may not realize they still have selected.
This also updates the
useSelected
hook to provideselectAll
andclearSelected
functions. Any lists that weren't yet already using this hook have been updated to do so.Addresses #6853 and #7509
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION
*Lists that do not include this change are modals where the user is expected to paginate through screens and make several selections along the way (e.g. Multi Credential select modal), and lists that still use PaginatedDataList and are yet to be converted to PaginatedTable
Note: I originally wanted to make the
clearSelected
prop on PaginatedTable required, so any lists that don't have this fix applied would fail loudly. Unfortunately that wasn't possible, as there were a few lists that should not have this behavior, so I had to leave it as an optional prop.