fix(review): show System in theme menu via shared mode list#1015
Conversation
|
@backnotprop mind taking a look when you get a chance? hope it's ok I skipped the issue since it's tiny QoL stuff.. |
The review Options menu offered only Light/Dark while the plan, annotate, and Settings surfaces offered System too. Four surfaces each hand-listed the theme modes independently, so the review picker had silently dropped `system` and could never highlight it (its `activeTheme` memo collapsed system to the resolved light/dark). Introduce `THEME_MODES` as one shared list of the mode set with each mode's label and icon; derive `Mode` from it so a mode cannot exist without a render entry, and no surface can omit one. All four pickers (plan, review, Settings, landing toggle) now map the same list; active state is the selected mode everywhere. 🩸 Co-Authored-By: Dexter - @gwynnnplaine's Pi (https://pi.dev/) assistant
Preserve the published Mode import, expose shared mode data through a supported package export, and keep System selection coherent with constrained palettes and live OS changes. Scope the wider menu geometry to the two theme pickers and add public-consumer and focused DOM coverage. Co-authored-by: sasha <kredich05@gmail.com>
eb83531 to
8f7913d
Compare
|
Maintainer repair pushed at exact head This keeps #1015's Light/Dark/System single source of truth while preserving the published Local validation is green: full typecheck; 1,946-test core suite; 108-test DOM CI suite; strict public-consumer compile; hook, review, OpenCode, Pi, VS Code, portal, marketing, and UI CSS builds. Browser checks covered plan, review, annotate, Settings, reload persistence, menu geometry, single-mode palettes, and populated Pierre shadow roots in Light/Dark/System. Live OS preference changes are covered by the focused Useful constrained-palette and menu-geometry design from #1047 was incorporated without its duplicated mode list or global ActionMenu widening. Its contributor, sasha ( Two independent exact-head reviews (Standards and Spec) are green with no remaining findings. Waiting for GitHub CI before squash merge. |
b08946e to
4fb9fe5
Compare
2 participants
Small QoL fix. I've sent a couple of little patches before (#523); while using code review I hit a papercut worth fixing.
The papercut
The code-review Options → Theme menu only offers Light/Dark, but plan review, annotate, and Settings all offer System too — so "follow OS theme" silently wasn't available in review.
Root cause
Each surface hand-listed its own theme modes (
PlanHeaderMenu,ReviewHeaderMenu,ThemeTab,ModeToggle). The review picker had droppedsystem, and itsactiveThemememo collapsedsystem→ the resolved light/dark, so a System button couldn't have shown as active anyway.Changes
packages/ui/components/themeModes.ts(THEME_MODES), carrying each mode's id + label + icon.Modeis now derived from that list, so a mode can't exist without a render entry and no surface can omit one (no separate completeness guard needed).ReviewHeaderMenu'sactiveThemememo and its local Sun/Moon icons.Verification
typecheck clean ·
apps/reviewbuild +build:hookgreen · manual:plannotator reviewnow shows Light/Dark/System (System selectable, follows the OS); plan/annotate/Settings/landing unchanged.Not in this PR
ThemeProviderstill exposes redundanttheme/setThemeandmode/setModealiases — happy to collapse those separately if you'd like.