From d4aedbbbcba93d6b70ef6d4d562d1be2be246110 Mon Sep 17 00:00:00 2001 From: Andrew Cherniavskyi Date: Mon, 14 Oct 2024 23:18:26 +0200 Subject: [PATCH 01/10] make filler grow and fill available space --- .../x-data-grid/src/components/containers/GridRootStyles.ts | 2 +- .../src/components/virtualization/GridVirtualScroller.tsx | 2 ++ .../hooks/features/virtualization/useGridVirtualScroller.tsx | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/x-data-grid/src/components/containers/GridRootStyles.ts b/packages/x-data-grid/src/components/containers/GridRootStyles.ts index f66edba975eac..75cc663e29944 100644 --- a/packages/x-data-grid/src/components/containers/GridRootStyles.ts +++ b/packages/x-data-grid/src/components/containers/GridRootStyles.ts @@ -742,7 +742,7 @@ export const GridRootStyles = styled('div', { }, [`& .${c.filler}`]: { - flex: 1, + flexGrow: 1, }, [`& .${c['filler--borderBottom']}`]: { borderBottom: '1px solid var(--DataGrid-rowBorderColor)', diff --git a/packages/x-data-grid/src/components/virtualization/GridVirtualScroller.tsx b/packages/x-data-grid/src/components/virtualization/GridVirtualScroller.tsx index 109bd7fbb879f..82296b07365fb 100644 --- a/packages/x-data-grid/src/components/virtualization/GridVirtualScroller.tsx +++ b/packages/x-data-grid/src/components/virtualization/GridVirtualScroller.tsx @@ -51,6 +51,8 @@ const Scroller = styled('div', { flexGrow: 1, overflow: 'scroll', scrollbarWidth: 'none' /* Firefox */, + display: 'flex', + flexDirection: 'column', '&::-webkit-scrollbar': { display: 'none' /* Safari and Chrome */, }, diff --git a/packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx b/packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx index f72c1896480c7..d17e8a8779610 100644 --- a/packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx +++ b/packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx @@ -524,7 +524,8 @@ export const useGridVirtualScroller = () => { const contentSize = React.useMemo(() => { const size: React.CSSProperties = { width: needsHorizontalScrollbar ? columnsTotalWidth : 'auto', - height: contentHeight, + flexBasis: contentHeight, + flexShrink: 0, }; if (rootProps.autoHeight && currentPage.rows.length === 0) { From 41ca1d5f74d31cacfa390957f878033b452848b4 Mon Sep 17 00:00:00 2001 From: Andrew Cherniavskyi Date: Mon, 14 Oct 2024 23:18:34 +0200 Subject: [PATCH 02/10] fix overlay height --- packages/x-data-grid/src/components/base/GridOverlays.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/x-data-grid/src/components/base/GridOverlays.tsx b/packages/x-data-grid/src/components/base/GridOverlays.tsx index e21561bcdf273..d3e9fb2bd6915 100644 --- a/packages/x-data-grid/src/components/base/GridOverlays.tsx +++ b/packages/x-data-grid/src/components/base/GridOverlays.tsx @@ -37,6 +37,7 @@ const GridOverlayWrapperRoot = styled('div', { overlayType === 'loadingOverlay' ? 5 // Should be above pinned columns, pinned rows, and detail panel : 4, // Should be above pinned columns and detail panel + flex: 1, } : {}, ); From 9c21841fe5f8b19daa18132f9ad31451c9ca5a24 Mon Sep 17 00:00:00 2001 From: Andrew Cherniavskyi Date: Wed, 23 Oct 2024 00:36:27 +0200 Subject: [PATCH 03/10] remove unnecessary offset for bottom container --- .../src/components/base/GridOverlays.tsx | 1 - .../virtualization/GridBottomContainer.tsx | 18 ------------------ 2 files changed, 19 deletions(-) diff --git a/packages/x-data-grid/src/components/base/GridOverlays.tsx b/packages/x-data-grid/src/components/base/GridOverlays.tsx index 0679c1cb5eb34..79baf162939db 100644 --- a/packages/x-data-grid/src/components/base/GridOverlays.tsx +++ b/packages/x-data-grid/src/components/base/GridOverlays.tsx @@ -37,7 +37,6 @@ const GridOverlayWrapperRoot = styled('div', { overlayType === 'loadingOverlay' ? 5 // Should be above pinned columns, pinned rows, and detail panel : 4, // Should be above pinned columns and detail panel - flex: 1, } : {}, ); diff --git a/packages/x-data-grid/src/components/virtualization/GridBottomContainer.tsx b/packages/x-data-grid/src/components/virtualization/GridBottomContainer.tsx index ca0c0d5179eb3..cc2a9c8c1789f 100644 --- a/packages/x-data-grid/src/components/virtualization/GridBottomContainer.tsx +++ b/packages/x-data-grid/src/components/virtualization/GridBottomContainer.tsx @@ -3,9 +3,6 @@ import clsx from 'clsx'; import { styled } from '@mui/system'; import composeClasses from '@mui/utils/composeClasses'; import { gridClasses, getDataGridUtilityClass } from '../../constants/gridClasses'; -import { gridDimensionsSelector } from '../../hooks/features/dimensions/gridDimensionsSelectors'; -import { useGridApiContext } from '../../hooks/utils/useGridApiContext'; -import { useGridSelector } from '../../hooks/utils/useGridSelector'; const useUtilityClasses = () => { const slots = { @@ -23,25 +20,10 @@ const Element = styled('div')({ export function GridBottomContainer(props: React.PropsWithChildren) { const classes = useUtilityClasses(); - const apiRef = useGridApiContext(); - const { viewportOuterSize, minimumSize, hasScrollX, scrollbarSize } = useGridSelector( - apiRef, - gridDimensionsSelector, - ); - const scrollHeight = hasScrollX ? scrollbarSize : 0; - const offset = Math.max( - viewportOuterSize.height - - minimumSize.height - - // Subtract scroll height twice to account for GridVirtualScrollerFiller and horizontal scrollbar - 2 * scrollHeight, - 0, - ); - return ( ); From 4a982cfd624946c91478d532b67e27ba7f0c6553 Mon Sep 17 00:00:00 2001 From: Andrew Cherniavskyi Date: Wed, 23 Oct 2024 00:38:57 +0200 Subject: [PATCH 04/10] add regression demo --- .../data-grid/DataGridPinnedColumns.js | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 test/regressions/data-grid/DataGridPinnedColumns.js diff --git a/test/regressions/data-grid/DataGridPinnedColumns.js b/test/regressions/data-grid/DataGridPinnedColumns.js new file mode 100644 index 0000000000000..743f70a7386f8 --- /dev/null +++ b/test/regressions/data-grid/DataGridPinnedColumns.js @@ -0,0 +1,62 @@ +import * as React from 'react'; +import DeleteIcon from '@mui/icons-material/Delete'; +import EditIcon from '@mui/icons-material/Edit'; +import { DataGridPro, GridActionsCellItem } from '@mui/x-data-grid-pro'; +import { + randomCreatedDate, + randomTraderName, + randomEmail, + randomUpdatedDate, +} from '@mui/x-data-grid-generator'; + +const columns = [ + { field: 'name', headerName: 'Name', width: 160, editable: true }, + { field: 'email', headerName: 'Email', width: 200, editable: true }, + { field: 'age', headerName: 'Age', type: 'number', editable: true }, + { + field: 'dateCreated', + headerName: 'Date Created', + type: 'date', + width: 180, + editable: true, + }, + { + field: 'lastLogin', + headerName: 'Last Login', + type: 'dateTime', + width: 220, + editable: true, + }, + { + field: 'actions', + type: 'actions', + width: 100, + getActions: () => [ + } label="Edit" />, + } label="Delete" />, + ], + }, +]; + +const rows = [ + { + id: 1, + name: randomTraderName(), + email: randomEmail(), + age: 25, + dateCreated: randomCreatedDate(), + lastLogin: randomUpdatedDate(), + }, +]; + +export default function BasicColumnPinning() { + return ( +
+ +
+ ); +} From 7f97a3ded10ab8f77b413763ed0767700b5d34de Mon Sep 17 00:00:00 2001 From: Andrew Cherniavskyi Date: Wed, 23 Oct 2024 00:55:58 +0200 Subject: [PATCH 05/10] fix unit tests --- .../src/tests/detailPanel.DataGridPro.test.tsx | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/x-data-grid-pro/src/tests/detailPanel.DataGridPro.test.tsx b/packages/x-data-grid-pro/src/tests/detailPanel.DataGridPro.test.tsx index 1f71824e507fa..b8e38ff9df4a6 100644 --- a/packages/x-data-grid-pro/src/tests/detailPanel.DataGridPro.test.tsx +++ b/packages/x-data-grid-pro/src/tests/detailPanel.DataGridPro.test.tsx @@ -86,10 +86,10 @@ describe(' - Detail panel', () => { await microtasks(); const virtualScrollerContent = $('.MuiDataGrid-virtualScrollerContent')!; - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${rowHeight + detailPanelHeight}px`, }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); const detailPanels = $$('.MuiDataGrid-detailPanel'); expect(detailPanels[0]).toHaveComputedStyle({ @@ -128,11 +128,9 @@ describe(' - Detail panel', () => { }); await waitFor(() => { - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', - height: `${rowHeight + 100}px`, - }); + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${rowHeight + 100}px` }); }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); const detailPanels = $$('.MuiDataGrid-detailPanel'); expect(detailPanels[0]).toHaveComputedStyle({ @@ -142,11 +140,9 @@ describe(' - Detail panel', () => { fireEvent.click(screen.getByRole('button', { name: 'Increase' })); await waitFor(() => { - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', - height: `${rowHeight + 200}px`, - }); + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${rowHeight + 200}px` }); }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); expect(detailPanels[0]).toHaveComputedStyle({ height: `200px`, From 7d2224c73c4113d71c6513435cc1bf31c7e649a0 Mon Sep 17 00:00:00 2001 From: Andrew Cherniavskyi Date: Wed, 23 Oct 2024 01:03:48 +0200 Subject: [PATCH 06/10] fix more unit tests --- .../src/tests/rows.DataGrid.test.tsx | 50 +++++++------------ 1 file changed, 18 insertions(+), 32 deletions(-) diff --git a/packages/x-data-grid/src/tests/rows.DataGrid.test.tsx b/packages/x-data-grid/src/tests/rows.DataGrid.test.tsx index 2efc26c7c9c34..f4dbb00e16b1a 100644 --- a/packages/x-data-grid/src/tests/rows.DataGrid.test.tsx +++ b/packages/x-data-grid/src/tests/rows.DataGrid.test.tsx @@ -645,11 +645,9 @@ describe(' - Rows', () => { ); const expectedHeight = baselineProps.rows.length * (contentHeight + border); await waitFor(() => { - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', - height: `${expectedHeight}px`, - }); + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${expectedHeight}px` }); }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); }); it('should use the default row height to calculate the content size when the row has not been measured yet', async () => { @@ -674,11 +672,9 @@ describe(' - Rows', () => { border + // Measured rows also include the border (baselineProps.rows.length - 1) * defaultRowHeight; await waitFor(() => { - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', - height: `${expectedHeight}px`, - }); + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${expectedHeight}px` }); }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); }); it('should use the value from getEstimatedRowHeight to estimate the content size', async () => { @@ -703,11 +699,9 @@ describe(' - Rows', () => { const expectedHeight = firstRowHeight + (baselineProps.rows.length - 1) * estimatedRowHeight; await waitFor(() => { - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', - height: `${expectedHeight}px`, - }); + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${expectedHeight}px` }); }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); }); it('should recalculate the content size when the rows prop changes', async () => { @@ -723,18 +717,14 @@ describe(' - Rows', () => { '.MuiDataGrid-virtualScrollerContent', ); await waitFor(() => { - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', - height: '101px', - }); + expect(virtualScrollerContent).toHaveComputedStyle({ height: '101px' }); }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); setProps({ rows: [{ clientId: 'c1', expanded: true }] }); await waitFor(() => { - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', - height: '201px', - }); + expect(virtualScrollerContent).toHaveComputedStyle({ height: '201px' }); }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); }); it('should set minHeight to "auto" in all rows with dynamic row height', () => { @@ -813,11 +803,11 @@ describe(' - Rows', () => { '.MuiDataGrid-virtualScrollerContent', )!; await waitFor(() => { - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${Math.floor(expectedHeight)}px`, }); }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); }); it('should position correctly the render zone when the 2nd page has less rows than the 1st page', async function test() { @@ -981,10 +971,8 @@ describe(' - Rows', () => { ); const virtualScrollerContent = document.querySelector('.MuiDataGrid-virtualScrollerContent'); const expectedHeight = rows.length * (rowHeight + spacingTop + spacingBottom); - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', - height: `${expectedHeight}px`, - }); + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${expectedHeight}px` }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); }); it('should update the content size when getRowSpacing is removed', () => { @@ -1000,15 +988,13 @@ describe(' - Rows', () => { ); const virtualScrollerContent = document.querySelector('.MuiDataGrid-virtualScrollerContent'); const expectedHeight = rows.length * (rowHeight + spacingTop + spacingBottom); - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', - height: `${expectedHeight}px`, - }); + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${expectedHeight}px` }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); setProps({ getRowSpacing: null }); - expect(virtualScrollerContent).toHaveInlineStyle({ - width: 'auto', + expect(virtualScrollerContent).toHaveComputedStyle({ height: `${rows.length * rowHeight}px`, }); + expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); }); it('should set the row margin to the value returned by getRowSpacing if rowSpacingType is not defined', () => { From debda11052933b46adcb4649aea2c327500c0784 Mon Sep 17 00:00:00 2001 From: Andrew Cherniavskyi Date: Wed, 23 Oct 2024 01:10:26 +0200 Subject: [PATCH 07/10] fix unit test --- .../x-data-grid/src/components/containers/GridRootStyles.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/x-data-grid/src/components/containers/GridRootStyles.ts b/packages/x-data-grid/src/components/containers/GridRootStyles.ts index 84ac52fbd96c8..eb1ccdc749965 100644 --- a/packages/x-data-grid/src/components/containers/GridRootStyles.ts +++ b/packages/x-data-grid/src/components/containers/GridRootStyles.ts @@ -743,7 +743,7 @@ export const GridRootStyles = styled('div', { }, [`& .${c.filler}`]: { - flexGrow: 1, + flex: '1 0 auto', }, [`& .${c['filler--borderBottom']}`]: { borderBottom: '1px solid var(--DataGrid-rowBorderColor)', From 991912cf3fa0dc34ed1f44af6bdecf4bee1cc745 Mon Sep 17 00:00:00 2001 From: Andrew Cherniavskyi Date: Wed, 23 Oct 2024 01:14:32 +0200 Subject: [PATCH 08/10] fix flaky screenshot --- docs/data/data-grid/list-view/ListViewAdvanced.js | 2 +- docs/data/data-grid/list-view/ListViewAdvanced.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/data/data-grid/list-view/ListViewAdvanced.js b/docs/data/data-grid/list-view/ListViewAdvanced.js index b27633b632708..4f83c53d2d2d7 100644 --- a/docs/data/data-grid/list-view/ListViewAdvanced.js +++ b/docs/data/data-grid/list-view/ListViewAdvanced.js @@ -258,7 +258,7 @@ export default function ListViewAdvanced() { return ( -
+
-
+
Date: Wed, 23 Oct 2024 01:35:57 +0200 Subject: [PATCH 09/10] Revert "fix flaky screenshot" This reverts commit 991912cf3fa0dc34ed1f44af6bdecf4bee1cc745. --- docs/data/data-grid/list-view/ListViewAdvanced.js | 2 +- docs/data/data-grid/list-view/ListViewAdvanced.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/data/data-grid/list-view/ListViewAdvanced.js b/docs/data/data-grid/list-view/ListViewAdvanced.js index 4f83c53d2d2d7..b27633b632708 100644 --- a/docs/data/data-grid/list-view/ListViewAdvanced.js +++ b/docs/data/data-grid/list-view/ListViewAdvanced.js @@ -258,7 +258,7 @@ export default function ListViewAdvanced() { return ( -
+
-
+
Date: Wed, 23 Oct 2024 01:43:11 +0200 Subject: [PATCH 10/10] skip tests asserting computed styles in jsdom --- .../x-data-grid/src/tests/rows.DataGrid.test.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/x-data-grid/src/tests/rows.DataGrid.test.tsx b/packages/x-data-grid/src/tests/rows.DataGrid.test.tsx index f4dbb00e16b1a..0952929261989 100644 --- a/packages/x-data-grid/src/tests/rows.DataGrid.test.tsx +++ b/packages/x-data-grid/src/tests/rows.DataGrid.test.tsx @@ -958,7 +958,11 @@ describe(' - Rows', () => { }); }); - it('should consider the spacing when computing the content size', () => { + it('should consider the spacing when computing the content size', function test() { + if (isJSDOM) { + // Need layouting + this.skip(); + } const spacingTop = 5; const spacingBottom = 10; const rowHeight = 50; @@ -975,7 +979,11 @@ describe(' - Rows', () => { expect(virtualScrollerContent).toHaveInlineStyle({ width: 'auto' }); }); - it('should update the content size when getRowSpacing is removed', () => { + it('should update the content size when getRowSpacing is removed', function test() { + if (isJSDOM) { + // Need layouting + this.skip(); + } const spacingTop = 5; const spacingBottom = 10; const rowHeight = 50;