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

Fix issue toggling row selection for a grouped row #4760

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mollykreis
Copy link

Resolves #4759

Toggling a grouped row can have up to two effects:

  1. Update the selected state of the grouped row if that grouped row supports selection
  2. Update the selected state of subrows if getCanSelectSubRows is true

toggleSelected() returned early if the selected state of the row wasn't going to change without any consideration of whether the selected state of subrows also needed to be updated. With this change, toggleSelected() now considers both if the rows selection state will change and if subrow selection state might change before returning early.

I've added some selection tests to table-core -- both a few basic tests and some tests that cover the issue I am fixing.

@vercel
Copy link

vercel bot commented Mar 17, 2023

@mollykreis is attempting to deploy a commit to the Tanstack Team on Vercel.

A member of the Team first needs to authorize it.

@KevinVandy
Copy link
Member

Unsure if this change is needed as discussed here: #4759 (comment)

@@ -353,7 +353,8 @@ export const RowSelection: TableFeature = {
table.setRowSelection(old => {
value = typeof value !== 'undefined' ? value : !isSelected

if (isSelected === value) {
const willUpdateSubRows = row.subRows?.length && row.getCanSelectSubRows()

Choose a reason for hiding this comment

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

row.subRows?.length can be undefined, how about using !! to explicitly convert it to a boolean?

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.

Cannot toggle selection of grouped row to deselect its children
3 participants