Skip to content

[data grid] Fix scroll jumping #19156

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Aug 12, 2025
Merged

[data grid] Fix scroll jumping #19156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7e0b2ab
fix: avoid running ResizeObserver effect needlessly
romgrk Aug 12, 2025
d236cd1
lint
romgrk Aug 12, 2025
9cfc478
fix: avoid dimensions warning message
romgrk Aug 12, 2025
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
3 changes: 2 additions & 1 deletion packages/x-data-grid/src/hooks/features/dimensions/useGridDimensions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import * as React from 'react';
import { RefObject } from '@mui/x-internals/types';
import { useStoreEffect } from '@mui/x-internals/store';
import { Size } from '@mui/x-virtualizer/models';
import { GridEventListener } from '../../../models/events';
import { ElementSize } from '../../../models';
import { GridPrivateApiCommunity } from '../../../models/api/gridApiCommunity';
Expand Down Expand Up @@ -136,7 +137,7 @@ export function useGridDimensions(apiRef: RefObject<GridPrivateApiCommunity>, pr
const errorShown = React.useRef(false);

useGridEventPriority(apiRef, 'resize', (size) => {
if (!getRootDimensions().isReady) {
if (!getRootDimensions().isReady || size === Size.EMPTY) {
return;
}
if (size.height === 0 && !errorShown.current && !props.autoHeight && !isJSDOM) {
Expand Down
11 changes: 5 additions & 6 deletions packages/x-virtualizer/src/features/dimensions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ function useDimensions(store: Store<BaseState>, params: VirtualizerParams, _api:
},
} = params;

const containerNode = refs.container.current;

const updateDimensions = React.useCallback(() => {
if (isFirstSizing.current) {
return;
Expand All @@ -132,10 +134,7 @@ function useDimensions(store: Store<BaseState>, params: VirtualizerParams, _api:
// All the floating point dimensions should be rounded to .1 decimal places to avoid subpixel rendering issues
// https://github.com/mui/mui-x/issues/9550#issuecomment-1619020477
// https://github.com/mui/mui-x/issues/15721
const scrollbarSize = measureScrollbarSize(
params.refs.container.current,
params.dimensions.scrollbarSize,
);
const scrollbarSize = measureScrollbarSize(containerNode, params.dimensions.scrollbarSize);

const topContainerHeight = topPinnedHeight + rowsMeta.pinnedTopRowsTotalHeight;
const bottomContainerHeight = bottomPinnedHeight + rowsMeta.pinnedBottomRowsTotalHeight;
Expand Down Expand Up @@ -234,7 +233,7 @@ function useDimensions(store: Store<BaseState>, params: VirtualizerParams, _api:
store.update({ dimensions: newDimensions });
}, [
store,
params.refs.container,
containerNode,
params.dimensions.scrollbarSize,
params.autoHeight,
rowHeight,
Expand All @@ -259,7 +258,7 @@ function useDimensions(store: Store<BaseState>, params: VirtualizerParams, _api:
);
React.useEffect(() => debouncedUpdateDimensions?.clear, [debouncedUpdateDimensions]);

useLayoutEffect(() => observeRootNode(refs.container.current, store), [refs, store]);
useLayoutEffect(() => observeRootNode(containerNode, store), [containerNode, store]);
Copy link
Member

Choose a reason for hiding this comment

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

This change broke horizontal virtualization: https://deploy-preview-19156--material-ui-x.netlify.app/x/react-data-grid/#pro-version

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I don't understand why it doesn't work as expected though:

const containerNode = refs.container.current;

console.log('render', containerNode);

React.useLayoutEffect(() => {
  console.log('useLayoutEffect', containerNode);
  return observeRootNode(containerNode, store);
}, [containerNode, store]);

Logs:

render null
dimensions.ts:261 render null
dimensions.ts:264 useLayoutEffect null
dimensions.ts:527 observeRootNode null
dimensions.ts:261 render null
dimensions.ts:261 render null
dimensions.ts:261 render <div class=​"MuiDataGrid-main MuiDataGrid-main--hiddenContent css-1bgckiu-MuiDataGrid-main" tabindex=​"-1" role=​"grid" aria-colcount=​"30" aria-rowcount=​"1" aria-multiselectable=​"true">​…​</div>​flex
dimensions.ts:261 render <div class=​"MuiDataGrid-main MuiDataGrid-main--hiddenContent css-1bgckiu-MuiDataGrid-main" tabindex=​"-1" role=​"grid" aria-colcount=​"30" aria-rowcount=​"1" aria-multiselectable=​"true">​…​</div>​flex
dimensions.ts:261 render <div class=​"MuiDataGrid-main MuiDataGrid-main--hiddenContent css-1bgckiu-MuiDataGrid-main" tabindex=​"-1" role=​"grid" aria-colcount=​"30" aria-rowcount=​"1" aria-multiselectable=​"true">​…​</div>​flex
dimensions.ts:261 render <div class=​"MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex=​"-1" role=​"grid" aria-colcount=​"30" aria-rowcount=​"100001" aria-multiselectable=​"true">​…​</div>​flex
dimensions.ts:261 render <div class=​"MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex=​"-1" role=​"grid" aria-colcount=​"30" aria-rowcount=​"100001" aria-multiselectable=​"true">​…​</div>​flex
dimensions.ts:261 render <div class=​"MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex=​"-1" role=​"grid" aria-colcount=​"30" aria-rowcount=​"100001" aria-multiselectable=​"true">​…​</div>​flex
dimensions.ts:261 render <div class=​"MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex=​"-1" role=​"grid" aria-colcount=​"30" aria-rowcount=​"100001" aria-multiselectable=​"true">​…​</div>​flex
dimensions.ts:261 render <div class=​"MuiDataGrid-main css-1bgckiu-MuiDataGrid-main" tabindex=​"-1" role=​"grid" aria-colcount=​"30" aria-rowcount=​"100001" aria-multiselectable=​"true">​…​</div>​flex

How come useLayoutEffect doesn't rerun when containerNode clearly changes and there are rerenders? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I tried moving this effect to the very top of the hook to make sure that it's not cancelled by another effect that has state update inside, but the behavior is the same.
This looks like a React bug to me.

Copy link
Member

Choose a reason for hiding this comment

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

Or a nextjs bug – I can't reproduce the issue in stackblitz using the packages built from this branch: https://stackblitz.com/edit/kygkymrj?file=package.json

Copy link
Member

@cherniavskii cherniavskii Aug 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Commenting out this line makes layout effect run after containerNode changes and everything seems to work fine:

mui-x/packages/x-virtualizer/src/features/dimensions.ts

Line 271 in 040047b

updateDimensions();

This means that the layout effect is indeed cancelled because of state update in useStoreEffect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason, the issue doesn't appear for me in the playground page, it only appears in the other docs pages.

I'll refactor the state update away from the store effect.

Copy link
Member

Choose a reason for hiding this comment

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

I can reproduce it on playground page:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How's your playground page setup? Mine works like this, which has been reported (by others) to avoid some nextjs issues, maybe I should configure it differently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can confirm that if I remove the shell that I have in my playground page, I observe the same issues that are otherwise present only in the rest of the pages.


useLayoutEffect(updateDimensions, [updateDimensions]);

Expand Down