fix(review): show System in theme menu via shared mode list#1015
Open
gwynnnplaine wants to merge 1 commit into
Open
fix(review): show System in theme menu via shared mode list#1015gwynnnplaine wants to merge 1 commit into
gwynnnplaine wants to merge 1 commit into
Conversation
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
Contributor
Author
|
@backnotprop mind taking a look when you get a chance? hope it's ok I skipped the issue since it's tiny QoL stuff.. |
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.
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.