mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.1k stars 1.27k forks source link

[data grid] adding a new row with the autoHeight prop causes first row flicker #13270

Closed Luis-Ramirez21x closed 4 weeks ago

Luis-Ramirez21x commented 4 months ago

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

michelengelen commented 4 months ago

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?

Luis-Ramirez21x commented 4 months ago

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!

michelengelen commented 4 months ago

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" };
}
Luis-Ramirez21x commented 4 months ago

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

https://github.com/mui/mui-x/assets/86748117/6fd5c8be-0264-492d-a48c-a56ed11f46d1

michelengelen commented 4 months ago

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 commented 4 months ago

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 commented 3 months ago

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 commented 3 months ago

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.

Luis-Ramirez21x commented 3 months ago

Thank you for looking into it @MBilalShafi ! :pray:

cherniavskii commented 3 months ago

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?

Luis-Ramirez21x commented 3 months ago

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

https://github.com/mui/mui-x/assets/86748117/409c5dcb-45a5-4803-bf63-02820b2c3819

KenanYusuf commented 2 months ago

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

Luis-Ramirez21x commented 1 month ago

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

KenanYusuf commented 1 month ago

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).

https://github.com/user-attachments/assets/43a81c67-4743-4f9b-b4dd-0598572ee240

github-actions[bot] commented 4 weeks ago

:warning: 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.