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] Update scrollbar positions to match native behavior #16379

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
2 changes: 0 additions & 2 deletions packages/x-data-grid/src/components/GridRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {
gridEditRowsStateSelector,
gridRowIsEditingSelector,
} from '../hooks/features/editing/gridEditingSelectors';
import { GridScrollbarFillerCell as ScrollbarFiller } from './GridScrollbarFillerCell';
import { getPinnedCellOffset } from '../internals/utils/getPinnedCellOffset';
import { useGridConfiguration } from '../hooks/utils/useGridConfiguration';
import { useGridPrivateApiContext } from '../hooks/utils/useGridPrivateApiContext';
Expand Down Expand Up @@ -473,7 +472,6 @@ const GridRow = forwardRef<HTMLDivElement, GridRowProps>(function GridRow(props,
{cells}
<div role="presentation" className={clsx(gridClasses.cell, gridClasses.cellEmpty)} />
{rightCells}
{scrollbarWidth !== 0 && <ScrollbarFiller pinnedRight={pinnedColumns.right.length > 0} />}
</div>
);
});
Expand Down
38 changes: 0 additions & 38 deletions packages/x-data-grid/src/components/GridScrollbarFillerCell.tsx

This file was deleted.

11 changes: 0 additions & 11 deletions packages/x-data-grid/src/components/GridSkeletonLoadingOverlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import { getDataGridUtilityClass, gridClasses } from '../constants/gridClasses';
import { getPinnedCellOffset } from '../internals/utils/getPinnedCellOffset';
import { shouldCellShowLeftBorder, shouldCellShowRightBorder } from '../utils/cellBorderUtils';
import { escapeOperandAttributeSelector } from '../utils/domUtils';
import { GridScrollbarFillerCell } from './GridScrollbarFillerCell';
import { rtlFlipSide } from '../utils/rtlFlipSide';
import { attachPinnedStyle } from '../internals/utils';

Expand Down Expand Up @@ -140,7 +139,6 @@ const GridSkeletonLoadingOverlay = forwardRef<HTMLDivElement, React.HTMLAttribut
const emptyCell = (
<slots.skeletonCell key={`skeleton-filler-column-${i}`} width={emptyCellWidth} empty />
);
const hasScrollbarFiller = isLastColumn && scrollbarWidth !== 0;

if (hasFillerBefore) {
rowCells.push(emptyCell);
Expand Down Expand Up @@ -170,15 +168,6 @@ const GridSkeletonLoadingOverlay = forwardRef<HTMLDivElement, React.HTMLAttribut
if (hasFillerAfter) {
rowCells.push(emptyCell);
}

if (hasScrollbarFiller) {
rowCells.push(
<GridScrollbarFillerCell
key={`skeleton-scrollbar-filler-${i}`}
pinnedRight={pinnedColumns.right.length > 0}
/>,
);
}
}

