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

Bug with switching to light mode for the first time with dark mode preference #2487

Closed
wookie184 opened this issue Mar 30, 2022 · 2 comments · Fixed by #2494
Closed

Bug with switching to light mode for the first time with dark mode preference #2487

wookie184 opened this issue Mar 30, 2022 · 2 comments · Fixed by #2494
Assignees
Labels
bug infra Core infrastructure for building and rendering PEPs

Comments

@wookie184
Copy link
Contributor

wookie184 commented Mar 30, 2022

When on a system with prefers-color-scheme as dark mode, switching to light mode for the first time requires two clicks.

The toggle colour scheme function:

const toggleColourScheme = () => {
if (localStorage.getItem("colour_scheme") === "dark") {
makeLight()
localStorage.setItem("colour_scheme", "light")
} else {
makeDark()
localStorage.setItem("colour_scheme", "dark")
}
}

If the page starts in dark mode, localStorage would not contain a theme value, so the second block there is called, and localStorage is set to dark meaning it works on the next click.

I think changing it to this would fix the issue:

 const toggleColourScheme = () => { 
     const colourScheme = localStorage.getItem("colour_scheme")
     const prefersDark = window.matchMedia("(prefers-color-scheme: dark)").matches

     if ((colourScheme === "dark") || (!colourScheme && prefersDark)) { 
         makeLight() 
         localStorage.setItem("colour_scheme", "light") 
     } else { 
         makeDark() 
         localStorage.setItem("colour_scheme", "dark") 
     } 
 } 

Would this be OK to PR?

@hugovk
Copy link
Member

hugovk commented Mar 31, 2022

Good point, when I try this in incognito on Android and starting with prefers dark mode, the first click flashes light mode and then back to dark.

Yes, please send a PR. It will create a build preview for ease of testing.

@CAM-Gerlach CAM-Gerlach added bug infra Core infrastructure for building and rendering PEPs labels Mar 31, 2022
@CAM-Gerlach
Copy link
Member

Agreed as well; I vaguely recall this being brought up before some time ago after the dark theme was first implemented. Yes, please send a PR, unless you'd prefer one of us to take care of it. Thanks!

@hugovk hugovk changed the title Very minor bug with switching to light mode for the first time with dark mode preference Bug with switching to light mode for the first time with dark mode preference Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug infra Core infrastructure for building and rendering PEPs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants