`getToggleAllRowsSelectedHandler` is selecting not allowed rows.

[jpedroh]

Describe the bug

Hello!

The table method getToggleAllRowsSelectedHandler is selecting all rows, even the ones which return false for the getCanSelect row check. I don't know if this is either a bug or intended behaviour though.

Your minimal, reproducible example

https://codesandbox.io/s/romantic-silence-sh1x95

Steps to reproduce

1. First, notice that by table definition in line 115, only rows with age greater than 18 should be able to be selected. enableRowSelection: (row) => row.original.age > 18,
2. Check that the checkboxes for ages smaller than 18 are disabled.
3. Tick select all. All the boxes will be checked, even the disabled ones.

Expected behavior

The disabled checkboxes should not be checked.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

React 18, Chrome 103, Windows 10.

react-table version

8.2.0

TypeScript version

No response

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.

[LetItRock]

hey @jpedroh, @tannerlinsley!

@tannerlinsley is this just a matter of a missed call to row.getCanSelect() in the RowSelection.ts:111?

[jpedroh]

Unfortunately, I didn't have the time to check the source code for Row Selection, but looking from outside, it indeed looks like a missing filter in the selectAll method.
I can try to hook into the code during the weekend and give it a try though.

[jpedroh]

All right, had the time to give the code a look. I agree with @LetItRock solution. However, I believe there's room for improvement in also adding this check in the mutateRowIsSelected function itself - so this bug is avoided in other places as well (toggleAll does not use it). I've opened a PR so we can discuss these changes there.

[jpedroh]

Not so fast on this one! This change introduced a bug in getIsAllRowsSelected, since the method also doesn't use a filter to only match the rows which are allowed to be selected. This breaks the toggle selection feature, because the toggle method - which was patched - calls this method.

Let's make this right. I would like to implement some tests to assert this behaviour is correct on my next PR. @tannerlinsley is it ok if I implement a test for this feature in the react package or would you rather have a test implemented in the core one? Perhaps setting up infrastructure for testing in the core package itself will be a little bit trickier because of state management?

[tannerlinsley]

Test wherever is easier. The react package already has some tests and yes, it’s easier there.

[ThomasLeveque]

Hi @jpedroh !
Did you had the chance to fix getIsAllRowsSelected ?
I still have the issue where the return value is never true if i have some disabled rows so the toggle between all/none selected is broken

[jpedroh]

Hi @ThomasLeveque. Yes I've had! I've implemented some test using React and it seems to be working as it should now. There were some substantial changes though and I'm afraid that might possibly break another behaviour. Unfortunately, I won't have much of the time to work in it until next week, but I will open a draft PR and feel free to proceed with implementation if you wish.