-
Notifications
You must be signed in to change notification settings - Fork 673
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
Use media query value when useColorSchemeMediaQuery is set to true #981
Use media query value when useColorSchemeMediaQuery is set to true #981
Conversation
And save to localStorage even if useColorScheme...: true, to make the flash prevention script work.
Thinking about an issue with this. The pr as is gives sense to the What if the user has Example use case: The end user having three options: light, dark and auto. Very popular pattern.
To enable "auto", the programmer just has to clear localStorage. Let me know what you guys think and I'll update. |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/systemui/theme-ui/gqe7nm3c3 |
Personally, I'd like to make this a separate option from the existing option. I think there are different usecases for these functionalities, & while personally I'd use it on most sites, I can definitely think of applications where you'd want other color modes but not use the media query live. I'm not quite sure how to name these differently:
|
@lachlanjc (insert state machines meme) What do you think about this?
I think I'd prefer to have a special color mode |
Brilliant. Yeah, let’s do the string solution, that’s way better than conflicting Booleans! |
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.
This is awesome work. Before we merge, let's implement this as a separate option from the existing functionality, using @hasparus's suggestion for syntax:
useColorSchemeMediaQuery: 'live' | 'initial' | true (same as initial for backwards compatibility) | false
hello there! |
@albertusdev We’d still love for this feature to be implemented, but as this PR is stale, if you’d like to update it with the API I mentioned, that would be much-appreciated! |
🚀 PR was released in |
Currently, even if
useColorSchemeMediaQuery: true
, the color mode stored in localStorage wins. Also, the color mode from the media query gets stored to localStorage.Update (4814af8): I made the media query color mode get stored in localStorage again, to make the initial flash prevention script still work.
Fixes #787
Absolutely loving theme-ui btw.