[pickers] Fix time picker scrollbar width #16774
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oliviertassinari
added
bug 🐛
|
Deploy preview: https://deploy-preview-16774--material-ui-x.netlify.app/ |
|
Thank you for taking care of this! 🙏 I have created a follow-up PR to align the widths between Digital Clocks. 😉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This one feels like a quick win, so going after it.
You can compare the scrollbar width with:
Help with #9311
On the solution space, Ant Design uses a similar solution, but it's different:
they also use
scrollbar-width: thinbut also:leave more width. It feels great because when we look at [pickers] 5. Time section scroll bar gets in front of section column #9311 (comment), it looks like someone who added a large zoom. So It feels like fixing [pickers] 5. Time section scroll bar gets in front of section column #9311 should also involve a change in this direction. One could argue that to full fix this, we need to have a custom scrollbar handling logic, like one we have on Base UI scroll area component or Material UI tabs component, but I don't know, this feel overkill, the pragmatic approach feels like to try those quick wins, and see if it's good enough. Adding more JS logic will slow the component, so could be a net worse overall.
change the background color to white. Because of point 1 of [scroll area] Style hover + active state base-ui#1283 it makes using the scrollbar harder, but it creates more contrast, it's easier to see the scroll position.
One could argue that, now, the scrollbar is too small, but honestly, it feels like that scrollbar is not meant to be used: it's more about indicating where the scroll is. If I'm a power user, I don't care, I use my mouse scroll wheel or my trackpad. If I'm a web user noob, I use the keyboard, input field, arrow. So what Ant Design did here isn't stupid, I would default to be lazy, not do changes, unless we feel it's a clear net win.
Preview: https://deploy-preview-16774--material-ui-x.netlify.app/x/react-date-pickers/time-picker/