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

Use OS color scheme to init light/dark mode (#2591) #2611

Merged
merged 2 commits into from Aug 9, 2022
Merged

Use OS color scheme to init light/dark mode (#2591) #2611

merged 2 commits into from Aug 9, 2022

Conversation

pt2121
Copy link
Contributor

@pt2121 pt2121 commented Aug 7, 2022

Closes #2591

I tried to implement what mentioned in the issue:

I think if you open up Dokka for the first time, it could choose light/dark mode based on OS theme, and then if the person decides to switch it to something else - remember that choice.

Copy link
Member

@IgnatBeresnev IgnatBeresnev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! I think you're on the right track! Thanks for deciding to implement this

@pt2121
Copy link
Contributor Author

pt2121 commented Aug 9, 2022

I updated my PR.
If a user has never selected the Dokka theme, do you want to change the theme when the user changes the OS theme as well?
If so, I can add the listener and toggle the class.

@IgnatBeresnev
Copy link
Member

If a user has never selected the Dokka theme, do you want to change the theme when the user changes the OS theme as well?

If you want, I don't think we'll object :) But I think it's fine the way it is - navigating to any page would trigger the script in the header, so the new theme should be picked up. We'll probably also have to re-do it anyway if/when Dokka migrates to react routing

Copy link
Member

@IgnatBeresnev IgnatBeresnev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, although I'm unable to test it properly (running a custom linux theme, so it would mess everything up). @vmishenev if you are able and have the theme toggle, could you please check?

Thanks once again for taking the initiative!

Copy link
Member

@vmishenev vmishenev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested it via simulation in a debugger. It works if a theme switcher has not been used.

@IgnatBeresnev IgnatBeresnev merged commit d3ff629 into Kotlin:master Aug 9, 2022
@pt2121 pt2121 deleted the pt/2591-auto-dark-mode branch August 9, 2022 15:41
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.

Automatic dark mode
3 participants