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] Skeleton Loading Overlay Triggers Error with Master Detail Panel Containing Another DataGrid #14061

Closed
1 task done
4everTonyStark opened this issue Jul 31, 2024 · 3 comments · Fixed by #14186
Closed
1 task done
Assignees
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Master-detail Related to the data grid Master-detail feature feature: Row loading Related to the data grid Row loading features

Comments

@4everTonyStark
Copy link

4everTonyStark commented Jul 31, 2024

Latest version

  • I have tested the 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

@4everTonyStark 4everTonyStark added bug 🐛 Something doesn't work status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 31, 2024
@github-actions github-actions bot added the component: data grid This is the name of the generic UI component, not the React module! label Jul 31, 2024
@4everTonyStark 4everTonyStark changed the title DataGrid Skeleton Loading Overlay Triggers Error with Master Detail Panel Containing another DataGrid DataGrid Skeleton Loading Overlay Triggers Error with Master Detail Panel Containing Another DataGrid Jul 31, 2024
@michelengelen michelengelen changed the title DataGrid Skeleton Loading Overlay Triggers Error with Master Detail Panel Containing Another DataGrid [data grid] Skeleton Loading Overlay Triggers Error with Master Detail Panel Containing Another DataGrid Aug 1, 2024
@michelengelen
Copy link
Member

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?

@michelengelen michelengelen added feature: Row loading Related to the data grid Row loading features feature: Master-detail Related to the data grid Master-detail feature labels Aug 2, 2024
@KenanYusuf
Copy link
Contributor

KenanYusuf commented Aug 2, 2024

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

@KenanYusuf KenanYusuf self-assigned this Aug 2, 2024
@michelengelen michelengelen removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 5, 2024
@github-project-automation github-project-automation bot moved this to 🆕 Needs refinement in MUI X Data Grid Aug 5, 2024
@michelengelen michelengelen moved this from 🆕 Needs refinement to 🏗 In progress in MUI X Data Grid Aug 5, 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.

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

@DanailH DanailH moved this from 🏗 In progress 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! feature: Master-detail Related to the data grid Master-detail feature feature: Row loading Related to the data grid Row loading features
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants