diff --git a/web_ui/e2e/boot.spec.ts b/web_ui/e2e/boot.spec.ts index d7e9a5e..c14ffaf 100644 --- a/web_ui/e2e/boot.spec.ts +++ b/web_ui/e2e/boot.spec.ts @@ -22,9 +22,16 @@ test("boots against the real backend and renders the canvas shell", async ({ pag await expect(page.getByTestId("scene-canvas")).toBeVisible(); await expect(page.locator(".react-flow__viewport")).toBeVisible(); - // The connection badge (connectionBadge.ts) reporting the real WS - // handshake's outcome, not just "the page loaded". - await expect(page.locator(".app-conn-open")).toHaveText("connected"); + // The real WS handshake's outcome (App.tsx's data-connection-status), + // not just "the page loaded". gotoApp already waited on this exact + // attribute to reach "open" before returning, so this is a direct + // re-assertion of that same state rather than a race - the app bar and + // canvas checks above ran after that wait, and a WS drop between then and + // now is exactly the kind of regression this line exists to catch. + // connectionBadge.ts's own label text is NOT asserted here any more: it + // now renders only for a degraded connection (App.tsx), so "connected" + // has no visible text to check on the happy path this test exercises. + await expect(page.locator('.app-shell[data-connection-status="open"]')).toBeAttached(); // A fresh session has zero nodes - SceneCanvas.tsx's own empty-state hint // is the honest "nothing broken, genuinely nothing here yet" signal for diff --git a/web_ui/e2e/helpers.ts b/web_ui/e2e/helpers.ts index 2f35771..b387586 100644 --- a/web_ui/e2e/helpers.ts +++ b/web_ui/e2e/helpers.ts @@ -6,11 +6,16 @@ import { expect, type Page } from "@playwright/test"; * * Two things every spec would otherwise have to repeat: * - * 1. Wait for the REAL WS round-trip to complete (App.tsx's connection - * badge, `.app-conn-`) before touching anything - a fresh - * `page.goto("/")` returns as soon as the SPA shell's static HTML/JS - * loads, well before the WsTransport handshake against the real backend - * (tests_e2e/run_backend.py) has actually completed. + * 1. Wait for the REAL WS round-trip to complete (App.tsx's + * `data-connection-status` attribute on `.app-shell`) before touching + * anything - a fresh `page.goto("/")` returns as soon as the SPA shell's + * static HTML/JS loads, well before the WsTransport handshake against the + * real backend (tests_e2e/run_backend.py) has actually completed. This + * used to wait on the visible connection badge (`.app-conn-open`), which + * stopped being a reliable signal once that badge became exception-only + * (App.tsx renders nothing there for a healthy connection by design) - + * the attribute is the same underlying signal without depending on + * whatever the topbar currently chooses to show for it. * 2. Dismiss the first-run onboarding wizard (chrome/OnboardingDialog.tsx). * Every E2E run boots against a BRAND NEW settings_state_file (see that * script's own docstring on why - full isolation from a real user's @@ -38,13 +43,13 @@ import { expect, type Page } from "@playwright/test"; */ export async function gotoApp(page: Page): Promise { await page.goto("/"); - await expect(page.locator(".app-conn-open")).toBeVisible(); + await expect(page.locator('.app-shell[data-connection-status="open"]')).toBeAttached(); // waitFor (not isVisible(), which resolves immediately either way) is // deliberate: the app-settings snapshot that decides whether onboarding - // auto-opens arrives asynchronously over the SAME WS connection - // app-conn-open just confirmed, so it can genuinely still be in flight at - // this exact line. A bare isVisible() check here would race it - "not + // auto-opens arrives asynchronously over the SAME WS connection the + // attribute above just confirmed, so it can genuinely still be in flight + // at this exact line. A bare isVisible() check here would race it - "not // visible yet" and "never opening" look identical at a single instant, // and picking the wrong one would leave the dialog to pop up mid-test // instead of being dismissed up front. diff --git a/web_ui/src/app/App.tsx b/web_ui/src/app/App.tsx index 13582b3..0381c79 100644 --- a/web_ui/src/app/App.tsx +++ b/web_ui/src/app/App.tsx @@ -379,7 +379,18 @@ function App() { -
+ {/* data-connection-status: the real WS connection state, always + present regardless of whether the topbar renders any visual + indicator for it (see .app-topbar-status below - a healthy + connection now renders nothing there by design). The E2E suite's + shared boot helper (e2e/helpers.ts) waits on this attribute to + know the real backend round-trip has completed before touching + anything, which it can no longer do by waiting for a badge that + is often absent on purpose. Not a UI affordance - never styled, + never meant to be seen - so this stays a plain attribute rather + than a class, the same "invisible hook, not a rendered element" + posture as aria-live regions elsewhere in this file. */} +
{/* ADR-012 stage 12.3: the very first focusable element in the page, per the standard skip-link convention - invisible until it itself receives focus (Tab from anywhere before the canvas @@ -400,12 +411,39 @@ function App() {
{announcement}
+ {/* Three declared grid tracks - brand, toolbar, status - so the + regions of a permanently-visible bar have fixed homes and + cannot encroach on one another. See .app-topbar in styles.css + and AppBar.tsx's own layout contract. */}
- Graphlink +
+ + Graphlink +
- - {connectionBadgeLabel(status)} - + {/* Exception-only: a healthy connection shows nothing at all. + A permanent "connected" badge reports the expected state on + every frame and carries no information. The degraded states + do carry information - "reconnecting" is the app's own + answer to "why did my click do nothing", since intents are + queued or refused while it shows (see connectionBadge.ts) - + so those still surface, and only those. */} +
+ {status !== "open" && ( + + + )} +
diff --git a/web_ui/src/app/chrome/AppBar.test.tsx b/web_ui/src/app/chrome/AppBar.test.tsx index fb8e795..bfaa2a8 100644 --- a/web_ui/src/app/chrome/AppBar.test.tsx +++ b/web_ui/src/app/chrome/AppBar.test.tsx @@ -197,19 +197,29 @@ describe("AppBar", () => { await user.click(screen.getByRole("button", { name: "More toolbar actions" })); const menu = screen.getByRole("dialog"); + // Tiers live on the GROUP wrapper, not the individual button: the + // bar collapses whole clusters so related actions stay together at + // every width instead of leaving fragments behind (see AppBar.tsx). + // The contract this pins is unchanged though - an inline action and + // its overflow duplicate must always collapse at the same tier. const pairs: [string, string][] = [ - ["Export PNG", "1"], - ["Pins", "2"], + ["Undo", "1"], + ["Redo", "1"], + ["Zoom In", "1"], + ["Zoom Out", "1"], + ["Reset", "1"], + ["Fit All", "1"], ["Organize", "2"], - ["View", "2"], - ["Plugins", "2"], - ["Zoom In", "3"], - ["Zoom Out", "3"], - ["Reset", "3"], - ["Fit All", "3"], - ["About", "1"], - ["Help", "1"], - ["Diagnostics", "1"], + ["Pins", "2"], + ["Export PNG", "2"], + ["View", "3"], + ["Plugins", "3"], + ["Global Search", "4"], + ["Knowledge", "4"], + ["Builder", "4"], + ["Diagnostics", "4"], + ["Help", "4"], + ["About", "4"], ]; for (const [label, tier] of pairs) { // Every duplicated pair shares its exact label except Plugins (the @@ -222,7 +232,7 @@ describe("AppBar", () => { const matches = screen.getAllByRole("button", { name: new RegExp(label) }); const inline = matches.find((el) => !menu.contains(el)); const overflowItem = matches.find((el) => menu.contains(el)); - expect(inline).toHaveAttribute("data-tier", tier); + expect(inline?.closest(".appbar-group")).toHaveAttribute("data-tier", tier); expect(overflowItem).toHaveAttribute("data-tier", tier); } diff --git a/web_ui/src/app/chrome/AppBar.tsx b/web_ui/src/app/chrome/AppBar.tsx index 0eadbae..19e849f 100644 --- a/web_ui/src/app/chrome/AppBar.tsx +++ b/web_ui/src/app/chrome/AppBar.tsx @@ -4,10 +4,36 @@ import { exportCanvasAsPng } from "../canvas/exportCanvasPng"; import { motionDuration } from "../reducedMotion"; import type { SceneStore } from "../canvas/sceneStore"; import { Popover, useOverlays } from "../overlays/overlays"; +import { AppBarIcon, type AppBarIconName } from "./AppBarIcon"; /** * The app bar (Qt-removal plan R2) - the toolbar island's SPA successor. * + * LAYOUT CONTRACT. This bar is on screen at all times, so its geometry is + * fixed rather than emergent: + * + * - `.app-topbar` (styles.css) is a THREE-COLUMN GRID - brand, this + * toolbar, connection status - so those three regions occupy declared + * tracks and cannot encroach on one another. The previous layout was one + * flex row in which the status badge sat outside the toolbar and relied + * on `margin-left: auto` against a `flex: 1` sibling that had already + * eaten the free space, which is why it ended up jammed against the last + * button instead of anchored to the window edge. + * - The row has a FIXED height and every control a fixed height, so the bar + * never changes size with its contents. + * - Related actions live in `.appbar-group` containers with uniform inner + * spacing and a shared surface. Grouping is what carries the visual + * organisation; the old bar was one undifferentiated run of twenty text + * buttons separated by ad-hoc 1px rules. + * + * ICONS vs LABELS. Frequent, app-specific verbs (Library, Save, Organize, + * View, Plugins) keep text - they are the vocabulary of the product and + * nothing draws them unambiguously. Universally-recognised mechanics (undo, + * redo, the four viewport controls) and the utility surfaces on the right + * become icons with `title` + `aria-label` carrying the exact same wording + * they had as text, which is what keeps them findable by keyboard, by + * screen reader, and by every existing test. + * * Intent routing, surface by surface, against the ToolbarBridge @Slot list: * - zoomIn/zoomOut/resetZoom/fitAll -> React Flow viewport ops (they were * pure ChatView viewport calls; the viewport lives HERE now) @@ -26,66 +52,112 @@ import { Popover, useOverlays } from "../overlays/overlays"; * * R8a (UI/UX issue list finding #8): the provider-mode