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] adding a new row with the autoHeight prop causes first row flicker #13270

Closed
Luis-Ramirez21x opened this issue May 27, 2024 · 15 comments · Fixed by #14235
Closed
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! support: commercial Support request from paid users support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@Luis-Ramirez21x
Copy link

Luis-Ramirez21x commented May 27, 2024

The problem in depth

I have a DataGrid component that lets you add a new row and auto-adds a new row after being saved.
The issue is a re-render on the first row is triggered when adding a row and it causes a flicker. Ths only occurs when the autoHeight prop is added.

See this demo. Simply add a row and notice the flicker on the first row.

Your environment

`npx @mui/envinfo`
  System:
    OS: Linux 6.1 Debian GNU/Linux 12 (bookworm) 12 (bookworm)
  Binaries:
    Node: 18.19.0 - /usr/bin/node
    npm: 9.2.0 - /usr/bin/npm
    pnpm: Not Found
  Browsers:
    Chrome: 124.0.6367.60
  npmPackages:
    @emotion/react: ^11.10.6 => 11.10.6 
    @emotion/styled: ^11.10.6 => 11.10.6 
    @mui/base: ^5.0.0-beta.34 => 5.0.0-beta.40 
    @mui/core-downloads-tracker:  5.15.18 
    @mui/icons-material: ^5.11.11 => 5.11.11 
    @mui/joy: ^5.0.0-beta.11 => 5.0.0-beta.11 
    @mui/lab: ^5.0.0-alpha.126 => 5.0.0-alpha.126 
    @mui/material: ^5.11.12 => 5.15.18 
    @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-charts: ^6.0.0-alpha.15 => 6.0.0-alpha.15 
    @mui/x-data-grid:  6.6.0 
    @mui/x-data-grid-generator: ^6.6.0 => 6.6.0 
    @mui/x-data-grid-premium:  6.6.0 
    @mui/x-data-grid-pro: ^7.4.0 => 7.4.0 
    @mui/x-date-pickers:  6.16.2 
    @mui/x-date-pickers-pro: ^6.16.2 => 6.16.2 
    @mui/x-license:  7.2.0 
    @mui/x-license-pro: ^6.6.0 => 6.6.0 
    @types/react:  18.0.28 
    react: ^18.2.0 => 18.2.0 
    react-dom: ^18.2.0 => 18.2.0 
    typescript:  4.9.5 

Search keywords: autoHeight, DataGrid, flicker
Order ID: 67382

@Luis-Ramirez21x Luis-Ramirez21x added status: waiting for maintainer These issues haven't been looked at yet by a maintainer support: commercial Support request from paid users labels May 27, 2024
@michelengelen michelengelen added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/ labels May 28, 2024
@michelengelen michelengelen changed the title adding a new row with the autoHeight prop causes first row flicker. [data grid] adding a new row with the autoHeight prop causes first row flicker. May 28, 2024
@michelengelen
Copy link
Member

Hey @Luis-Ramirez21x it works if you make the click handler asynchronous:

  const handleClick = async () => {
    const id = randomId();
    await apiRef.current.updateRows([{ id, isNew: true, name: "" }]);
    apiRef.current.startRowEditMode({ id, fieldToFocus: "name" });
  };

Does that solve your problem?

@michelengelen michelengelen added status: waiting for author Issue with insufficient information and removed bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 28, 2024
@Luis-Ramirez21x
Copy link
Author

Hey @michelengelen ! Thanks for the quick reply.
This does solve the flicker when clicking the add button but not the auto-added row flicker.
See this demo. You can recreate it by adding a row, inputting a name, and saving it. A new row is added but still causes the first row to flicker.
Thanks in advance!

@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 28, 2024
@michelengelen
Copy link
Member

Hey @michelengelen ! Thanks for the quick reply. This does solve the flicker when clicking the add button but not the auto-added row flicker. See this demo. You can recreate it by adding a row, inputting a name, and saving it. A new row is added but still causes the first row to flicker. Thanks in advance!

