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

Fix: current param not being respected by the dark OS color mode #207

Conversation

beltranrengifo
Copy link
Contributor

@beltranrengifo beltranrengifo commented Dec 21, 2022

Issue #206

Motivation

As described in the issue, the current param seems not to be respected.
When loading the Storybook with OS color schema as dark, a default initialMode is always created with the dark value, so the re-evaluation of the current param value never happens again.
I wrote my initial thoughts here in the issue.

Solution proposed

Respect the param

For this purpose, I added a new property in the store to explicitly control if the user has clicked in the theme handler. If so, we respect the user decision. If not, we respect the current param, and at the end we look at the preferred color schema.

Why a new property? I think relying on the initialMode is not safe because it is set from multiple side effects. Having a unique property ensure we know if the user has set a preference.

Not reacting to OS color schema changes

I decided to add a check to NOT react to color schema changes in the OS because of this two cases:

  • To respect current property if set
  • To respect the user decision, if interaction has occur.

How to test

  • Run the example Storybook with the current param set to light
// /examples/basic/.storybook/preview.js
export const parameters = {
  darkMode: {
    current: 'light',
  },
};
  • Set your color schema to dark in the OS
  • Open the SB
  • Is expected the SB is in light mode
  • Change the theme using the handler and reload
  • Is expected the SB reloads with the dark theme, as it is the user choice
  • Clear the storage localStorage.clear()
  • Remove the current param
  • Open SB
  • Is expected the SB loads in dark mode (or your current choice in OS color schema)
  • At this point, the changes in the OS color schema should affect the SB theme, should be reactive

@beltranrengifo beltranrengifo changed the title Fix: current param not being respected by the dark OS color mode Fix: current param not being respected by the dark OS color mode Dec 21, 2022
@beltranrengifo
Copy link
Contributor Author

hi @hipstersmoothie !
hope you could take a look at this soon
let me know if I can help
😃

@hipstersmoothie hipstersmoothie merged commit 82228c0 into hipstersmoothie:master Dec 29, 2022
@hipstersmoothie
Copy link
Owner

🚀 PR was released in v2.0.5 🚀

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.

2 participants