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\)/); + }); +});