Skip to content

feat(themes): named custom color schemes - #569

Merged
Juliusolsson05 merged 10 commits into
mainfrom
feat/named-custom-themes
Jul 18, 2026
Merged

Juliusolsson05 merged 10 commits into
mainfrom
feat/named-custom-themes

Conversation

@Juliusolsson05

Copy link
Copy Markdown
Owner

Closes #566.

Replaces the single unnamed customAppearanceJson slot with named, selectable, editable saved themes.

Theme
┌────────────────────────────┬────────────────────────────┐
│ ▪ Dark                     │   Gray Dark                │
├────────────────────────────┼────────────────────────────┤
│   Tokyonight               │   Light                    │
├────────────────────────────┼────────────────────────────┤
│   Soft Light               │   Nord Night        CUSTOM │
├────────────────────────────┼────────────────────────────┤
│   Paper Warm        CUSTOM │   + New theme…             │
└────────────────────────────┴────────────────────────────┘

Saved themes sit in the same grid as built-ins, tagged CUSTOM, with Edit/Delete revealed on hover — mirroring how custom prompt templates already render in the command palette. Hover-only actions also respond to focus-within so keyboard users aren't stranded.

Design

Spec: docs/superpowers/specs/2026-07-18-named-custom-themes-design.md
Plan: docs/superpowers/plans/2026-07-18-named-custom-themes.md

The theme id doubles as Settings.mode. mode holds either a built-in id or theme:<uuid>. The alternative — mode: 'custom' plus a separate activeSavedThemeId — admits an invalid state (mode is dark but a theme id is also set) that every consumer would have to defend against. One field still answers "what am I looking at" for applyTheme, useThemeSync, and the paired phone client alike.

resolveThemePayload(settings) is the new seam between "which theme" and "what colors". applyTheme calls it instead of branching on mode === 'custom'; everything downstream (the 81 inline custom properties, the theme-changed event, the xterm/Monaco re-reads) is untouched.

+ New theme… seeds from getComputedStyle on <html>. This is the only way to reach built-in palette values — they exist solely as CSS in [data-mode] blocks, and THEME_MODES carries just {id, label, family}. Same mechanism readXtermTheme() already uses. It makes "duplicate Tokyonight and tweak it" work without copying palettes into TS, and means a new theme never starts as an unreadable blank.

Themes store raw JSON strings, preserving the documented reason the single slot did: the editor is a text editor, and formatting/key order should survive a round trip.

Settings version 4 → 5 with a real migration. An existing mode: 'custom' user gets their payload converted into a saved theme named "Custom" and stays on it. The conversion lives in coerceSettings rather than only in zustand's migrate, so it also repairs same-version-but-stale blobs; a name guard makes it idempotent.

Opportunistic cleanup

  • relativeLuminance moved from private-in-xtermTheme.ts to lib/color/luminance.ts — the chrome's light/dark decision and xterm's ANSI ramp now answer from the same math instead of two copies.
  • Deleted the stale THEME_MODES comment explaining that Custom sat at index 5 to make the Appearance grid an even 3×2. That arrangement no longer exists.
  • Replaced (rather than deleted) the AppearanceMenu comment excluding Custom. Its three reasons — "not a one-click preset", "needs the JSON editor", "default payload starts as the dark theme" — all stopped being true, so saved themes now appear there. Accent/contrast stay hidden while a saved theme is active, because inline properties outrank both.

Verification

Type-checked clean, both projects: tsc -p tsconfig.node.json --noEmit and tsc -p tsconfig.web.json --noEmit each report 0 errors. (remote-client is inside tsconfig.web.json, so the phone client compiled too.) Note this required building the node project once first — a fresh worktree otherwise reports only TS6305 project-reference staleness and never checks real code.

App boots clean: npm run dev builds main + preload + renderer and starts Electron with no errors.

Migration and coercion logic exercised with a temporary vitest harness — 9 cases, all passing, deleted before commit per the no-new-test-files convention:

  • v4 mode: 'custom' → a saved theme named "Custom", still selected, colors preserved
  • re-coercing is idempotent (no second "Custom")
  • unparseable legacy payload → falls back to Dark
  • mode pointing at a never-persisted theme id → falls back
  • the ordering bug: a theme dropped for bad JSON whose id mode still references → both fall back together (validating mode against the raw array instead of the coerced one would boot the user to an unstyled Dark that looks like a silent reset)
  • malformed/duplicate entries dropped without poisoning good ones
  • built-in modes resolve to a null payload so the accent path runs

Not verified by driving the UI: creating/editing/deleting through the actual Settings panel, switching between themes without stale inline properties leaking, and the phone client rendering a saved theme. These need a human at the window — an Electron GUI isn't something I can click. The clear-all-tokens teardown they depend on is unchanged code that now simply runs whenever the payload is null.

🤖 Generated with Claude Code

Juliusolsson05 and others added 10 commits July 18, 2026 23:05
Save, name, pick, edit, and delete custom color schemes instead of the
single unnamed customAppearanceJson slot.

Key decisions: saved theme id doubles as Settings.mode so there stays one
source of truth for "what am I looking at"; themes store raw JSON strings
to preserve user formatting; new themes seed from getComputedStyle on the
live DOM, which is the only way to reach built-in palettes that exist only
as CSS in [data-mode] blocks.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Eight tasks from the SavedTheme model through the picker grid, editor,
v5 migration, and end-to-end verification. Gates on tsc plus driving the
app rather than committed test files, per repo convention.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
readAppliedAppearance reads the 81 live token values off <html>, which is
the only way to reach built-in palettes that exist solely as CSS in
[data-mode] blocks. relativeLuminance moves to lib/color so xterm's ANSI
ramp and a saved theme's light/dark family answer from the same math.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tinel

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e to v5

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Five findings from an adversarial review of this branch.

