fix: report the active theme in the toggle menu - #56
Merged
Conversation
ryaneggz
changed the base branch from
fix/theme-auto-os-dark-and-cross-tab
to
development
August 3, 2026 00:23
The theme menu offered three destinations and reported no state. Plain DropdownMenuItems carry role="menuitem" with no checked semantics, and the trigger's accessible name was the constant "Toggle theme" - so there was nowhere in the website UI that told a non-sighted user which theme was active. Sighted users read it off the page itself; screen-reader users had nothing. Switched to DropdownMenuRadioGroup/RadioItem, which the design system already exports and which nothing was using. Each option now reports role="menuitemradio" with aria-checked, and the trigger names the current selection - including which way System currently resolves, since "System" alone still does not say whether the page is light or dark right now. This is pre-existing debt, not fallout from the time-of-day default: the timezone change only wrapped the onClick handlers, and left the plain-item structure untouched. It is fixed here because it is the same control. The state-dependent name is gated behind a mounted flag. The active theme is unknowable during SSR and is resolved from the DOM on the client, so rendering it before mount would hydrate against markup the server could not produce. The server-rendered name stays "Toggle theme" - verified: the built HTML contains that string and zero occurrences of "Theme: ". Verified in a browser by reading the accessibility properties, not the source: role=menuitemradio on all three with aria-checked true/false/false; trigger "Theme: Light" under the automatic default, "Theme: System (currently light)" after choosing System, and "Theme: System (currently dark)" once the OS preference flips - which is the one case where following the OS live is correct, because the user asked for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ryaneggz
force-pushed
the
fix/theme-toggle-state-aria
branch
from
August 3, 2026 00:24
6aa2493 to
78bd428
Compare
✅ Deploy Preview for promptengineers ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This was referenced Aug 3, 2026
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
1 participant
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.
The theme menu offered three destinations and reported no state.
DropdownMenuItemcarriesrole="menuitem"with no checked semantics, and the trigger's accessible name was the constant"Toggle theme". So there was nowhere in the website UI that reported which theme is active. Sighted users read it off the page; screen-reader users had nothing.Slightly ironic against the console, which already exposes
aria-pressedand a literal "On"/"Off" in its authenticated menu.Fix
DropdownMenuRadioGroup/DropdownMenuRadioItem— both already exported by the design system (ui/dropdown-menu.tsx:19,120) and used nowhere. Each option now reportsrole="menuitemradio"witharia-checked, and gets the existingItemIndicatordot for free.The trigger names the current selection. "System" alone still doesn't tell you whether the page is light or dark right now, so that case resolves:
Theme: System (currently dark).This is pre-existing debt
git show 2b66e7a -- mode-toggle.tsxshows the time-of-day change only wrapped theonClickhandlers. The plain-item structure predates it and was untouched. It ships here because it is the same control, not because that feature caused it.Hydration
The state-dependent name is gated behind a
mountedflag. The active theme is unknowable during SSR and is resolved from the DOM on the client, so rendering it before mount would hydrate against markup the server could not have produced.Verified against the built HTML: it contains
Toggle themeand zero occurrences ofTheme:.Verification
Accessibility properties read from a live browser, not from source.
Theme: Lightmenuitemradio×3 —aria-checked=true/false/falseTheme: System (currently light)theme-choice: systempersistedTheme: System (currently dark)Note on the automatic default
Under the automatic default the menu shows Light or Dark checked, because that is genuinely the active theme. It does not claim the user chose it — nothing is persisted until they do.