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

Subrow unselect doesn't affect parent checkbox #3965

Closed
2 tasks done
Jonyberry opened this issue May 25, 2022 · 9 comments
Closed
2 tasks done

Subrow unselect doesn't affect parent checkbox #3965

Jonyberry opened this issue May 25, 2022 · 9 comments

Comments

@Jonyberry
Copy link

Describe the bug

If you unselect child row and you already has selected its parent row, parent row stay selected, but it should go to indeterminate state.

Your minimal, reproducible example

https://codesandbox.io/s/summer-lake-2qklsp?file=/src/App.js

Steps to reproduce

  1. Expand row
  2. Select parent row
  3. Unselect child row -> parent row stay selected

Expected behavior

  1. Expand row
  2. Select parent row
  3. Unselect child row -> parent row go to indeterminate state

How often does this bug happen?

Every time

Screenshots or Videos

Snímek obrazovky 2022-05-25 v 13 41 35

Platform

I tried it on

  • OS: macOS
  • browser: Chrome

But I don't think it is browser issue.

react-table version

7.7.12

TypeScript version

4.5.5

Additional context

No response

Terms & Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
@nelsonrds
Copy link

nelsonrds commented May 25, 2022

Hello Jony,

You are building the parent Checkbox with the wrong properties. You have to differentiate the selected rows from the "parent" selected rows.

So we have this:
image

This image was taken from here: https://codesandbox.io/s/summer-lake-2qklsp?file=/src/App.js with the fix.

Regards,
Nelson.

EDIT: This won't work if you add more rows, I just duplicated the toggle all rows selected props to the "parent" row:
image

@Jonyberry
Copy link
Author

Hi Nelson,

thanks for quick answer!

I updated my sandbox and also I added one more row. When I tried your fix my parent checkbox now selects all rows but it should select only subrows.

Have you got any other idea how to fix it?

@nelsonrds
Copy link

nelsonrds commented May 26, 2022

Hello Jony,
To be honest, I think that there is something missing on the row property, something like row toggle all rows selected properties. This way you could toggle all properties from that specific row + his sub rows.

image

Also, expanding + row selection is not properly connected and you cannot do what you are trying to do, maybe Tan can help us out here.

Good luck Jony!

@tannerlinsley
Copy link
Collaborator

Feel free to open a discussion or use the Discord for more help!

@Jonyberry
Copy link
Author

Alright, I create discussion here

@itsmejoeeey
Copy link
Contributor

itsmejoeeey commented Sep 4, 2023

When following the reproduction steps, this same issue is visible in the "Expanding" CodeSandbox example from the guide.

This issue seems to have the same/similar symptoms to these other issues: #4720 #4759 #4878 #4879

This naive UI example here (cascade checking) has the behavior we are expecting:
https://www.naiveui.com/en-US/os-theme/components/tree#cascade.vue

Could you please advise if we are missing something @tannerlinsley?

@itsmejoeeey
Copy link
Contributor

See my comment on the above linked discussion that outlines a way I managed to implement this sort of "cascading" behaviour.

@VincEnterprise
Copy link

VincEnterprise commented Dec 21, 2023

bump. I'm still waiting for a row selection that is leaf node only and cascading with proper indeterminate state for parent rows.

It seems alot of people expect this behavior:

#4298
#4349
#4720
#4759
#4878
#4879
trimble-oss/modus-web-components#1672

@R-Bower
Copy link

R-Bower commented Jan 27, 2024

I've fixed this in a local fork. The issue stems from row selection functioning independently of parent row selection. In other words, the current implementation allows a parent row to be selected independently of its child rows. This doesn't align with what I expect from nested checkbox behavior, namely:

  • When all children are selected, the parent should be selected.
  • When only some children are selected, the parent should be indeterminate.
  • When no children are selected, the parent should be unselected.

The fixes below pertain to the RowSelection.ts feature.

row.toggleSelected = (value, opts) => {
  const isSelected = row.getIsSelected()

  table.setRowSelection((old) => {
    value = typeof value !== "undefined" ? value : !isSelected

    if (row.getCanSelect() && isSelected === value) {
      return old
    }

    const selectedRowIds = {...old}

    mutateRowIsSelected(
      selectedRowIds,
      row.id,
      value,
      opts?.selectChildren ?? true,
      table,
    )
    // could also be enabled through a new config option.
    return syncParentRowSelection(row, selectedRowIds, table)
  })
}
/**
 * When child rows are updated, the parent state is not properly synced.
 * Assumptions:
 * When all children are selected, the parent should be selected.
 * When only some children are selected, the parent should be indeterminate.
 * When no children are selected, the parent should be unselected.
 */
function syncParentRowSelection<TData extends RowData>(
  row: Row<TData>,
  selection: RowSelectionState,
  table: Table<TData>,
): RowSelectionState {
  const parentRow = row.getParentRow()
  if (!parentRow) {
    return selection
  }

  const selected = isRowSelected(parentRow, selection)
  const isAllSubRowsSelected =
    isSubRowSelected(parentRow, selection, table) === "all"

  if (isAllSubRowsSelected && !selected) {
    mutateRowIsSelected(selection, parentRow.id, true, false, table)
  } else if (!isAllSubRowsSelected && selected) {
    mutateRowIsSelected(selection, parentRow.id, false, false, table)
  }

  return syncParentRowSelection(parentRow, selection, table)
}

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

No branches or pull requests

6 participants