[DataGrid] Fix flex columns and resize #2110

dtassone · 2021-07-09T10:50:11Z

follow up from #2104
fix #2011
fix #2103

Preview: https://deploy-preview-2110--material-ui-x.netlify.app/components/data-grid/columns/#fluid-width

oliviertassinari

We miss a test case
There is a CSS regression, see argos once it has finish running

oliviertassinari · 2021-07-09T11:13:29Z

packages/storybook/src/stories/grid-columns.stories.tsx

+export function FlexLayoutGrid() {
+  const { data } = useDemoData({
+    dataSet: 'Commodity',
+    rowLength: 15,
+    maxColumns: 6,
+  });
+
+  const tmpData = JSON.parse(JSON.stringify(data));
+  tmpData.columns.forEach((item) => {
+    item.flex = 1;
+  });
+  return (
+    <div style={{ height: '400px', width: '400px' }}>
+      <div style={{ display: 'flex', height: '100%' }}>
+        <div style={{ flexGrow: 1 }}>
+          <XGrid
+            autoHeight
+            showCellRightBorder
+            showColumnRightBorder
+            disableExtendRowFullWidth
+            rows={tmpData.rows}
+            columns={tmpData.columns}
+          />
+        </div>
+      </div>
+    </div>
+  );
+}


We can already reproduce the issue in the docs. What's the use case for the story?

Suggested change

export function FlexLayoutGrid() {

const { data } = useDemoData({

dataSet: 'Commodity',

rowLength: 15,

maxColumns: 6,

});

const tmpData = JSON.parse(JSON.stringify(data));

tmpData.columns.forEach((item) => {

item.flex = 1;

});

return (

<div style={{ height: '400px', width: '400px' }}>

<div style={{ display: 'flex', height: '100%' }}>

<div style={{ flexGrow: 1 }}>

<XGrid

autoHeight

showCellRightBorder

showColumnRightBorder

disableExtendRowFullWidth

rows={tmpData.rows}

columns={tmpData.columns}

/>

</div>

</div>

</div>

);

}

working on flex columns, and see if it works. Where is #2011 in the doc?

Ok, could we make it a Snap story in this case? This could complement the function test to make sure the rendering with sub-pixels is still correct

oliviertassinari · 2021-07-09T11:13:45Z

packages/grid/_modules_/grid/components/cell/GridEmptyCell.tsx

