Skip to content

[DataGrid] Update vertical scrollbar position to cover pinned rows #16476

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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

KenanYusuf
Copy link
Member

@KenanYusuf KenanYusuf commented Feb 5, 2025

Continuation of #16379

Fixes #13286

Browser Before After
Chrome, macOS macOS-chrome-before Screenshot 2025-02-06 at 16 30 58
Edge, Windows 11 windows-11-edge-before windows-11-edge-after
Chrome, Windows 11 windows-11-chrome-before windows-11-chrome-after

@KenanYusuf KenanYusuf added component: data grid This is the name of the generic UI component, not the React module! design This is about UI or UX design, please involve a designer labels Feb 5, 2025
@KenanYusuf KenanYusuf changed the title [data grid] Update vertical position to cover pinned rows [data grid] Update vertical scrollbar position to cover pinned rows Feb 5, 2025
@mui-bot
Copy link

mui-bot commented Feb 5, 2025

Deploy preview: https://deploy-preview-16476--material-ui-x.netlify.app/

Bundle size report

Total Size Change:${\tiny{\color{green}▼}}$-6.31KB(-0.05%) - Total Gzip Change:${\tiny{\color{green}▼}}$-1.46KB(-0.04%)
Files: 118 total (0 added, 0 removed, 19 changed)

@mui/x-data-grid-premiumparsed:${\tiny{\color{green}▼}}$-1.06KB(-0.19%) gzip:${\tiny{\color{green}▼}}$-248B(-0.15%)
@mui/x-data-grid-proparsed:${\tiny{\color{green}▼}}$-1.06KB(-0.23%) gzip:${\tiny{\color{green}▼}}$-238B(-0.18%)
@mui/x-data-grid-premium/DataGridPremiumparsed:${\tiny{\color{green}▼}}$-1.06KB(-0.20%) gzip:${\tiny{\color{green}▼}}$-267B(-0.18%)
@mui/x-data-grid-pro/DataGridProparsed:${\tiny{\color{green}▼}}$-1.06KB(-0.24%) gzip:${\tiny{\color{green}▼}}$-226B(-0.18%)
@mui/x-data-gridparsed:${\tiny{\color{green}▼}}$-1.04KB(-0.27%) gzip:${\tiny{\color{green}▼}}$-239B(-0.22%)
@mui/x-data-grid/DataGridparsed:${\tiny{\color{green}▼}}$-1.04KB(-0.29%) gzip:${\tiny{\color{green}▼}}$-236B(-0.22%)

Show 13 more bundle changes

@mui/x-charts-proparsed: 0B(0.00%) gzip:${\tiny{\color{red}▲}}$+1B(0.00%)
@mui/x-charts-pro/BarChartProparsed: 0B(0.00%) gzip:${\tiny{\color{green}▼}}$-2B(0.00%)
@mui/x-charts-pro/ChartContainerProparsed: 0B(0.00%) gzip:${\tiny{\color{green}▼}}$-1B(0.00%)
@mui/x-charts-pro/ChartDataProviderProparsed: 0B(0.00%) gzip:${\tiny{\color{red}▲}}$+1B(0.00%)
@mui/x-charts-pro/FunnelChartparsed: 0B(0.00%) gzip:${\tiny{\color{red}▲}}$+1B(0.00%)
@mui/x-charts-pro/ScatterChartProparsed: 0B(0.00%) gzip:${\tiny{\color{green}▼}}$-1B(0.00%)
@mui/x-date-pickers-pro/DateRangeCalendarparsed: 0B(0.00%) gzip:${\tiny{\color{green}▼}}$-2B(-0.01%)
@mui/x-date-pickers-pro/DateRangePickerparsed: 0B(0.00%) gzip:${\tiny{\color{red}▲}}$+1B(0.00%)
@mui/x-date-pickers-pro/DateRangePickerDayparsed: 0B(0.00%) gzip:${\tiny{\color{red}▲}}$+1B(+0.01%)
@mui/x-date-pickers-pro/DesktopDateTimeRangePickerparsed: 0B(0.00%) gzip:${\tiny{\color{green}▼}}$-1B(0.00%)
@mui/x-date-pickers-pro/MobileDateRangePickerparsed: 0B(0.00%) gzip:${\tiny{\color{red}▲}}$+1B(0.00%)
@mui/x-tree-view-proparsed: 0B(0.00%) gzip:${\tiny{\color{green}▼}}$-1B(0.00%)
@mui/x-tree-view-pro/RichTreeViewProparsed: 0B(0.00%) gzip:${\tiny{\color{red}▲}}$+1B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against de7bfb5

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Feb 5, 2025
Copy link

github-actions bot commented Feb 5, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Feb 6, 2025
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Feb 17, 2025
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 15, 2025
bottom: 0,
});

export const ScrollbarCorner = styled(Scrollbar)({
Copy link
Member Author

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.

Without With
Screenshot 2025-05-21 at 17 01 47 Screenshot 2025-05-21 at 17 01 16

@KenanYusuf KenanYusuf changed the title [data grid] Update vertical scrollbar position to cover pinned rows [DataGrid] Update vertical scrollbar position to cover pinned rows May 22, 2025
[`& .${c['main--hiddenContent']}`]: {
// Position vertical scrollbar and fillers out of grid viewport
marginRight: 'calc(var(--DataGrid-scrollbarSize) * -1)',
Copy link
Member Author

@KenanYusuf KenanYusuf May 22, 2025

Choose a reason for hiding this comment

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

This is the easiest way to ensure the horizontal scrollbar is the correct width and the vertical and corner scrollbar are hidden. Also removes the need for fillers in the skeleton loader rows.

@KenanYusuf KenanYusuf marked this pull request as ready for review May 22, 2025 12:59
@KenanYusuf KenanYusuf force-pushed the vertical-scrollbar-include-pinned-rows branch from 89a113a to 9707912 Compare May 22, 2025 14:10
@romgrk
Copy link
Contributor

romgrk commented May 23, 2025

Tbh I love the design as it is. I feel like it makes the grid with pinned rows a bit more readable as to which is the pinned vs unpinned section. I wonder if we could use the current styling for a grid with pinned rows, and the new styling for the case when there's no pinned rows?

@KenanYusuf
Copy link
Member Author

I feel like it makes the grid with pinned rows a bit more readable as to which is the pinned vs unpinned section

@romgrk it does, but I think scroll shadows are a better indicator — I plan to add this back in at the same time #10001

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! design This is about UI or UX design, please involve a designer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data grid] Scrollbar overlap
3 participants