-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Addon-docs: Fix style bleeding #21150
Addon-docs: Fix style bleeding #21150
Conversation
…g-storybook-docs-css-cascading-into-our-components
I have an issue with our UI Storybook here that I'm unsure how to solve. Basically this PR moves the styles from the DocsPage component to be global instead with Emotion's The problem is with the stories for any doc block, and the theme switcher. Our theme switcher tool has this "stacked" mode that shows the story in both light and dark theme, by rendering the story twice (see https://github.com/storybookjs/storybook/blob/20600-bug-storybook-docs-css-cascading-into-our-components/code/ui/.storybook/preview.tsx/#L196-L214). But when you render the DocsPage twice, it will set the global styles first with the light theme and then with the dark theme. And because they're global styles it means that both instances will end up with the dark theme because the global styles will override eachother, which makes the stacked view unusable. As an example, see this story in light theme: I can think of a few ways to get around this, but none of them are great so I'm hoping someone else will come up with better ideas.
|
I don't really have an opinion on this solution but it does seem like a fairly large change to make if you aren't sure it's super beneficial.
We probably only need one or two stories per doc block in the dark theme, I'm guessing? So maybe this solution isn't so onerous? The stacked approach is a bit of a hack in any case, I guess a better thing would be for Chromatic to just support multiple themes already ... |
…nts' of github.com:storybookjs/storybook into 20600-bug-storybook-docs-css-cascading-into-our-components
You're right, let's not overcomplicate things if we don't know that it brings value. I reverted that part. |
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.
Don't entirely grok this but it seems like an improvement
Inspired by storybookjs/storybook#21150
Closes #20600
What I did
Change the "global" docs styles from being set on the DocsContent component to being actually global. This allows us to wrap them in a CSS Cascade Layer, which has lower specificity than un-layered styles that the user provides.Reverted.sb-unstyled
+ known blocks to only targeting.sb-unstyled
, and then let the blocks add that class. This simplified the global selector greatly. See initial discussion (Docs: using targetted styles, not components to style MDX #19958 (comment)How to test
Here's an example MDX I wrote that shows how styles no longer bleed into stories. There's a global
font-family: Cambria
, which has ultra-low specificity but is still being respected in the button. It wasn't previously.Checklist
MIGRATION.MD
Maintainers
make sure to add the
ci:merged
orci:daily
GH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]