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: Default white preview background #21593

Conversation

JReinhold
Copy link
Contributor

Closes #21080

What I did

Set the preview iframe to default to a white background. Previously it was transparent, meaning that Storybook in dark mode would cause a dark background, which is unexpected given that browsers default to a white background - only the user's code should change the background color, not Storybook's code.

Setting the color on the iframe element ensures that user code that sets it on html, body, or something else within the iframe still overrides it.

I've tested that

  1. this still works with the backgrounds addon
  2. it's still possible for user code to change the background, even based on browser theme, with:
@media screen and (prefers-color-scheme: dark) {
  body {
    background-color: darkblue;
    color: lightgray;
  }
}

How to test

  1. Open one of the published sandboxes (eg. this one:)
  2. Navigate to one of the page stories:
  3. Change your browser color scheme to dark (see screenshot) and reload
  4. see that Storybook UI goes dark, but the preview stays white.

Feel free to test this out locally with custom css in stories.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a note to MIGRATION.md?

@JReinhold
Copy link
Contributor Author

I felt I heard someone saying that SB dark mode (in manager UI) was only introduced in 7.0, which is what's caused this bug. But my memory might be wrong?

@MichaelArestad
Copy link
Contributor

I am noticing on cra/default, the light and dark themes don't seem to do anything, but I don't think that's a regression with this PR. The backgrounds addon still works just fine. Nice work!

@MichaelArestad
Copy link
Contributor

Can you also change the Backgrounds addon to not default to light mode? (in the "cleared" state)

@JReinhold
Copy link
Contributor Author

Can you also change the Backgrounds addon to not default to light mode? (in the "cleared" state)

I'm not sure what you mean by this @MichaelArestad . When it's cleared it doesn't set anything, as if it wasn't there?

@MichaelArestad
Copy link
Contributor

@JReinhold
@yannbf mentioned that Before you made this fix, the Backgrounds addon (used on the Example storybook) was changed to default to light so that the background color wouldn't change when dark mode was enabled. If that's the case, we should revert that change.

@JReinhold
Copy link
Contributor Author

JReinhold commented Mar 14, 2023

Aha I see what you mean, are you referring to #20982? I've reverted it as I think it makes sense. Any objections @ndelangen?

@ndelangen
Copy link
Member

None @JReinhold

@ndelangen ndelangen merged commit 09ae40f into next Mar 15, 2023
56 checks passed
@ndelangen ndelangen deleted the 21080-bug-preview-has-dark-background-when-browser-is-set-to-dark-mode branch March 15, 2023 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Preview has dark background when browser is set to dark mode
4 participants