-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
base: master
Are you sure you want to change the base?
Conversation
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Deploy preview: https://deploy-preview-16379--material-ui-x.netlify.app/ |
height: 'var(--size)', | ||
bottom: 0, | ||
right: 0, | ||
overflow: 'scroll', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Signed-off-by: Kenan Yusuf <[email protected]>
FYI, I believe the whole idea of fake scrollbars was to avoid scrollbars showing up over the headers with Would make sense though if the scrollarea included sticky rows. But it would make even more sense if sticky rows would be part of the |
TIL it is partially a design decision 😅 . There's this point though, which I think is still relevant:
Right, we have some open issues relating to sticky rows (not pinned). Not sure if there's a reason we didn't go for this approach initially—will experiment. |
Correct! |
I don't think so. This is my main argument against which would also apply in that case: The scrollbars act as an indicator as to where users can use their touchpad/mouse wheel to scroll the grid. Currently, it looks like you can only scroll in the area of non-sticky rows. |
Right, I can't argue with that. Our Data Grid is quite unique – most of the competition doesn't allow scrolling when the cursor is over the column headers. I'm honestly not sure the scrollbar should cover column headers though. @joserodolfofreitas do you have an opinion? |
I quite like it TBH, except for the scrollbar over column headers. |
It doesn't look too bad with macOS scrollbars, but on other OS:s it looks more heavy. Overall I still prefer the current implementation, I like the design better, even if there is the minor UX issue of where it's scrollable. |
On other OS:s the result is less pretty when there's touching scrollbars, that's part of the reason why I used that implementation. Even with your last update, I think I still prefer keeping the design as it is. |
That's a case where function needs to come over form though. The current one makes me think I need to move my cursor outside the pinned rows to be able to scroll, and that's not a great UX imho. It's less of an issue with headers (which I agree looks much better the way it is), since it's a small difference compared to the viewport. |
Really nice exploration 👏
This point is convincing, though both the proposed version and current have their merits and demerits. For me, a visual differentiation of the scrollable (regular) and unscrollable (pinned) sections makes the overall design aesthetics clearer. An opinionated thing, but having to scroll over sticky areas seems an added benefit and not usually how I am used to scroll. I think the sheets' version is less confusing in that regard. If we could adapt it, it would make this visual differentiation true for the horizontal scroll too. About extending the dead scrollbar areas (like the second image in #16379 (comment)), it looks even better as it seems to solve the height issue for the last pinned column cell, though I'm not sure how it'll look in other OS's. My preference for the update would be: 1) a version similar to sheets version with dead scrollbar areas 2) 1 but with fillers (like the master version) 3) native scrollbar behavior without including the scrollbar over header |
Experimenting with making the grid scroll area better match native behavior. You can see how scrollbars should look in a native scroll area here: https://codepen.io/KenanYusuf/pen/mybvJxV
Why?
Grid with a sticky header row, sticky rows and sticky columns in a native scroll area:
Before
After