Conversation
Dark colors are generated at build time by a small PostCSS plugin (src/build/dark-theme.ts) instead of hand-written overrides for ~560 hard-coded colors. Every rule with color declarations gets a twin under :root[data-theme='dark'] that flips OKLab lightness while keeping hue, lifts mid-tones for AA contrast, keeps shadows dark, and preserves media queries and cascade order. Intentionally dark surfaces (the call view) are marked /* theme: fixed */. - public/theme.js applies the saved theme before first paint (an external file because the CSP disallows inline scripts), so there is no flash. - Settings has an Appearance control; System follows the device and updates live. The choice is stored per browser. - Measured in dark: secondary text 4.98:1, notes 4.83:1, headings 8.4:1. Light mode is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jerelvelarde
left a comment
There was a problem hiding this comment.
Dark mode has good template value, and the build-time approach works with the separate accent refinement PR. Hold merge for two reproduced theme behavior regressions. Seven focused theme tests and 171 full tests passed; typecheck, lint, formatting, and the rerun production build passed. The generated CSS, CSP/static asset loading, editor variables, exclusions, and system/prepaint agreement were reviewed.
| } catch { | ||
| // Applies for this visit only. | ||
| } | ||
| apply(); |
There was a problem hiding this comment.
[P2] Apply the selected theme even when storage is unavailable. With localStorage methods throwing and the system set to light, choosing Dark leaves data-theme=light while Settings shows Dark. apply() rereads storage instead of using the selected preference. Keep the active preference in memory and treat persistence failure separately.
There was a problem hiding this comment.
Fixed in 79b4fb5. The selected preference is now kept in memory for the page: setThemePreference sets it before trying localStorage, and apply() reads that in-memory choice first. If storage throws, Dark still applies; it just isn't remembered after a reload.
Regression test: applies the chosen theme even when storage is unavailable. Every storage method throws and the system is light. Choosing Dark sets data-theme=dark, and a later system-preference event doesn't undo it.
| if (/^\s*theme:\s*end\s*$/.test(node.text)) fixed = false; | ||
| return; | ||
| } | ||
| if (fixed) return; |
There was a problem hiding this comment.
[P2] Preserve fixed call styles against generated global rules. Skipping fixed rules leaves them with their original specificity while generated global button rules gain root specificity. Chromium with the actual stylesheet shows call controls changing from transparent to #1b1b1b in Dark mode, adding rectangular backgrounds around the circular controls. Preserve the fixed surface's effective cascade when generating dark rules.
There was a problem hiding this comment.
Fixed in 79b4fb5. Rules inside /* theme: fixed */ now get dark twins with unchanged values, so they keep the same specificity as the generated global rules, and the global button twin can't override them.
Checked in Chromium with the app's stylesheet: the call controls' backgrounds are now identical in light and dark (.call-controls button transparent, .call-minimize 7% white). Before this fix they became #1b1b1b.
The test now asserts the fixed twin (.call-view { background: #1c544c; }) is emitted unchanged.
jerelvelarde
left a comment
There was a problem hiding this comment.
One additional merge blocker: the generated dark palette needs readable text without depending on the separate appearance PR. Chromium computed-style measurements confirm the service setup text and enabled Save label below 4.5:1. This supplements the existing request for changes.
| export function darkColor(value: string) { | ||
| const { rgb, alpha } = parseHex(value); | ||
| const [L, a, b] = rgbToOklab(rgb); | ||
| const next = 0.95 - 0.73 * L ** 1.3; |
There was a problem hiding this comment.
Please keep normal text readable in the generated dark palette without relying on PR #82. With this PR alone, Settings service setup text has 2.44:1 contrast and the enabled Save label has 3.28:1, both below 4.5:1. These ratios were measured from Chromium computed styles with transparent backgrounds composited through ancestors. Adjust the mapping or add explicit accessible dark foreground/background pairs, and verify this PR independently.
There was a problem hiding this comment.
Fixed in 79b4fb5, verified with this PR on its own, without #82.
- Text floor: every text color now gets a minimum dark-mode lightness (OKLab L ≥ 0.72), which is AA on surfaces from
#1b1b1bto about#2a2a2a. - Text/background pairs: when a rule sets both its text and background, the pair is checked after mapping and adjusted to 4.5:1. The text moves to the better extreme, and a mid-tone background darkens if that alone isn't enough.
Measured in Chromium from computed styles, with transparent backgrounds composited through ancestors:
- Service setup text: 2.44 → 6.45:1
- Enabled Save label: 3.28 → 4.69:1
- Permission descriptions: 6.92:1
New tests check the config-note pair, a matrix of light-theme text colors against dark surfaces, and Save-style pairs (lavender, green, red) at 4.5:1 or better.
On the integration note: understood. When #82 lands, its polish layer stays after /* theme: end */. I've also restored the lockfile: it's now upstream's lockfile plus only the postcss devDependency line, with the libc metadata intact, and npm ci passes.
jerelvelarde
left a comment
There was a problem hiding this comment.
Two integration notes from follow-up review:
- If #82 is integrated later, keep its appended polish layer after the theme:end marker at src/client/style.css:3997. Placing that layer inside the fixed call-style region prevents its rules from receiving dark variants. A generated-CSS fixture confirms the primary/config-note dark rules appear only with the correct placement. This is future merge guidance, not another current blocker.
- Please restore the unrelated native-package libc metadata removed from package-lock.json. npm platform filtering otherwise accepts both GNU and musl variants for the same Linux architecture. Runtime binding selection still chooses the matching variant; I have not reproduced a Linux build failure, so this is lockfile hygiene rather than a merge blocker.
# Conflicts: # package-lock.json
…t storage Review fixes for the dark mode PR. - Readable without the polish PR: text colors get a lightness floor in dark (AA on #1b1b1b-#2a2a2a surfaces), and a rule that sets both its text and background is checked as a pair and adjusted to 4.5:1 (better text extreme, then a darker mid-tone background if needed). Measured in Chromium with this PR alone: service setup text 2.44 -> 6.45:1, enabled Save label 3.28 -> 4.69:1. - Fixed call surface: rules in /* theme: fixed */ now get dark twins with unchanged values, so they keep the generated rules' specificity and the global button twin no longer paints the call controls (#1b1b1b -> unchanged transparent / 7% white, same as light). - The chosen theme is kept in memory, so Dark applies even when localStorage throws; persistence failure is separate. - package-lock.json is upstream's plus only the postcss devDependency line (restores the libc metadata). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Workflow
Settings has a new Appearance control: System, Light or Dark. System follows the device and updates live. The choice is saved per browser, and the theme is applied before the first paint, so there's no white flash.
Approach: dark colors derived at build time
style.cssandeditor.csscontain about 560 hard-coded colors, most of them unique. Hand-writing dark overrides for all of them would be large and would drift with every style change. Instead, a small PostCSS plugin (src/build/dark-theme.ts, registered invite.config.ts) generates the dark theme::root[data-theme='dark']that contains only those color declarations, inside the same media query.#1b1b1b, black becomes#eeeeee), with a curve that lifts mid-tones so secondary text keeps AA contrast. Hue is preserved and chroma is softened slightly. Alpha is kept.url()values are left alone.background: noneorvar(--x). Without that, an earlier rule's dark twin could beat a later light rule that has no literal color./* theme: fixed */ … /* theme: end */.New styles get a dark variant automatically.
public/theme.jssetsdata-themebefore React loads. It's an external file because the CSP disallows inline scripts.src/client/theme.tshandles later changes and the system preference.postcssis now listed as a direct dev dependency; it was already installed through Vite.Verification
npm run check-format,lint,typecheck,test(171 passing, 7 new intests/dark-theme.test.ts) andbuildall pass.🤖 Generated with Claude Code