array.push(
Expand Down
30 changes: 1 addition & 29 deletions packages/x-data-grid/src/components/containers/GridRootStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,6 @@ export const GridRootStyles = styled('div', {
{ [`& .${c.scrollbar}`]: styles.scrollbar },
{ [`& .${c['scrollbar--horizontal']}`]: styles['scrollbar--horizontal'] },
{ [`& .${c['scrollbar--vertical']}`]: styles['scrollbar--vertical'] },
{ [`& .${c.scrollbarFiller}`]: styles.scrollbarFiller },
{ [`& .${c['scrollbarFiller--borderBottom']}`]: styles['scrollbarFiller--borderBottom'] },
{ [`& .${c['scrollbarFiller--borderTop']}`]: styles['scrollbarFiller--borderTop'] },
{ [`& .${c['scrollbarFiller--header']}`]: styles['scrollbarFiller--header'] },
{ [`& .${c['scrollbarFiller--pinnedRight']}`]: styles['scrollbarFiller--pinnedRight'] },
{ [`& .${c.sortIcon}`]: styles.sortIcon },
{ [`& .${c.treeDataGroupingCell}`]: styles.treeDataGroupingCell },
{
Expand Down Expand Up @@ -544,8 +539,7 @@ export const GridRootStyles = styled('div', {

/* Bottom border of the top-container */
[`& .${c['row--borderBottom']} .${c.columnHeader},
& .${c['row--borderBottom']} .${c.filler},
& .${c['row--borderBottom']} .${c.scrollbarFiller}`]: {
& .${c['row--borderBottom']} .${c.filler}`]: {
borderBottom: `1px solid var(--DataGrid-rowBorderColor)`,
},
[`& .${c['row--borderBottom']} .${c.cell}`]: {
Expand Down Expand Up @@ -598,11 +592,6 @@ export const GridRootStyles = styled('div', {
[`& .${c.pinnedRows}`]: {
backgroundColor: 'var(--DataGrid-pinnedBackground)',
},
[`& .${c['pinnedRows--top']} :first-of-type`]: {
[`& .${c.cell}, .${c.scrollbarFiller}`]: {
borderTop: 'none',
},
},
[`&.${c['root--disableUserSelection']} .${c.cell}`]: {
userSelect: 'none',
},
Expand Down Expand Up @@ -773,23 +762,6 @@ export const GridRootStyles = styled('div', {
marginRight: t.spacing(2),
},

/* ScrollbarFiller styles */
[`.${c.scrollbarFiller}`]: {
minWidth: 'calc(var(--DataGrid-hasScrollY) * var(--DataGrid-scrollbarSize))',
alignSelf: 'stretch',
[`&.${c['scrollbarFiller--borderTop']}`]: {
borderTop: '1px solid var(--DataGrid-rowBorderColor)',
},
[`&.${c['scrollbarFiller--borderBottom']}`]: {
borderBottom: '1px solid var(--DataGrid-rowBorderColor)',
},
[`&.${c['scrollbarFiller--pinnedRight']}`]: {
backgroundColor: 'var(--DataGrid-pinnedBackground)',
position: 'sticky',
right: 0,
},
},

[`& .${c.filler}`]: {
flex: '1 0 auto',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ const GridToolbarContainerRoot = styled('div', {
alignItems: 'center',
flexWrap: 'wrap',
gap: theme.spacing(1),
padding: theme.spacing(0.5, 0.5, 0),
padding: theme.spacing(0.5),
borderBottom: `1px solid ${theme.palette.divider}`,
}));

const GridToolbarContainer = forwardRef<HTMLDivElement, GridToolbarContainerProps>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,19 @@ const Scrollbar = styled('div')({

const ScrollbarVertical = styled(Scrollbar)({
width: 'var(--size)',
height:
'calc(var(--DataGrid-hasScrollY) * (100% - var(--DataGrid-topContainerHeight) - var(--DataGrid-bottomContainerHeight) - var(--DataGrid-hasScrollX) * var(--DataGrid-scrollbarSize)))',
overflowY: 'auto',
overflowX: 'hidden',
// Disable focus-visible style, it's a scrollbar.
outline: 0,
'& > div': {
width: 'var(--size)',
},
top: 'var(--DataGrid-topContainerHeight)',
right: '0px',
top: 0,
bottom: 0,
right: 0,
});

const ScrollbarHorizontal = styled(Scrollbar)({
width: '100%',
height: 'var(--size)',
overflowY: 'hidden',
overflowX: 'auto',
Expand All @@ -73,7 +71,16 @@ const ScrollbarHorizontal = styled(Scrollbar)({
'& > div': {
height: 'var(--size)',
},
bottom: '0px',
bottom: 0,
left: 0,
});

export const ScrollbarCorner = styled(Scrollbar)({
width: 'var(--size)',
height: 'var(--size)',
bottom: 0,
right: 0,
overflow: 'scroll',
Copy link
Member Author

@KenanYusuf KenanYusuf Jan 29, 2025

Choose a reason for hiding this comment

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

This corner piece covers up the dead zone between the two scrollbars—works nicely on a Mac, needs to be tested on other browsers + OS.

Without With
localhost_3001_playground_ (1) localhost_3001_playground_ (3)

});

const GridVirtualScrollbar = forwardRef<HTMLDivElement, GridVirtualScrollbarProps>(
Expand All @@ -90,18 +97,8 @@ const GridVirtualScrollbar = forwardRef<HTMLDivElement, GridVirtualScrollbarProp
const propertyDimension = props.position === 'vertical' ? 'height' : 'width';
const propertyScroll = props.position === 'vertical' ? 'scrollTop' : 'scrollLeft';
const propertyScrollPosition = props.position === 'vertical' ? 'top' : 'left';
const hasScroll = props.position === 'vertical' ? dimensions.hasScrollX : dimensions.hasScrollY;

const contentSize =
dimensions.minimumSize[propertyDimension] + (hasScroll ? dimensions.scrollbarSize : 0);

const scrollbarSize =
props.position === 'vertical'
? dimensions.viewportInnerSize.height
: dimensions.viewportOuterSize.width;

const scrollbarInnerSize =
scrollbarSize * (contentSize / dimensions.viewportOuterSize[propertyDimension]);
const scrollbarInnerSize = dimensions.minimumSize[propertyDimension];

const onScrollerScroll = useEventCallback(() => {
const scrollbar = scrollbarRef.current;
Expand All @@ -123,8 +120,7 @@ const GridVirtualScrollbar = forwardRef<HTMLDivElement, GridVirtualScrollbarProp
}
isLocked.current = true;

const value = scrollPosition[propertyScrollPosition] / contentSize;
scrollbar[propertyScroll] = value * scrollbarInnerSize;
scrollbar[propertyScroll] = scrollPosition[propertyScrollPosition];
});

const onScrollbarScroll = useEventCallback(() => {
Expand All @@ -141,8 +137,7 @@ const GridVirtualScrollbar = forwardRef<HTMLDivElement, GridVirtualScrollbarProp
}
isLocked.current = true;

const value = scrollbar[propertyScroll] / scrollbarInnerSize;
scroller[propertyScroll] = value * contentSize;
scroller[propertyScroll] = scrollbar[propertyScroll];
});

useOnMount(() => {
Expand All @@ -164,15 +159,23 @@ const GridVirtualScrollbar = forwardRef<HTMLDivElement, GridVirtualScrollbarProp

const Container = props.position === 'vertical' ? ScrollbarVertical : ScrollbarHorizontal;

// TODO: Check why this is needed
const listViewStyle =
props.position === 'vertical' && rootProps.unstable_listView
? { bottom: 0, top: 0 }
: undefined;

return (
<Container
ref={useForkRef(ref, scrollbarRef)}
className={classes.root}
style={
props.position === 'vertical' && rootProps.unstable_listView
? { height: '100%', top: 0 }
: undefined
}
style={{
bottom:
props.position === 'vertical' && dimensions.hasScrollX ? 'var(--size)' : undefined,
right:
props.position === 'horizontal' && dimensions.hasScrollY ? 'var(--size)' : undefined,
...listViewStyle,
}}
tabIndex={-1}
aria-hidden="true"
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { GridBottomContainer as BottomContainer } from './GridBottomContainer';
import { GridVirtualScrollerContent as Content } from './GridVirtualScrollerContent';
import { GridVirtualScrollerFiller as SpaceFiller } from './GridVirtualScrollerFiller';
import { GridVirtualScrollerRenderZone as RenderZone } from './GridVirtualScrollerRenderZone';
import { GridVirtualScrollbar as Scrollbar } from './GridVirtualScrollbar';
import { GridVirtualScrollbar as Scrollbar, ScrollbarCorner } from './GridVirtualScrollbar';
import { GridLoadingOverlayVariant } from '../GridLoadingOverlay';

type OwnerState = Pick<DataGridProcessedProps, 'classes'> & {
Expand Down Expand Up @@ -127,6 +127,7 @@ function GridVirtualScroller(props: GridVirtualScrollerProps) {
<Scrollbar position="horizontal" {...getScrollbarHorizontalProps()} />
)}
{dimensions.hasScrollY && <Scrollbar position="vertical" {...getScrollbarVerticalProps()} />}
{dimensions.hasScrollX && dimensions.hasScrollY && <ScrollbarCorner />}
{props.children}
</Container>
);
Expand Down
30 changes: 0 additions & 30 deletions packages/x-data-grid/src/constants/gridClasses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -554,31 +554,6 @@ export interface GridClasses {
* Styles applied to the horizontal scrollbar.
*/
'scrollbar--vertical': string;
/**
* @ignore - do not document.
* Styles applied to the scrollbar filler cell.
*/
scrollbarFiller: string;
/**
* @ignore - do not document.
* Styles applied to the scrollbar filler cell, in header position.
*/
'scrollbarFiller--header': string;
/**
* @ignore - do not document.
* Styles applied to the scrollbar filler cell, with a border top.
*/
'scrollbarFiller--borderTop': string;
/**
* @ignore - do not document.
* Styles applied to the scrollbar filler cell, with a border bottom.
*/
'scrollbarFiller--borderBottom': string;
/**
* @ignore - do not document.
* Styles applied to the scrollbar filler cell.
*/
'scrollbarFiller--pinnedRight': string;
/**
* Styles applied to the footer selected row count element.
*/
Expand Down Expand Up @@ -815,11 +790,6 @@ export const gridClasses = generateUtilityClasses<GridClassKey>('MuiDataGrid', [
'scrollbar',
'scrollbar--vertical',
'scrollbar--horizontal',
'scrollbarFiller',
'scrollbarFiller--header',
'scrollbarFiller--borderTop',
'scrollbarFiller--borderBottom',
'scrollbarFiller--pinnedRight',
'selectedRowCount',
'sortIcon',
'toolbarContainer',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
} from '../columns';
import { GridGroupingStructure } from '../columnGrouping/gridColumnGroupsInterfaces';
import { gridColumnGroupsUnwrappedModelSelector } from '../columnGrouping/gridColumnGroupsSelector';
import { GridScrollbarFillerCell as ScrollbarFiller } from '../../../components/GridScrollbarFillerCell';
import { getPinnedCellOffset } from '../../../internals/utils/getPinnedCellOffset';
import { GridColumnHeaderSeparatorSides } from '../../../components/columnHeaders/GridColumnHeaderSeparator';
import { gridClasses } from '../../../constants/gridClasses';
Expand Down Expand Up @@ -176,13 +175,8 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
leftOverflow: number,
borderBottom: boolean = false,
) => {
const isPinnedRight = params?.position === PinnedColumnPosition.RIGHT;
const isNotPinned = params?.position === undefined;

const hasScrollbarFiller =
(pinnedColumns.right.length > 0 && isPinnedRight) ||
(pinnedColumns.right.length === 0 && isNotPinned);

const leftOffsetWidth = offsetLeft - leftOverflow;

return (
Expand All @@ -198,14 +192,6 @@ export const useGridColumnHeaders = (props: UseGridColumnHeadersProps) => {
)}
/>
)}
{hasScrollbarFiller && (
<ScrollbarFiller
header
pinnedRight={isPinnedRight}
borderBottom={borderBottom}
borderTop={false}
/>
)}
</React.Fragment>
);
};
Expand Down