1. isDarkThemeValue's luminance branch was unreachable at both call
   sites: reaching it required a saved theme that both resolves and
   parses, which is exactly the condition that yields a payload and takes
   the other branch. The spec's Risk #1 was wrong to begin with — accent
   and accentFg are themselves custom-appearance tokens, so a saved theme
   always writes its own concrete accent and the accent-pair branch is
   only reachable on a built-in mode. Deleted the helper, reverted the
   lib/color/luminance.ts extraction and the xtermTheme.ts churn it
   justified, and corrected the spec rather than leaving a wrong risk
   analysis in the repo.

2. applyTheme wrote a raw `theme:<uuid>` into data-mode when a theme id
   no longer resolved. Not a black screen (:root doubles as dark), but
   every high-contrast rule is a compound selector, so contrast silently
   switched off while its toggle still read on. Now falls back to a
   built-in id. Unreachable in-app today, but applyTheme is called by the
   phone client on an uncoerced blob.

3. Migration idempotency keyed on the theme NAME "Custom", which is
   non-unique and user-editable — so naming a theme "Custom" and
   round-tripping through a v4 build could orphan the payload the user
   was actually looking at. Now keyed on a migratedFrom marker that
   renaming cannot forge.

4. parseCustomAppearanceJson threw on unknown keys and coerceSavedThemes
   dropped anything that threw, so a future token rename would silently
   delete every saved theme at boot. That was survivable for one
   throwaway slot; named themes are durable artifacts seeded dense with
   all 81 keys. Split strict (editor — a typo must not look applied) from
   lenient (load — keep the theme, ignore what we cannot read). The
   isSafeCssColorValue allowlist is enforced in both modes; leniency
   recovers themes, it does not relax what reaches a CSS property.

5. Dropped THEME_MODES / ThemeMode imports left dead in settingsRegistry
   by the THEME_MODE_OPTIONS deletion. tsc misses these — noUnusedLocals
   is not set in any tsconfig.

Verified: tsc clean on both projects; 12-case throwaway harness covering
the marker-keyed idempotency, the reviewer's exact downgrade sequence,
lenient survival of a renamed token, and that lenient still rejects
url(). Harness deleted, not committed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Review findings addressed

An adversarial review of this branch surfaced five findings. All five are fixed in 762cdbd.

1. isDarkThemeValue's luminance branch was dead code — and the risk it mitigated wasn't real.

Reaching the luminance line required a saved theme that both resolves and parses, which is exactly the condition that makes resolveThemePayload return non-null and take the other branch. Unreachable at both call sites.

The deeper problem was the spec, not the code: Risk #1 claimed a light saved theme would "pick the wrong half of the accent pair." It can't — accent and accentFg are themselves two of the 81 tokens, backfilled by the parser, so every saved theme writes its own concrete accent. The accent-pair branch is only reachable on a built-in mode.

Deleted the helper, reverted the lib/color/luminance.ts extraction and the xtermTheme.ts churn that only existed to justify it (now byte-identical to main), and corrected the spec rather than leaving wrong analysis in the repo.

2. Invalid data-mode silently disabled high contrast. A raw theme:<uuid> matched no CSS block; :root doubles as dark so it wasn't a black screen, but every high-contrast rule is a compound selector, so contrast switched off while its toggle still read on. Now falls back to a built-in id. Unreachable in-app today — but applyTheme is called by the phone client on an uncoerced blob, so it defends itself.

3. Migration idempotency keyed on a user-editable name. Guarding on name === 'Custom' meant a user who names a theme "Custom" and round-trips through a v4 build could convince the migration it had already run, orphaning the payload they were looking at. Now keyed on a migratedFrom marker that renaming cannot forge.

4. Named themes made unknown-key rejection destructive. parseCustomAppearanceJson threw on unknown keys and coerceSavedThemes dropped anything that threw — so the day a token is renamed, every saved theme carrying the old key would be silently deleted at boot. Survivable for one throwaway slot; not for durable named artifacts seeded dense with all 81 keys.

Split the parser: strict for the editor (a typo must not look successfully applied — the original reason the check exists) and lenient for load (keep the theme, ignore what we can't read). isSafeCssColorValue is enforced in both — leniency recovers themes, it never relaxes what reaches a CSS custom property.

5. Dead imports left in settingsRegistry.ts by the THEME_MODE_OPTIONS deletion. tsc misses them; noUnusedLocals isn't set anywhere.

What the review cleared

Stale inline properties (tried to break it, couldn't — symmetric key iteration plus dense backfill), coercion ordering, and the getComputedStyle seeding path — the last verified against all 81 tokens rather than assumed. It flagged that seeding is load-bearing on custom properties being unregistered; one @property registration would break "+ New theme…" with an error naming a token the user never typed. That dependency is now documented in the spec.

Verification

tsc clean on both projects. A 12-case throwaway harness covered marker-keyed idempotency, the reviewer's exact downgrade sequence, lenient survival of a renamed token, and that lenient still rejects url(). Deleted, not committed, per the no-new-test-files convention.

@Juliusolsson05
Juliusolsson05 merged commit 25d6a5c into main Jul 18, 2026
1 check passed
@Juliusolsson05
Juliusolsson05 deleted the feat/named-custom-themes branch July 18, 2026 21:40
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.

Named custom color themes: save, name, pick, edit, delete

1 participant