@@ -22,6 +22,7 @@ export const GridEmptyCell = React.memo(function GridEmptyCell({
        lineHeight: `${height - 1}px`,
        minHeight: height,
        maxHeight: height,
+        padding: 0,


Do we need?

Suggested change

padding: 0,

The issue is that the cell always have a padding of 10px
But we do not want it on the GridEmptyCell.

Maybe we should add a class to this cell and set the padding to 0 in the rootStyle instead of here.

dtassone · 2021-07-09T11:47:19Z

We miss a test case

There is a CSS regression, see argos once it has finish running

Yes I moved the separator to avoid negative margin that creates issue on resize

m4theushw · 2021-07-09T12:27:37Z

About #2011, the double border is still there.

The separator is being displayed when showColumnRightBorder is used.

dtassone · 2021-07-09T12:40:24Z

About #2011, the double border is still there.

A separate issue that can be fixed in another PR. We need to keep PR small

m4theushw · 2021-07-09T13:40:06Z

I'm confused, we have this PR and #2104 both trying to fix #2103.

About the solution proposed to fix it here, it's still trying to scroll, but since we removed the padding from the empty cell it can't. We're not fixing the root cause #2103 (comment). To reproduce, use fixed size columns.

siTztA0h5p.mp4

flaviendelangle · 2021-07-09T13:42:11Z

@m4theushw for me this PR is a follow up on #2104 because there are several issues to be fixed around the column width.

The scroll event should be fixed here #2104 (comment)

dtassone · 2021-07-09T13:46:55Z

the separator creates an extra 10px on the columns header due to its positioning. The problem is the icon shape that is a square 24x24 instead of just a bar 24x2 which would be easier to position

dtassone · 2021-07-12T10:13:01Z

I managed to fix the icon and put the bar on the right side of the icon so we can position it properly without using negative margin. let me know what you think

oliviertassinari

Please isolate the fix of #2011 and #2103. Looking at the changes, here are no reason to bundle them.

oliviertassinari · 2021-07-17T17:15:39Z

packages/storybook/src/stories/grid-columns.stories.tsx

+export function FlexLayoutGrid() {
+  const { data } = useDemoData({
+    dataSet: 'Commodity',
+    rowLength: 15,
+    maxColumns: 6,
+  });
+
+  const tmpData = JSON.parse(JSON.stringify(data));
+  tmpData.columns.forEach((item) => {
+    item.flex = 1;
+  });
+  return (
+    <div style={{ height: '400px', width: '400px' }}>
+      <div style={{ display: 'flex', height: '100%' }}>
+        <div style={{ flexGrow: 1 }}>
+          <XGrid
+            autoHeight
+            showCellRightBorder
+            showColumnRightBorder
+            disableExtendRowFullWidth
+            rows={tmpData.rows}
+            columns={tmpData.columns}
+          />
+        </div>
+      </div>
+    </div>
+  );
+}


Ok, could we make it a Snap story in this case? This could complement the function test to make sure the rendering with sub-pixels is still correct

oliviertassinari · 2021-07-24T20:46:51Z

We can resume the effort later on

fix flex columns and resize

16b34d5

dtassone requested a review from a team July 9, 2021 10:53

oliviertassinari requested changes Jul 9, 2021

View reviewed changes

oliviertassinari changed the title ~~[DataGrid] fix flex columns and resize~~ [DataGrid] Fix flex columns and resize Jul 9, 2021

oliviertassinari requested a review from a team July 9, 2021 11:14

dtassone mentioned this pull request Jul 9, 2021

[DataGrid] Column header is moving when keyboard arrow keys are pressed #2103

Closed

2 tasks

add test

f074658

fix separator position and icon

54c22dc

dtassone added 2 commits July 12, 2021 12:22

fix margin and menu click

6e53a2e

fix padding on title container

4a81695

flaviendelangle mentioned this pull request Jul 12, 2021

[DataGrid] Avoid trying to scroll right when focusing a cell in the last column if 100% in viewport #2104

Closed

oliviertassinari requested changes Jul 17, 2021

View reviewed changes

dtassone mentioned this pull request Jul 23, 2021

[DataGrid] Won't be merged - Do not loose the resized columns width when running the flex allocation #2181

Closed

oliviertassinari closed this Jul 24, 2021

oliviertassinari mentioned this pull request Jul 24, 2021

[DataGrid] Gap after the last column when all the column widths are set to fluid #2011

Closed

2 tasks

oliviertassinari added the component: data grid This is the name of the generic UI component, not the React module! label Jan 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Fix flex columns and resize #2110

[DataGrid] Fix flex columns and resize #2110

dtassone commented Jul 9, 2021 •

edited by oliviertassinari

Loading

oliviertassinari left a comment

oliviertassinari Jul 9, 2021

dtassone Jul 9, 2021

oliviertassinari Jul 17, 2021

oliviertassinari Jul 9, 2021 •

edited

Loading

flaviendelangle Jul 9, 2021

dtassone Jul 9, 2021

dtassone commented Jul 9, 2021

m4theushw commented Jul 9, 2021

dtassone commented Jul 9, 2021

m4theushw commented Jul 9, 2021

flaviendelangle commented Jul 9, 2021 •

edited

Loading

dtassone commented Jul 9, 2021

dtassone commented Jul 12, 2021

oliviertassinari left a comment

oliviertassinari Jul 17, 2021

oliviertassinari commented Jul 24, 2021

[DataGrid] Fix flex columns and resize #2110

[DataGrid] Fix flex columns and resize #2110

Conversation

dtassone commented Jul 9, 2021 • edited by oliviertassinari Loading

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari Jul 9, 2021

Choose a reason for hiding this comment

dtassone Jul 9, 2021

Choose a reason for hiding this comment

oliviertassinari Jul 17, 2021

Choose a reason for hiding this comment

oliviertassinari Jul 9, 2021 • edited Loading

Choose a reason for hiding this comment

flaviendelangle Jul 9, 2021

Choose a reason for hiding this comment

dtassone Jul 9, 2021

Choose a reason for hiding this comment

dtassone commented Jul 9, 2021

m4theushw commented Jul 9, 2021

dtassone commented Jul 9, 2021

m4theushw commented Jul 9, 2021

flaviendelangle commented Jul 9, 2021 • edited Loading

dtassone commented Jul 9, 2021

dtassone commented Jul 12, 2021

oliviertassinari left a comment

Choose a reason for hiding this comment

oliviertassinari Jul 17, 2021

Choose a reason for hiding this comment

oliviertassinari commented Jul 24, 2021

dtassone commented Jul 9, 2021 •

edited by oliviertassinari

Loading

oliviertassinari Jul 9, 2021 •

edited

Loading

flaviendelangle commented Jul 9, 2021 •

edited

Loading