search

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.55k stars 1.33k forks source link

[data grid] Skeleton Loading Overlay Triggers Error with Master Detail Panel Containing Another DataGrid #14061

Closed 4everTonyStark closed 3 months ago

4everTonyStark commented 3 months ago

Latest version

Steps to reproduce

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

Steps:

  1. Open a row's Detail Panel.
  2. Click "Toggle Loading" to show the Skeleton Loading Overlay.
  3. Open the console to observe the error that is logged from the DataGrid.

Current behavior

When the Skeleton Loading Overlay renders over a DataGrid with an open detail panel, and the detail panel contains a DataGrid component, an error is logged from the DataGrid within the row detail panel that it has no intrinsic size.

Expected behavior

No errors should be logged. I would think either the overlay should be a true overlay on top of the existing content, or detail panels should be removed or hidden before the overlay is applied. I've tried switching the getDetailPanelContent prop to undefined when loading is true, but that doesn't seem to fix the issue.

Context

Allowing the DataGrid to show the Skeleton Loading Overlay while a master detail panel is opened and not log any errors.

Your environment

npx @mui/envinfo ``` System: OS: macOS 14.5 Binaries: Node: 20.11.1 - ~/Library/Caches/fnm_multishells/173_1722440156564/bin/node npm: 10.2.4 - ~/Library/Caches/fnm_multishells/173_1722440156564/bin/npm pnpm: 9.6.0 - ~/Library/Caches/fnm_multishells/173_1722440156564/bin/pnpm Browsers: Chrome: 127.0.6533.73 Edge: Not Found Safari: 17.5 ```

I see this bug in Chrome. I have not tested Safari.

Search keywords: skeleton loading overlay, master detail

michelengelen commented 3 months ago

The "overlay" is not actually overlaying the data grid content. The rows are still present, but will be hidden with display: none;. I guess this is due to performance reasons. @KenanYusuf can you elaborate?

KenanYusuf commented 3 months ago

@4everTonyStark @michelengelen

Correct that the skeleton loading overlay is not actually an overlay, the name is misleading but the main reasons are:

  1. We do not enforce or have a default background-color on the data grid, so displaying the skeleton loader over the top of rows does not work as you can see through the skeleton elements. It's not safe for us to assume and set a background-color on the overlay either right now.
  2. Virtualized scrolling - if the skeleton is rendered in an absolutely positioned element over the top of actual rows, we would have to listen to scroll events within the virtual scroller, and there would be a slight delay between seeing the headers scroll and the skeleton content.
  3. For performance reasons (and just logically), it doesn't make much sense to display the actual grid cells behind the skeleton loader, because they will never be visible whilst it is active.

All that to say, it was a simpler implementation and a better UX to just render the skeleton loader as a statically positioned element and hide the content.

Onto your actual issue, looks like we need prevent rendering master details whilst the skeleton loader is active, I'll look into it.

github-actions[bot] commented 3 months 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.

@4everTonyStark: 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.