Skip to content
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

UI: Fix scrollbar color to be visible in dark theme #16345

Merged
merged 2 commits into from
Oct 16, 2021

Conversation

MichaelArestad
Copy link
Contributor

@yannbf pointed out to me the scrollbar handles are nearly invisible when using the dark theme. Let's fix that!

What I did

I modified the color to work well in both dark and light themes. I looked at the MacOS scrollbars in both MacOS dark/light modes to roughly match the contrast in both modes. It is also trivial to make each mode have a different color (that's what I did at first), but I think this works well alongside the MacOS scrollbar handles.

How to test

  1. Modify examples/official-storybook/manager.js to enable dark mode.
  2. Build and run Storybook.
  3. Open the addon panel and dock it to the side. It might help to resize it so it is fairly narrow.
  4. Scroll the tabs.
  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

Before

image

image

After

image

image

@MichaelArestad MichaelArestad added the maintenance User-facing maintenance tasks label Oct 13, 2021
@MichaelArestad MichaelArestad added this to the 6.4 improvements milestone Oct 13, 2021
@MichaelArestad MichaelArestad self-assigned this Oct 13, 2021
@nx-cloud
Copy link

nx-cloud bot commented Oct 13, 2021

Nx Cloud Report

CI ran the following commands for commit af9b9ef. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status Command
#000000 nx run-many --target=prepare --all --parallel --max-parallel=15

Sent with 💌 from NxCloud.

@domyen
Copy link
Member

domyen commented Oct 13, 2021 via email

@shilman shilman added the ui label Oct 14, 2021
@yannbf yannbf self-requested a review October 14, 2021 20:17
@yannbf
Copy link
Member

yannbf commented Oct 14, 2021

Super awesome seeing your contributions @MichaelArestad! I'm wondering if the color will be set for other scenarios as well like here

@MichaelArestad
Copy link
Contributor Author

Super awesome seeing your contributions @MichaelArestad! I'm wondering if the color will be set for other scenarios as well like here

You know, I'm not sure what's going on there, @yannbf. I am not entirely clear what Bar is for. From what I saw, I think theme.barBg is used for background colors of toolbars.

@yannbf
Copy link
Member

yannbf commented Oct 15, 2021

Super awesome seeing your contributions @MichaelArestad! I'm wondering if the color will be set for other scenarios as well like here

You know, I'm not sure what's going on there, @yannbf. I am not entirely clear what Bar is for. From what I saw, I think theme.barBg is used for background colors of toolbars.

I will try this out right now and get back to you!

@yannbf
Copy link
Member

yannbf commented Oct 15, 2021

Alright I just got confused! LGTM!!

@shilman shilman changed the title Update custom scrollbar color to be visible with the dark theme enabled UI: Fix custom scrollbar color to be visible in dark theme Oct 16, 2021
@shilman shilman changed the title UI: Fix custom scrollbar color to be visible in dark theme UI: Fix scrollbar color to be visible in dark theme Oct 16, 2021
@shilman shilman modified the milestones: 6.4 improvements, 6.4 PRs Oct 16, 2021
@shilman shilman merged commit 744efa6 into next Oct 16, 2021
@shilman shilman deleted the update-scroll-handle-color branch October 16, 2021 13:57
@MichaelArestad
Copy link
Contributor Author

Thanks, @yannbf !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance User-facing maintenance tasks ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants