-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 global CSS switcher #42747
Conversation
Size Change: +625 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
dab6f36
to
f73988b
Compare
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.
Thanks for putting this together @mirka 👍
This tests as advertised for me. I tried several components within the Storybook and all appeared to get the correct styles when switching between the presets.
I did notice with the BorderBoxControl
that it was missing a box-sizing
reset as discussed within #42415.
I'm not certain exactly what you had in mind for a boxSizingReset
mixin but I've knocked up a PR (#42754) based off this branch that adds one and fixes the BorderBoxControl
. Happy to abandon it though if I've gone down the wrong track.
'https://wordpress.org/gutenberg/wp-admin/css/common.min.css', | ||
'https://wordpress.org/gutenberg/wp-admin/css/forms.min.css', |
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.
Do you think that we may end up in an "out-of-sync" situation between the version of the styles hosted on wordpress.org
and the styles that part of a given PR ?
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.
And we could be in trouble either way since components need to work with all versions of WP 😂
I don't expect there to be frequent changes to the kind of broadly scoped styles that are affecting us, but if we do encounter issues we could consider the Trac or GitHub-hosted mirror as a source.
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.
Fair point 😅 Let's keep it as it is, and make changes if necessary
Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
Closes #42717
Fixes #42414
What?
Implements a global CSS switcher add-on so we can test our component styles in different CSS environments.
Why?
Recently we've made some unexpected discoveries (#37555, #42414, #42415) about the CSS contexts that wp-components exist in. I was also bitten by this when working on #42000, when I wrote CSS for a component that looked good in Storybook but completely broke down when rendered in wp-admin, due to some broadly scoped styles in a globally loaded WP stylesheet.
We need to ensure that our components have sufficient styles to work in a non-WP context. At the same time, we need to ensure that components have styles that are defensive enough to hold up in a (sometimes adverse) WP context.
The purpose of this Storybook add-on is to help us easily switch between WP and non-WP CSS environments.
How?
First of all, we fix the style leakage issue in the Block Editor Playground. The reset.css that is meant for the Playground will now only be loaded in the Playground story.
Then, we implement a Storybook add-on where we can configure some presets to load and unload CSS files dynamically. I've set up three presets to begin with:
none
,basic
, andwordpress
. We can tweak these as we discover new surprises or our testing needs change.Testing Instructions
npm run storybook:dev
input
element for example, and see how styles from forms.css is affecting it when choosing the WordPress preset.Screenshots or screencast
CleanShot.2022-07-28.at.01.08.22.mp4