[DataGrid] Improve custom overlay slots positioning #3832

cherniavskii · 2022-02-03T15:42:42Z

Preview: https://codesandbox.io/s/fullfeatureddemo-material-demo-forked-efmqf

Our layout requires overlays to have position absolute. Otherwise things like horizontal scrolling won't work (overlay might push render zone down below the grid footer).
At the same time, we allow to override overlays using slots.
So in order to make custom overlays work as good as default ones, we have to make sure custom slots are positioned in the way as default ones.
I've solved it by wrapping overlays with a component that has all position-related styles applied, so that they stay there when using custom overlays.

UPDATE: I just noticed, that in our demos we use GridOverlay component.
https://mui.com/components/data-grid/components/#loading-overlay
I think this PR makes it unnecessary. Also, it's not obvious GridOverlay is required - as seen in #3795
I'll update the demos.

Our layout requires overlays to have position absolute. otherwise e.g. horizontal scrolling won't work. At the same time we allow to override overlays using slots. So in order to make custom overlays work as good as default ones, we have to make sure custom slots are positioned the way default ones are. This is solved by wrapping overlays with a component that has all position-related styles applied, so that they stay there when using custom overlays

cherniavskii · 2022-02-03T15:46:52Z

packages/grid/_modules_/grid/components/base/GridOverlaysRoot.tsx

