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

[DataGrid] autoHeight has zero height #604

Closed
2 tasks done
henrikq opened this issue Nov 19, 2020 · 7 comments · Fixed by #940
Closed
2 tasks done

[DataGrid] autoHeight has zero height #604

henrikq opened this issue Nov 19, 2020 · 7 comments · Fixed by #940
Assignees
Labels
component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request
Milestone

Comments

@henrikq
Copy link

henrikq commented Nov 19, 2020

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

<DataGrid autoHeight {...stuff} /> has style="height: 0px; .... Therefore, siblings after data-grid overlap/overflow, e.g. in

<div>
    <DataGrid autoHeight {...stuff} />
    <p style={{ backgroundColor: "red" }}>after DataGrid</p>
<div>

the paragraph element and DataGrid dom overlap:

image

Expected Behavior 🤔

The paragraph should be appear after the DataGrid, with no overlap.

Steps to Reproduce 🕹

https://codesandbox.io/s/datagrid-autosize-bug-6uhmp?file=/demo.js

Context 🔦

I need DataGrid to have height, such that the parent element (Paper) resizes accordingly.

Your Environment 🌎

image

Tech Version
Material-UI v4.11.0
React
Browser Chrome
TypeScript
etc.
@oliviertassinari oliviertassinari transferred this issue from mui/material-ui Nov 19, 2020
@oliviertassinari oliviertassinari added component: data grid This is the name of the generic UI component, not the React module! discussion new feature New feature or request and removed discussion labels Nov 19, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 19, 2020

@henrikq Thanks for raising. I agree, I think that it's an opportunity to start moving the AutoSizer deeper in the rendering tree. What about this change?:

diff --git a/packages/grid/_modules_/grid/GridComponent.tsx b/packages/grid/_modules_/grid/GridComponent.tsx
index 84562673..2ebccd1b 100644
--- a/packages/grid/_modules_/grid/GridComponent.tsx
+++ b/packages/grid/_modules_/grid/GridComponent.tsx
@@ -39,6 +39,7 @@ import { useResizeContainer } from './hooks/utils/useResizeContainer';
 import { useVirtualRows } from './hooks/features/virtualization/useVirtualRows';
 import { RootContainerRef } from './models/rootContainerRef';
 import { getCurryTotalHeight } from './utils/getTotalHeight';
+import { classnames } from './utils';
 import { ApiContext } from './components/api-context';
 import { OptionsContext } from './components/options-context';
 import { RenderContext } from './components/render-context';
@@ -103,19 +104,19 @@ export const GridComponent = React.forwardRef<HTMLDivElement, GridComponentProps
     );

     return (
+      <GridRoot
+        className={classnames('MuiDataGrid-root', props.className)}
+        ref={handleRef}
+        role="grid"
+        aria-colcount={gridState.columns.visible.length}
+        aria-rowcount={gridState.rows.totalRowCount}
+        tabIndex={0}
+        aria-label="grid"
+        aria-multiselectable={!gridState.options.disableMultipleSelection}
+      >
       <AutoSizer onResize={onResize}>
         {(size: any) => (
-          <GridRoot
-            ref={handleRef}
-            className={props.className}
-            style={{ width: size.width, height: getTotalHeight(size) }}
-            role="grid"
-            aria-colcount={gridState.columns.visible.length}
-            aria-rowcount={gridState.rows.totalRowCount}
-            tabIndex={0}
-            aria-label="grid"
-            aria-multiselectable={!gridState.options.disableMultipleSelection}
-          >
+          <div className="MuiDataGrid-sized" style={{ width: size.width, height: getTotalHeight(size) }}>
             <ErrorBoundary
               hasError={errorState != null}
               componentProps={errorState}
@@ -193,9 +194,10 @@ export const GridComponent = React.forwardRef<HTMLDivElement, GridComponentProps
                 </OptionsContext.Provider>
               </ApiContext.Provider>
             </ErrorBoundary>
-          </GridRoot>
+          </div>
         )}
       </AutoSizer>
+      </GridRoot>
     );
   },
 );
