[DataGrid] Select All feature does not work on column filtering

[Nate-Gage]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Select All feature accurately selects active rows when only 1 filter is active. If a subsequent filter is added, Select All feature breaks.

Expected Behavior 🤔

Select All feature should update total number of active rows when additional filters are added.

Steps to Reproduce 🕹

Steps:

1. Add a single column filter to X Grid table https://material-ui.com/components/data-grid/#commercial-version
2. Select All checkbox on top left
3. Add a second column filter.
4. Uncheck and check Select All checkbox again.

Context 🔦

Your Environment 🌎

System:
OS: Windows 10 10.0.18363
Binaries:
Node: 12.18.4 - ~\PMT-App\node_modules.bin\node.CMD
Yarn: Not Found
npm: 6.14.10 - C:\Program Files\nodejs\npm.CMD
Browsers:
Chrome: 88.0.4324.190
Edge: Spartan (44.18362.449.0)
npmPackages:
@material-ui/core: 4.11.0 => 4.11.0
@material-ui/icons: 4.11.2 => 4.11.2
@material-ui/lab: 4.0.0-alpha.56 => 4.0.0-alpha.56
@material-ui/pickers: 3.2.10
@material-ui/styles: 4.11.3
@material-ui/system: 4.11.3
@material-ui/types: 5.1.0
@material-ui/utils: 4.11.2
@material-ui/x-grid: 4.0.0-alpha.20 => 4.0.0-alpha.20
@material-ui/x-license: 4.0.0-alpha.20
@types/react: 17.0.0
react: 16.13.1 => 16.13.1
react-dom: 16.13.1 => 16.13.1

Order id 💳

n/a

[oliviertassinari]

Regarding the pain of @Nate-Gage. The first note is that it can be reproduced with a single filter, so it's DataGrid related, not XGrid.

I have seen this aspect slightly covered in https://ag-grid.com/javascript-grid/row-selection/#select-everything-or-just-filtered. By default, they have the select all checkbox ignore the filtering. You have to apply headerCheckboxSelectionFilteredOnly: true to get the current behavior of our data grid.

We have almost the same issue but for pagination in: #605.

I would personally vote to optimize for avoiding surprises. Meaning, having the select all feature to be as close as possible to what's currently visible on-screen, and avoiding states. For instance:

• If you apply one filter, and select all, then select the rows of the filter. Ask for selecting all rows outside of the filter.
• If you apply one filter, and unselect all, then unselect the visible rows. Ask for unselecting all rows outside of the filter.
• If you have multiple pages, only select the current page. Ask for selecting all the other pages.

We could imagine an opt-in prop to bypass the "Ask" part and behave like ag-Grid, with headerCheckboxSelectionFilteredOnly=false (default).

[matandro]

We are also seeing this issue with a single filter in x-grid. It seems that the intermediate state lets you pick anything but the filter but not only the filter.

so when we:

1. Use any kind of filter
2. checkbox => selects all inc unfiltered (blue checkmark)
3. checkbox => selects all but filtered (blue - sign)
4. checkbox => nothing change in selection (gray - sign)
5. checkbox => back to case 2
   selection is mainly based on the number of select shown in bottom bar.
   similar to what @oliviertassinari showed but without clicking away at any point.

EDIT:
We reverted back to alpha 24 to 29 and it still occurs

[matandro]

Hey,

I know you guys are busy but is there a timeline for this one?
Our users are pretty reliant on multi column filtering -> selecting all to get samples into analysis and we are using x-grid for our tables.

Out current workaround is telling then to filter the negative and then click the select all twice but it creates lots of confusion. Also doing the negative of multi column select is hard for 🙃

[oliviertassinari]

@dtassone I have realized that it's currently impossible to know why onSelectionChange fires. I propose the following plan of action, we can do, at least, 4 PRs:

• 1. At the very least, I think that we should allow developers to control the selection, so they can implement whatever they need. In order to do so, I would assume that the simpler is to add a reason?: 'selectAll' | 'unselectAll' argument to the selectionChange event.
• 2. Then, we can fix this bug [DataGrid] Select All feature does not work on column filtering #1141 (comment): the impossible checkbox state. Fixed in [DataGrid] Fix double-click issue #1919.
• 3. Then, we can fix this bug [DataGrid] Select All feature does not work on column filtering #1141 (comment), it's likely linked to 2. The current behavior doesn't make any sense. The first click on select all behaves as "Select everything", but the following clicks behave as "Just Filtered". It's strongly confusing. Fixed in [DataGrid] Select all checkbox click should select only filtered rows after filter #1879
• 4. Last, we can improve the default behavior/add new props.

[ZeeshanTamboli]

Hey guys. So I was looking into this from past few days. So as per my investigation, this stopped working from 4.0.0-alpha.27. But it works in previous version prior to alpha-27.

