DataTable 'Select All' does not overwrites previously selected

[HardikChoudhary24]

What does this PR do?

Fixes #6946

• DataTable 'Select All' does not overwrites previously selected items

Where should the reviewer start?

• Create a test story with the code provided in https://codesandbox.io/s/grommet-filtered-datatable-3b37fe?file=/src/App.js and execute the steps.

What testing has been done on this PR?

• Ran locally on storybook

How should this be manually tested?

• By Creating a test story with the code provided in https://codesandbox.io/s/grommet-filtered-datatable-3b37fe?file=/src/App.js and executing the steps.

Do Jest tests follow these best practices?

• screen is used for querying.
• The correct query is used. (Refer to this list of queries)
• userEvent is used in place of fireEvent.
• asFragment() is used for snapshot testing.

Any background context you want to provide?

What are the relevant issues?

#6946

Screenshots (if appropriate)

Screen.Recording.2023-10-15.at.11.36.45.PM.mov

Do the grommet docs need to be updated?

• NO

Should this PR be mentioned in the release notes?

• No

Is this change backwards compatible or is it a breaking change?

• Yes

[jcfilben]

Thanks for working on this @HardikChoudhary24!

I think that when we have an option selected and then we search and the results doesn't include any of the selected options then we should just display an empty checkbox in the header rather than a checkbox with a - symbol. Also if we search and then select all options, clicking the header checkbox should only unselect the filtered options rather than unselecting all options.

@taysea @halocline feel free to jump in if you have any alternative opinions here

[MikeKingdom]

Thanks for working on this @HardikChoudhary24!

I think that when we have an option selected and then we search and the results doesn't include any of the selected options then we should just display an empty checkbox in the header rather than a checkbox with a - symbol. Also if we search and then select all options, clicking the header checkbox should only unselect the filtered options rather than unselecting all options.

@taysea @halocline feel free to jump in if you have any alternative opinions here

This one probably warrants some discussion. When in a controlled mode, especially with onUpdate, the select list can contain keys that aren't in the current data and the header checkbox should reflect there are still some selections, but not all items are selected. This has ramifications for pagination situations.

[jcfilben]

Thanks for working on this @HardikChoudhary24!
I think that when we have an option selected and then we search and the results doesn't include any of the selected options then we should just display an empty checkbox in the header rather than a checkbox with a - symbol. Also if we search and then select all options, clicking the header checkbox should only unselect the filtered options rather than unselecting all options.
@taysea @halocline feel free to jump in if you have any alternative opinions here

This one probably warrants some discussion. When in a controlled mode, especially with onUpdate, the select list can contain keys that aren't in the current data and the header checkbox should reflect there are still some selections, but not all items are selected. This has ramifications for pagination situations.

Good callout @MikeKingdom lets chat more about this Thursday during our grommet checkpoint meeting

[jcfilben]

Hey @HardikChoudhary24 we really like the direction that this PR is heading! We chatted more as a team and want to evaluate some more complex examples of DataTable with pagination. No action items are needed on your end at this point, we just need some time to test and think through what is a reasonable default for different types of DataTables.

Are you participating in Hacktoberfest? If so I would be happy to mark this pull request as 'hacktoberfest-accepted'

[HardikChoudhary24]

Hey @HardikChoudhary24 we really like the direction that this PR is heading! We chatted more as a team and want to evaluate some more complex examples of DataTable with pagination. No action items are needed on your end at this point, we just need some time to test and think through what is a reasonable default for different types of DataTables.

Are you participating in Hacktoberfest? If so I would be happy to mark this pull request as 'hacktoberfest-accepted'

Thanks @jcfilben for your feedback. I appreciate the offer to mark this PR as 'hacktoberfest-accepted', but I've already completed my Hacktoberfest contributions. Thank you for your understanding, Have a great day!