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.

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

[DataGrid] Fix scroll error #15521

Merged
merged 3 commits into from
Nov 28, 2024
Merged

[DataGrid] Fix scroll error #15521

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
10aae09
return early if no scroller
cherniavskii Nov 20, 2024
62ef54d
Merge branch 'master' into fix-scroll-top-error
cherniavskii Nov 28, 2024
805c47c
lint
cherniavskii Nov 28, 2024
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
9 changes: 7 additions & 2 deletions packages/x-data-grid/src/hooks/features/virtualization/useGridVirtualScroller.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,14 @@ export const useGridVirtualScroller = () => {
);

const triggerUpdateRenderContext = useEventCallback(() => {
const scroller = scrollerRef.current;
if (!scroller) {
Copy link
Member

@oliviertassinari oliviertassinari Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I still can't help myself asking if this is not hiding something.

It could be about useTimeout that uses useOnMount, that uses useEffect so call the clear function async, so after the DOM node was unmounted, ref was set to null. Changing useTimeout to use useLayoutEffect to run clean-up in sync mode could solve this.

Copy link
Member Author

Choose a reason for hiding this comment

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

after the DOM node was unmounted, ref was set to null

I tried reproducing it #14987 (comment), but I couldn't get any scrolling callbacks called after the grid was unmounted.

return undefined;
}

const newScroll = {
top: scrollerRef.current!.scrollTop,
left: scrollerRef.current!.scrollLeft,
top: scroller.scrollTop,
left: scroller.scrollLeft,
Comment on lines -271 to +277
Copy link
Contributor

Choose a reason for hiding this comment

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

The ! type annotation causing yet another bug.

};

const dx = newScroll.left - scrollPosition.current.left;
Expand Down