just use await on the handleAddRow fixed it for me:

if (isNew) {
  apiRef.current.updateRows([{ ...res }]);

  await handleAddRow();
  return { ...oldRow, _action: "delete" };
}

@michelengelen michelengelen 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 29, 2024
@Luis-Ramirez21x
Copy link
Author

The first row continues to re-render despite your suggestion. See the demo and attached recording.

first_row_flicker.mp4

@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 29, 2024
@michelengelen
Copy link
Member

It doesn't happen for me on the first 2-3 rows I add ... but then it does.
I am not sure if we can fully get rid of this flicker tbh.
It happens because we do a updateRows followed by handleAddRow (which also calls updateRows) and return the new row from processRowUpdate (which will also call updateRows). This is at least 3 consecutive times we call that in a quick succession.

Maybe @romgrk or @cherniavskii know if this might be possible.

@michelengelen
Copy link
Member

Maybe we could even handle this as a feature request to allow for an additional empty row that sits at the bottom (or top)?

Thoughts @mui/xgrid ?

@Luis-Ramirez21x
Copy link
Author

Hello @michelengelen @romgrk @cherniavskii , I wanted to know if you guys had any more insight on this?
I haven't been able to come up with a solution.

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

Thanks @Luis-Ramirez21x for reporting. I can confirm the issue. I can reproduce the problem with the CRUD example in the docs (after adding autoHeight prop): https://stackblitz.com/edit/react-eer4hb

On initial investigation, the cells in the first row seem to be unnecessarily re-rendering. I'll move it to the board for prioritization.

@MBilalShafi MBilalShafi removed their assignment Jun 12, 2024
@MBilalShafi MBilalShafi added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 12, 2024
@github-project-automation github-project-automation bot moved this to 🆕 Needs refinement in MUI X Data Grid Jun 12, 2024
@Luis-Ramirez21x
Copy link
Author

Thank you for looking into it @MBilalShafi ! 🙏

@cherniavskii
Copy link
Member

I can't reproduce it using the @mui/x-data-grid-pro@7.7.0: https://stackblitz.com/edit/react-eer4hb-ugghju?file=Demo.tsx,package.json,index.tsx
Am I missing something?

@cherniavskii cherniavskii added the status: waiting for author Issue with insufficient information label Jun 21, 2024
@Luis-Ramirez21x
Copy link
Author

Hey @cherniavskii
Seems that the flicker only occurs when the grid is not overflowing.
See the attached video with your demo.

Grid_Flicker.mp4

@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 21, 2024
@KenanYusuf
Copy link
Member

KenanYusuf commented Jul 3, 2024

Seems that the flicker only occurs when the grid is not overflowing

I am also seeing this behaviour when the grid is overflowing. I think there is enough information for us to reproduce and take a look at this issue after prioritisation

@KenanYusuf KenanYusuf removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 3, 2024
@Luis-Ramirez21x
Copy link
Author

Hey guys, just wanted to follow up and see if there are any updates on this. 🙂

@KenanYusuf
Copy link
Member

The issue seems to be related to switching focus to an editable cell. I have narrowed it down to this effect:

https://github.com/mui/mui-x/blob/master/packages/x-data-grid/src/components/cell/GridEditInputCell.tsx#L117-L121

But still unsure why this would create a flicker (which is also only occuring with autoHeight enabled).

flicker.mp4

@oliviertassinari oliviertassinari changed the title [data grid] adding a new row with the autoHeight prop causes first row flicker. [data grid] adding a new row with the autoHeight prop causes first row flicker Aug 17, 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.

@Luis-Ramirez21x: 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.

@DanailH DanailH moved this from 🆕 Needs refinement to ✅ Done in MUI X Data Grid Nov 1, 2024
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! support: commercial Support request from paid users support: pro standard Support request from a Pro standard plan user. https://mui.com/legal/technical-support-sla/
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants