Skip to content

[site] Replace theme menu with synced toggle - #1485

Open
e111077 wants to merge 4 commits into
mainfrom
theme-toggle-shared-worker
Open

[site] Replace theme menu with synced toggle#1485
e111077 wants to merge 4 commits into
mainfrom
theme-toggle-shared-worker

Conversation

@e111077

@e111077 e111077 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • replace the light/dark/system menu with a WAI-ARIA toggle button that starts from the resolved system preference
  • synchronize temporary theme overrides across open tabs with a SharedWorker, while resetting to the system preference after all tabs close
  • keep system media-query changes authoritative and preserve SSR hydration without an initial icon mismatch
  • audit every litdev-ripple-icon-button usage and fix disclosure state, accessible names, link semantics, and keyboard focus indication
  • Decreases 130kb raw bytes from homepage (20kb gzipped)

Testing

  • npm run build -w lit-dev-content
  • npm run build -w lit-dev-tests
  • focused Playwright coverage for theme SSR, keyboard toggling, media changes, cross-tab sync/reset, mobile navigation disclosure state, playground controls, and tutorial navigation
  • complete tutorial Playwright spec, including light/dark goldens
  • Chrome accessibility-tree verification with no console errors

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

A live preview of this PR will be available at the URL(s) below.
The latest URL will be appended to this comment on each push.
Each build takes ~5-10 minutes, and will 404 until finished.

https://pr1485-cbca93e---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1485-f69af27---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1485-816c64d---lit-dev-5ftespv5na-uc.a.run.app/
https://pr1485-46a45d3---lit-dev-5ftespv5na-uc.a.run.app/

@e111077

e111077 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

Updated all screenshot goldens from the macOS CI artifacts produced by Integration tests run 33547534756. The follow-up Integration tests run 33548989291 passed all 41 tests with no screenshot diffs.

@e111077

e111077 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed same-tab navigation resetting the theme in commit 816c64d3. Manual override state now uses per-tab sessionStorage as a navigation bridge, while the SharedWorker continues syncing open tabs; system preference changes clear the override. Verified on the deployed preview: light system → toggle dark → navigate to /docs/ remains dark with no console errors. Preview: https://pr1485-816c64d---lit-dev-5ftespv5na-uc.a.run.app/ . Remote integration run 33553807401 passed all 41 tests.

@e111077

e111077 commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

Removed the navigation icon flash in commit 46a45d3f. A parser-blocking classic initializer now applies any stored manual override to the SSR theme-switcher hosts immediately after the header and before main, while keeping the invariant two-icon SSR template intact. Added a regression test that blocks hydration and verifies the stored icon/mode/aria-pressed state is already correct. Remote integration run 33556991582 passed all 41 tests with no screenshot diffs. Verified on https://pr1485-46a45d3---lit-dev-5ftespv5na-uc.a.run.app/ with no console errors.

@e111077
e111077 marked this pull request as ready for review September 2, 2026 19:00
const storeColorMode = (mode: ColorMode | undefined) => {
try {
if (mode) {
sessionStorage.setItem(COLOR_MODE_STORAGE_KEY, mode);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

previously this was stored in local storage. with session storage the user's choice is lost when the tab is closed, though i suppose that is the only way to revert to "auto" with this new toggle.

i kind of think if the user did explicitly click the toggle to switch mode, we should persist that across sessions?

@e111077 e111077 Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is following Lea Verou's latest blogpost which I really vibed with.

If the user really cares about a specific mode, they probably would change their OS to that mode. SessionStorage stores it for the current tab for fast loads (if overridden), and SharedWorker should sync it across tabs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It looks like the current implementation violates this: https://lea.verou.me/blog/2026/dark-mode-toggles/#tri-state-toggles-are-implementation-driven-ui:~:text=When%20you%20press%20it%20for%20the%20first%20time%2C%20it%20toggles%20to%20the%20opposite%20of%20what%20you%E2%80%99re%20currently%20seeing%2C%20and%20stores%20the%20literal%20value%20(light%20or%20dark).%20The%20next%20time%20you%20press%20it%2C%20it%20toggles%20back%20to%20the%20system%20default%2C%20and%20removes%20the%20stored%20value.

Right now in the latest preview, with my system on light mode, opening the page there's no value stored. Clicking the toggle sets "dark". Clicking it again sets "light" where I would expect the value to be unset to revert to system setting.

I also don't see any rationale in the blog post that an override should only last the session. If the user chose to override, I think it should stay in localStorage to persist across sessions, as the first graphic in the blog post shows as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Additionally, if the state changes due to a change in system, it should reset the state to auto

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.

2 participants