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

Row Grouping collapsed state returns to default on row/cell changes. #10085

Closed
2 tasks done
andreskimlee opened this issue Aug 18, 2023 · 9 comments
Closed
2 tasks done
Labels
component: data grid This is the name of the generic UI component, not the React module! plan: Premium Impact at least one Premium user support: commercial Support request from paid users

Comments

@andreskimlee
Copy link

andreskimlee commented Aug 18, 2023

Order ID

72370

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

The problem in depth 🔍

Whenever a row/cell updates, I am having an issue where the collapsed state of the row grouping to (expand/close) is set back to the table's original state.

My guess is the parent component is re-rendering the entire table due to a prop change. (Currently we have an action button that triggers an update to that row. We are currently not using something like updateRow() but instead the rows are recalculated) Since there is no way to access the state of the row grouping (if collapsed or not). I'm not entirely sure how to persist the states between re-renders.

Alternatively, the collapsed state isnt persisting between renders

Screen.Recording.2023-08-18.at.7.39.45.PM.mov

anyone have any ideas?

EDIT: Some other things I've I noticed,
a. sorting model persists between renders, so it looks like the data grid isn't being entirely re-rendered. (Maybe the state gets reset on certain re-renders for when a row grouping is collapsed?)
b. I noticed certain event listeners will trigger the row to return back to a default state. I've tested out preventDefaults/stop propogations on those said buttons event listeners, and it does not seem to work. Here is a video example

Screen.Recording.2023-08-21.at.11.03.59.AM.mov

Your environment 🌎

`npx @mui/envinfo`
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.
@andreskimlee andreskimlee added status: waiting for maintainer These issues haven't been looked at yet by a maintainer support: commercial Support request from paid users labels Aug 18, 2023
@zannager zannager added component: data grid This is the name of the generic UI component, not the React module! plan: Premium Impact at least one Premium user labels Aug 21, 2023
@cherniavskii
Copy link
Member

Hi @andreskimlee
This looks similar to #7771
Could you check if using updateRows solves your issue?

@cherniavskii cherniavskii added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 22, 2023
@andreskimlee
Copy link
Author

Hi @andreskimlee This looks similar to #7771 Could you check if using updateRows solves your issue?

That seems to fix the issue in regards to updating an actual row, Any ideas on why opening a popover/menu causes the collapsed state to change back to default?

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Aug 29, 2023
@MBilalShafi
Copy link
Member

Any ideas on why opening a popover/menu causes the collapsed state to change back to default?

I was not able to reproduce the mentioned issue in this codesandbox demo, probably you are updating some external variable on popover/menu open which is causing the Grid to re-render and recompute the Grid tree?

Could you provide a minimal reproduction test case with the latest version? This would help us understand the issue better. 👷
A live example would be perfect. This codesandbox example may be a good starting point. Feel free to fork it or start over from scratch.

Thank you!

@MBilalShafi MBilalShafi added the status: waiting for author Issue with insufficient information label Aug 29, 2023
@andreskimlee
Copy link
Author

andreskimlee commented Aug 30, 2023

Any ideas on why opening a popover/menu causes the collapsed state to change back to default?

I was not able to reproduce the mentioned issue in this codesandbox demo, probably you are updating some external variable on popover/menu open which is causing the Grid to re-render and recompute the Grid tree?

Could you provide a minimal reproduction test case with the latest version? This would help us understand the issue better. 👷 A live example would be perfect. This codesandbox example may be a good starting point. Feel free to fork it or start over from scratch.

Thank you!

Was unable to reproduce via condesandbox so Its most likely an issue on our end. What im struggling to understand is why the sort state persists between re-renders but the collapsed state gets reset? Might be insightful to know how the row collapsed state in row grouping behaves differently from the sort state.

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Aug 30, 2023
@MBilalShafi
Copy link
Member

That's actually a good question. It's by design, whenever the rows prop is updated or apiRef.current.setRows is called, the whole tree of rows is regenerated, as this update is supposed to be done when a full Grid row recalculation is needed, otherwise it's recommended to use partial rows update method apiRef.current.updateRows which doesn't cause the expansion state to be lost. (Though I think this section of docs deserves some more explanation)

Regarding the application state other than rows (like filterModel for example) it's not updated unless there's a trigger to update it, like controlled state being changed or some event firing which updates a part of the state.

I'd recommend you to read a more detailed explanation of the related problem faced by some users: #210 (comment)

Does that somewhat addresses your concern?

@MBilalShafi MBilalShafi added the status: waiting for author Issue with insufficient information label Aug 31, 2023
@andreskimlee
Copy link
Author

That's actually a good question. It's by design, whenever the rows prop is updated or apiRef.current.setRows is called, the whole tree of rows is regenerated, as this update is supposed to be done when a full Grid row recalculation is needed, otherwise it's recommended to use partial rows update method apiRef.current.updateRows which doesn't cause the expansion state to be lost. (Though I think this section of docs deserves some more explanation)

Regarding the application state other than rows (like filterModel for example) it's not updated unless there's a trigger to update it, like controlled state being changed or some event firing which updates a part of the state.

I'd recommend you to read a more detailed explanation of the related problem faced by some users: #210 (comment)

Does that somewhat addresses your concern?

Appreciate the quick response, that was super insightful.

So we found a solution (Albeit not optimal since we're masking the re-rendering issue lol) but for now,

the way we solved this issue was managing the row collapsed states ourselves

useEffect(() => {
    apiRef.current.subscribeEvent('rowExpansionChange', (node) => {
      expansionState.current[node.id] = node.childrenExpanded ?? false
    })
  }, [apiRef])

  const isGroupExpanded = useCallback(
    (node: GridGroupNode) => expansionState.current[node.id] ?? false,
    [expansionState],
  )

<DataGridPremium
    isGroupExpandedByDefault={isGroupExpanded}

@github-actions github-actions bot removed the status: waiting for author Issue with insufficient information label Aug 31, 2023
@MBilalShafi
Copy link
Member

So we found a solution (Albeit not optimal since we're masking the re-rendering issue lol)

Glad you found one, yeah, it would cause the Grid to re-render every time the rows prop updates.
If possible, I'd recommend you to port to the more optimal solution to achieve a better performance.

Since the question is answered, I'll close the issue. Feel free to reopen if you face an issue again on the same topic or open a separate one if you have one on another topic.

Thank you again for using the X Data Grid. 🙂

@secretwpn
Copy link

secretwpn commented Dec 15, 2023

@andreskimlee thanks for the idea. it was not obvious how to subscribe an event and what event to subscribe to follow these changes. overall whole expanded state management situation (at least user-facing part of it feels very immature. I would argue that persisting the expanded state should be a built in feature that user can control via prop)
Do you know if you have to explicitly unsubscribe the event on useEffect return (cleanup)? I could not find a way to do that.

@cherniavskii
Copy link
Member

@secretwpn subscribeEvent returns a cleanup function, so you can return it in useEffect - see https://mui.com/x/react-data-grid/events/#with-apiref-current-subscribeevent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! plan: Premium Impact at least one Premium user support: commercial Support request from paid users
Projects
None yet
Development

No branches or pull requests

5 participants