[DataGrid] Add scroll shadows and fix scrollbar overlap #16476
There are no files selected for viewing
cherniavskii marked this conversation as resolved.
Show resolved
Hide resolved
cherniavskii marked this conversation as resolved.
Show resolved
Hide resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| 'use client'; | ||
| import { styled } from '@mui/system'; | ||
| import * as React from 'react'; | ||
| import { | ||
| gridDimensionsSelector, | ||
| gridPinnedColumnsSelector, | ||
| useGridEvent, | ||
| useGridSelector, | ||
| } from '../hooks'; | ||
| import { gridPinnedRowsSelector } from '../hooks/features/rows/gridRowsSelector'; | ||
| import { useGridRootProps } from '../hooks/utils/useGridRootProps'; | ||
| import { DataGridProcessedProps } from '../models/props/DataGridProps'; | ||
| import { GridEventListener } from '../models/events'; | ||
| import { vars } from '../constants/cssVariables'; | ||
| import { useGridPrivateApiContext } from '../hooks/utils/useGridPrivateApiContext'; | ||
|
|
||
| interface GridScrollShadowsProps { | ||
| position: 'vertical' | 'horizontal'; | ||
| } | ||
|
|
||
| type OwnerState = Pick<DataGridProcessedProps, 'classes'> & { | ||
| position: 'vertical' | 'horizontal'; | ||
| }; | ||
|
|
||
| const ScrollShadow = styled('div')<{ ownerState: OwnerState }>(({ theme }) => ({ | ||
|
There was a problem hiding this comment. The shadows look quite dim in dark mode: 
I can't tell that there are pinned rows and columns just looking at the screenshot 😅 It looks great in light mode for comparison: 
There was a problem hiding this comment. Shadows on dark mode are hard 🫠 We need to have a lighter background on the grid for shadows to be effective. Investigated a little and found that on the Material UI Table component, it has the paper overlay making it a dark grey as opposed to the black we currently have: 
I've updated the background color on dark mode for the data grid to match and refined the shadow size/opacity: 
Feels like an improvement — WDYT? There was a problem hiding this comment. Looks better! There was a problem hiding this comment. I think so? Not sure. There are a few substantial design changes here, including removing borders on pinned columns by default. @joserodolfofreitas any thoughts? There was a problem hiding this comment. Good question. Since we follow SemVer, visual changes like background color or removing borders can be breaking if they affect existing layouts or theming expectations, especially for users relying on visual consistency. So even though they might feel “minor”, ideally, we should treat these as breaking changes and target them for the next major version. That said, if we can opt-in* to the new visuals via a prop or theme variable (and keep the old behavior as default), then we could consider shipping it in a minor instead. *(An opt-out prop is not the ideal, because it essentilly still breaking, but it's fine as well, since it'd not break for most people) There was a problem hiding this comment. I have added a prop to control borders/shadows https://deploy-preview-16476--material-ui-x.netlify.app/x/react-data-grid/column-pinning/#pinned-columns-appearance There was a problem hiding this comment. @cherniavskii thanks for picking this up. Would it make sense to have a similar option for pinned rows since they can have shadows now too? Or we could perhaps condense it into one prop: There was a problem hiding this comment.
In case of pinned rows, the border is always there. Do you think we should support the same list of options for the pinned rows as for the pinned columns? There was a problem hiding this comment. Added a separate There was a problem hiding this comment. Yeah, I don't think a
arminmeh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| position: 'absolute', | ||
| inset: 0, | ||
| pointerEvents: 'none', | ||
| transition: vars.transition(['box-shadow'], { duration: vars.transitions.duration.short }), | ||
| '--length': '5px', | ||
| '--length-end': 'calc(var(--length) * -1)', | ||
| '--opacity': theme.palette.mode === 'dark' ? '0.8' : '0.18', | ||
| '--blur': 'var(--length)', | ||
| '--spread': 'calc(var(--length) * -1)', | ||
| '--color-start': 'rgba(0, 0, 0, calc(var(--hasScrollStart) * var(--opacity)))', | ||
| '--color-end': 'rgba(0, 0, 0, calc(var(--hasScrollEnd) * var(--opacity)))', | ||
| variants: [ | ||
| { | ||
| props: { position: 'vertical' }, | ||
| style: { | ||
| top: 'var(--DataGrid-topContainerHeight)', | ||
| bottom: | ||
| 'calc(var(--DataGrid-bottomContainerHeight) + var(--DataGrid-hasScrollX) * var(--DataGrid-scrollbarSize))', | ||
| boxShadow: | ||
| 'inset 0 var(--length) var(--blur) var(--spread) var(--color-start), inset 0 var(--length-end) var(--blur) var(--spread) var(--color-end)', | ||
| }, | ||
| }, | ||
| { | ||
| props: { position: 'horizontal' }, | ||
| style: { | ||
| left: 'var(--DataGrid-leftPinnedWidth)', | ||
| right: | ||
| 'calc(var(--DataGrid-rightPinnedWidth) + var(--DataGrid-hasScrollY) * var(--DataGrid-scrollbarSize))', | ||
| boxShadow: | ||
| 'inset var(--length) 0 var(--blur) var(--spread) var(--color-start), inset var(--length-end) 0 var(--blur) var(--spread) var(--color-end)', | ||
| }, | ||
| }, | ||
| ], | ||
| })); | ||
|
|
||
| function GridScrollShadows(props: GridScrollShadowsProps) { | ||
| const { position } = props; | ||
| const rootProps = useGridRootProps(); | ||
| const ownerState = { classes: rootProps.classes, position }; | ||
| const ref = React.useRef<HTMLDivElement>(null); | ||
| const apiRef = useGridPrivateApiContext(); | ||
| const dimensions = useGridSelector(apiRef, gridDimensionsSelector); | ||
| const pinnedRows = useGridSelector(apiRef, gridPinnedRowsSelector); | ||
| const pinnedColumns = useGridSelector(apiRef, gridPinnedColumnsSelector); | ||
| const initialScrollable = | ||
| position === 'vertical' | ||
| ? dimensions.hasScrollY && pinnedRows?.bottom?.length > 0 | ||
| : dimensions.hasScrollX && | ||
| pinnedColumns?.right?.length !== undefined && | ||
| pinnedColumns?.right?.length > 0; | ||
|
|
||
| const updateScrollShadowVisibility = (scrollPosition: number) => { | ||
| if (!ref.current) { | ||
| return; | ||
| } | ||
| const scroll = Math.round(scrollPosition); | ||
| const maxScroll = Math.round( | ||
| dimensions.contentSize[position === 'vertical' ? 'height' : 'width'] - | ||
| dimensions.viewportInnerSize[position === 'vertical' ? 'height' : 'width'], | ||
| ); | ||
| const hasPinnedStart = | ||
| position === 'vertical' | ||
| ? pinnedRows?.top?.length > 0 | ||
| : pinnedColumns?.left?.length !== undefined && pinnedColumns?.left?.length > 0; | ||
| const hasPinnedEnd = | ||
| position === 'vertical' | ||
| ? pinnedRows?.bottom?.length > 0 | ||
| : pinnedColumns?.right?.length !== undefined && pinnedColumns?.right?.length > 0; | ||
| ref.current.style.setProperty('--hasScrollStart', hasPinnedStart && scroll > 0 ? '1' : '0'); | ||
| ref.current.style.setProperty('--hasScrollEnd', hasPinnedEnd && scroll < maxScroll ? '1' : '0'); | ||
| }; | ||
|
|
||
| const handleScrolling: GridEventListener<'scrollPositionChange'> = (scrollParams) => { | ||
| updateScrollShadowVisibility(scrollParams[position === 'vertical' ? 'top' : 'left']); | ||
| }; | ||
|
|
||
| const handleColumnResizeStop: GridEventListener<'columnResizeStop'> = () => { | ||
| if (position === 'horizontal') { | ||
| updateScrollShadowVisibility(apiRef.current.virtualScrollerRef?.current?.scrollLeft || 0); | ||
| } | ||
| }; | ||
|
|
||
| useGridEvent(apiRef, 'scrollPositionChange', handleScrolling); | ||
| useGridEvent(apiRef, 'columnResizeStop', handleColumnResizeStop); | ||
|
|
||
| return ( | ||
| <ScrollShadow | ||
| ownerState={ownerState} | ||
| ref={ref} | ||
| style={ | ||
| { | ||
| '--hasScrollStart': 0, | ||
| '--hasScrollEnd': initialScrollable ? '1' : '0', | ||
| } as React.CSSProperties | ||
| } | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| export { GridScrollShadows }; | ||
Uh oh!
There was an error while loading. Please reload this page.