-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Storybook: Add theme switcher tool #44715
Conversation
5f5984f
to
ce15b70
Compare
Storybook-related code changes look good, but I'll give this PR a final look once #44668 has been merged and this PR rebased.
Good point! This also made me realize that components (like Button) where the background color changes dinamically (i.e. on hover, focus...) may pose a challenge when it comes to text color contrast: would there ever be a situation in which the test has enough contrast in the button's default state, but doesn't have enough contrast when the button is hovered? Is that something that we'd need to take care of, or can we ignore it at least for now? |
It's an interesting problem! I wouldn't say it's a requirement, given that one can always unhover or unfocus, and there will be by definition a very limited number of primary buttons on any given page. Let's see what happens in practice and decide if it's worth the extra complexity to address. |
67ec72d
to
f2d504f
Compare
Size Change: 0 B Total Size: 1.27 MB ℹ️ View Unchanged
|
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.
🚀
a38b6d3
to
a4697c8
Compare
Part of #44116
Based on #44668
What?
Adds a theme switcher tool to the Storybook toolbar to test component support of the new experimental
Theme
component.Why?
We need to test whether the new configurable colors we introduce are supported correctly by all components.
How?
Adds a new toolbar add-on with some preset accent colors.
The orange accent color ("Sunrise") was chosen intentionally, because for sufficient contrast, the foreground text should be black when this orange is used as a background color. As can be seen with the primary variant of the Button component in the video, we don't support automatic text color switching yet. This is one of our next steps to work on.
Testing Instructions
npm run storybook:dev
Screenshots or screencast
CleanShot.2022-10-05.at.17.27.21.mp4