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] Fix scroll jumping #14929

Merged
merged 24 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/pages/x/api/data-grid/data-grid-premium.json
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,11 @@
"rowGroupingModel": { "type": { "name": "arrayOf", "description": "Array<string>" } },
"rowHeight": { "type": { "name": "number" }, "default": "52" },
"rowModesModel": { "type": { "name": "object" } },
"rowPositionsDebounceMs": { "type": { "name": "number" }, "default": "166" },
"rowPositionsDebounceMs": {
"type": { "name": "number" },
"default": "166",
"deprecated": true
},
"rowReordering": { "type": { "name": "bool" }, "default": "false" },
"rows": {
"type": { "name": "arrayOf", "description": "Array<object>" },
Expand Down
6 changes: 5 additions & 1 deletion docs/pages/x/api/data-grid/data-grid-pro.json
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,11 @@
"rowCount": { "type": { "name": "number" } },
"rowHeight": { "type": { "name": "number" }, "default": "52" },
"rowModesModel": { "type": { "name": "object" } },
"rowPositionsDebounceMs": { "type": { "name": "number" }, "default": "166" },
"rowPositionsDebounceMs": {
"type": { "name": "number" },
"default": "166",
"deprecated": true
},
"rowReordering": { "type": { "name": "bool" }, "default": "false" },
"rows": {
"type": { "name": "arrayOf", "description": "Array<object>" },
Expand Down
6 changes: 5 additions & 1 deletion docs/pages/x/api/data-grid/data-grid.json
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,11 @@
"rowCount": { "type": { "name": "number" } },
"rowHeight": { "type": { "name": "number" }, "default": "52" },
"rowModesModel": { "type": { "name": "object" } },
"rowPositionsDebounceMs": { "type": { "name": "number" }, "default": "166" },
"rowPositionsDebounceMs": {
"type": { "name": "number" },
"default": "166",
"deprecated": true
},
"rows": {
"type": { "name": "arrayOf", "description": "Array<object>" },
"default": "[]"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,7 @@ DataGridPremiumRaw.propTypes = {
* Setting it to a lower value could be useful when using dynamic row height,
* but might reduce performance when displaying a large number of rows.
* @default 166
* @deprecated
*/
rowPositionsDebounceMs: PropTypes.number,
/**
Expand Down
1 change: 1 addition & 0 deletions packages/x-data-grid-pro/src/DataGridPro/DataGridPro.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,7 @@ DataGridProRaw.propTypes = {
* Setting it to a lower value could be useful when using dynamic row height,
* but might reduce performance when displaying a large number of rows.
* @default 166
* @deprecated
*/
rowPositionsDebounceMs: PropTypes.number,
/**
Expand Down
Copy link
Contributor Author

@romgrk romgrk Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidenote but the detail panels code is terribly unefficient. IIUC, it calls getDetailPanel(Height|Content) for all the rows of the grid, regardless if they're displayed or not. For large datasets, the performance must be horrible.

Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,13 @@ export const useGridDetailPanel = (

const isFirstRender = React.useRef(true);
if (isFirstRender.current) {
isFirstRender.current = false;
updateCachesIfNeeded();
}
React.useEffect(() => {
if (!isFirstRender.current) {
updateCachesIfNeeded();
apiRef.current.hydrateRowsMeta();
}
isFirstRender.current = false;
}, [apiRef, updateCachesIfNeeded]);
};
71 changes: 0 additions & 71 deletions packages/x-data-grid-pro/src/tests/rows.DataGridPro.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -887,77 +887,6 @@ describe('<DataGridPro /> - Rows', () => {
});
});

describe('apiRef: setRowHeight', () => {
const ROW_HEIGHT = 52;

before(function beforeHook() {
if (isJSDOM) {
// Need layouting
this.skip();
}
});

beforeEach(() => {
baselineProps = {
rows: [
{
id: 0,
brand: 'Nike',
},
{
id: 1,
brand: 'Adidas',
},
{
id: 2,
brand: 'Puma',
},
],
columns: [{ field: 'brand', headerName: 'Brand' }],
};
});

let apiRef: React.MutableRefObject<GridApi>;

function TestCase(props: Partial<DataGridProProps>) {
apiRef = useGridApiRef();
return (
<div style={{ width: 300, height: 300 }}>
<DataGridPro {...baselineProps} apiRef={apiRef} rowHeight={ROW_HEIGHT} {...props} />
</div>
);
}

it('should change row height', () => {
const resizedRowId = 1;
render(<TestCase />);

expect(getRow(1).clientHeight).to.equal(ROW_HEIGHT);

act(() => apiRef.current.unstable_setRowHeight(resizedRowId, 100));
expect(getRow(resizedRowId).clientHeight).to.equal(100);
});

it('should preserve changed row height after sorting', () => {
const resizedRowId = 0;
const getRowHeight = spy();
render(<TestCase getRowHeight={getRowHeight} />);

const row = getRow(resizedRowId);
expect(row.clientHeight).to.equal(ROW_HEIGHT);

getRowHeight.resetHistory();
act(() => apiRef.current.unstable_setRowHeight(resizedRowId, 100));
expect(row.clientHeight).to.equal(100);

// sort
fireEvent.click(getColumnHeaderCell(resizedRowId));

expect(row.clientHeight).to.equal(100);
expect(getRowHeight.neverCalledWithMatch({ id: resizedRowId })).to.equal(true);
});
});

describe('prop: rowCount', () => {
function TestCase(props: DataGridProProps) {
return (
Expand Down
1 change: 1 addition & 0 deletions packages/x-data-grid/src/DataGrid/DataGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,7 @@ DataGridRaw.propTypes = {
* Setting it to a lower value could be useful when using dynamic row height,
* but might reduce performance when displaying a large number of rows.
* @default 166
* @deprecated
*/
rowPositionsDebounceMs: PropTypes.number,
/**
Expand Down
60 changes: 16 additions & 44 deletions packages/x-data-grid/src/components/GridRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { fastMemo } from '@mui/x-internals/fastMemo';
import { GridRowEventLookup } from '../models/events';
import { GridRowId, GridRowModel } from '../models/gridRows';
import { GridEditModes, GridRowModes, GridCellModes } from '../models/gridEditRowModel';
import { useGridApiContext } from '../hooks/utils/useGridApiContext';
import { gridClasses } from '../constants/gridClasses';
import { composeGridClasses } from '../utils/composeGridClasses';
import { useGridRootProps } from '../hooks/utils/useGridRootProps';
Expand All @@ -29,6 +28,7 @@ import { PinnedPosition, gridPinnedColumnPositionLookup } from './cell/GridCell'
import { GridScrollbarFillerCell as ScrollbarFiller } from './GridScrollbarFillerCell';
import { getPinnedCellOffset } from '../internals/utils/getPinnedCellOffset';
import { useGridConfiguration } from '../hooks/utils/useGridConfiguration';
import { useGridPrivateApiContext } from '../hooks/utils/useGridPrivateApiContext';

export interface GridRowProps extends React.HTMLAttributes<HTMLDivElement> {
row: GridRowModel;
Expand Down Expand Up @@ -111,7 +111,7 @@ const GridRow = React.forwardRef<HTMLDivElement, GridRowProps>(function GridRow(
onMouseOver,
...other
} = props;
const apiRef = useGridApiContext();
const apiRef = useGridPrivateApiContext();
const configuration = useGridConfiguration();
const ref = React.useRef<HTMLDivElement>(null);
const rootProps = useGridRootProps();
Expand Down Expand Up @@ -153,37 +153,19 @@ const GridRow = React.forwardRef<HTMLDivElement, GridRowProps>(function GridRow(

React.useLayoutEffect(() => {
if (currentPage.range) {
// The index prop is relative to the rows from all pages. As example, the index prop of the
// first row is 5 if `paginationModel.pageSize=5` and `paginationModel.page=1`. However, the index used by the virtualization
// doesn't care about pagination and considers the rows from the current page only, so the
// first row always has index=0. We need to subtract the index of the first row to make it
// compatible with the index used by the virtualization.
const rowIndex = apiRef.current.getRowIndexRelativeToVisibleRows(rowId);
// pinned rows are not part of the visible rows
if (rowIndex != null) {
// Pinned rows are not part of the visible rows
if (rowIndex !== undefined) {
apiRef.current.unstable_setLastMeasuredRowIndex(rowIndex);
}
}

const rootElement = ref.current;
const hasFixedHeight = rowHeight !== 'auto';
if (!rootElement || hasFixedHeight || typeof ResizeObserver === 'undefined') {
return undefined;
if (ref.current && rowHeight === 'auto') {
return apiRef.current.observeRowHeight(ref.current, rowId);
}

const resizeObserver = new ResizeObserver((entries) => {
const [entry] = entries;
const height =
entry.borderBoxSize && entry.borderBoxSize.length > 0
? entry.borderBoxSize[0].blockSize
: entry.contentRect.height;
apiRef.current.unstable_storeRowHeightMeasurement(rowId, height);
});

resizeObserver.observe(rootElement);

return () => resizeObserver.disconnect();
}, [apiRef, currentPage.range, index, rowHeight, rowId]);
return undefined;
}, [apiRef, currentPage.range, rowHeight, rowId]);

const publish = React.useCallback(
(
Expand Down Expand Up @@ -254,22 +236,12 @@ const GridRow = React.forwardRef<HTMLDivElement, GridRowProps>(function GridRow(

const rowReordering = (rootProps as any).rowReordering as boolean;

const sizes = useGridSelector(
const heightEntry = useGridSelector(
apiRef,
() => ({ ...apiRef.current.unstable_getRowInternalSizes(rowId) }),
() => ({ ...apiRef.current.getRowHeightEntry(rowId) }),
objectShallowCompare,
);

let minHeight = rowHeight;
if (minHeight === 'auto' && sizes) {
const numberOfBaseSizes = 1;
const maximumSize = sizes.baseCenter ?? 0;

if (maximumSize > 0 && numberOfBaseSizes > 1) {
minHeight = maximumSize;
}
}

const style = React.useMemo(() => {
if (isNotVisible) {
return {
Expand All @@ -282,28 +254,28 @@ const GridRow = React.forwardRef<HTMLDivElement, GridRowProps>(function GridRow(
const rowStyle = {
...styleProp,
maxHeight: rowHeight === 'auto' ? 'none' : rowHeight, // max-height doesn't support "auto"
minHeight,
minHeight: rowHeight,
'--height': typeof rowHeight === 'number' ? `${rowHeight}px` : rowHeight,
};

if (sizes?.spacingTop) {
if (heightEntry.spacingTop) {
const property = rootProps.rowSpacingType === 'border' ? 'borderTopWidth' : 'marginTop';
rowStyle[property] = sizes.spacingTop;
rowStyle[property] = heightEntry.spacingTop;
}

if (sizes?.spacingBottom) {
if (heightEntry.spacingBottom) {
const property = rootProps.rowSpacingType === 'border' ? 'borderBottomWidth' : 'marginBottom';
let propertyValue = rowStyle[property];
// avoid overriding existing value
if (typeof propertyValue !== 'number') {
propertyValue = parseInt(propertyValue || '0', 10);
}
propertyValue += sizes.spacingBottom;
propertyValue += heightEntry.spacingBottom;
rowStyle[property] = propertyValue;
}

return rowStyle;
}, [isNotVisible, rowHeight, styleProp, minHeight, sizes, rootProps.rowSpacingType]);
}, [isNotVisible, rowHeight, styleProp, heightEntry, rootProps.rowSpacingType]);

const rowClassNames = apiRef.current.unstable_applyPipeProcessors('rowClassName', [], rowId);
const ariaAttributes = rowNode ? getRowAriaAttributes(rowNode, index) : undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
import { GridRowEntry, GridRowId } from '../../../models/gridRows';
import { GridHydrateRowsValue } from '../../features/rows/gridRowsInterfaces';
import { GridPreferencePanelsValue } from '../../features/preferencesPanel';
import { HeightEntry } from '../../features/rows/gridRowsMetaInterfaces';

export type GridPipeProcessorGroup = keyof GridPipeProcessingLookup;

Expand All @@ -41,7 +42,7 @@ export interface GridPipeProcessingLookup {
value: GridRestoreStatePreProcessingValue;
context: GridRestoreStatePreProcessingContext<GridInitialStateCommunity>;
};
rowHeight: { value: Record<string, number>; context: GridRowEntry };
rowHeight: { value: HeightEntry; context: GridRowEntry };
scrollToIndexes: {
value: Partial<GridScrollParams>;
context: Partial<GridCellIndexCoordinates>;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { GridRowId } from '../../../models/gridRows';

export type HeightEntry = {
content: number;
spacingTop: number;
spacingBottom: number;
detail: number;

autoHeight: boolean;
needsFirstMeasurement: boolean;
};

export type HeightCache = Map<GridRowId, HeightEntry>;

export interface GridRowsMetaInternalCache {
/**
* Map of height cache entries.
*/
heights: HeightCache;
}
5 changes: 5 additions & 0 deletions packages/x-data-grid/src/hooks/features/rows/gridRowsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,3 +464,8 @@ export const rowHeightWarning = [
`MUI X: The \`rowHeight\` prop should be a number greater than 0.`,
`The default value will be used instead.`,
].join('\n');

export const getRowHeightWarning = [
`MUI X: The \`getRowHeight\` prop should return a number greater than 0 or 'auto'.`,
`The default value will be used instead.`,
].join('\n');
Loading