Rework the theme selector ahead of the design refresh - #3259
Open
enf0rc3 wants to merge 3 commits into
Open
Conversation
Move theme handling onto a single contract in src/lib/theme.ts, shared by the pre-paint inline script, the client controller and the stylesheets. <html> now always carries data-theme="light" or "dark" (never absent, never "system"), so incoming design token stylesheets can key off the attribute with no :root fallback. data-theme-preference records the raw choice, including "system", ready for a three-position control. Fixes along the way: - The two switchers each cached their own state, so toggling one left the other stale. Both now read the same attribute. - The knob position came from a JS-applied class, so it painted on the light side and slid across on every load. It is now driven by html[data-theme]. - tabindex="-1" on the input plus tabindex="0" on the label made the focusable element a bare <label> with no role, name or state. The input is focusable again and the deprecated keyCode handler is gone. - A localStorage throw in the head script left the site permanently light. - color-scheme is set per theme, so native controls and scrollbars follow. - OS preference changes are followed until the user picks a side; other tabs stay in sync via the storage event. The controller moves from public/ into src/ so Astro bundles, minifies and dedupes it, and an astro:after-swap handler keeps it working if <ClientRouter /> is ever added. Adds tests/theme.spec.ts (7 tests) and PLAYWRIGHT_PORT so the suite can run beside a dev server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
enf0rc3
force-pushed
the
wl/theme-selector-rework
branch
from
July 28, 2026 04:41
84e1d85 to
8f97202
Compare
|
Pull request environment is available at https://stoctodocspr3259.z22.web.core.windows.net. You can view the ephemeral environment status in Octopus Deploy. This environment will be automatically deprovisioned when the pull request is closed, or after 7 days of inactivity. |
Moving to a native checkbox meant only Space toggled it - a checkbox ignores Enter. The switch role treats Enter as an optional second activation key and the control supported it before, so bind it explicitly on the input. Tests now press each key twice to check it toggles both ways. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
enf0rc3
marked this pull request as ready for review
July 28, 2026 05:29
- Route the Enter handler through box.click() so the change listener stays the only writer of theme state, instead of flipping box.checked separately. - Scope the dark slider rule to .theme-switcher so a future .switch-slider elsewhere does not inherit it. - Use auto-retrying toHaveAttribute assertions in the theme spec rather than one-shot getAttribute reads. - Document that theme:change does not fire at boot, and that the src/pages/report/*.astro pages render their own <html> and sit outside the data-theme contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Groundwork for the design refresh: makes theme management solid before the new colour/spacing/typography token CSS lands.
The contract for the incoming token files
<html>always carriesdata-theme="light"ordata-theme="dark"— never absent, never"system", set before first paint.[data-theme=light]/[data-theme=dark]selectors always match, so token files need no:rootfallback and no ordering rules.<html>also carriesdata-theme-preference(light/dark/system). Nothing styles off it; it is there so a future three-position control renders correctly.theme:changefires on<html>with{ theme, preference }.src/lib/theme.ts. Change them there and the pre-paint script, controller and tests follow.Bugs fixed
isDarkModeat construction, so toggling the mobile one left the header one stale. Resize past 1130px and it showed the wrong position; clicking it then setdata-themeto the value it already had..dark-modeclass, so in dark mode it painted on the light side and slid across after hydration. Now driven byhtml[data-theme].tabindex="-1"on the input plustabindex="0"on the label meant the focusable element was a bare<label>— no role, name or state. The realinput[role=switch]is focusable again, the deprecatedkeyCodehandler is gone, and the focus ring is drawn viainput:focus-visible + label.localStoragewas unguarded. A throw in the head script (Safari private mode, cookie-blocked iframes) killed the IIFE and left the site permanently light.<body data-theme="dark">was dead and misleading — all CSS targetshtml[data-theme].Also
color-schemeset per theme, so scrollbars and native controls follow.storageevent.public/intosrc/scripts/, so Astro bundles, minifies and dedupes it instead of shipping an unhashed public file loaded viais:inline.astro:after-swaphandler so it keeps working if<ClientRouter />is ever added.src/components/ThemeSwitcher.astro.Testing
tests/theme.spec.ts— 7 tests, all passing: OS default in both schemes, explicit choice surviving navigation, both switchers staying in sync, keyboard operation, storage-throws fallback, and a regression guard that the knob position is attribute-driven rather than script-driven.pnpm buildpasses (2697 pages).playwright.config.tsnow honoursPLAYWRIGHT_PORTso the suite can run alongside a dev server on 3000.Not touched:
class="initial"on<html>has no matching CSS anywhere, but I could not prove nothing external reads it.I have also manually tested the above bugs and can confirm they are resolved, i found one trivial thing that was fixed being pressing enter on the toggle when tabbing through ui
🤖 Generated with Claude Code