[DataGrid] Prevent the grid from cleaning up the selectionModel when selectionModel prop is provided

[tforssander]

• 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 😯

When using paginationMode="server"together with a controlled checkbox selection, it does not retain the selection model when changing pages.

https://codesandbox.io/s/material-demo-forked-k5bnx?file=/demo.tsx

Expected Behavior 🤔

It should retain selection model when changing pages, as it does with paginationMode="client".

https://codesandbox.io/s/material-demo-forked-s28wx?file=/demo.tsx

Steps to Reproduce 🕹

Steps:

1. Navigate to https://codesandbox.io/s/material-demo-forked-k5bnx?file=/demo.tsx
2. Click on one or more of the rows
3. Note that "X rows selected" is visible in table footer
4. Click on the next arrow to jump to the second page
5. Note that "X rows selected" is no longer visible in the table footer
6. Note that the checkbox in the header is no longer in the intermediate state
7. Click on the previous arrow to jump back to the first page
8. Note that all checkbox selections are missing, but the selectionModel state is still intact

Context 🔦

I'm trying to implement server-side pagination and together with the controlled selection model, keep track of all selected row IDs.

Environement

v4.0.0-alpha.30

[dtassone]

Thanks for reporting this bug. We need to reapply the selection on page change.

As a workaround, you can reset the selection model on page change.

  const handlePageChange = (params: GridPageChangeParams) => {
    setSelectionModel([...selectionModel]);
    setPage(params.page);
  };

https://codesandbox.io/s/material-demo-forked-s30xp

[tforssander]

@dtassone Thanks! That works, much appreciated.

[oliviertassinari]

It looks like it impacts the DataGrid too. We miss a dependency on the state change, somewhere in the logic.

[m4theushw]

The state is updated even if the selection is controlled. Whenever the rowsLookup change, we unselect those rows that are not in the new row set: https://github.com/mui-org/material-ui-x/blob/b7e84aaa031a355075eccc19749173a3583d16e3/packages/grid/_modules_/grid/hooks/features/selection/useGridSelection.ts#L188-L197

I think that a simple solution could be to not unselect rows when in controlled mode, but it seems like this is also an opportunity to better enforce the idea of using the prop (in controlled mode) as the source of truth, not the state. @oliviertassinari already did something like that in the pagination: https://github.com/mui-org/material-ui-x/blob/b7e84aaa031a355075eccc19749173a3583d16e3/packages/grid/_modules_/grid/hooks/features/pagination/useGridPagination.ts#L41-L46

[oliviertassinari]

I think that a simple solution could be to not unselect rows when in controlled mode

@m4theushw I think that the behavior should always be identical if a state is controlled or not. Why? Because otherwise, it creates surprises and branch logic in the code. I'm not aware of any precedent in this direction in the components, so far. IMHO, it's a dead end.

it seems like this is also an opportunity to better enforce the idea of using the prop (in controlled mode) as the source of truth, not the state.

Agree, there is a systematic issue here. It's like this for most of the states of the grid, not great. However, it's not specific to #1782, so I think that we can ignore it. @dtassone Is working on this problem in #1823, and not on solving #1782.

---

@DanailH Looking at where #1864 was heading. It looks like we labeled this issue a "bug" but we didn't clarify what should be the correct behavior (what developed say is expected is only suppose to help us understand the root issue). From what I understand, there are two options for a sound solution:

1. The problem is that the onSelectionModelChange callback is never called with the event that empties the selection on rows prop change.
2. The problem is that the selection shouldn't be emptied when the rows change. Controlled or not when in server-side pagination mode.

https://github.com/mui-org/material-ui-x/blob/2d8f6a3116de58e447475169149f84d27b1604cb/packages/grid/_modules_/grid/hooks/features/selection/useGridSelection.ts#L205

I don't know which option would be better.
If we go in the direction of option 1, then we need to add a reason argument to allow @tforssander to implement its logic. He will need to ignore the reset event.
If we go in the direction of option 2, then does it mean some of the selected rows can leak between different filter values? E.g. if implemented outside of the logic of the data grid? It might be riskier.

@tforssander Do you have a preference?

---

I'm trying to implement server-side pagination and together with the controlled selection model, keep track of all selected row IDs.

@tforssander Actually, we have seen multiple related issues:

