Skip to content

feat(web): light theme — three-way picker, paper tokens, AA xterm palette - #89

Open
dchaudhari7177 wants to merge 1 commit into
burakgon:mainfrom
dchaudhari7177:feat/light-theme
Open

feat(web): light theme — three-way picker, paper tokens, AA xterm palette#89
dchaudhari7177 wants to merge 1 commit into
burakgon:mainfrom
dchaudhari7177:feat/light-theme

Conversation

@dchaudhari7177

Copy link
Copy Markdown
Contributor

Closes #67

Adds the light theme along the exact seams the issue points at:

pwa/theme.tsThemeName is now "dark" | "oled" | "light", with TERMINAL_BG and THEME_COLOR entries for light (#f6f6f7). loadTheme keeps the stored-value validation pattern: anything other than "oled"/"light" falls back to dark. applyTheme sets data-theme="light"; dark still removes the attribute so the :root default stays the single source of truth.

styles/tokens.css — a [data-theme="light"] block flips ink → paper: --bg #f6f6f7, white cards, black-wash hairlines, text at 15.9:1 / 6.2:1 / 4.7:1. Coral handling follows the issue's AA note: coral fills (primary buttons, brand glyph) keep brand coral — dark ink on coral reads ~7:1 — while coral used as text/status (--accent, --iris, --awaiting) darkens to #c2481d (4.6:1 on paper). color-scheme: light flips native form controls/scrollbars. Every ratio below was computed against #f6f6f7.

settings/SettingsPanel.tsx — the OLED checkbox becomes a three-way Theme select (Dark / True black (OLED) / Light), using the existing rc-settings__field + rc-settings__control idiom from Session order. Still applies instantly through setTheme (no save button, no reload).

chat/TerminalView.tsx — a full LIGHT_THEME xterm palette (same shape as THEME): ink-on-paper foreground #383a42 (10.5:1), selection #c9d0da, and ANSI hues darkened so ordinary terminal text meets WCAG AA — red 5.0, green 4.6, yellow 4.9, blue 5.0, magenta 5.7, cyan 5.5, white-gray 4.7. terminalTheme() resolves the palette at mount and in the rc-theme-change listener, so an open terminal restyles live without a remount. The .xterm-viewport seam already follows var(--bg).

Teststheme.test.ts extended per acceptance criteria: light round-trip + data-theme, unknown stored value ("solarized") falls back to dark, theme-color meta mirrors #f6f6f7, TERMINAL_BG covers all three themes.

Verification

  • pnpm typecheck clean; theme + storage-migration tests pass (10/10).
  • Full packages/web suite: the only failures are the known flaky-under-load Windows ones (App/TerminalView/SessionList timeouts + keeps compact stacked remaining-limit rows), reproduced identically on a clean main checkout on this machine.
  • Rendered the screenshot.html?scene=newsession harness with data-theme="light": body paints paper rgb(246,246,247), ink text rgb(27,27,31), picker surfaces flip to translucent white.

Left the stretch "system" (prefers-color-scheme) option out per the issue — happy to follow up with it as a separate PR.

Three-way theme choice in Settings (Dark / True black / Light),
replacing the OLED checkbox:

- theme.ts: ThemeName gains "light"; stored-value validation keeps
  unknown values falling back to dark; applyTheme sets data-theme for
  any non-default theme.
- tokens.css: a [data-theme=light] block — paper surfaces, ink text,
  darkened hairlines; the coral accent shifts to its deep value for
  contrast on paper; warn/err darkened for AA. The code card stays a
  dark panel deliberately.
- TerminalView: light mode swaps the FULL xterm palette (One
  Light-derived), not just the background — the dark ANSI ramp would
  be unreadable on paper. Live switch keeps working via rc-theme-change.
- theme.test.ts extended for the new value, fallback, and meta mirror.

Closes burakgon#67
@lemkyz

lemkyz commented Jul 26, 2026

Copy link
Copy Markdown

Thanks for putting this together. I pulled the PR branch locally and tested it through the real screenshot.html?scene=terminal canvas, not just the theme unit tests. The picker, persistence, surface tokens, theme-color update, and live switching all behaved as expected.

I did find one gap in the terminal contrast guarantee. LIGHT_THEME replaces the first 16 ANSI colors, but Ghostty also renders xterm 256-color indexes and arbitrary truecolor values. Those paths bypass the 16-color palette entirely. The Claude frame included in the repository already exercises this with indexes 153, 246, 244, and 211. Against the light terminal background, their contrast ratios are approximately 1.39:1, 2.81:1, 3.65:1, and 2.08:1. Faint text can reduce the effective contrast even further, so parts of the real terminal canvas still remain below AA despite the base palette being sound.

I implemented the fix as a small extension to the Ghostty canvas renderer rather than attempting to rewrite all 256 palette entries, since that still would not cover truecolor output. The renderer now accepts an optional minimum contrast value and evaluates the final foreground against the actual cell background after inverse and selection handling. It also accounts for faint alpha, understands both the runtime's rgb() output and hex theme colors, and follows native Ghostty's black-or-white fallback behavior when a color does not meet the requested ratio. Light mode requests 4.5:1, while dark and OLED remain at 1 so their existing rendering is unchanged. The existing theme-change path updates the value on an already open terminal without requiring a remount.

The focused follow-up is here: dchaudhari7177#1

I verified the result with all 87 affected tests, the workspace typecheck, scoped ESLint, git diff --check, and the real terminal screenshot scene. The previously washed-out indexed and faint text becomes readable immediately in light mode. I deliberately left the picker, storage behavior, meta theme color, surface tokens, and base ANSI palette from this PR untouched.

@dchaudhari7177

Copy link
Copy Markdown
Contributor Author

@lemkyz thank you for the depth of that — pulling the branch and testing through the real screenshot.html?scene=terminal canvas rather than the unit tests is exactly how you find the gap you found, and you are right about it. Replacing the first 16 ANSI entries never touches the 256-index or truecolor paths, and 1.39:1 on index 153 is not a rounding error.

I have to be straight about where this leaves the PR, though, because it is not good news for it.

main has replaced the theme system underneath this. packages/web/src/pwa/theme.ts no longer has a three-value ThemeName union at all — it is ThemeName = string, backed by the generated Ghostty theme catalog in packages/web/src/appearance/, with surfaces derived from each theme's own background and foreground:

isLightTheme(theme)  ->  relativeLuminance(theme.background) > 0.42
surface  = mix(background, foreground, light ? 0.045 : 0.075)
surface2 = mix(background, foreground, light ? 0.08  : 0.125)

That is a superset of what this PR does, arrived at from a better direction. My LIGHT_THEME is one hand-tuned light palette plus hand-written paper tokens; main now supports every light Ghostty theme and computes the paper surfaces for each. Rebasing this would mean deleting the palette, deleting the tokens, and deleting the three-way picker — there is nothing left of the PR afterwards.

So I think this should be closed as superseded, and I am happy to close it — say the word, or close it yourself, whichever you prefer.

Your contrast fix should not go with it. It is the one part of this that main did not subsume: isLightTheme decides light-vs-dark and derives surfaces, but nothing enforces a minimum contrast for indexed or truecolor cells, so the 153/246/244/211 problem you measured is still live on main today — now reachable through any light Ghostty theme rather than just my one. Your approach also survives the rewrite intact, since it works at the renderer against the final resolved cell colours rather than against a palette.

Two things, then:

  1. I would rather you open that renderer change directly against burakgon/roamcode than through my fork — it is your work, it stands on its own, and it should not be gated on a PR that is about to be closed. If you would prefer I carried it, tell me and I will, with attribution to you in the commit and PR.
  2. The one thing worth revisiting against the new architecture is the threshold: light ? 4.5 : 1 was a two-theme decision. With isLightTheme() available, the natural form is to key the minimum off that rather than off a theme name, so it applies to every light theme in the catalog.

I have not merged the PR on my fork — it should go where it can actually be reviewed by the maintainer.

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.

Theme: add a light terminal theme

2 participants