diff --git a/packages/app/e2e/snap/command-palette-header.snap.ts b/packages/app/e2e/snap/command-palette-header.snap.ts new file mode 100644 index 000000000..e5a355a07 --- /dev/null +++ b/packages/app/e2e/snap/command-palette-header.snap.ts @@ -0,0 +1,69 @@ +import type { Locator, Page } from "@playwright/test" +import { test, expect } from "../fixtures" +import { openPalette } from "../actions" +import { applyDarkModeForTests } from "../utils" +import { composeGrid, snapOutputPath, type Shot } from "./_compose" + +// Visual contract for the command palette's grouped section headers: +// +// In dark mode --surface-base (#1a1917) is noticeably darker than the +// palette body's --surface-raised (#2d2a27). The List component paints its +// sticky group header on its own surface, exposed as the inherited +// --list-surface custom property (default --surface-base). The palette sits +// on --surface-raised, so palette-content must set --list-surface to match; +// otherwise the "Suggested / Navigation / Panels" headers read as black +// bands across an otherwise warm-grey palette. +// +// This is the regression guard for that custom-property contract — earlier +// the palette tried to win the same effect with a low-specificity +// background override that lost to list.css's 0,4,0 chain (and to its later +// import order), so the band stayed black in dark mode. +// +// Light + dark are both captured because the bug only appears in dark. +test.use({ viewport: { width: 900, height: 700 }, deviceScaleFactor: 2 }) + +async function assertHeaderMatchesPaletteSurface(page: Page) { + const header = page.locator('[data-component="command-palette"] [data-slot="list-header"]').first() + await expect(header).toBeVisible() + + // Header surface must match the palette body's --surface-raised, not List's + // default --surface-base. In dark mode the two differ, so a regression of + // the --list-surface contract fails here. Poll to absorb first-paint races + // on a cold dev server, where styles can settle a frame after the header + // becomes visible. + await expect + .poll(() => + header.evaluate((el) => { + // Let the browser resolve the token in context instead of copying its + // raw value off documentElement. + const probe = document.createElement("span") + probe.style.color = "var(--surface-raised)" + document.body.appendChild(probe) + const expectedRgb = window.getComputedStyle(probe).color + probe.remove() + const headerBg = window.getComputedStyle(el).backgroundColor + return headerBg === expectedRgb + }), + ) + .toBe(true) +} + +test("command-palette-header", async ({ page, gotoSession }) => { + test.setTimeout(180_000) + + await gotoSession() + const lightDialog = await openPalette(page) + await assertHeaderMatchesPaletteSurface(page) + const lightShot: Shot = { name: "light", buf: await (lightDialog as Locator).screenshot() } + await page.keyboard.press("Escape") + + await applyDarkModeForTests(page) + await gotoSession() + const darkDialog = await openPalette(page) + await assertHeaderMatchesPaletteSurface(page) + const darkShot: Shot = { name: "dark", buf: await (darkDialog as Locator).screenshot() } + + const out = snapOutputPath("command-palette-header") + await composeGrid([lightShot, darkShot], out, { cols: 2 }) + process.stdout.write(`\n[snap] command-palette-header grid -> ${out}\n\n`) +}) diff --git a/packages/app/e2e/snap/dialog-grouped-header.snap.ts b/packages/app/e2e/snap/dialog-grouped-header.snap.ts new file mode 100644 index 000000000..fd8eaa765 --- /dev/null +++ b/packages/app/e2e/snap/dialog-grouped-header.snap.ts @@ -0,0 +1,80 @@ +import type { Locator, Page } from "@playwright/test" +import { test, expect } from "../fixtures" +import { openSettings } from "../actions" +import { applyDarkModeForTests } from "../utils" +import { composeGrid, snapOutputPath, type Shot } from "./_compose" + +// Visual contract for grouped Lists rendered inside a Dialog (the twin of the +// command-palette case). The provider picker dialog groups providers into +// "Popular" / "Other", so each group paints a sticky List header. +// +// In dark mode --surface-base (#1a1917) is darker than the dialog body's +// --surface-raised (#2d2a27). The List paints its header on the inherited +// --list-surface custom property (default --surface-base), so dialog-content +// must set --list-surface to its own raised surface; otherwise the group +// labels read as black bands across the warm-grey dialog. +// +// The shared list.css mechanism is already guarded by +// command-palette-header.snap.ts; this is the dialog-local guard, so deleting +// the one --list-surface line from dialog.css fails here rather than slipping +// through to a dark-mode regression. +// +// Light + dark are both captured because the bug only appears in dark. +test.use({ viewport: { width: 900, height: 700 }, deviceScaleFactor: 2 }) + +async function openProviderDialog(page: Page): Promise { + const settings = await openSettings(page) + await settings.getByRole("tab", { name: "Providers" }).click() + await settings.getByRole("button", { name: "Show more providers" }).click() + + const dialog = page.locator('[data-component="dialog"] [data-slot="dialog-content"]').first() + await expect(dialog).toBeVisible() + // The grouped headers are the subject under test; wait for at least one. + await expect(page.locator('[data-component="dialog"] [data-slot="list-header"]').first()).toBeVisible() + return dialog +} + +async function assertHeaderMatchesDialogSurface(page: Page) { + const header = page.locator('[data-component="dialog"] [data-slot="list-header"]').first() + await expect(header).toBeVisible() + + // Header surface must match the dialog body's --surface-raised, not List's + // default --surface-base. In dark mode the two differ, so a regression of + // the --list-surface contract fails here. Poll to absorb first-paint races + // on a cold dev server, where styles can settle a frame after the header + // becomes visible. + await expect + .poll(() => + header.evaluate((el) => { + // Let the browser resolve the token in context instead of copying its + // raw value off documentElement. + const probe = document.createElement("span") + probe.style.color = "var(--surface-raised)" + document.body.appendChild(probe) + const expectedRgb = window.getComputedStyle(probe).color + probe.remove() + const headerBg = window.getComputedStyle(el).backgroundColor + return headerBg === expectedRgb + }), + ) + .toBe(true) +} + +test("dialog-grouped-header", async ({ page, gotoSession }) => { + test.setTimeout(180_000) + + await gotoSession() + const lightDialog = await openProviderDialog(page) + await assertHeaderMatchesDialogSurface(page) + const lightShot: Shot = { name: "light", buf: await lightDialog.screenshot() } + + await applyDarkModeForTests(page) + await gotoSession() + const darkDialog = await openProviderDialog(page) + await assertHeaderMatchesDialogSurface(page) + const darkShot: Shot = { name: "dark", buf: await darkDialog.screenshot() } + + const out = snapOutputPath("dialog-grouped-header") + await composeGrid([lightShot, darkShot], out, { cols: 2 }) + process.stdout.write(`\n[snap] dialog-grouped-header grid -> ${out}\n\n`) +}) diff --git a/packages/ui/src/components/command-palette.css b/packages/ui/src/components/command-palette.css index 3255c5c8f..8095639f7 100644 --- a/packages/ui/src/components/command-palette.css +++ b/packages/ui/src/components/command-palette.css @@ -25,6 +25,8 @@ border-radius: var(--radius-lg); background: var(--surface-raised); + /* Tell the nested List its sticky headers sit on the raised surface. */ + --list-surface: var(--surface-raised); background-clip: padding-box; box-shadow: var(--shadow-floating); border: 1px solid var(--border-base); @@ -118,7 +120,7 @@ letter-spacing: 0.5px; line-height: 1; color: var(--fg-weak); - background: var(--surface-raised); + /* Background comes from --list-surface set on palette-content (see top). */ &::after { display: none; diff --git a/packages/ui/src/components/dialog.css b/packages/ui/src/components/dialog.css index 3e9523b70..657af764f 100644 --- a/packages/ui/src/components/dialog.css +++ b/packages/ui/src/components/dialog.css @@ -50,6 +50,8 @@ /* padding: 8px 8px 0 8px; */ border-radius: var(--radius-lg); background: var(--surface-raised); + /* Grouped Lists inside a dialog read this for their sticky headers. */ + --list-surface: var(--surface-raised); background-clip: padding-box; box-shadow: var(--shadow-modal); diff --git a/packages/ui/src/components/list.css b/packages/ui/src/components/list.css index 605c9e9e4..4ed36a7ff 100644 --- a/packages/ui/src/components/list.css +++ b/packages/ui/src/components/list.css @@ -195,7 +195,12 @@ justify-content: space-between; align-items: center; align-self: stretch; - background: var(--surface-base); + /* Sticky headers need an opaque backdrop matching the surface the list + sits on. List defaults to the page canvas (--surface-base); popover + consumers (picker, command palette) sit on --surface-raised and set + --list-surface on their container so the header tracks them without + a per-consumer specificity fight. */ + background: var(--list-surface, var(--surface-base)); position: sticky; top: 0; @@ -215,7 +220,7 @@ left: 0; right: 0; height: 16px; - background: linear-gradient(to bottom, var(--surface-base), transparent); + background: linear-gradient(to bottom, var(--list-surface, var(--surface-base)), transparent); pointer-events: none; opacity: 0; transition: opacity 0.15s ease; diff --git a/packages/ui/src/components/picker.css b/packages/ui/src/components/picker.css index 706c6e7cd..b8288ad12 100644 --- a/packages/ui/src/components/picker.css +++ b/packages/ui/src/components/picker.css @@ -30,6 +30,8 @@ [data-picker-content] { background-color: var(--surface-raised); + /* Tell the nested List its headers sit on the raised surface. */ + --list-surface: var(--surface-raised); border-radius: var(--radius-md); padding: 4px; box-shadow: var(--ring-base), var(--shadow-floating); @@ -46,18 +48,16 @@ Let labels scroll with their group, and add a small gap so the label doesn't read as glued to the first item. + The header surface is handled by --list-surface on [data-picker-content] + above; this rule only undoes the sticky positioning and its gradient. Selector matches list.css's default header chain ([data-component="list"] [data-slot="list-scroll"] [data-slot="list-group"] [data-slot="list-header"], specificity 0,4,0) so picker.css — imported after list.css — wins on equal specificity. A shorter selector here - loses to the default and leaves the header sticky. Background swaps to - picker's raised surface (default --surface-base reads darker than the - picker body in dark mode), and the 16px ::after stuck-gradient is - hidden so it cannot paint over the first item. */ + loses to the default and leaves the header sticky. */ [data-picker-content] [data-slot="list-scroll"] [data-slot="list-group"] [data-slot="list-header"] { position: static; z-index: auto; - background: var(--surface-raised); &::after { display: none;