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 to cell logic for keyboard navigating cells and drag selection with pinned columns #14550

Merged
merged 11 commits into from
Sep 23, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ export const useGridCellSelection = (
}

const { x: mouseX, y: mouseY } = mousePosition.current;
const { height, width } = dimensions.viewportInnerSize;
const { width, height: viewportOuterHeight } = dimensions.viewportOuterSize;
const height = viewportOuterHeight - totalHeaderHeight;

let deltaX = 0;
let deltaY = 0;
Expand Down Expand Up @@ -329,7 +330,7 @@ export const useGridCellSelection = (
}

autoScroll();
}, [apiRef, dimensions]);
}, [apiRef, dimensions, totalHeaderHeight]);

const handleCellMouseOver = React.useCallback<GridEventListener<'cellMouseOver'>>(
(params, event) => {
Expand All @@ -353,7 +354,8 @@ export const useGridCellSelection = (
}

const { x, y } = virtualScrollerRect;
const { height, width } = dimensions.viewportInnerSize;
const { width, height: viewportOuterHeight } = dimensions.viewportOuterSize;
const height = viewportOuterHeight - totalHeaderHeight;
const mouseX = event.clientX - x;
const mouseY = event.clientY - y - totalHeaderHeight;
mousePosition.current = { x: mouseX, y: mouseY };
Expand Down
40 changes: 20 additions & 20 deletions packages/x-data-grid/src/hooks/features/scroll/useGridScroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,24 @@ import { gridDimensionsSelector } from '../dimensions';
// Logic copied from https://www.w3.org/TR/wai-aria-practices/examples/listbox/js/listbox.js
// Similar to https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollIntoView
function scrollIntoView(dimensions: {
clientHeight: number;
scrollTop: number;
offsetHeight: number;
offsetTop: number;
containerSize: number;
scrollPosition: number;
elementSize: number;
elementOffset: number;
Copy link
Member Author

@KenanYusuf KenanYusuf Sep 10, 2024

Choose a reason for hiding this comment

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

Changed the param names to be direction agnostic, since this is used for horizontal and vertical scrolling

}) {
const { clientHeight, scrollTop, offsetHeight, offsetTop } = dimensions;
const { containerSize, scrollPosition, elementSize, elementOffset } = dimensions;

const elementBottom = offsetTop + offsetHeight;
const elementEnd = elementOffset + elementSize;
// Always scroll to top when cell is higher than viewport to avoid scroll jump
// See https://github.com/mui/mui-x/issues/4513 and https://github.com/mui/mui-x/issues/4514
if (offsetHeight > clientHeight) {
return offsetTop;
if (elementSize > containerSize) {
return elementOffset;
}
if (elementBottom - clientHeight > scrollTop) {
return elementBottom - clientHeight;
if (elementEnd - containerSize > scrollPosition) {
return elementEnd - containerSize;
}
if (offsetTop < scrollTop) {
return offsetTop;
if (elementOffset < scrollPosition) {
return elementOffset;
}
return undefined;
}
Expand Down Expand Up @@ -96,10 +96,10 @@ export const useGridScroll = (
}
// When using RTL, `scrollLeft` becomes negative, so we must ensure that we only compare values.
scrollCoordinates.left = scrollIntoView({
clientHeight: dimensions.viewportInnerSize.width,
scrollTop: Math.abs(virtualScrollerRef.current!.scrollLeft),
offsetHeight: cellWidth,
offsetTop: columnPositions[params.colIndex],
containerSize: dimensions.viewportOuterSize.width,
scrollPosition: Math.abs(virtualScrollerRef.current!.scrollLeft),
elementSize: cellWidth,
elementOffset: columnPositions[params.colIndex],
});
}
if (params.rowIndex !== undefined) {
Expand All @@ -116,10 +116,10 @@ export const useGridScroll = (
: rowsMeta.currentPageTotalHeight - rowsMeta.positions[elementIndex];

scrollCoordinates.top = scrollIntoView({
clientHeight: dimensions.viewportInnerSize.height,
scrollTop: virtualScrollerRef.current!.scrollTop,
offsetHeight: targetOffsetHeight,
offsetTop: rowsMeta.positions[elementIndex],
containerSize: dimensions.viewportInnerSize.height,
scrollPosition: virtualScrollerRef.current!.scrollTop,
elementSize: targetOffsetHeight,
elementOffset: rowsMeta.positions[elementIndex],
});
}

Expand Down