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

Add system to ThemeSwitch #699

Merged
merged 1 commit into from Apr 17, 2024
Merged

Add system to ThemeSwitch #699

merged 1 commit into from Apr 17, 2024

Conversation

pavelzw
Copy link
Contributor

@pavelzw pavelzw commented Aug 26, 2023

The current implementation doesn't allow changing back to system once you changed the theme once since it's stored in localstorage by next-themes. This PR adds a RadioGroup to choose from all three states similar to https://tailwindcss.com/.

For an example of how this looks like in action see https://pavel.pink/ / https://github.com/pavelzw/blog

Bildschirmaufnahme.2023-08-26.um.14.46.21.mov

@vercel
Copy link

vercel bot commented Aug 26, 2023

Someone is attempting to deploy a commit to a Personal Account owned by @timlrx on Vercel.

@timlrx first needs to authorize it.

@timlrx
Copy link
Owner

timlrx commented Aug 28, 2023

Hi @pavelzw, thanks for the contribution. I intentionally did it this way - by default it follows your system theme but once a user selects a preference, I don't see a situation where the user would revert back to system instead of just switching to the preferred theme.

Happy to link your blog instead in the examples list for a reference to the implementation for those that might prefer it this way.

@Mood93
Copy link

Mood93 commented Dec 3, 2023

Hi @timlrx, what about if they clicked dark/light out of curiosity, then decided they won't it to go with their system. For example, on my Mac I have it set to switch to dark mode at sunset, then light mode at sunrise.

Am I explaining the scenario well? In this situation, I would be annoyed with the site if I couldn't switch to 'System'.

@timlrx
Copy link
Owner

timlrx commented Dec 3, 2023

@Mood93 Did not realised that's how auto works on the Mac. I am convinced by your argument and it appears the upvotes tilt in favour of supporting it as well 😄

@pavelzw do you mind handling the merge conflict? You could use resolvedTheme from useTheme as per the new code to deduce the settings based on prefers-color-scheme instead of adding an additional hook. Since it has been a while since the PR was created, let me know if you are are busy and I can also do it. Many thanks!

@pavelzw
Copy link
Contributor Author

pavelzw commented Dec 3, 2023

do you mind handling the merge conflict

yeah, I can take a look at it later 👍🏻

@pavelzw pavelzw force-pushed the theme-selector branch 2 times, most recently from 0b7403f to 9f6ef66 Compare December 4, 2023 09:13
@pavelzw
Copy link
Contributor Author

pavelzw commented Dec 4, 2023

@timlrx ready for review

@pavelzw
Copy link
Contributor Author

pavelzw commented Jan 24, 2024

@timlrx what's the status of this PR? From my side it can be merged.

@timlrx timlrx merged commit 7ffdcc8 into timlrx:main Apr 17, 2024
1 check failed
@timlrx
Copy link
Owner

timlrx commented Apr 17, 2024

Apologies for the delay! I have merged it in. Thanks for the contribution!

@pavelzw pavelzw deleted the theme-selector branch April 17, 2024 08:39
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.

None yet

3 participants