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

[Bug]: Preview has dark background when browser is set to dark mode #21080

Closed
vanessayuenn opened this issue Feb 14, 2023 · 6 comments · Fixed by #21593
Closed

[Bug]: Preview has dark background when browser is set to dark mode #21080

vanessayuenn opened this issue Feb 14, 2023 · 6 comments · Fixed by #21593
Assignees
Labels

Comments

@vanessayuenn
Copy link
Contributor

Is your feature request related to a problem? Please describe

https://discord.com/channels/486522875931656193/489892645087215636/1016356422956630016 In 7.0, we automatically set the SB theme based on user preferences. But this ends up visually breaking the example components that come out of the box with sb init.

Describe the solution you'd like

I think this could be fixed by setting the background: white. That way, all example components stick with a light theme.
(Open to other suggestions)

Describe alternatives you've considered

No response

Are you able to assist to bring the feature to reality?

no

Additional context

@domyen:
Stackblitz had the exact same problem we have. They set the iframe bg color to white when color-scheme was in dark mode.

Something like this:

:root.theme-dark {
  color-scheme: dark;
}

.preview-iframe {
  /* tell browsers to make the iframe opaque
     for documents with color-scheme:dark */
  color-scheme: light;
  background-color: white;
}

More context? See the long discussion in Slack here.

@JReinhold:
I think that is a sound solution, let's try that out when we work on this.

One thing I don't understand, is why we can't just always set the iframe to a white background, regardless of the color scheme. In my head that would mimic browsers more closely, which always have white backgrounds - and I would assume that the story's style would override the background color anyway because they're children of the iframe.

But it's just a note to self, I'll experiment with both.

@MichaelArestad
Copy link
Contributor

One thing I don't understand, is why we can't just always set the iframe to a white background, regardless of the color scheme.

This is definitely what we should do. I would consider it a blocker as it's a bit unexpected to see the background color change when the user happens to have their OS setting in Dark Mode.

@domyen
Copy link
Member

domyen commented Mar 3, 2023

Agree with it being a blocker to 7.0. When developing UI, I'd be surprised if the background of the canvas changed from under me.

@JReinhold
Copy link
Contributor

cc @shilman @vanessayuenn regarding priority.

@shilman
Copy link
Member

shilman commented Mar 6, 2023

I would classify this as a bug, not a feature and that it's the most important UI bug on our plate since (1) it's the very first thing a user will see when they upgrade to 7 and have dark mode system settings, (2) it renders SB useless in many cases, even if it's cosmetic. I'd say required for GA and ideally even before RC

@shilman
Copy link
Member

shilman commented Mar 6, 2023

@JReinhold discussed with @vanessayuenn and we'd prioritize this over #21415

@JReinhold JReinhold changed the title [Feature Request]: Fix default CLI template for dark theme [Bug]: Preview has dark background when browser is set to dark mode Mar 10, 2023
@JReinhold JReinhold linked a pull request Mar 14, 2023 that will close this issue
5 tasks
@shilman
Copy link
Member

shilman commented Mar 17, 2023

Yay!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-rc.4 containing PR #21593 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb@next upgrade --prerelease

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants