Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions packages/app/e2e/snap/command-palette-header.snap.ts
Original file line number Diff line number Diff line change
@@ -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`)
})
80 changes: 80 additions & 0 deletions packages/app/e2e/snap/dialog-grouped-header.snap.ts
Original file line number Diff line number Diff line change
@@ -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<Locator> {
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`)
})
4 changes: 3 additions & 1 deletion packages/ui/src/components/command-palette.css
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions packages/ui/src/components/dialog.css
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
9 changes: 7 additions & 2 deletions packages/ui/src/components/list.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
10 changes: 5 additions & 5 deletions packages/ui/src/components/picker.css
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down