feat: retire the old Manifold themes, promote Royal into the Manifold name - #913
Merged
Conversation
svenmalvik
force-pushed
the
manifold-theme-palette-and-default
branch
from
August 8, 2026 16:23
3cb262f to
e2800fd
Compare
A fresh install now starts on Royal Dark instead of Manifold Dark. `DEFAULT_SETTINGS.theme` is what `SettingsStore.loadFromDisk()` returns when `~/.manifold/config.json` does not exist yet, so this only affects first-time users — an existing config's theme is merged over the defaults and is left alone. Left deliberately unchanged: `DEFAULT_THEME` in `themes/registry.ts` is the fallback for an unresolvable theme id and the target of `migrateLegacyTheme()` for old 'dark'/'light' values, which is an upgrade path rather than the first-run default. The registry test also pins that the default is a *registered* theme id: `loadTheme()` silently falls back to manifold-dark for an unknown id, so a typo in the default would otherwise ship the old theme with nothing failing. `SettingsModal.test.tsx` restated the default theme in an assertion about a modal rendered from `DEFAULT_SETTINGS`; since that test covers Save round- tripping settings unchanged, it now reads the value from `DEFAULT_SETTINGS`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… name Deletes the original Manifold Dark/Light and renames the Royal pair into their place, so the shipped set is five pairs and the default is the deep navy/gold palette under the Manifold name. Supersedes the previous commit's `royal-dark` default. The theme JSONs carry no internal name field — the label comes purely from the filename map in theme-data.ts — so this is a file move plus a barrel edit. Migration: `migrateLegacyTheme()` maps saved `royal-dark`/`royal-light` ids to `manifold-dark`/`manifold-light`. This is load-bearing for `royal-light`: without it that id falls through to `loadTheme()`'s unknown-id fallback, which is dark, silently flipping a light-theme user to dark on upgrade. Anyone who had the old `manifold-dark`/`manifold-light` selected keeps that id and now sees the new palette. That is inherent to retiring the old themes — the previous colors no longer ship. Test ids that referenced the removed themes were repointed: the screenshot script's `loadThemeVars` cases to `manifold-dark` (it rejects unknown ids), its `parseArgs` cases to `jade-dark` (any real non-default id proves parsing), and the three fixture header comments to `--theme manifold-dark` so the documented command still runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two places still described a theme Manifold has not shipped in a long time. `window-factory.ts` fell back to the literal id 'dracula' when picking the window's pre-paint background. It is unreachable through SettingsStore (the defaults always supply a theme) but it named a theme that does not exist, so the fallback would have thrown into loadTheme()'s catch. It now uses the exported DEFAULT_THEME. `theme.css`'s `:root` block held Dracula's palette as the pre-JS fallback. Those values are what paints between the window's first frame and App mounting, so they were a visible flash of grey-purple before the real theme landed — confirmed by driving the built app, where the pre-mount `--bg-primary` read #282a36 while the applied theme was #06080F. Both now read #06080F. Repointing the obvious block turned up four more strays that a new drift test caught: `--sidebar-active-border` and `--accent-subtle` still carried the old #007acc accent blue, and `--control-bg-hover`/`--statusbar-hover-bg` had drifted to generic white overlays. The test pins colour fallbacks against the default theme's converted vars so this cannot rot again. Shadows are deliberately excluded: theme.css layers an extra inset top highlight onto each that the adapter does not emit, which is a stylesheet design choice rather than palette drift, and pinning it would force the richer shadow to be dropped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
svenmalvik
force-pushed
the
manifold-theme-palette-and-default
branch
from
August 8, 2026 16:34
f13006d to
7abadb2
Compare
Copied in with the import line from screenshot-component.test.ts; the file only uses resolve(dirname(...)). Flagged by the code-quality bot on #913. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
## Summary Restores the theme **family dropdown** and **light/dark toggle** to the title bar's trailing edge. They were dropped in a1023f6 (*"remove theme controls from TitleBar"*, 2026-07-27), which reached `main` with the workspaces rebuild (#880) — so from a user's point of view they vanished in that merge. Theme switching survived only in Settings and the command palette. Both controls are back on every shell branch, including the pre-setup and no-project screens, matching where they sat before. **The family list is now derived from the theme registry** (`getThemeFamilies()`) instead of the hardcoded array the old code carried. That array is exactly what would have gone stale here — it still listed **Royal**, the family the base branch retires, so restoring it verbatim would have shipped a dead option that silently resolved to nothing. Switching family preserves the current light/dark variant, and the toggle flips the variant within the family, so the two controls stay independent. ## Stacked on #913 Targets `manifold-theme-palette-and-default`, not `main`. It depends on that branch's theme set: the derived family list is what keeps Royal out. Merge #913 first and this retargets to `main` cleanly. ## Testing - `npm test` — 376 files / 2931 tests pass. - `npm run typecheck` — clean across web, node, and plugins. - Drove the **built** app under Playwright with an isolated `HOME` + `CFFIXED_USER_HOME`, asserting against the real DOM: families `["Manifold","Garfield","Neon","Jade","Platinum"]` (no Royal), selection `manifold`, toggle labelled *Switch to Light theme*, and the select positioned inside the 38px title bar past the window midpoint — i.e. actually in the upper right, not merely present. New coverage: - `TitleBar.test.tsx` — selecting a family calls back with the family id; the option list contains Manifold and Jade but **not** Royal and holds one entry per family rather than per theme; the toggle offers and reports the opposite variant. The pre-existing *"carries no controls"* test encoded the removal, so it is now scoped to "omits the controls when no handlers are wired", with a separate test keeping search out of the title bar. - `registry.test.ts` — `themeFamilyOf` strips the variant suffix and is idempotent; `getThemeFamilies()` collapses each dark/light pair into one entry, stays in step with the shipped theme list, and every family combined with either variant names a real theme. `docs/architecture/renderer.md` (covers `src/renderer`) gains an entry for `TitleBar.tsx` describing the controls and why the family list is derived. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <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
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.
Summary
Retires the original Manifold Dark/Light and renames the Royal pair into their place. The shipped set goes from six pairs to five, and a first run now opens on the deep navy/gold palette — under the Manifold name. Also clears out the Dracula-era leftovers the rename exposed.
The theme JSONs carry no internal
namefield (the label comes purely from the filename map intheme-data.ts), so the rename itself is a file move plus a barrel edit.Note that
defaults.tsis untouched —DEFAULT_SETTINGS.themewas alreadymanifold-darkand still is. What changed is the palette that id resolves to. The new-default requirement is satisfied entirely by the rename.Migration
migrateLegacyTheme()maps savedroyal-dark/royal-lighttomanifold-dark/manifold-light. This is load-bearing forroyal-light: without it that id falls through toloadTheme()'s unknown-id fallback, which is dark, silently flipping a light-theme user to dark on upgrade.Worth a reviewer's explicit sign-off: anyone who currently has the old
manifold-dark/manifold-lightselected keeps that id and will see the new palette after upgrading. That's inherent to retiring the old themes — those colors no longer ship anywhere — but it is a visible change for existing users who never opted into Royal.Dracula cleanup
Two places still described a theme Manifold hasn't shipped in a long time:
window-factory.tsfell back to the literal id'dracula'for the window's pre-paint background. Unreachable throughSettingsStore(the defaults always supply a theme), but it named a theme that no longer exists, so the fallback would have dropped intoloadTheme()'s catch. Now uses the exportedDEFAULT_THEME.theme.css's:rootblock held Dracula's palette as the pre-JS fallback. Those values are what paints between the window's first frame and App mounting, so they were a real flash of grey-purple before the theme landed — driving the built app showed pre-mount--bg-primary: #282a36against an applied#06080F. Both now read#06080F.Repointing the obvious block surfaced four more strays that the new drift test caught:
--sidebar-active-borderand--accent-subtlestill carried the old#007accaccent blue, and--control-bg-hover/--statusbar-hover-bghad drifted to generic white overlays.theme-css-fallbacks.test.tspins the colour fallbacks against the default theme's converted vars so this can't rot again. Shadows are deliberately excluded —theme.csslayers an extrainset 0 1px 0 rgba(255,255,255,…)top highlight onto each that the adapter doesn't emit. That reads as a stylesheet design choice rather than palette drift, and pinning it would force the richer shadow to be dropped. Happy to reconcile them either direction if you'd rather they matched.Still not changed
migrateLegacyTheme()maps the legacy'light'/'vs'values to a dark default (pre-existing, untouched).Testing
npm test— 376 files / 2923 tests pass.npm run typecheck— clean across web, node, and plugins.HOME+CFFIXED_USER_HOMEfor a genuine first run (noconfig.json, andapp.getPath('home')asserted to be the scratch dir so the real profile stayed untouched). Pre-mount fallback#06080f, applied--bg-primary: #06080F,--bg-sidebar: #090D18,--accent: #E2C275, and the fresh config persistedtheme: manifold-dark.manifold-lightto confirm the light variant is the cream/navy former Royal Light.New coverage in
registry.test.ts: the default is Manifold Dark and a registered id;manifold-dark/manifold-lightresolve to the Royal palette (#06080F/#F6F1E7) — this is what would fail if the rename regressed, since the id itself never changed; theroyal-*ids are gone from the picker; and both migrate to the Manifold id of the same type, withroyal-lightasserted to stay light.settings-store.test.tspins the fresh-install theme.Test ids referencing removed themes were repointed:
loadThemeVarscases tomanifold-dark(it rejects unknown ids),parseArgscases tojade-dark(any real non-default id proves parsing), and three fixture header comments to--theme manifold-darkso the documented command still runs.Rebased onto
mainafter #909/#911/#912 landed. Supersedes #910, which GitHub auto-closed when its head branch was renamed to match the final change.🤖 Generated with Claude Code