[DataGrid] Rework column headers and virtual scroller positioning

[cherniavskii]

Closes #602

TODO:

• Cherry-pick [DataGrid] Rework column headers and virtual scroller positioning #7001 (comment) to the master branch: [DataGridPro] Fix missing column headers border with top-pinned rows #7399

Changelog

Breaking changes

• The headerHeight prop was renamed to columnHeaderHeight

[mui-bot]

	Messages
📖	Netlify deploy preview: https://deploy-preview-7001--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case	Unit	Min	Max	Median	Mean	σ
Filter 100k rows	ms	600.2	1,078	643.3	781.82	167.444
Sort 100k rows	ms	585.4	1,164.9	821.8	888.32	188.38
Select 100k rows	ms	208.3	296.1	231.7	241.14	30.567
Deselect 100k rows	ms	176.4	367.5	202.4	227.1	71.111

Generated by 🚫 dangerJS against 07b9cf3

[m4theushw]

I checked the other regressions and it seems that the demo is wrong and this PR uncovered this problem. Before, with position: absolute, it appeared to behave correctly because the dimensions of the absolute positioned element were not taken into account.

diff --git a/docs/data/data-grid/layout/FlexLayoutGrid.js b/docs/data/data-grid/layout/FlexLayoutGrid.js
index 807b56922..647ceed4a 100644
--- a/docs/data/data-grid/layout/FlexLayoutGrid.js
+++ b/docs/data/data-grid/layout/FlexLayoutGrid.js
@@ -11,7 +11,7 @@ export default function FlexLayoutGrid() {
 
   return (
     <div style={{ height: 400, width: '100%' }}>
-      <div style={{ display: 'flex', height: '100%' }}>
+      <div style={{ display: 'flex', flexDirection: 'column', height: '100%' }}>
         <div style={{ flexGrow: 1 }}>
           <DataGrid {...data} />
         </div>

diff --git a/docs/data/data-grid/row-height/ExpandableCells.js b/docs/data/data-grid/row-height/ExpandableCells.js
index 919ac9ff1..5df89012c 100644
--- a/docs/data/data-grid/row-height/ExpandableCells.js
+++ b/docs/data/data-grid/row-height/ExpandableCells.js
@@ -84,7 +84,7 @@ for (let i = 0; i < 10; i += 1) {
 
 export default function ExpandableCells() {
   return (
-    <div style={{ height: 400, width: 800 }}>
+    <div style={{ height: 400, width: '100%' }}>
       <DataGrid
         rows={rows}
         columns={columns}

The demo above tries to use width: 800px but the test viewer has max-width: 500px.

[m4theushw]

https://app.argos-ci.com/mui/mui-x/builds/6190/29684886 can be fixed with

Suggested change

	<div ref={handleRef} style={{ flex: 1, overflow: 'auto', ...style }} {...other}>
	<div ref={handleRef} style={{ flex: disableHeight ? 0 : '1 1 0px', overflow: 'auto', ...style }} {...other}>

The reason for flex: 0 is because when auto height is enabled we don't want the content to shrink.

[cherniavskii]

Thanks for your help!

[cherniavskii]

This is a bugfix actually, you can see that in v5 the column header border is not visible when you have pinned rows:
https://codesandbox.io/s/strange-sea-6ul544?file=/demo.tsx
You cannot reproduce this in the same demo in the docs because in the docs we use CssBaseline which sets * { box-sizing: border-box }.

I didn't want to extract this into a separate PR to save time.
I'll cherry-pick this change to master once this PR is merged.

[cherniavskii]

Alright, I've finally managed to fix the regressions caused by the layout change 🎉

There are only 3 regressions left in https://app.argos-ci.com/mui/mui-x/builds/6762/31501643:

All 3 are caused by height rounding that we did before.
Before this PR, the virtual scroller had the explicit height set and the value was rounded to a whole pixel:

Now the virtual scroller uses the available space and sometimes it may have subpixel value:

So I believe we should accept the screenshot changes since this is now the expected behavior 👍

[cherniavskii]

I really like this change!

[cherniavskii]

I wonder what is the point of this demo.
There are 3 wrapper divs, 2 of which seem to be redundant since it's equivalent of

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

What are we trying to cover in this section? https://mui.com/x/react-data-grid/layout/#flex-layout

[cherniavskii]

@m4theushw could you take a look at this one? It's off-topic, but it's not clear for me what's the intention of that demo

[m4theushw]

I don't know why it has this structure. It was created already that way: https://github.com/mui/mui-x/pull/267/files#diff-b4e854d1aab47be29ed3d21847dbed928f8d36d3cafa0162b2ae35293fb9d6da

I suppose the outermost div (the one with height: 400px) was added first to constrain the height so it doesn't extend the full page height. In a real scenario this div wouldn't be needed. Then, the intermediary div (the one with display: flex) was added is to simulate how flex layout is used in real applications. I think the outermost and the intermediary divs can be merged, and height: 100% -> height: 400px.

[oliviertassinari]

If I recall correctly, I used https://ag-grid.com/javascript-data-grid/grid-size/#inside-flexbox-container as inspiration for this demo. But in hindsight, it feels that developers learn nothing from https://next.mui.com/x/react-data-grid/layout/#flex-layout. It could be good to delete this h2 section. This would make more sense

but really? Do we need to teach developer CSS Grid/Flexbox?

[cherniavskii]

Thanks, @oliviertassinari
Few minor concerns I also had about the flex demo:

1. Developers might think that a flex container is required for the grid to work (especially on a quick look at the docs, since it's the first demo on the page).
2. Developers might copy the whole snippet with 2 nested divs, which is ... unnecessarily complicated 🤷🏻‍♂️

but really? Do we need to teach developer CSS Grid/Flexbox?

I think we can remove the whole section. The main requirement for the parent container is covered in the first few sentences anyway

[oliviertassinari]

It sounds like this is fixed now 👍 https://stackblitz.com/edit/react-8ahs3n?file=src%2FApp.js

https://mui-org.slack.com/archives/C041SDSF32L/p1674915288809749