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(www): themes toggle #34

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

emiliosheinz
Copy link

Theme changes were only being applied when made from the system; however, when attempting to change it from the application header, the changes were not reflecting within the app.

Before:

Screen.Recording.2024-03-14.at.20.47.25.mov

After

Screen.Recording.2024-03-14.at.20.46.19.mov

Copy link

vercel bot commented Mar 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rodz-input-otp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 14, 2024 11:54pm

@guilhermerodz
Copy link
Owner

guilhermerodz commented Mar 15, 2024

hi @emiliosheinz now the system's prefers-color-scheme stopped working

CleanShot.2024-03-15.at.10.46.27.mp4

@emiliosheinz
Copy link
Author

emiliosheinz commented Mar 15, 2024

@guilhermerodz thank you for noticing. Weird that in my second video, when I change my system theme, it successfully changes the website theme. I'll look into that shortly but any thoughts are welcome.

@emiliosheinz
Copy link
Author

emiliosheinz commented Mar 15, 2024

@guilhermerodz, I have tested it locally, and based on my understanding, it is working as expected. The implementation I have employed ensures that the selection made on the website will always take precedence. In essence:

  1. If users select Light or Dark on the website's header it will take precedence over what is selected on the System (aka prefers-colors-scheme or OS theme).
  2. If users select System on the website's header but have no preferred color scheme it will fallback to what is set on the OS.
  3. If users select system on the website's header and have a preferred color scheme, the preferred color scheme will be applied, no matter what's the theme applied to the OS.

IMO having the preferred color scheme taking precedence over what is selected on the website will make the feature confusing for users since they might have a prefer color scheme set and when they try to change the theme on the header nothing will happen.

Basically, in your example the preferred color scheme didn't work because Dark was selected. To have your preferred color scheme applied you need to select System.

Video showcasing what I explained above:

Screen.Recording.2024-03-15.at.16.10.28.mov

Please, let me know your thoughts or concerns about this approach.

@guilhermerodz
Copy link
Owner

The theme isn't updating when there's no prefers-color-scheme, or if there's prefers-color-scheme:dark

CleanShot.2024-03-15.at.23.31.14.mp4

I still haven't debugged deeply to understand what's happening

@emiliosheinz
Copy link
Author

emiliosheinz commented Mar 16, 2024

@guilhermerodz Sorry for asking but are you sure you are testing the right branch? 😅

I'm asking because the behavior you described in your previous message and video is exactly what I'm fixing with this PR. When testing the theme toggle on https://input-otp.rodz.dev/ the behavior is exactly as in your video, but when trying the same thing on the branch with the fixes I applied it works as expected. Could you please double check and let me know the results?

Sorry if I'm missing something really obvious here. Thanks!

See the proof below:

Screen.Recording.2024-03-16.at.10.31.35.mov

@emiliosheinz
Copy link
Author

@guilhermerodz Did you have the chance of circling back to this? I would love to hear your thoughts.

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

Successfully merging this pull request may close these issues.

2 participants