+import { useGridApiContext } from '../../hooks/utils/useGridApiContext';
+import { useGridRootProps } from '../../hooks/utils/useGridRootProps';
+
+export function GridOverlaysRoot(props: React.PropsWithChildren<{}>) {


It's basically extracted from GridOverlay.tsx

mui-bot · 2022-02-03T15:52:22Z

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	243.3	460.2	360.9	344.76	87.536
Sort 100k rows	ms	401.6	918.2	611	621.48	177.935
Select 100k rows	ms	162	320.4	217.1	237.44	66.87
Deselect 100k rows	ms	135.1	314.7	162.8	187.54	64.574

Generated by 🚫 dangerJS against 0c8d1cd

cherniavskii · 2022-02-03T16:10:22Z

docs/src/pages/components/data-grid/components/CustomLoadingOverlayGrid.js

@@ -24,7 +14,7 @@ export default function CustomLoadingOverlayGrid() {
    <div style={{ height: 400, width: '100%' }}>
      <DataGrid
        components={{
-          LoadingOverlay: CustomLoadingOverlay,
+          LoadingOverlay: LinearProgress,


I love this change!

github-actions · 2022-02-17T15:39:00Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

m4theushw

It was a bit complicate to understand how these overlay components work together. That's a signal that we should simplify the tree. What do you think about removing GridOverlay from the default overlays (e.g. no result overlay) and integrate the styles into GridOverlays? GridOverlay would still be exported but it wouldn't have styles, only kept for backwards compatibility.

m4theushw · 2022-02-21T14:07:08Z

packages/grid/x-data-grid/src/internals/components/base/GridOverlaysRoot.tsx

+        top: headerHeight,
+        left: 0,
+        right: 0,
+        bottom: 0,


bottom and right are not necessary if we also define the height and width.

turns out bottom: 0 is needed here when height: auto, otherwise overlay won't take the whole height

m4theushw · 2022-02-21T15:41:48Z

packages/grid/x-data-grid/src/internals/components/base/GridBody.tsx

@@ -72,7 +73,9 @@ function GridBody(props: GridBodyProps) {

  return (
    <GridMainContainer>
-      <GridOverlays />
+      <GridOverlaysRoot>
+        <GridOverlays />


Could GridOverlaysRoot be the root element of GridOverlays? This approach kinda breaks the internal convention for styled components:

<GridOverlays> <GridOverlaysRoot> {currentOverlay} </GridOverlaysRoot> </GridOverlays>

For the error overlay we can make an exception and export GridOverlaysRoot.

I've kept GridOverlayWrapper in separate file though to avoid exporting it in the package

m4theushw · 2022-02-23T17:41:07Z

packages/grid/x-data-grid/src/internals/components/base/GridOverlayWrapper.tsx

+        bottom: 0,
+        left: 0,


Not needed.

Suggested change

bottom: 0,

left: 0,

bottom is needed when height is auto, because otherwise overlay won't take the full height and some tests will fail

True, I forgot #3832 (comment). Then, maybe keep bottom=0 only when height=auto, otherwise it's undefined. That way we don't have two properties competing.

packages/grid/x-data-grid/src/internals/components/base/GridOverlays.tsx

cherniavskii · 2022-02-24T10:08:43Z

packages/grid/x-data-grid/src/internals/components/base/GridOverlayWrapper.tsx

+        height,
+        width: viewportInnerSize?.width ?? 0,
+        position: 'absolute',
+        top: headerHeight,


Looking at #4026, I think we should set top: 0 and full height (including footer) for ErrorOverlay, since neither grid column header nor footer are rendered.
At this point, I don't think we need GridOverlayWrapper for ErrorOverlay at all, since scrolling is not an issue if there's an error.

cc @m4theushw

And since GridOverlayWrapper is used only in GridOverlays, I've moved it to GridOverlays

m4theushw

Much better. 👍

cherniavskii added component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request feature: Rendering layout Related to the data grid Rendering engine labels Feb 3, 2022

cherniavskii commented Feb 3, 2022

View reviewed changes

get rid of GridOverlay in demos

dfb8600

cherniavskii commented Feb 3, 2022

View reviewed changes

cherniavskii added 3 commits February 3, 2022 17:13

fix failing test

bbc6459

fix error overlay position

e45a68e

fix failing test

ffb967d

This comment was marked as resolved.

Sign in to view

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 8, 2022

Merge branch 'master' into custom-overlays

5042267

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 11, 2022

cherniavskii mentioned this pull request Feb 14, 2022

[DataGrid] Horizontal Scrollbar disappears when NoResultsOverlay is used and no results in filter #3795

Closed

2 tasks

Merge branch 'master' into custom-overlays

6bac367

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 17, 2022

m4theushw self-requested a review February 18, 2022 14:44

Merge branch 'master' into custom-overlays

f13a620

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 18, 2022

fix failed merge

09d049a

m4theushw reviewed Feb 21, 2022

View reviewed changes

cherniavskii added 5 commits February 21, 2022 18:15

remove needless styles

e8d5dc3

move GridOverlaysRoot to GridOverlays

6eebb7d

rename GridOverlaysRoot

51b9336

fix failing test

c6c46ec

move GridOverlayWrapper to separate file

16c3940

cherniavskii requested a review from m4theushw February 22, 2022 14:22

m4theushw reviewed Feb 23, 2022

View reviewed changes

cherniavskii added 4 commits February 23, 2022 20:28

Merge branch 'master' into custom-overlays

8b6946a

remove unnecessary style

3c31d73

return null if there's no overlay

7aeb191

apply bottom value conditionally

c2c046e

cherniavskii requested a review from m4theushw February 23, 2022 20:16

flaviendelangle mentioned this pull request Feb 24, 2022

[DataGrid] Starting v5.3.0 till latest version components ErrorOverlay gets wrapped into div with 0px x 0px size #4026

Closed

2 tasks

cherniavskii commented Feb 24, 2022

View reviewed changes

cherniavskii added 2 commits February 24, 2022 11:19

make ErrorOverlay to take full width and height of the grid

4d3ea29

move GridOverlayWrapper to GridOverlays

0c8d1cd

m4theushw approved these changes Feb 24, 2022

View reviewed changes

cherniavskii merged commit 0f09f20 into mui:master Feb 25, 2022

cherniavskii deleted the custom-overlays branch February 25, 2022 06:02

cherniavskii mentioned this pull request Mar 5, 2022

[DataGrid] ErrorOverlay not visible with autoHeight #4088

Closed

2 tasks

m4theushw mentioned this pull request Mar 7, 2022

[DataGrid] Fix error overlay not visible when autoHeight is enabled #4110

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Improve custom overlay slots positioning #3832

[DataGrid] Improve custom overlay slots positioning #3832

cherniavskii commented Feb 3, 2022 •

edited

Loading

cherniavskii Feb 3, 2022

mui-bot commented Feb 3, 2022 •

edited

Loading

cherniavskii Feb 3, 2022

This comment was marked as resolved.

This comment was marked as resolved.

github-actions bot commented Feb 17, 2022

m4theushw left a comment

m4theushw Feb 21, 2022

cherniavskii Feb 22, 2022

m4theushw Feb 21, 2022

cherniavskii Feb 21, 2022

cherniavskii Feb 22, 2022

m4theushw Feb 23, 2022

cherniavskii Feb 23, 2022

m4theushw Feb 23, 2022

cherniavskii Feb 23, 2022

cherniavskii Feb 24, 2022

cherniavskii Feb 24, 2022

cherniavskii Feb 24, 2022

m4theushw left a comment

[DataGrid] Improve custom overlay slots positioning #3832

[DataGrid] Improve custom overlay slots positioning #3832

Conversation

cherniavskii commented Feb 3, 2022 • edited Loading

Choose a reason for hiding this comment

mui-bot commented Feb 3, 2022 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

github-actions bot commented Feb 17, 2022

m4theushw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m4theushw left a comment

Choose a reason for hiding this comment

cherniavskii commented Feb 3, 2022 •

edited

Loading

mui-bot commented Feb 3, 2022 •

edited

Loading