-
Notifications
You must be signed in to change notification settings - Fork 843
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
Fixed scrollbar colors for dark mode #4969
Conversation
Uses the `euiScrollBar()` mixin at the `html` level and adding a 3rd parameter for `$size` with the default being `thin` (as it were) and allowing `thick` to maintain the larger size for the overall page. Caveat: This could mean Firefox instances of scrolling containers are no longer “thin” if they don’t use the mixin.
Preview documentation changes for this PR: https://eui.elastic.co/pr_4969/ |
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.
Tested and checked code. LGTM.
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.
Tested in Chrome, Safari, Edge, Brave and Firefox. I also looked at the code and LGTM! 🎉
But I noticed something... In all browsers, the scrollbar only appears when we start scrolling except in Chrome.
This is a screenshot of what I'm seeing without even interacting with the page in Chrome. The scrollbar is visible by default.
Before this change, the scrollbar would only appear once we started scrolling.
Is this new behavior expected?
Preview documentation changes for this PR: https://eui.elastic.co/pr_4969/ |
I'm gonna merge in, and keep an ear out for any concerns. Thanks both! |
Thanks for this! |
Fixes #4423
Uses the
euiScrollBar()
mixin at thehtml
level and adding a 3rd parameter for$size
with the default beingthin
(as it were) and allowingthick
to maintain the larger size for the overall page.Firefox: The
thin
usage used to be applied at the root level for Firefox, but now it's thethick
version. This could mean Firefox instances of scrolling containers are no longer “thin” if consumers didn't manually apply the mixin. But Chrome would have been using the stock scrollbar anyway.Safari: None of these styles affect Safari at the
html
level. But that's ok because it was already adhering to the theme switch.BEFORE

AFTER

Checklist
[ ] Props have proper autodocs and playground toggles[ ] Added documentation[ ] Checked Code Sandbox works for the any docs examples[ ] Added or updated jest tests[ ] Checked for breaking changes and labeled appropriately[ ] Checked for accessibility including keyboard-only and screenreader modes