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] Column autosizing calculated on old data instead of new data #12940

Closed
frans-sqills opened this issue Apr 29, 2024 · 9 comments · Fixed by #13587
Closed

[data grid] Column autosizing calculated on old data instead of new data #12940

frans-sqills opened this issue Apr 29, 2024 · 9 comments · Fixed by #13587
Assignees
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!

Comments

@frans-sqills
Copy link

frans-sqills commented Apr 29, 2024

Steps to reproduce

Link to live example: https://stackblitz.com/edit/react-e7tkn5?file=Demo.tsx

Current behavior

The community version of data grid uses the old data set to set the --DataGrid-rowWidth css variable when using the autosizeColumns functionality, resulting in inconsistencies between the header and data rows.

Row exceeds header length:
image

Header exceeds row length:
image

The pro version does set the --DataGrid-rowWidth based on the new data, as shown in the screenshot below. The pro version above the community version below, both using the same data but different widths:
image

Expected behavior

The community version should also set the --DataGrid-rowWidth based on the new data just like the pro version does.

Context

The live example is a fork of autosizing asynchronously with extended data and the community version added below to the pro version, both use the same logic to retrieve the data and execute the autosizeColumns.

With the browser inspector open, clicking the refresh button I see the style attribute of the MuiDataGrid-root div refreshed immediately for the community version which is too soon. With the pro version the style attribute of the MuiDataGrid-root div is refreshed after the new data is loaded.

Your environment

npx @mui/envinfo
  System:
    OS: macOS 14.5
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
    pnpm: Not Found
  Browsers:
    Chrome: 125.0.6422.113
    Edge: Not Found
    Safari: 17.5
  npmPackages:
    @emotion/react:  11.11.4 
    @emotion/styled:  11.11.5 
    @mui/base:  5.0.0-beta.40 
    @mui/core-downloads-tracker:  5.15.17 
    @mui/icons-material:  5.15.17 
    @mui/lab:  5.0.0-alpha.170 
    @mui/material:  5.15.17 
    @mui/private-theming:  5.15.14 
    @mui/styled-engine:  5.15.14 
    @mui/system:  5.15.15 
    @mui/types:  7.2.14 
    @mui/utils:  5.15.14 
    @mui/x-data-grid:  7.4.0 
    @mui/x-date-pickers:  7.4.0 
    @types/react:  18.3.2 
    react:  18.3.1 
    react-dom:  18.3.1 
    typescript: >=5.4.5 => 5.4.5 

Search keywords: Autosizing

@frans-sqills frans-sqills added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 29, 2024
@zannager zannager added the component: data grid This is the name of the generic UI component, not the React module! label Apr 29, 2024
@romgrk romgrk self-assigned this Apr 29, 2024
@romgrk romgrk added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Apr 29, 2024
@romgrk
Copy link
Contributor

romgrk commented May 30, 2024

I'm unable to reproduce an issue with the steps above, would it be possible to explain which steps to take to experience any of the issues you list?

@romgrk romgrk added the status: waiting for author Issue with insufficient information label May 30, 2024
@frans-sqills
Copy link
Author

Please have a look at the example here https://stackblitz.com/edit/react-e7tkn5. After retrieval of the data the CSS var --DataGrid-rowWidth should be calculated and set based on the widest row. The pro version does this correctly but the community version sets the --DataGrid-rowWidth based on the widest row of the old data and not of the newly retrieved data.

Some examples:

First data retrieval after opening the example, the header and hover backgrounds of the community version do not cover the "Branding", "Representative" and "Rating" columns on the far right:
image

After clicking refetch data,
either the widest row is smaller thus the header and hover backgrounds exceed the widest row (the row underline does not cover the full width)
image

or the widest row is wider thus the header and hover backgrounds does not cover the "Rating" column on the far right:
image

@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 30, 2024
@romgrk
Copy link
Contributor

romgrk commented May 30, 2024

Still can't reproduce, can you fill the env info section? You have the instructions in the top comment.

@romgrk romgrk 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 May 30, 2024
@frans-sqills
Copy link
Author

I have added the env info section in the top comment

@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 30, 2024
@romgrk romgrk removed their assignment Jun 10, 2024
@romgrk
Copy link
Contributor

romgrk commented Jun 10, 2024

@mui/xgrid Can someone on macOS take this one? Might have an impact.

@MBilalShafi
Copy link
Member

@mui/xgrid Can someone on macOS take this one? Might have an impact.

I'll check it out.

@MBilalShafi MBilalShafi self-assigned this Jun 12, 2024
@MBilalShafi
Copy link
Member

MBilalShafi commented Jun 22, 2024

After a few tries, I was finally able to reproduce this error. It didn't reproduce so easily for me.

To reproduce I needed to meet these conditions:

  1. Use one of Edge or Safari browsers
  2. It only reproduces on a specific screen width (roughly when 4 columns are visible after autoSize) I needed to resize the window several times to reproduce.

When it reproduces, this useEnhancedEffect doesn't get triggered after the autosizeColumns action.

useEnhancedEffect(() => {
if (!root) {
return;
}
const set = (k: string, v: string) => root.style.setProperty(k, v);
set('--DataGrid-width', `${dimensions.viewportOuterSize.width}px`);
set('--DataGrid-hasScrollX', `${Number(dimensions.hasScrollX)}`);
set('--DataGrid-hasScrollY', `${Number(dimensions.hasScrollY)}`);
set('--DataGrid-scrollbarSize', `${dimensions.scrollbarSize}px`);
set('--DataGrid-rowWidth', `${dimensions.rowWidth}px`);
set('--DataGrid-columnsTotalWidth', `${dimensions.columnsTotalWidth}px`);
set('--DataGrid-leftPinnedWidth', `${dimensions.leftPinnedWidth}px`);
set('--DataGrid-rightPinnedWidth', `${dimensions.rightPinnedWidth}px`);
set('--DataGrid-headerHeight', `${dimensions.headerHeight}px`);
set('--DataGrid-headersTotalHeight', `${dimensions.headersTotalHeight}px`);
set('--DataGrid-topContainerHeight', `${dimensions.topContainerHeight}px`);
set('--DataGrid-bottomContainerHeight', `${dimensions.bottomContainerHeight}px`);
set('--height', `${dimensions.rowHeight}px`);
}, [root, dimensions]);

Since the autosizeColumns has already resized the individual columns, but the root CSS variables are not updated, it yields a weird UX.

I suspect it's because of the non-reactive nature of the dimensions dependency of this useEnhnacedEffect. Probably using the dimensions selectors along with useGridSelector will fix it (by making it reactive).

@froons can you reproduce it consistently or on a specific screen width? Also which browser do use to reproduce it?


Update: I tried to make the dimensions reactive and the error doesn't reproduce for me anymore.
@Frooons can you check if it still reproduces for you in this codesandbox: https://codesandbox.io/p/sandbox/gifted-lederberg-l4vx2h

@MBilalShafi MBilalShafi 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 Jun 22, 2024
@frans-sqills
Copy link
Author

@MBilalShafi To answer the first question, I am able to reproduce it consistently using Chrome and Safari where the table has to be wider than the screen width.
And for the second question, the issue does not reproduce for me anymore in the codesandbox!

@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 Jun 24, 2024
@MBilalShafi MBilalShafi removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 24, 2024
Copy link

⚠️ This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@Frooons: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants