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 dark mode support in twenty twenty-one theme #6874

Conversation

thelovekesh
Copy link
Collaborator

Summary

Fixes #6783

  • Add support to toggle dark and light mode using AMP.toggleTheme component
  • Add dark mode class name to $class_names[] so that dark mode class used by AMP.toggleTheme can be prevented from being tree-shaken.
  • Remove the dark mode toggler button notice from the dark mode customizer.

See recorded screencast

c2011e824f.mp4

Checklist

  • My code is tested and passes existing tests.
  • My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

@CLAassistant
Copy link

CLAassistant commented Feb 1, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@delawski delawski left a comment

Choose a reason for hiding this comment

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

The dark mode toggle is now rendered on AMP pages when Twenty Twenty-One theme is activated. The color mode preference is persisted across visits.

There's a minor difference between AMP and non-AMP versions: the mode toggle is not hidden/shown on scroll:

tt1-dark-mode-toggle.mp4

I don't think it's a blocker but if it's something relatively easy to achieve, it'd be nice to have.

includes/sanitizers/class-amp-core-theme-sanitizer.php Outdated Show resolved Hide resolved
includes/sanitizers/class-amp-core-theme-sanitizer.php Outdated Show resolved Hide resolved
@delawski delawski added the Enhancement New feature or improvement of an existing one label Feb 9, 2022
@delawski delawski added this to the v2.2.2 milestone Feb 9, 2022
@delawski
Copy link
Collaborator

@thelovekesh It seems that the unit tests need to be updated. Can you have a look into that?

@thelovekesh
Copy link
Collaborator Author

thelovekesh commented Feb 21, 2022

It seems that the unit tests need to be updated. Can you have a look into that?

@delawski Updated the test cases.

Copy link
Collaborator

@delawski delawski left a comment

Choose a reason for hiding this comment

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

@westonruter I think this is now ready for your review.

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

Thanks a lot for working on this. Great that this includes not only dark mode support for Twenty Twenty-One but also updating the style sanitizer so other themes can implement dark mode.

includes/sanitizers/class-amp-core-theme-sanitizer.php Outdated Show resolved Hide resolved
@westonruter westonruter merged commit fd1efd8 into ampproject:develop Mar 1, 2022
westonruter added a commit that referenced this pull request Mar 1, 2022
Co-authored-by: thelovekesh <lovekesh.kumar@rtcamp.com>
@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Apr 4, 2022
@thelovekesh thelovekesh deleted the enhancement/6783-dark-mode-support-in-twenty-twenty-one branch June 8, 2022 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelogged Whether the issue/PR has been added to release notes. Enhancement New feature or improvement of an existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for dark mode user toggle in Twenty Twenty-One
4 participants