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

dark mode for AMP pages (it should be automatic) #20951

Closed
willfs84 opened this issue Feb 20, 2019 · 14 comments · Fixed by #36958
Closed

dark mode for AMP pages (it should be automatic) #20951

willfs84 opened this issue Feb 20, 2019 · 14 comments · Fixed by #36958

Comments

@willfs84
Copy link

apologies if this is already being worked on or has already been ruled out but AMP provides an amazing opportunity to easily, quickly and properly make a 'dark mode' for a huge amount of web pages. More companies are looking at doing this for normal webpages (Samsung Internet Browser, Firefox on iOS and now Google Chrome, see this post for more info https://9to5google.com/2019/02/19/android-chrome-webview-web-dark-mode/ ) . Of course it would be ideal if this would happen automatically (if your OS or browser is set to dark mode, AMP should be able to see this and serve up a dark version of the AMP page).

@archon810
Copy link

We're looking to do this as well, plus give the user a choice of auto / dark / normal, and save the preference in a cookie or localstorage.

What needs to happen to make these work?

@westonruter
Copy link
Member

I think there also needs to be support for a user to set some persistent preference to indicate whether they want dark mode or light mode. This preference could then be applied in the page, for example, by amp-dynamic-css-classes. But maybe this can be done at the framework level.

cc @tomayac

@westonruter
Copy link
Member

It would be great if the dark mode preference could persist with the user as they navigate between arbitrary AMP pages across multiple origins, though this has some privacy implications.

@tomayac
Copy link
Contributor

tomayac commented Oct 4, 2019

The requirements you mentioned are all covered by my <dark-mode-toggle>. In an AMP context you would use it with a class that you can toggle on and off, which is called method ② in the documentation. I’m not sure what it would take to convert my custom element into an AMP element, but maybe the logic at least could be reused. Happy to help out or answer questions you may have.

@westonruter
Copy link
Member

The only way to do this in AMP currently as I can tell is use amp-script to get/set the user preference for dark/light mode in localStorage. Since amp-script only allows mutations to children elements, it would not be able to change a class name on the body. In order to do this, amp-script would need to call AMP.setState() to modify the page state as a whole to set some flag which is then used to update the body class. However, I'm not clear on whether setting state in this way should be permissible on page load before a user gesture. See #24862 (comment).

@westonruter
Copy link
Member

OK, this approach using amp-script will not work. Using amp-script to update state would only update bindings inside the subtree, so this could not be used to update the class on the body. See #24862 (comment).

@archon810
Copy link

Do we maybe need a special component for dark mode? One that would support storing preferences for auto/light/dark?

@a-haan
Copy link

a-haan commented Oct 27, 2019

I've implemented Prefers-color-scheme into my AMP site, this works perfectly for enabling dark mode on my site when the system is set to dark mode.

@tomayac
Copy link
Contributor

tomayac commented Oct 27, 2019

I've implemented Prefers-color-scheme into my AMP site

Sure, getting it to work is perfectly feasible. The discussion in the lower part of the thread is around a toggle as in the footer on v8.dev.

@tomayac
Copy link
Contributor

tomayac commented Nov 19, 2019

Here's a codepen with a simple theme switcher for dark and light mode (note that this does not adhere to prefers-color-scheme, but acts as a manual switch):

Screen Shot 2019-11-19 at 13 59 23

Screen Shot 2019-11-19 at 13 59 14

@willfs84
Copy link
Author

Also, the AMP UI that appears in browsers should respect dark mode too (it doesn't, see attached screenshot). Surely this an easy and fast fix?
AMPbrowserdarkUI

@ProH4Ck
Copy link

ProH4Ck commented May 2, 2020

Any news on this?

@asadkn
Copy link

asadkn commented Jan 19, 2021

Definitely need a fix for this. While relying on prefers-color-scheme is an option, it's not ideal. Just because I prefer light scheme globally doesn't mean I don't wish to have some websites in dark mode.

@deepaklalwani97
Copy link
Contributor

Hello 👋, I have added an initial draft PR #36958 to add support for the dark theme. I have registered a global action that can be used to toggle the dark theme mode classes on the body and store the user's preferences in local storage and by default relies on prefers-color-scheme value if the local storage is not set. As the usage of localStorage is forbidden, how can proceed to get permission for the usage of localStorage in this scenario?

dark-mode

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

Successfully merging a pull request may close this issue.

10 participants