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

[data grid] All grouped rows collapses when rows updating (by state, not by updateRows) #13064

Open
iOrcohen opened this issue May 9, 2024 · 14 comments
Labels
component: data grid This is the name of the generic UI component, not the React module! feature: Row grouping Related to the data grid Row grouping feature support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@iOrcohen
Copy link

iOrcohen commented May 9, 2024

Steps to reproduce

Link to live example: code sandbox

Steps:

  1. Expand one or more of the groups.
  2. Click on one of the buttons - update row, update rows, delete row or add row.
  3. All the groups collapses.

Current behavior

All groups collapses after the rows state changed.

Expected behavior

Groups expansion won't be effected by the rows change.

Context

We update the table rows with fresh data every one minute in our application (It's include coins prices data which require often refresh).
After every refresh all the groups collapses automatically, which make unpleasant user experience.

Your environment

npx @mui/envinfo
  System:
    OS: macOS 14.4.1
  Binaries:
    Node: 18.16.0 - /usr/local/bin/node
    npm: 9.5.1 - /usr/local/bin/npm
    pnpm: Not Found
  Browsers:
    Chrome: 124.0.6367.119
    Edge: Not Found
    Safari: 17.4.1
  npmPackages:
    @emotion/react: ^11.11.4 => 11.11.4 
    @emotion/styled: ^11.11.0 => 11.11.0 
    @mui/base:  5.0.0-beta.28 
    @mui/core-downloads-tracker:  5.15.14 
    @mui/lab: ^5.0.0-alpha.127 => 5.0.0-alpha.157 
    @mui/material: ^5.15.14 => 5.15.14 
    @mui/private-theming:  5.15.14 
    @mui/styled-engine:  5.15.14 
    @mui/system:  5.15.14 
    @mui/types:  7.2.14 
    @mui/utils:  5.15.14 
    @mui/x-data-grid:  6.19.11 
    @mui/x-data-grid-premium: ^6.19.11 => 6.19.11 
    @mui/x-data-grid-pro:  6.19.11 
    @mui/x-date-pickers: ^6.10.2 => 6.18.5 
    @mui/x-date-pickers-pro: ^6.18.0 => 6.18.5 
    @mui/x-license-pro:  6.10.2 
    @types/react:  18.2.45 
    react: ^18.2.0 => 18.2.0 
    react-dom: ^18.2.0 => 18.2.0 
    typescript: ^4.9.5 => 4.9.5 

Search keywords: expansion, grouping
Order ID: 66471 + 90144

@iOrcohen iOrcohen added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 9, 2024
@zannager zannager added component: data grid This is the name of the generic UI component, not the React module! support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ labels May 9, 2024
@michelengelen michelengelen changed the title MUI Datagrid V6 - All grouped rows collapses when rows updating (by state, not by updateRows) [data grid] All grouped rows collapses when rows updating (by state, not by updateRows) May 13, 2024
@michelengelen
Copy link
Member

michelengelen commented May 13, 2024

Hey @iOrcohen thanks for opening this issue.

The behavior you are describing is actually to be expected, when providing a new rows prop, the data grid will reset the initialState and the collapsed rows.

This can be avoided when using the updateRows() method from the apiRef or, if this is not an option, you can always create a custom state and function for isGroupExpandedByDefault. This is from the group expansion: https://mui.com/x/react-data-grid/row-grouping/#group-expansion part of the docs.

@cherniavskii is there any plan of including the row-expansion state in the grid state?

@michelengelen michelengelen added status: waiting for author Issue with insufficient information feature: Row grouping Related to the data grid Row grouping feature and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 13, 2024
@iOrcohen
Copy link
Author

@michelengelen Thanks for your comment!
We will try solve it by using the updateRows function, but since the common usage of the rows change is by changing the state, I think the row expansion should be part of the grid state, and remain the same from render to render as same as filtering, grouping and all the others.

If the plan is to the add the row-expansion to the grid state, let me know and I might contribute it myself :)

Thanks again.

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels May 13, 2024
@michelengelen michelengelen removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 14, 2024
@github-project-automation github-project-automation bot moved this to 🆕 Needs refinement in MUI X Data Grid May 14, 2024
@michelengelen

This comment was marked as resolved.

@iOrcohen

This comment was marked as resolved.

@michelengelen

This comment was marked as resolved.

@cherniavskii
Copy link
Member

Related discussion: #7771

We might reconsider the default behavior for the next major since the current behavior is clearly confusing to many users.

@SamJB123
Copy link

SamJB123 commented Jun 2, 2024

Hi all, I've developed a band-aid solution to this, leveraging (a) the internal state's storage of whether rows belonging to a group are visible pre-update and (b) the fact that group rows can be dynamically identified by their prefix.

Sharing in case it helps anybody else, as this was driving me nuts!

Initialise a dynamic state for expanded row groups

    const [expandedGroups, setExpandedGroups] = useState<string[]>([]);

...

The main function

  const HandleNodeClick_withSavedExpandState = useCallback(
    debounce((node) => {
      // Get current apiRef state
      const CurrentGridState = apiRef.current.state;

      // Dynamically identify the group headings that are currently visible by using the visibleRowsLookup object, and the fact that group headings start with "auto-generated-row-type/"
      const visibleGroups = Object.keys(CurrentGridState.visibleRowsLookup).filter(row => row.startsWith('auto-generated-row-type/'));

      // Store the identified group headings for reference
      const visibleGroupsWithoutPrefix = visibleGroups.map(row => row.replace('auto-generated-row-type/', ''));

      // To check if each group is currently expanded, check if the first row of each group is visible
      const firstRowOfEachGroup = visibleGroups.map(groupId => 
        apiRef.current.getRowGroupChildren({groupId})[0]
      );      
      const firstRowVisibility = firstRowOfEachGroup.map(row => CurrentGridState.visibleRowsLookup[row]);

      // Build an array of the visible groups (without prefix), based on those for which which the first row is visible
      const visibleExpandedGroups = visibleGroupsWithoutPrefix.filter((_, index) => firstRowVisibility[index]);
      console.log('Visible Expanded Groups: ', visibleExpandedGroups);

      // Pass this array of expanded groups to the state for the next render
      setExpandedGroups(visibleExpandedGroups);

      // Call the original trigger for a state update
      originalHandleNodeClick(node);

    }, 300),
    []
  );

...

Define the 'isGroupExpandedByDefault' parameter dynamically

  const isGroupExpandedByDefault = useCallback((node: GridGroupNode) =>
    node.groupingField === '[YOUR_GROUP_FIELD_NAME_HERE]' && expandedGroups.includes(String(node.groupingKey)), [expandedGroups]);

...

Include the parameter in your DataGrid output

    <DataGridPremium
        isGroupExpandedByDefault={isGroupExpandedByDefault} />

@tunganh-phamba
Copy link

I am also experiencing this bug and it significantly impacts my work. It would be greatly appreciated if a fix could be provided soon. Thank you for your attention to this matter.

@kseileraltus
Copy link

Thanks Sam for the workaround! For what its worth, I would also greatly appreciate official support to keep the rows expanded on interaction.

@tonynchen

This comment was marked as resolved.

@Matthieudegny

This comment was marked as resolved.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 22, 2024

Related discussion: #7771. We might reconsider the default behavior for the next major since the current behavior is clearly confusing to many users.

@cherniavskii Yeah, I think that it would make sense to reopen #7771 too. As far as I remember, the updateRows() API was introduced to support high frequency updates use cases, not to solve problems of this type, it looks off.

As a developer, I would expect those:

  • reactivity: the derived states are retained when rows changes. What happen instead is clean-up: a diff of the removed rows, and a removal of the dead value from the derived state. If this adds too much complexity, then we could simplify the logic with continuously expending the derived state.
  • control: the expansion state is a controllable state. Having a defaut value isGroupExpandedByDefault is a nice start but it doesn't look enough. It seems to be [data grid] Allow to change to expansion status of several groups at once #4930.

Now, I guess tree data and row grouping are powered by the same state, so #7771 and #13064 are the same issues.

@HansBrende
Copy link

HansBrende commented Nov 5, 2024

@oliviertassinari is this a new bug? As far as I can tell, this was not happening until we upgraded to data grid premium v7. I think we were on data grid pro v5 before... pretty sure everything was working just fine up until that upgrade, as complaints from our users about this issue have only started very recently. (Although, I can't say for sure that upgrading was the reason since other changes have gone in as well since then--however no changes that I can see which would affect whether or not rows are memoized... we have never memoized rows, only columns.)

(Note: we are not using row grouping, but tree data. Is it possible that previously, in v5, the expansion state was correctly linked to either the row IDs or the tree data path--which I would expect--but in v7, the expansion state was erroneously linked to the row's object identity instead? Like I said, we have never memoized rows and the expansion state was definitely working correctly prior to our upgrade to v7.)

@AviRaboah
Copy link

do you know if the next release is going to incudes a fix for that issue?

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! feature: Row grouping Related to the data grid Row grouping feature support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/
Projects
Status: 🆕 Needs refinement
Development

No branches or pull requests