• Improve the selectionModel state to support rows id that we don't have access to (somewhere in the DB): [DataGrid] Allow developers to differentiate the checkbox selection all state #1876 (comment)
• Add a mode to change the behavior of the select all event between pages: [DataGrid] Allow "select all" to only select the active page #605

[tforssander]

@dtassone When updating to alpha.36 your workaround no longer works, since the selectionModel is now always cleared. Not sure if this is intended due to other changes.

@oliviertassinari So does that mean the fix is still under discussion until those related issues have been resolved?

[oliviertassinari]

@tforssander Are you able to control the selection and ignore the update that are emptying the selection? I guess you are not able to differentiate between a user unselecting with the checkbox and changing page with the footer.
Would it mean that we need to introduce a detail argument for doing such? cc @mui-org/x

[tforssander]

@oliviertassinari I'm using a controlled selection, with paginationMode="server". What I'm currently trying to understand is why my selectionModel state is cleared as soon as I change page. In previous example (#1782 (comment)) the workaround was working, but not anymore.

Example: https://codesandbox.io/s/material-demo-forked-n3rem?file=/demo.tsx

[oliviertassinari]

@tforssander Thanks for the reproduction. We miss a details.reason argument that you can use to ignore change events. It was first suggested in #1782 (comment).

  const handleSelectionModelChange = (
    newSelectionModel: GridSelectionModel,
    events,
    details
  ) => {
    if (details.reason === 'pageChange') {
      return
    }

    setSelectionModel(newSelectionModel);
  };

[tforssander]

@tforssander Thanks for the reproduction. We miss a details.reason argument that you can use to ignore change events. It was first suggested in #1782 (comment).

  const handleSelectionModelChange = (
    newSelectionModel: GridSelectionModel,
    events,
    details
  ) => {
    if (details.reason === 'pageChange') {
      return
    }

    setSelectionModel(newSelectionModel);
  };

@oliviertassinari Ah, that alternative would be great! Is this still a suggestion or is it something that's currently being developed?

[oliviertassinari]

@tforssander It's a suggestion, something that the community could be working on (It's part of the MIT component).

[DanailH]

@tforssander I investigated the problem but and tried to solve it with the current version. It can be done you can check a working demo here -> https://codesandbox.io/s/material-demo-forked-ok333?file=/demo.tsx

To summarise - the selectionModel is updated not because of the page change but because the rows that have been loaded are removed entirely from the data set. By just appending the next page rows to the already loaded ones the problem is solved and the onSelectionModelChange returns the correct values.

Let me know if this solution will work for you. We can look into adding a reason for that case but it will take time as the real reason is not that the page changes but the fact that the rows data has changed so it will be misleading.

[Josephaberry54]

We have a similar issue, not using server side pagination but using custom column filtering alongside controlled row selection. Our filter logic updates the rows data that is being passed to the datagrid, which results in the selected ids no longer being in the row data. When the filter is removed and the row data is present again the selection is missing in the same way as described for this bug. Therefore if its possible to have a solution that covers this too then that would be great.

The workaround we have in place is to spread the ids we pass as the selectionModel prop in to a new array whenever the row data changes and this seems to trigger a re render but would be nice not to have to do this

[flaviendelangle]

The workaround we have in place is to spread the ids we pass as the selectionModel prop in to a new array whenever the row data changes and this seems to trigger a re render but would be nice not to have to do this

If adding a rerender is fixing it, I think this is due to this bug https://github.com/mui-org/material-ui-x/pull/2433/files#diff-87fa96c5d959b8acbc3c76f30e1750e8293f462996dd1a1e8cba8e72553d2f5aR157
If my PR is not ready for the next release, I'll do a one-liner fix 👍

Could you give me a reproduction example on Codesandbox so that I check if it works with my PR ?

My PR does not solve the issue with paginationMode="server" though.

[DanailH]

A fix for the documentation has been added with an example of how to work around this issue for now.

A more comprehensive solution will follow up as part of a separate PR. I'm reopening this issue so we can use it to track the new effort.

I'll also rename the issue to be more relevant to what the problem actually is.

[flaviendelangle]

I am closing this
The behavior is now respecting what was wanted on this issue
See https://codesandbox.io/s/datagrid-v5-quick-start-forked-zelec