From 62a816ccf60af6b609a7d1714cd4842fe436b97d Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Mon, 2 Nov 2026 21:49:01 +0100 Subject: [PATCH] fix(theme): buttons follow the theme, and system follows macOS A transition started by a theme change does not advance while WebKit is not rendering. After a switch in a hidden or minimized window the page was in the new theme while the title bar buttons kept the old colour. Theme switches now run with transitions off and commit their styles before they return. In the Mac app "system" read back the page's own last choice, because prefers-color-scheme follows the window and the window's appearance was always forced. System now leaves the appearance unset. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 4 +++ DESIGN.md | 11 +++++++ packaging/macos/main.swift | 6 ++-- src/client/app.ts | 6 ++-- src/client/styles.css | 6 ++++ src/client/theme.ts | 19 ++++++++++++ test/client.theme.test.ts | 59 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 src/client/theme.ts create mode 100644 test/client.theme.test.ts diff --git a/AGENTS.md b/AGENTS.md index 69f6907..93d9540 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,6 +79,10 @@ bun run build:local # binaries + the app for this machine (needs X diffs with no full paint. `PtySession.requestRepaint` makes the program redraw with a real size change; a same-size resize, or two in a row, does nothing. WebKit has no canvas `contextlost`/`contextrestored` events: repaint on `TerminalView.onReveal` instead of relying on them. +- **Theme switches do not animate.** Set the theme with `setDocumentTheme` (`theme.ts`): a transition + started by a theme change freezes while WebKit is not rendering, leaving old colours on the new + background. In the Mac app "system" must leave `window.appearance` nil, because + `prefers-color-scheme` follows the window. - **Terminal input is the user's.** On macOS ⌘-shortcuts belong to the app; on Linux Ctrl+key combinations stay with the shell unless the README lists them. diff --git a/DESIGN.md b/DESIGN.md index 20e32eb..a838da2 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -928,3 +928,14 @@ The "Codeviewer X is available" dialog showed only the verify link instead of wh | Hard-wrapped paragraphs would break mid-sentence in `.upd-notes` (`white-space: pre-wrap`). | They are unwrapped in the script. | | The verify line is noise in the dialog (the updater verifies by itself) but useful on the GitHub page. | `RELEASE_FOOTER` in `update.ts` is the single definition; `Updater.check()` strips it before the 4000 cap, so a footer-only body (all older releases) gives empty notes and the dialog shows no box. | | Fetching the tag's commit at runtime would be a new network request. | Not done (privacy rule). Existing releases keep their old body and are not edited. | + +## 23. Round 14: buttons out of step with the theme + +The title bar buttons could look missing. They were drawn, in the previous theme's colour. An +on-screen `WKWebView` set up like the Mac window reproduced it, and a second theme bug with it. + +| Finding | Fix | +| --- | --- | +| `.icon-btn` has `transition: color`. A transition started by a theme change does not advance while WebKit is not rendering, so after a switch in a hidden or minimized window the page was already in the new theme and the buttons were not (computed `rgb(139, 148, 165)`, the dark colour, on the light title bar at 2.8:1). The omnibox, sash and other transitioned colours are exposed the same way. | `setDocumentTheme` applies a theme under `data-theme-switching`, which turns all transitions off, and forces a layout so the new styles commit before they return. Buttons match on reveal in all four flips tried, in computed colours and in pixels. | +| In the Mac app "system" never followed macOS. WKWebView's `prefers-color-scheme` follows the window, and the window's appearance was always forced from the page, so "system" read back the page's last choice: light on a Dark Mac after Light, dark on a light Mac from launch. | The theme message carries the mode, and system sets `window.appearance = nil`. The `matchMedia` listener then re-applies the theme with the real value. | +| Not checked. `takeSnapshot` draws web content only and window capture needs Screen Recording permission, so the native toolbar over the title bar was not captured in the light theme. | None needed so far: the user's screenshot of the dark state matches the snapshot pixels. | diff --git a/packaging/macos/main.swift b/packaging/macos/main.swift index 7b196ba..a57b203 100644 --- a/packaging/macos/main.swift +++ b/packaging/macos/main.swift @@ -240,7 +240,7 @@ final class ViewerWindow: NSObject, NSWindowDelegate, WKNavigationDelegate, WKUI } // Messages from the page: - // {t: "theme", dark, bg} sync the window appearance + // {t: "theme", mode, dark, bg} sync the window appearance // {t: "drag"} mouse-down on empty title bar: move the window // {t: "dblclick"} double-click on the title bar: system zoom/minimize action func userContentController(_ ucc: WKUserContentController, didReceive message: WKScriptMessage) { @@ -263,7 +263,9 @@ final class ViewerWindow: NSObject, NSWindowDelegate, WKNavigationDelegate, WKUI } guard t == "theme" else { return } let dark = body["dark"] as? Bool ?? true - window.appearance = NSAppearance(named: dark ? .darkAqua : .aqua) + // WKWebView's prefers-color-scheme follows the window: forcing it in system mode would make + // the page read back its own last choice and never see macOS. + window.appearance = body["mode"] as? String == "system" ? nil : NSAppearance(named: dark ? .darkAqua : .aqua) if let hex = body["bg"] as? String, let c = NSColor(hex: hex) { window.backgroundColor = c webView.underPageBackgroundColor = c diff --git a/src/client/app.ts b/src/client/app.ts index 4738d8d..ae68b79 100644 --- a/src/client/app.ts +++ b/src/client/app.ts @@ -7,6 +7,7 @@ import { markdownToggleCommand } from "./mdcommands.ts"; import { TerminalPanel } from "./terminals.ts"; import type { Theme } from "./term/renderer.ts"; import { icons } from "./icons.ts"; +import { nativeThemeMessage, setDocumentTheme, type ThemeName } from "./theme.ts"; import { GitState, relativeTime } from "./git.ts"; import { Updates } from "./updates.ts"; import { basename, debounce, dragSplitter, esc, formatBytes, h, IS_MAC, key, MOD, store } from "./util.ts"; @@ -20,7 +21,6 @@ import { reference, referenceWithCode, unsafePath } from "./reference.ts"; import { headingText } from "./symbols.ts"; import { slugify } from "./markdown.ts"; -type ThemeName = "dark" | "light" | "system"; interface Layout { sidebar: boolean; @@ -643,9 +643,9 @@ class App { private applyTheme() { const t = this.L.theme; const dark = t === "dark" || (t === "system" && matchMedia("(prefers-color-scheme: dark)").matches); - document.documentElement.dataset.theme = dark ? "dark" : "light"; + setDocumentTheme(document.documentElement, dark ? "dark" : "light"); // Codeviewer.app: match the native window (traffic lights, background) to the theme. - nativeBridge()?.postMessage({ t: "theme", dark, bg: cssVar("--bg") }); + nativeBridge()?.postMessage(nativeThemeMessage(t, dark, cssVar("--bg"))); if (this.themeBtn) { this.themeBtn.innerHTML = dark ? icons.moon : icons.sun; this.themeBtn.title = `Theme: ${t} (click to change)`; diff --git a/src/client/styles.css b/src/client/styles.css index 8dc843a..dde613e 100644 --- a/src/client/styles.css +++ b/src/client/styles.css @@ -298,6 +298,12 @@ svg { user-select: none !important; } +:root[data-theme-switching] *, +:root[data-theme-switching] *::before, +:root[data-theme-switching] *::after { + transition: none !important; +} + .icon-btn { display: inline-grid; place-items: center; diff --git a/src/client/theme.ts b/src/client/theme.ts new file mode 100644 index 0000000..21b220e --- /dev/null +++ b/src/client/theme.ts @@ -0,0 +1,19 @@ +export type ThemeName = "dark" | "light" | "system"; + +/** + * Applies a theme with transitions off, so nothing fades into it. A transition started by a theme + * change does not advance while WebKit is not rendering (hidden or minimized window), which left + * buttons in the old theme's colour on the new background. The forced layout read commits the new + * styles before transitions come back; without it they would all start once the attribute is gone. + */ +export function setDocumentTheme(root: HTMLElement, theme: "dark" | "light") { + root.dataset.themeSwitching = ""; + root.dataset.theme = theme; + void root.offsetHeight; + delete root.dataset.themeSwitching; +} + +/** Codeviewer.app: syncs the native window (traffic lights, background) with the theme. */ +export function nativeThemeMessage(mode: ThemeName, dark: boolean, bg: string) { + return { t: "theme", mode, dark, bg }; +} diff --git a/test/client.theme.test.ts b/test/client.theme.test.ts new file mode 100644 index 0000000..cb89330 --- /dev/null +++ b/test/client.theme.test.ts @@ -0,0 +1,59 @@ +// A theme switch must land at once on everything, and "system" must follow macOS in the Mac app. +import { describe, expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { nativeThemeMessage, setDocumentTheme } from "../src/client/theme.ts"; + +const read = (p: string) => readFileSync(new URL(`../${p}`, import.meta.url), "utf8"); + +/** Stands in for : reading offsetHeight is what commits styles, so record the state at that moment. */ +function fakeRoot(theme = "dark") { + const flushes: Array<{ theme?: string; switching: boolean }> = []; + const dataset: Record = { theme }; + const root = { + dataset, + get offsetHeight() { + flushes.push({ theme: dataset.theme, switching: "themeSwitching" in dataset }); + return 0; + }, + }; + return { root: root as unknown as HTMLElement, dataset, flushes }; +} + +describe("switching theme", () => { + test("styles commit with the new theme while transitions are off", () => { + const { root, flushes } = fakeRoot("dark"); + setDocumentTheme(root, "light"); + expect(flushes).toEqual([{ theme: "light", switching: true }]); + }); + + test("transitions come back afterwards", () => { + const { root, dataset } = fakeRoot("dark"); + setDocumentTheme(root, "light"); + expect(dataset.theme).toBe("light"); + expect("themeSwitching" in dataset).toBe(false); + }); + + test("the stylesheet turns every transition off during a switch", () => { + const css = read("src/client/styles.css"); + const rule = /:root\[data-theme-switching\] \*,\s*:root\[data-theme-switching\] \*::before,\s*:root\[data-theme-switching\] \*::after \{\s*transition: none !important;\s*\}/; + expect(css).toMatch(rule); + }); + + test("the app goes through setDocumentTheme", () => { + const app = read("src/client/app.ts"); + expect(app).toContain("setDocumentTheme(document.documentElement"); + expect(app).not.toContain("dataset.theme ="); + }); +}); + +describe("the Mac window's appearance", () => { + test("the page tells the window which mode is chosen", () => { + expect(nativeThemeMessage("system", true, "#0c0f15")).toEqual({ t: "theme", mode: "system", dark: true, bg: "#0c0f15" }); + expect(nativeThemeMessage("light", false, "#f4f5f8")).toEqual({ t: "theme", mode: "light", dark: false, bg: "#f4f5f8" }); + }); + + test("system clears the forced appearance, or prefers-color-scheme only echoes what the page last set", () => { + const swift = read("packaging/macos/main.swift"); + expect(swift).toMatch(/window\.appearance = body\["mode"\] as\? String == "system" \? nil : NSAppearance\(named: dark \? \.darkAqua : \.aqua\)/); + }); +});