diff --git a/docs/internal/DESIGN.md b/docs/internal/DESIGN.md index 1ca6ddf3..2f7b0d8c 100644 --- a/docs/internal/DESIGN.md +++ b/docs/internal/DESIGN.md @@ -167,7 +167,7 @@ put together from them and draws nothing of its own that one of them already dra a device's shows the device's state as a status, word and square. - **Menu.** A table row ends in a **⋯** `IconButton` that opens a small list; a right-click on the row opens the same list, and a drawer or page header shows its item's first safe action as a - secondary button beside its own **⋯**. An entry is 36px tall with `--text`; the highlighted one + secondary button beside its own **⋯**, and that list does not repeat the button's action. An entry is 36px tall with `--text`; the highlighted one takes `--surface` and the focus ring inside its edge. Dangerous entries come last, after a 1px `--border` line, in `--status-error`, never `--accent`. A disabled entry is `--text-muted`. A list with no entry draws no **⋯**. Arrow keys move, Escape closes. It floats above drawers and @@ -178,7 +178,8 @@ put together from them and draws nothing of its own that one of them already dra its progress ("Releasing…"). A dialog that asks for a typed name keeps the action disabled until the text equals it. - **Toast.** At the bottom right, newest at the bottom, at most three (older ones wait hidden), each - a title, an optional line and an optional action button, and a **Close** button. A success toast + a title, an optional line and an optional action button, and a **Close** button beside the text, + never over it. A success toast leaves after 6 seconds, paused while hovered or focused. An error toast has a `--status-error` edge and title, stays until closed and is announced at once (`role="alert"`); the rest are announced politely. A toast moves in once, when it appears. diff --git a/e2e/console/actions.spec.ts b/e2e/console/actions.spec.ts index 6fbb7265..06e128cb 100644 --- a/e2e/console/actions.spec.ts +++ b/e2e/console/actions.spec.ts @@ -478,15 +478,13 @@ test.describe("the console acts", () => { await page.getByRole("button", { name: "Drain", exact: true }).click(); await expect(page.getByText("bar-a drained", { exact: true })).toBeVisible(); await expect(page.getByRole("button", { name: "Undrain", exact: true })).toBeVisible(); - await page.getByRole("button", { name: "Actions for bar-a" }).click(); - await expect(page.getByRole("menuitem", { name: "Undrain" })).toBeVisible(); - await page.keyboard.press("Escape"); + await expect(page.getByRole("button", { name: "Actions for bar-a" })).toHaveCount(0); const [stopped] = fleet.workers as [RunningDaemon]; await stopped.stop(); await waitForDisconnected(fleet.gateway); await page.getByRole("button", { name: "Actions for bar-a" }).click(); - await expect(page.getByRole("menuitem")).toHaveText(["Undrain", "Remove worker"]); + await expect(page.getByRole("menuitem")).toHaveText(["Remove worker"]); await page.keyboard.press("Escape"); await expect(page.getByRole("button", { name: "Undrain", exact: true })).toBeVisible(); await expect(page.getByRole("button", { name: "Remove worker" })).toHaveCount(0); diff --git a/e2e/console/inbox.spec.ts b/e2e/console/inbox.spec.ts index 7fbbced8..f32a3dee 100644 --- a/e2e/console/inbox.spec.ts +++ b/e2e/console/inbox.spec.ts @@ -278,6 +278,10 @@ test.describe("the Inbox view", () => { expect((await fleet.gateway.cli(["worker", "drain", a?.id ?? ""])).code).toBe(0); await openInbox(page, fleet.gateway); + const row = page.getByRole("region", { name: "Needs a look" }); + await expect(row.getByRole("button", { name: "Undrain", exact: true })).toBeVisible(); + await expect(row.getByRole("button", { name: /^Actions for/ })).toHaveCount(0); + await item(page, "drained", "worker-a").click(); await expect(page).toHaveURL(new RegExp(`/inbox/${encodeURIComponent(`${a?.id}/drained`)}$`)); @@ -286,6 +290,7 @@ test.describe("the Inbox view", () => { await expect(drawer.getByRole("region", { name: "What happened" })).toContainText( "worker-a is drained", ); + await expect(drawer.getByRole("button", { name: "Undrain", exact: true })).toBeVisible(); await expect(facts(page)).toContainText(/Leases still held\s*0/); await expect(drawer.getByRole("link", { name: "Open worker" })).toHaveAttribute( "href", diff --git a/e2e/console/layout.spec.ts b/e2e/console/layout.spec.ts new file mode 100644 index 00000000..72051d42 --- /dev/null +++ b/e2e/console/layout.spec.ts @@ -0,0 +1,161 @@ +import type { Page } from "@playwright/test"; + +import { disposeFleet, expect, type RunningDaemon, signIn, startFleet, test } from "./fixtures.js"; + +const IOS = { availableOsVersions: ["18.4"], knownModels: ["iPhone 16"] }; + +async function openPage( + page: Page, + daemon: RunningDaemon, + path: string, + title: string, + signedIn: boolean, +) { + await page.goto(new URL(path, daemon.url).toString()); + if (!signedIn) await signIn(page, daemon.tokens.operator); + await expect(page.getByRole("heading", { level: 1, name: title })).toBeVisible(); +} + +test.describe("the console's layout", () => { + test("the page reserves room for a scrollbar on every page", async ({ page }) => { + const fleet = await startFleet([{ driverScript: { ios: IOS }, label: "layout-a" }]); + try { + const listed = await fleet.gateway.cli(["worker", "list", "--json"]); + const id = (listed.json as { workers: { id: string }[] }).workers[0]?.id ?? ""; + const pages = [ + { path: "/", title: "Overview" }, + { path: "/fleet", title: "Fleet" }, + { path: `/fleet/${id}`, title: "layout-a" }, + ]; + for (const [index, { path, title }] of pages.entries()) { + await openPage(page, fleet.gateway, path, title, index > 0); + const gutter = await page.evaluate( + () => getComputedStyle(document.documentElement).scrollbarGutter, + ); + expect(gutter, path).toBe("stable"); + } + } finally { + await disposeFleet(fleet); + } + }); + + test("a worker page has no breadcrumb, and its Warm pool tab draws a 1px --border line between the facts and the table", async ({ + page, + }) => { + const fleet = await startFleet([{ driverScript: { ios: IOS }, label: "layout-a" }]); + try { + const listed = await fleet.gateway.cli(["worker", "list", "--json"]); + const id = (listed.json as { workers: { id: string }[] }).workers[0]?.id ?? ""; + await openPage(page, fleet.gateway, `/fleet/${id}?tab=warm-pool`, "layout-a", false); + + await expect(page.getByRole("navigation", { name: "Breadcrumb" })).toHaveCount(0); + const panel = page.getByRole("region", { name: "Warm pool", exact: true }); + await expect(panel.locator(".facts")).toBeVisible(); + const line = await panel.locator(".table-card").evaluate((card) => { + const style = getComputedStyle(card); + const probe = document.createElement("span"); + probe.style.color = "var(--border)"; + document.body.append(probe); + const border = getComputedStyle(probe).color; + probe.remove(); + return { border, color: style.borderTopColor, width: style.borderTopWidth }; + }); + expect(line.width).toBe("1px"); + expect(line.color).toBe(line.border); + } finally { + await disposeFleet(fleet); + } + }); + + test("a fleet row's ⋯ lies inside its cell with 24px to the table's right edge, and an Inbox row's button has 8px above and below", async ({ + page, + }) => { + const fleet = await startFleet([{ driverScript: { ios: IOS }, label: "layout-a" }]); + try { + const listed = await fleet.gateway.cli(["worker", "list", "--json"]); + const id = (listed.json as { workers: { id: string }[] }).workers[0]?.id ?? ""; + await openPage(page, fleet.gateway, "/fleet", "Fleet", false); + const menu = page.locator("main tbody tr").getByRole("button", { name: /^Actions for/ }); + const box = await menu.evaluate((button) => { + const cell = button.closest("td")?.getBoundingClientRect(); + const table = button.closest("table")?.getBoundingClientRect(); + const own = button.getBoundingClientRect(); + return { + cellLeft: cell?.left ?? 0, + cellRight: cell?.right ?? 0, + left: own.left, + right: own.right, + tableRight: table?.right ?? 0, + }; + }); + expect(box.left).toBeGreaterThanOrEqual(box.cellLeft); + expect(box.right).toBeLessThanOrEqual(box.cellRight); + expect(box.tableRight - box.right).toBe(24); + + expect((await fleet.gateway.cli(["worker", "drain", id])).code).toBe(0); + await openPage(page, fleet.gateway, "/inbox", "Inbox", true); + const undrain = page.getByRole("button", { name: "Undrain", exact: true }); + const gaps = await undrain.evaluate((button) => { + const row = button.closest("li")?.getBoundingClientRect(); + const own = button.getBoundingClientRect(); + return { bottom: (row?.bottom ?? 0) - own.bottom, top: own.top - (row?.top ?? 0) }; + }); + expect(gaps).toEqual({ bottom: 8, top: 8 }); + } finally { + await disposeFleet(fleet); + } + }); + + test("a toast's close button lies beside its text, inside the toast, and a one-line toast centres both", async ({ + page, + }) => { + const fleet = await startFleet([{ driverScript: { ios: IOS }, label: "layout-a" }]); + try { + const listed = await fleet.gateway.cli(["worker", "list", "--json"]); + const id = (listed.json as { workers: { id: string }[] }).workers[0]?.id ?? ""; + await openPage(page, fleet.gateway, `/fleet/${id}`, "layout-a", false); + + const measure = (title: string) => + page + .locator(".toast") + .filter({ has: page.getByText(title, { exact: true }) }) + .evaluate((toast) => { + const box = toast.getBoundingClientRect(); + const close = toast.querySelector(".toast-close")?.getBoundingClientRect(); + const title = toast.querySelector(".toast-title"); + if (close === undefined || title === null) throw new Error("toast has no parts"); + const range = document.createRange(); + range.selectNodeContents(title); + const text = range.getBoundingClientRect(); + return { + closeInside: + close.left >= box.left && + close.right <= box.right && + close.top >= box.top && + close.bottom <= box.bottom, + centres: Math.abs(close.top + close.height / 2 - (text.top + text.height / 2)), + overlap: + text.right > close.left && text.left < close.right && text.bottom > close.top, + hasAction: toast.querySelector(".toast-action") !== null, + }; + }); + + await page.getByRole("button", { name: "Drain", exact: true }).click(); + await expect(page.getByText("layout-a drained", { exact: true })).toBeVisible(); + const drained = await measure("layout-a drained"); + expect(drained.hasAction).toBe(true); + expect(drained.closeInside).toBe(true); + expect(drained.overlap).toBe(false); + + await page.getByRole("button", { name: "Undrain", exact: true }).click(); + await expect(page.getByText("layout-a takes leases again", { exact: true })).toBeVisible(); + const undrained = await measure("layout-a takes leases again"); + expect(undrained.hasAction).toBe(false); + expect(undrained.closeInside).toBe(true); + expect(undrained.overlap).toBe(false); + expect(undrained.centres).toBeLessThanOrEqual(4); + } finally { + await disposeFleet(fleet); + } + }); +}); diff --git a/e2e/console/leases.spec.ts b/e2e/console/leases.spec.ts index c2f68243..84303aa3 100644 --- a/e2e/console/leases.spec.ts +++ b/e2e/console/leases.spec.ts @@ -360,7 +360,11 @@ test.describe("the leases page", () => { const drawer = page.getByRole("dialog"); await expect(drawer).toBeVisible(); - expect((await drawer.boundingBox())?.width).toBeCloseTo(360, 0); + // why: the page reserves a scrollbar gutter, so the page is narrower than the 360px viewport. + const available = await page.evaluate( + () => document.documentElement.getBoundingClientRect().width, + ); + expect((await drawer.boundingBox())?.width).toBeCloseTo(available, 0); expect(await hasHorizontalScroll(page)).toBe(false); } finally { await host.dispose(); diff --git a/e2e/console/long-lists.spec.ts b/e2e/console/long-lists.spec.ts index f23ef78f..4f5052b4 100644 --- a/e2e/console/long-lists.spec.ts +++ b/e2e/console/long-lists.spec.ts @@ -389,7 +389,10 @@ test.describe("long lists page", () => { await page.reload(); await expect(pager(page, "Devices").locator(".pager-range")).toHaveText("51–60 of 60"); - await page.getByRole("navigation", { name: "Breadcrumb" }).getByRole("link").click(); + await page.goBack({ waitUntil: "commit" }); + await page.goBack({ waitUntil: "commit" }); + await page.goBack({ waitUntil: "commit" }); + await expect(page).toHaveURL(/\/fleet\?page=2$/); await expect(pager(page, "Workers").locator(".pager-range")).toHaveText("26–30 of 30"); }); diff --git a/e2e/console/workers.spec.ts b/e2e/console/workers.spec.ts index 06f293f1..038aae0a 100644 --- a/e2e/console/workers.spec.ts +++ b/e2e/console/workers.spec.ts @@ -931,7 +931,7 @@ test.describe("the workers page", () => { await expect(figure(page, "Leases").locator(".figure-count")).toHaveText("1", { timeout: 10_000, }); - await page.getByRole("navigation", { name: "Breadcrumb" }).getByRole("link").click(); + await page.goBack(); await expect(running()).toHaveText(/^1 \/ \d+$/, { timeout: 10_000 }); } finally { await disposeFleet(fleet); @@ -1132,7 +1132,7 @@ test.describe("the workers page", () => { timeout: 10_000, }); - await page.getByRole("navigation", { name: "Breadcrumb" }).getByRole("link").click(); + await page.goBack(); await expect(row(page, "worker-b").getByRole("cell").nth(1)).toHaveText("disconnected"); await expect(row(page, "worker-a").getByRole("cell").nth(1)).not.toHaveText("disconnected"); await expect(page.getByText("1 / 2 up")).toBeVisible(); @@ -1289,10 +1289,7 @@ test.describe("the workers page", () => { await page.keyboard.press("Enter"); await expect(host_tab).toHaveAttribute("aria-selected", "true"); - await follow( - page.getByRole("navigation", { name: "Breadcrumb" }).getByRole("link", { name: "Fleet" }), - "backward", - ); + await follow(page.getByRole("link", { name: "Fleet", exact: true }), "backward"); await expect(page).toHaveURL(/\/fleet$/); }); }); diff --git a/ui/src/ds/foundation.css b/ui/src/ds/foundation.css index f1d34bde..f912bbef 100644 --- a/ui/src/ds/foundation.css +++ b/ui/src/ds/foundation.css @@ -78,6 +78,10 @@ box-sizing: border-box; } +html { + scrollbar-gutter: stable; +} + html, body { margin: 0; diff --git a/ui/src/ds/master-detail.css b/ui/src/ds/master-detail.css index 0a2bbba1..550745b4 100644 --- a/ui/src/ds/master-detail.css +++ b/ui/src/ds/master-detail.css @@ -1,4 +1,4 @@ -/* A list of rows, and the breadcrumb a worker's page carries. */ +/* A list of rows. */ .list { margin: 0; @@ -57,25 +57,3 @@ font-size: var(--text-xs); text-overflow: ellipsis; } - -.crumbs { - display: flex; - flex-wrap: wrap; - gap: var(--space-2); - color: var(--text-muted); - font-family: var(--font-mono); - font-size: var(--text-xs); -} - -.crumb + .crumb::before { - content: "/"; - margin-right: var(--space-2); -} - -.crumb { - min-width: 0; - max-width: 100%; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; -} diff --git a/ui/src/ds/menu.test.tsx b/ui/src/ds/menu.test.tsx new file mode 100644 index 00000000..7ae94ad4 --- /dev/null +++ b/ui/src/ds/menu.test.tsx @@ -0,0 +1,31 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it } from "vitest"; + +import { ActionBar, type MenuEntry } from "./menu"; + +const entry = (label: string, danger: boolean): MenuEntry => ({ + danger, + disabled: false, + key: label, + label, + onSelect: () => {}, +}); + +describe("the action bar", () => { + it("draws no ⋯ when its only safe entry is the button, and keeps the ⋯ for any other entry", () => { + const draw = (entries: readonly MenuEntry[]) => + renderToStaticMarkup(); + + const onlyButton = draw([entry("Undrain", false)]); + expect(onlyButton).toContain("Undrain"); + expect(onlyButton).not.toContain('aria-label="Actions for bar-a"'); + + const withDanger = draw([entry("Undrain", false), entry("Remove worker", true)]); + expect(withDanger).toContain("Undrain"); + expect(withDanger).toContain('aria-label="Actions for bar-a"'); + + const dangerOnly = draw([entry("Release lease", true)]); + expect(dangerOnly).not.toContain("button-secondary"); + expect(dangerOnly).toContain('aria-label="Actions for bar-a"'); + }); +}); diff --git a/ui/src/ds/menu.tsx b/ui/src/ds/menu.tsx index cf942830..fbcf4519 100644 --- a/ui/src/ds/menu.tsx +++ b/ui/src/ds/menu.tsx @@ -87,8 +87,8 @@ export function RowMenu(props: { readonly label: string; readonly entries: reado ); } -// why: a drawer or a page shows its item's first safe action as a button and every action under -// its own ⋯, the dangerous ones last. +// why: a drawer or a page shows its item's first safe action as a button and the other actions +// under its own ⋯, the dangerous ones last; the ⋯ does not repeat the button. export function ActionBar(props: { readonly label: string; readonly entries: readonly MenuEntry[]; @@ -103,7 +103,7 @@ export function ActionBar(props: { {first.label} )} - + entry !== first)} /> ); } diff --git a/ui/src/ds/table.css b/ui/src/ds/table.css index 4f529408..84b8db1a 100644 --- a/ui/src/ds/table.css +++ b/ui/src/ds/table.css @@ -212,7 +212,7 @@ } .table .table-menu { - width: 3.5rem; + width: 5rem; text-align: right; } diff --git a/ui/src/ds/toast.css b/ui/src/ds/toast.css index 443281f7..0a964d7f 100644 --- a/ui/src/ds/toast.css +++ b/ui/src/ds/toast.css @@ -34,11 +34,17 @@ .toast-content { display: grid; - gap: var(--space-1); - padding: var(--space-3) var(--space-4); + grid-template-columns: minmax(0, 1fr) auto; + gap: var(--space-1) var(--space-2); + padding: var(--space-1) var(--space-1) var(--space-3) var(--space-4); +} + +.toast-content > :not(.toast-close) { + grid-column: 1; } .toast-title { + align-self: center; margin: 0; font-size: var(--text-sm); font-weight: 600; @@ -69,8 +75,9 @@ display: none; } -.toast-close { - position: absolute; - top: var(--space-2); - right: var(--space-2); +/* why: the close keeps its 40px hit area but not its border, so it reads as part of the toast. */ +.toast .toast-close { + grid-column: 2; + grid-row: 1; + border-color: transparent; } diff --git a/ui/src/screens/fleet/fleet.css b/ui/src/screens/fleet/fleet.css index 4a8fffd2..2db02ceb 100644 --- a/ui/src/screens/fleet/fleet.css +++ b/ui/src/screens/fleet/fleet.css @@ -165,3 +165,8 @@ .worker-events li > :last-child { grid-column: 2; } + +.panel .facts + .table-card { + margin-top: var(--space-4); + border-top: 1px solid var(--border); +} diff --git a/ui/src/screens/fleet/worker-page.tsx b/ui/src/screens/fleet/worker-page.tsx index 702cbc9d..d9db8ad2 100644 --- a/ui/src/screens/fleet/worker-page.tsx +++ b/ui/src/screens/fleet/worker-page.tsx @@ -70,11 +70,6 @@ export function WorkerPageBody({ page }: { readonly page: WorkerPageView }) { {...(page.version === undefined ? {} : { note: page.version })} actions={} > -

{page.health.word} {page.system === undefined ? null : <> · {page.system}} diff --git a/ui/src/screens/inbox/inbox.css b/ui/src/screens/inbox/inbox.css index 67ff3279..56a2f2e3 100644 --- a/ui/src/screens/inbox/inbox.css +++ b/ui/src/screens/inbox/inbox.css @@ -97,7 +97,7 @@ display: flex; align-items: center; gap: var(--space-2); - padding-right: var(--space-4); + padding: var(--space-2) var(--space-4) var(--space-2) 0; } .inbox-row { diff --git a/ui/src/screens/inbox/index.tsx b/ui/src/screens/inbox/index.tsx index 6e9058d7..fceefbc9 100644 --- a/ui/src/screens/inbox/index.tsx +++ b/ui/src/screens/inbox/index.tsx @@ -129,6 +129,7 @@ export function InboxBody(props: { function InboxActions(props: { readonly row: InboxRow; readonly actions: InboxRowActions }) { const { actions, row } = props; + const rest = actions.menu.filter((entry) => entry.key !== actions.fix?.key); return ( {actions.fix === undefined ? null : ( @@ -136,9 +137,7 @@ function InboxActions(props: { readonly row: InboxRow; readonly actions: InboxRo {actions.fix.label} )} - {actions.menu.length === 0 ? null : ( - - )} + ); }