I was debugging and found that visibleRowIds in GridHeaderCheckbox returns old state.
https://github.com/mui-org/material-ui-x/blob/46a31d161034733963125bec771c3720d219ad62/packages/grid/_modules_/grid/components/columnSelection/GridHeaderCheckbox.tsx#L19

Example for 200 rows:
When the DataGrid is loaded, the visibleRowIds is initially 200.
When the selectAll checkbox is clicked after filtering is applied, the visibleRowIds inside handleChange is still 200. It should be the filtered Ids.
https://github.com/mui-org/material-ui-x/blob/46a31d161034733963125bec771c3720d219ad62/packages/grid/_modules_/grid/components/columnSelection/GridHeaderCheckbox.tsx#L40-L44
It then gets updated and then the visibleRowIds is returned with the filtered number which is passed to selectRows on the next time of on click of selectAll which is point 3 of this #1141 (comment) I think.

I haven't found any solution yet, but I think the selector visibleSortedGridRowsSelector is not running again due to mutability of Map wherein createSelector is not computing again due to the reference being same.
https://github.com/mui-org/material-ui-x/blob/46a31d161034733963125bec771c3720d219ad62/packages/grid/_modules_/grid/hooks/features/filter/gridFilterSelector.ts#L12-L25

As per my research Map object is not immutable.

1. https://stackoverflow.com/a/53606339
2. https://stackoverflow.com/a/46433041

Reselect's why input selector is not running again: https://github.com/reduxjs/reselect#q-why-isnt-my-selector-recomputing-when-the-input-state-changes

I am not 100% sure if Map here is causing issues. But I posted what all I can find. I still couldn't find a complete solution. Not sure how to re-render GridHeaderCheckbox after filter is applied and get new updated visibleRowIds so that it is passed correctly to selectRows method.

cc: @oliviertassinari @m4theushw . Notifying Matheus since he worked on introducing Map.

Edit: Also Map was introduced in PR #1377 which was released during v4.0.0.alpha-25 https://github.com/mui-org/material-ui-x/releases/tag/v4.0.0-alpha.25
But for me it is working in alpha-25 but stops working from alpha-27. And @Nate-Gage posted version XGrid's4.0.0-alpha.20 in which it works for me in codesandbox. Really confused!
Note that I am discussing DataGrid (single filter) not XGrid.

[dtassone]

The issue with the state not being refreshed, comes from the memoization of GridColumnHeaderItem
As the parent component of the select all checkbox is memoized and the props don't change the select all does not rerender.
Removing React.memo of the GridColumnHeaderItem component should fix the state of the checkbox.
Also, I believe the perf should not be hit as the header cells are virtualized so only the visible ones are rendered.

[ZeeshanTamboli]

The issue with the state not being refreshed, comes from the memoization of GridColumnHeaderItem
As the parent component of the select all checkbox is memoized and the props don't change the select all does not rerender.
Removing React.memo of the GridColumnHeaderItem component should fix the state of the checkbox.
Also, I believe the perf should not be hit as the header cells are virtualized so only the visible ones are rendered.

Thanks @dtassone . It works. Seems like I was looking at the wrong place. React.memo was added in #1421 which was released in v4.0.0.alpha-27. And it started causing issue since alpha-27. Removing React.memo works.

[oliviertassinari]

I have updated #1618 (comment) to link this new instance of the problem.

[ZeeshanTamboli]

Removing React.memo works for selecting rows after filter is applied. I am still trying to fix the main issue #1141 (comment) , This is the case after Select all. So I am calling apiRef.current.selectRows within applyFilter to select only the filtered rows after filter is applied if the filtered rows is less than the total selected rows. It gets updated in selection state, (also involves deselecting the other rows). But somehow/somewhere selection state goes back to previous state of all rows getting selected without a filter.

[matandro]

#1879 (comment)
I feel this is the correct way to look at it. A common use case is to filter, select all, filter again, select all in order to select specific items. It is also consistent with the fact that selected items are not being deselected (which is great) after filtering. But I understand if you select not to look at it this way since we can use multiple filters with and / or relationship in X-Grid any way.

I would think the simplest way to look at the select all checkbox as:
Empty -> nothing is selected in this current filter, next click will select all in this filter
Gray -> Some are selected within this filter, next click will select all
Blue -> All are selected in the current filter, next click will unselect all in this filter
That makes the select all to be a state machine of the current selection model based on the current filter model.

If this PR does get broken down the select all on the current filter is our biggest issue.

[m4theushw]

@matandro Reading the expected behavior, it appears that this issue has already been fixed indirectly by another PR. Do you still experience it using the latest version?

[matandro]

The base behavior is good for our use. (We are currently one version behind the latest)
It seems that selectAll unselects all if at lease one is selected so it simplifies what we had with the additional middle state.

At the end of the day, the counts and selection behave as expected based on the current filtered subset. 👍