From dd106d27518d7b94d4136b6b35f69ec10b5b554e Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Wed, 27 May 2026 21:40:04 +0800 Subject: [PATCH 1/5] fix(ui): render tooltip shortcut hints as plain sans glyphs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shortcut suffix in icon tooltips (新建会话 ⇧⌘S etc.) rendered in --font-family-mono, whose fixed cell width crammed the modifier glyphs (⇧⌘) into one squished blob. This affected every TooltipKeybind site (titlebar, sidebar, composer, session tabs, right-panel review) because they all share [data-slot="tooltip-keybind-key"]. Switch that slot to the sans stack at 12/500 — the macOS-menu convention (plain glyphs, no keycap). A .kbd keycap was considered but rejected as too heavy for a tertiary hover hint; the keycap stays for dense scannable surfaces (popover, command palette item-tail). Add e2e/snap/keybind-tooltip: asserts the suffix uses the sans stack (never mono) and renders the light/dark grid for visual review. Verified red (mono) -> green (sans) on the real renderer. --- packages/app/e2e/snap/keybind-tooltip.snap.ts | 74 +++++++++++++++++++ packages/ui/src/components/tooltip.css | 11 ++- 2 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 packages/app/e2e/snap/keybind-tooltip.snap.ts diff --git a/packages/app/e2e/snap/keybind-tooltip.snap.ts b/packages/app/e2e/snap/keybind-tooltip.snap.ts new file mode 100644 index 000000000..1f2764191 --- /dev/null +++ b/packages/app/e2e/snap/keybind-tooltip.snap.ts @@ -0,0 +1,74 @@ +import { expect } from "@playwright/test" +import { test } from "../fixtures" +import { applyDarkModeForTests } from "../utils" +import { composeGrid, snapOutputPath, type Shot } from "./_compose" + +test.use({ viewport: { width: 480, height: 200 }, deviceScaleFactor: 2 }) + +// The keybind tooltip (titlebar / sidebar / composer shortcuts) only appears on +// hover over a real button, and the shortcut suffix is the same DOM everywhere: +// . Like worktree-tooltip, we inject that +// DOM after the global CSS loads instead of standing up the app shell — the +// shortcut's font/color come entirely from tooltip.css tokens, no Solid state. +// +// Guards two things: (1) the suffix renders in the sans stack, never monospace — +// the mono cell width is what crammed ⇧⌘ into one squished blob; (2) the visual +// grid lets a human confirm the glyphs read as separate keys, light and dark. + +const TOOLTIP_HTML = ` +
+
+
+ 新建会话 + ⇧⌘S +
+
+
+` + +async function mountTooltip(page: import("@playwright/test").Page): Promise { + await page.evaluate((html) => { + document.body.innerHTML = html + }, TOOLTIP_HTML) + await page.locator('[data-component="tooltip"][data-expanded]').waitFor({ state: "visible", timeout: 10_000 }) +} + +async function waitForThemeBoot(page: import("@playwright/test").Page): Promise { + await page.waitForFunction( + () => getComputedStyle(document.documentElement).getPropertyValue("--bg-base").trim().length > 0, + null, + { timeout: 30_000 }, + ) +} + +test("keybind-tooltip", async ({ page }) => { + test.setTimeout(120_000) + + await page.goto("/") + await waitForThemeBoot(page) + await mountTooltip(page) + + // Regression lock: the shortcut suffix must use the sans stack, never mono. + const fontFamily = await page + .locator('[data-slot="tooltip-keybind-key"]') + .evaluate((el) => getComputedStyle(el).fontFamily) + expect(fontFamily).toContain("system-ui") + expect(fontFamily.toLowerCase()).not.toContain("mono") + + const lightShot: Shot = { + name: "light", + buf: await page.locator('[data-component="tooltip"]').screenshot(), + } + + await applyDarkModeForTests(page) + await waitForThemeBoot(page) + await mountTooltip(page) + const darkShot: Shot = { + name: "dark", + buf: await page.locator('[data-component="tooltip"]').screenshot(), + } + + const out = snapOutputPath("keybind-tooltip") + await composeGrid([lightShot, darkShot], out) + process.stdout.write(`\n[snap] keybind-tooltip grid -> ${out}\n\n`) +}) diff --git a/packages/ui/src/components/tooltip.css b/packages/ui/src/components/tooltip.css index 4832a8482..1f440f65c 100644 --- a/packages/ui/src/components/tooltip.css +++ b/packages/ui/src/components/tooltip.css @@ -9,10 +9,13 @@ } [data-slot="tooltip-keybind-key"] { - font-family: var(--font-family-mono); - font-size: var(--font-size-kbd); - font-weight: var(--font-weight-kbd); - line-height: var(--line-height-kbd); + /* Plain OS-native glyphs (⇧⌘S), not a keycap — see DESIGN.md Tooltip. + Sans, not mono: the monospace cell width crammed the modifier glyphs + together, which read as one squished blob. */ + font-family: var(--font-family-sans); + font-size: 12px; + font-weight: 500; + line-height: 1.2; color: var(--bg-cream); } From 51927939a6865bd34e3ba139bba8a7cf6116f9cf Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Wed, 27 May 2026 21:45:49 +0800 Subject: [PATCH 2/5] test(ui): cover en label in keybind-tooltip snap grid The chrome ships bilingual; the keybind suffix font is the regression we guard, but the grid only showed the zh label. Render zh + en tooltips stacked (light + dark) and assert the sans-not-mono lock on every suffix, not just the first. --- packages/app/e2e/snap/keybind-tooltip.snap.ts | 54 ++++++++++++------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/packages/app/e2e/snap/keybind-tooltip.snap.ts b/packages/app/e2e/snap/keybind-tooltip.snap.ts index 1f2764191..09933fbb5 100644 --- a/packages/app/e2e/snap/keybind-tooltip.snap.ts +++ b/packages/app/e2e/snap/keybind-tooltip.snap.ts @@ -3,7 +3,7 @@ import { test } from "../fixtures" import { applyDarkModeForTests } from "../utils" import { composeGrid, snapOutputPath, type Shot } from "./_compose" -test.use({ viewport: { width: 480, height: 200 }, deviceScaleFactor: 2 }) +test.use({ viewport: { width: 520, height: 240 }, deviceScaleFactor: 2 }) // The keybind tooltip (titlebar / sidebar / composer shortcuts) only appears on // hover over a real button, and the shortcut suffix is the same DOM everywhere: @@ -11,26 +11,38 @@ test.use({ viewport: { width: 480, height: 200 }, deviceScaleFactor: 2 }) // DOM after the global CSS loads instead of standing up the app shell — the // shortcut's font/color come entirely from tooltip.css tokens, no Solid state. // -// Guards two things: (1) the suffix renders in the sans stack, never monospace — -// the mono cell width is what crammed ⇧⌘ into one squished blob; (2) the visual -// grid lets a human confirm the glyphs read as separate keys, light and dark. +// Renders both UI languages (zh + en) stacked, because the chrome ships +// bilingual: the label font switches by lang while the shortcut glyphs stay the +// same Latin sans. Guards two things: (1) every suffix renders in the sans +// stack, never monospace — the mono cell width is what crammed ⇧⌘ into one +// squished blob; (2) the visual grid lets a human confirm the glyphs read as +// separate keys across language and light/dark. -const TOOLTIP_HTML = ` -
-
+const STAGE_HTML = ` +
+
新建会话 ⇧⌘S
+
+
+ New session + ⇧⌘S +
+
` -async function mountTooltip(page: import("@playwright/test").Page): Promise { +async function mountStage(page: import("@playwright/test").Page): Promise { await page.evaluate((html) => { document.body.innerHTML = html - }, TOOLTIP_HTML) - await page.locator('[data-component="tooltip"][data-expanded]').waitFor({ state: "visible", timeout: 10_000 }) + }, STAGE_HTML) + await page.locator("[data-snap-stage]").waitFor({ state: "visible", timeout: 10_000 }) } async function waitForThemeBoot(page: import("@playwright/test").Page): Promise { @@ -46,26 +58,30 @@ test("keybind-tooltip", async ({ page }) => { await page.goto("/") await waitForThemeBoot(page) - await mountTooltip(page) + await mountStage(page) - // Regression lock: the shortcut suffix must use the sans stack, never mono. - const fontFamily = await page + // Regression lock: every shortcut suffix (zh + en) must use the sans stack, + // never mono. + const fontFamilies = await page .locator('[data-slot="tooltip-keybind-key"]') - .evaluate((el) => getComputedStyle(el).fontFamily) - expect(fontFamily).toContain("system-ui") - expect(fontFamily.toLowerCase()).not.toContain("mono") + .evaluateAll((els) => els.map((el) => getComputedStyle(el).fontFamily)) + expect(fontFamilies.length).toBe(2) + for (const family of fontFamilies) { + expect(family).toContain("system-ui") + expect(family.toLowerCase()).not.toContain("mono") + } const lightShot: Shot = { name: "light", - buf: await page.locator('[data-component="tooltip"]').screenshot(), + buf: await page.locator("[data-snap-stage]").screenshot(), } await applyDarkModeForTests(page) await waitForThemeBoot(page) - await mountTooltip(page) + await mountStage(page) const darkShot: Shot = { name: "dark", - buf: await page.locator('[data-component="tooltip"]').screenshot(), + buf: await page.locator("[data-snap-stage]").screenshot(), } const out = snapOutputPath("keybind-tooltip") From d4d059b2c6796c421ff9f94e1f8b34886832006d Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Wed, 27 May 2026 21:48:43 +0800 Subject: [PATCH 3/5] refactor(ui): use --font-weight-emphasis token in tooltip keybind Address review: the tooltip container two rules below already uses var(--font-weight-emphasis) (= 500); the keybind suffix should reuse the same semantic token instead of a hardcoded 500. --- packages/ui/src/components/tooltip.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/components/tooltip.css b/packages/ui/src/components/tooltip.css index 1f440f65c..f21b331e9 100644 --- a/packages/ui/src/components/tooltip.css +++ b/packages/ui/src/components/tooltip.css @@ -14,7 +14,7 @@ together, which read as one squished blob. */ font-family: var(--font-family-sans); font-size: 12px; - font-weight: 500; + font-weight: var(--font-weight-emphasis); line-height: 1.2; color: var(--bg-cream); } From a773c70051f5d24815f68ab8ffeff2d6d286d685 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Wed, 27 May 2026 21:56:39 +0800 Subject: [PATCH 4/5] refactor(ui): tooltip shortcut uses caption role (13/400), drop off-system 12px MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sans has no 12px size token (theme.css: 12px is mono-only); the smallest sans is 13px. Use the tooltip's own 13px sans at caption weight (400) — one step lighter than the label's 500 emphasis, so the shortcut reads as secondary. Express only the two intentional properties (sans-not-mono + caption weight); size/line-height/color inherit the tooltip container. --- packages/ui/src/components/tooltip.css | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/ui/src/components/tooltip.css b/packages/ui/src/components/tooltip.css index f21b331e9..ae07d01b0 100644 --- a/packages/ui/src/components/tooltip.css +++ b/packages/ui/src/components/tooltip.css @@ -9,14 +9,15 @@ } [data-slot="tooltip-keybind-key"] { - /* Plain OS-native glyphs (⇧⌘S), not a keycap — see DESIGN.md Tooltip. - Sans, not mono: the monospace cell width crammed the modifier glyphs - together, which read as one squished blob. */ + /* The shortcut is plain OS-native glyphs (⇧⌘S), not a keycap (see DESIGN.md + Tooltip). Only two properties are set; size / line-height / color inherit + the tooltip's 13px cream text: + - sans, never mono (--type-kbd) — the monospace cell width crammed the + modifier glyphs into one squished blob; + - caption weight (400), one step lighter than the label's 500 emphasis, + so the shortcut reads as secondary metadata. */ font-family: var(--font-family-sans); - font-size: 12px; - font-weight: var(--font-weight-emphasis); - line-height: 1.2; - color: var(--bg-cream); + font-weight: var(--font-weight-caption); } [data-component="tooltip"] { From eb7114541808d5967ec4de5f966b0be04702b301 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Wed, 27 May 2026 22:10:01 +0800 Subject: [PATCH 5/5] chore(ui): drop dead token name from tooltip keybind comment The no-dead-tokens guardrail scans all of packages/ (comments included) for retired typography custom properties. The keybind comment named the old keycap token to explain the anti-mono choice; reword without it. --- packages/ui/src/components/tooltip.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/components/tooltip.css b/packages/ui/src/components/tooltip.css index ae07d01b0..8b7d6e1d7 100644 --- a/packages/ui/src/components/tooltip.css +++ b/packages/ui/src/components/tooltip.css @@ -12,7 +12,7 @@ /* The shortcut is plain OS-native glyphs (⇧⌘S), not a keycap (see DESIGN.md Tooltip). Only two properties are set; size / line-height / color inherit the tooltip's 13px cream text: - - sans, never mono (--type-kbd) — the monospace cell width crammed the + - sans, never the mono keycap font — its fixed cell width crammed the modifier glyphs into one squished blob; - caption weight (400), one step lighter than the label's 500 emphasis, so the shortcut reads as secondary metadata. */