diff --git a/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts b/packages/grid/_modules_/grid/components/styled-wrappers/Gri
dRootStyles.ts
index 7b97dcb4..0f18560a 100644
--- a/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts
+++ b/packages/grid/_modules_/grid/components/styled-wrappers/GridRootStyles.ts
@@ -9,19 +9,23 @@ export const useStyles = makeStyles(

     const gridStyle: { root: any } = {
       root: {
-        flex: 1,
         boxSizing: 'border-box',
-        position: 'relative',
-        border: `1px solid ${borderColor}`,
-        borderRadius: theme.shape.borderRadius,
+        width: '100%',
+        height: '100%',
         color: theme.palette.text.primary,
         ...theme.typography.body2,
         outline: 'none',
-        display: 'flex',
-        flexDirection: 'column',
         '& *, & *::before, & *::after': {
           boxSizing: 'inherit',
         },
+        '& .MuiDataGrid-sized': {
+          border: `1px solid ${borderColor}`,
+          borderRadius: theme.shape.borderRadius,
+          flex: 1,
+          position: 'relative',
+          display: 'flex',
+          flexDirection: 'column',
+        },
         '& .MuiDataGrid-mainGridContainer': {
           position: 'relative',
           flexGrow: 1,

It would move us a step closer to the architecture of ag-grid, and solve your issue: https://codesandbox.io/s/datagrid-autosize-bug-forked-ixtgo?file=/demo.js, at least, make it more obvious. It also gets us a step closer to the convention we have on the main repository, ref on the root & class name on the root element.

@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Nov 19, 2020
@oliviertassinari oliviertassinari changed the title DataGrid autoSize has zero height [DataGrid] autoSize has zero height Nov 19, 2020
@henrikq
Copy link
Author

henrikq commented Nov 20, 2020

@oliviertassinari Thanks for investigating.

I don't know if you are asking me. I'm not familiar with the implementation, so can't tell if the suggested approach is good. On the surface it appears reasonable assuming getTotalHeight is reliable.

That said, i took a look at the codesandbox you posted and it appears to not function perfectly yet
image
as it seems the distance between the DataGrid and paragraph is much to large.

@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 20, 2020

@henrikq The gap is expected. The grid relies on virtualization, it grows to take all the space available given by its container, you can't share the container.

@henrikq
Copy link
Author

henrikq commented Nov 20, 2020

@oliviertassinari Thanks for the prompt reply.

I took another look at the sandbox you provided. I found that the containing div is sized by a constant

    <div style={{ height: 400, width: "100%" }}>
      <div style={{ width: "100%", height: "100%" }}>
        <DataGrid ...

If we increase the number of rows in the DataGrid, then i would expect the <p> to overlap again.

How am i suppose to create a div containing DataGrid, such that the div is of the correct height to contain the DataGrid without overflow and without space between where the DataGrid visually ends and the div ends, as the number of rows change?

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! and removed component: data grid This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted new feature New feature or request labels Nov 20, 2020
@oliviertassinari
Copy link
Member

@henrikq Yeah ok, you are right, the grid should have an intrinsic height when autoHeight is used.

@oliviertassinari oliviertassinari changed the title [DataGrid] autoSize has zero height [DataGrid] autoHeight has zero height Nov 23, 2020
dtassone added a commit to dtassone/material-ui-x that referenced this issue Nov 23, 2020
@maksimf
Copy link

maksimf commented Jan 7, 2021

Is there any plans to resolve this any time soon?

@Coridyn
Copy link

Coridyn commented Jan 8, 2021

In case it's useful for other people, here is a workaround I've been using to clear the height: 0px style from the rendered DataGrid element.

    import * as react from React;
    import { DataGrid } from '@material-ui/data-grid';
    
    const MuiDataGrid = (props) => {
        /*
        2021-01-08
        Work around MUI DataGrid issue that sets `height: 0px;` when autoHeight is enabled 
        https://github.com/mui-org/material-ui-x/issues/604
        
        Get the first div (which is the MUI datagrid element) and clear the 0px CSS height style
        */
        const gridWrapperRef = react.useRef<HTMLDivElement>(null);
        react.useLayoutEffect(() => {
            const gridDiv = gridWrapperRef.current;
            if (gridDiv){
                const gridEl: HTMLDivElement = gridDiv.querySelector('div')!;
                gridEl.style.height = '';
            }
        });
        
        return (
            <div ref={gridWrapperRef}>
                <DataGrid
                    rows={props.rows}
                    columns={props.columns}
                    autoHeight={true}
                />
            </div>
        );
    }

buckethead1986 added a commit to buckethead1986/star-wars-hot-wheels that referenced this issue Jan 14, 2021
@oliviertassinari oliviertassinari added new feature New feature or request and removed bug 🐛 Something doesn't work labels Jan 18, 2021
@oliviertassinari oliviertassinari added this to the Sprint 9 milestone Jan 18, 2021
@DanailH DanailH self-assigned this Jan 19, 2021
@oliviertassinari oliviertassinari modified the milestones: Sprint 9, Sprint 10 Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
5 participants