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

Detect if user has prefers-dark-mode setting on #20

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

Conversation

popoway
Copy link

@popoway popoway commented Dec 2, 2019

initTheme() reads system dark mode settings and apply, if no darkSwitch localStorage is found.

@coliff coliff added the enhancement New feature or request label Dec 6, 2019
@coliff
Copy link
Owner

coliff commented Dec 12, 2019

hey there! thanks for the PR. This looks good! A couple of things though:

  • If a user has a system/browser dark-mode setting ON then when they visit the page the page will be dark. If they'd switch the dark-mode switch off on the page then that setting won't be saved as it removes the localStorage 'darkSwitch: dark' key.
    I think we should change the key name from 'darkSwicth' to 'darkMode' - and then have settings which are saved for both 'on' and 'off'. That way a user in dark mode could browse the site in light-mode if they wish. What do you think?

  • Using the system-preference for dark-mode is a great feature and I think setting the theme based on that will be useful for many sites, but I think it should be able to be opt-in (or opt-out) by the developer. I'm not too sure where that setting should be set though. Any thoughts on that? Maybe something like this does it: https://darkmodejs.learn.uno/#%E2%9A%99%EF%B8%8F-options

@rugk
Copy link

rugk commented Aug 12, 2021

This PR seems to be abandoned? Any news on that? I think this is a useful feature/good to merge…

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

Successfully merging this pull request may close these issues.

3 participants