From daa5d4c8ef7d01893b00e5a2d73de454a40ca1e4 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya Date: Sun, 9 Aug 2026 20:56:09 -0700 Subject: [PATCH] feat(annotator): one notice surface top-right, and the forward actions become a pair MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every sentence the editor floats over the stage now comes out of one anchor (`EditorNotice`), inset 16px from the stage's top and right edges. It replaces four placements in three treatments: two destructive badges inside the top bar's microtext, a full-bleed strip under the header, and the suggest card bottom-right, which had been clearing the zoom cluster with a hard-coded offset. The column is a stack, most-blocking first, because a suggest session and a refused save can both be true at once. Its body wraps mid-token and its width is `max-w-md`. A model reference is one unbroken string, so no fixed width guarantees the next one fits: wrapping is the invariant and the width is comfort. `Save and stay` moves to the resolve group, immediately after the primary next-action, filled in `success`. Advance and persist-in-place are two halves of one gesture and were a bar apart, the second of them the quietest control on the row. `DESIGN.md` records the two-fill exception, and the count test now sweeps both fills rather than one. The `⌘S` keycap goes — a chip inverts into a smudge inside a fill — and the chord is taught by the button's tooltip instead. The top bar's save state keeps three readings, not four: it says where the work is, and after a refused save the honest answer there is `unsaved`. --- DESIGN.md | 87 ++++- frontend/app/e2e/annotate.spec.ts | 128 ++++++- .../ui-core/src/annotator/AnnotationPage.tsx | 313 ++++++++++-------- .../ui-core/src/annotator/EditorNotice.tsx | 132 ++++++++ .../ui-core/src/annotator/SuggestPanel.tsx | 85 ++--- .../src/annotator/editorNotice.test.tsx | 233 +++++++++++++ .../ui-core/src/annotator/topBar.test.tsx | 104 +++++- frontend/ui-core/src/primitives/Button.tsx | 13 + frontend/ui-core/src/styles.css | 10 +- frontend/ui-core/src/tokens.ts | 5 + 10 files changed, 880 insertions(+), 230 deletions(-) create mode 100644 frontend/ui-core/src/annotator/EditorNotice.tsx create mode 100644 frontend/ui-core/src/annotator/editorNotice.test.tsx diff --git a/DESIGN.md b/DESIGN.md index 59513fac..2539a02e 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -219,8 +219,22 @@ Sibling actions are the outlined `secondary` variant — `card` fill, `input` bo so those are `secondary` too. The known exceptions, all legitimate: a modal `Dialog`'s confirm button (it overlays the -page, so it is its own view), and the steps of the ingest stepper (only the active step -renders). +page, so it is its own view), the steps of the ingest stepper (only the active step +renders), and **the annotation editor's paired forward actions**. + +That last one is the only place two filled buttons sit side by side, and it is recorded +rather than tolerated. The editor's top bar carries the primary next-action (`Save and +next` / `Next`, or `Finish job` on the last frame) in `primary`, and **Save and stay** in +`success`, immediately to its right. Two filled controls compete when they are two +answers to *what do I do next*; these are two halves of **one** answer — having finished +with this frame, advance or persist in place — and a person choosing between them is not +choosing between two calls to action. Colour is what separates their intent: a second +near-black beside the first would contend with it rather than pair with it, and the two +would read as a bar that could not decide. `success` is the only other fill in the +product, and this is its only filled use. + +The count test therefore reads the bar as `bg-primary` exactly once and `bg-success` +exactly once — still a count, still asserted from both sides. **The rule is a count, so it is tested as one — in both directions.** A test asserting that *the* CTA is present, or that a sibling is `secondary`, passes just as happily with two @@ -239,7 +253,7 @@ annotator and near-black in the gallery, and a *skipped* frame was painted `dest | Family | Token | |---|---| -| Done, settled, succeeded — `annotated`, `accepted`, batch `completed`, ingest `completed`, export `succeeded` | `success` | +| Done, settled, succeeded — `annotated`, `accepted`, batch `completed`, ingest `completed`, export `succeeded` — **and the annotation editor's `Save and stay` fill** | `success` | | Waiting on a person — `review_pending` | `warning` | | Failed — ingest `failed`, export `failed`, a corrupt file, a refusal | `destructive` | | Nothing has happened, or a decision was taken and nothing is wrong — `unannotated`, `skipped`, batch `draft`, export `queued`/`cancelled` | neutral | @@ -273,6 +287,14 @@ sixth colour is a compile error rather than a diff nobody notices. sanctioned hardcoded colour anywhere in the frontend, and `design_tokens.test.mjs` was already refusing one. +It gained two companions when the annotation editor's **Save and stay** became a filled +control: `success-foreground` (`#ffffff`, the ink on that fill) and `success-hover` +(`#3a896b`). The hover is derived rather than picked — `primary-hover` lifts `primary` by +(+12, +12, +16) per channel, and the same deltas applied to `success` give this — so the +bar's two filled controls brighten by the same amount under a pointer instead of each +having its own feel. They exist for that one control; a status *ink* still uses plain +`success`. + ## Typography System font stack (`-apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, …`), **14px @@ -532,8 +554,8 @@ The page the reference design shows (#56), with measurements verified in v1's so | Zone | Contents | | --- | --- | | Left | back · pinned `v{n}` badge · the frame's identity as a label (the content-hash head — there is no filename on the wire) · the frame microtext `● annotated · Saved` | - | Centre | the **navigation cluster**: `[⊞] [‹] n/m [›] │ [Skip] [Save and next]` | - | Right | `n / m annotated` · **Save and stay** (ghost, ⌘S) · the review move (outline) · overflow `⋯` | + | Centre | the **navigation cluster**: `[⊞] [‹] n/m [›] │ [Skip] [Save and next] [Save and stay]` | + | Right | `n / m annotated` · the review move (outline) · overflow `⋯` | **Everything that changes the picture on screen is in the centre cluster, and nothing else is** (#416). The gallery and `‹` `›` used to sit at the far left beside the back @@ -605,11 +627,21 @@ The page the reference design shows (#56), with measurements verified in v1's so tooltip saying what it means, because this product has **no annotator identity** (cf. #282) — a submitted frame is marked for a review pass, not routed to a person. - **Save is a ghost button again** (#383). #368 removed it on the grounds that it - duplicated an automatic behaviour — ⌘S saves, navigating saves, settling saves — and - dogfooding showed what that missed: the chord is invisible, and the overflow put the one - press meaning *store this now, without going anywhere* two clicks from the work. Ghost - is the honest weight for a control most people never need. + **Save and stay is the second half of the forward gesture, and it sits beside the + first.** #368 removed the explicit save on the grounds that it duplicated an automatic + behaviour — ⌘S saves, navigating saves, settling saves — and dogfooding showed what that + missed: the chord is invisible, and the overflow put the one press meaning *store this + now, without going anywhere* two clicks from the work. #383 brought it back as a ghost + in the right zone, which fixed the reachability and left the reading wrong: *advance* + and *persist in place* are one decision read two ways, and they were a bar apart with + the second of them the quietest control on the row. + + It is now the third member of the resolve group — `Skip · [primary next-action] · Save + and stay` — **filled in `success`**, which is the recorded exception to *one filled + button per view* above. It keeps the frame verbs' lifetime rather than the mode's: a + closed batch or a finished job has nothing to save on any frame, so it leaves with Skip + and the flow verb; inside a working job it holds its slot, disabled, so the cluster does + not change width as somebody walks a mixed job. **Reabsorption order when the bar runs out of room**: `Save and stay` first (below `xl`), the review move second (below `lg`), into the overflow; the Skip/Save-and-next @@ -618,14 +650,33 @@ The page the reference design shows (#56), with measurements verified in v1's so readout has no overflow row and needs none — it truncates, which is what a sentence may do and a button may not. - **Hotkey chips go on the ghost and outline controls and on nothing else** — `⌘S` on - Save and stay, `X` on Skip, both rows in `core/input/bindings.ts`. A chip is a muted - box on a bordered ground, which is a *lighter-than-the-surface* treatment: on the one - filled control it inverts into a dark box inside a dark button and reads as a smudge - rather than as a key, so the flow verb carries none (#385). Its chord is not the loser - — `enter` is the one key with two meanings, the polygon ring close while a shape is in - progress and *finish the frame* otherwise, and both are in the shortcut sheet, which - derives its rows from the live registry rather than from a hand-written table. + **Hotkey chips go on the ghost and outline controls and on nothing else** — `X` on + Skip, from `core/input/bindings.ts`. A chip is a muted box on a bordered ground, which + is a *lighter-than-the-surface* treatment: inside a filled control it inverts into a + dark box on a dark ground and reads as a smudge rather than as a key, so neither the + flow verb (#385) nor Save and stay carries one. Neither chord is the loser. `⌘S` is + taught by Save and stay's tooltip — the tool strip's own pattern (`Box (B)`) — and + `enter` is the one key with two meanings, the polygon ring close while a shape is in + progress and *finish the frame* otherwise. Both are in the shortcut sheet, which derives + its rows from the live registry rather than from a hand-written table. + + **In-editor messages have one surface, top-right of the stage** (`EditorNotice`). Every + sentence the editor floats over the picture goes into one column inset 16px from the + stage's top and right edges: a suggest session, a refused save, a refused progress move, + and a batch or job that could not be opened. They were four placements in three + treatments — two destructive badges inside the top bar's microtext, a full-bleed strip + under the header, and the suggest card bottom-right — so *where* a sentence appeared + depended on which mutation produced it. Top-right is the corner nothing else occupies: + the tool strip is top-left, the object counter bottom-left, and the zoom cluster + bottom-right, which the suggest card had been clearing with a hard-coded offset. The + column is a stack, most-blocking first, because more than one of those can be true at + once. Its width is `max-w-md` and its body wraps mid-token — a model reference is one + unbroken string and **no fixed width guarantees the next one fits**, so wrapping is the + invariant and the width is comfort. + + The top bar keeps the frame microtext `● annotated · Saved`, which says *where the work + is* and has three readings, not four: after a refused save the honest answer there is + `unsaved`, and the reason is a sentence in the notice column. The frame microtext replaces the dot-with-a-tooltip: the word is on the bar beside the save state, because **status is never colour alone** and a tooltip is a place a word diff --git a/frontend/app/e2e/annotate.spec.ts b/frontend/app/e2e/annotate.spec.ts index f9bde407..845ecfbc 100644 --- a/frontend/app/e2e/annotate.spec.ts +++ b/frontend/app/e2e/annotate.spec.ts @@ -108,6 +108,16 @@ interface Lifecycle { refuseJobStart?: string; /** When set, every `PUT .../progress` refuses 409 with this code instead. */ refuseProgress?: string; + /** + * When set, every write to `/annotations` refuses 409 with this code and this + * message. + * + * The message is the interesting half: a code with no entry in `REFUSAL_PROSE` + * falls through to the server's own wording, which is how an install command — + * or a model reference — reaches a person verbatim. It is also the only way to + * put an arbitrarily long unbroken token on screen. + */ + refuseSave?: { code: string; message: string }; /** When set, `POST /jobs/{id}/complete` refuses 409 with this code instead. */ refuseJobComplete?: string; /** @@ -258,6 +268,9 @@ async function serveApi( const mine = stored.filter((one) => one.asset_id === assetId); return route.fulfill({ json: { items: mine, total: mine.length } }); } + if (path.endsWith("/annotations") && request.method() !== "GET" && lifecycle.refuseSave !== undefined) { + return route.fulfill({ status: 409, json: lifecycle.refuseSave }); + } if (path.endsWith("/annotations") && request.method() === "POST") { // Kept, and stamped with a server id — the kernel mints its own and the page // refetches to learn them (`jobQueries.ts`). A stub that answered an empty @@ -2372,8 +2385,9 @@ test("a refusal on the bar is a sentence now, not a kernel identifier", async ({ }); // A raw code rendered as a destructive badge is a kernel identifier in front of - // a user. This is an *opening* refusal, which has its own badge beside the save - // state. Both render through `refusalProse`, so both carry the prose. + // a user. This is an *opening* refusal, which takes its own notice at the top + // of the stage's column. Every refusal renders through `refusalProse`, so all + // of them carry the prose and keep the code in `title`. const state = page.getByTestId("opening-refusal"); await expect(state).toContainText(/not open for annotation/i); await expect(state).not.toContainText("BATCH_NOT_IN_ANNOTATION"); @@ -2707,3 +2721,113 @@ test("the no-connection panel now has somewhere to send you (#424 D6)", async ({ await expect(page).toHaveURL(/\/inference$/); await expect(page.getByTestId("inference-screen")).toBeVisible(); }); + +/** + * The notice surface, measured — which is the only way any of it can be claimed. + * + * Every assertion below is about geometry, and jsdom has none: + * `getBoundingClientRect` answers all zeros, and `scrollWidth` / `clientWidth` + * answer zero with it. A component test asserting "the notice does not overflow" + * would pass with the wrap rule deleted, the width halved and the anchor moved + * back to the corner the zoom widget lives in. + * + * A save refusal is the vehicle because it is the one refusal whose *text* the + * stub controls: a code with no entry in `REFUSAL_PROSE` falls through to the + * server's own message, so an arbitrarily long unbroken token can be put on + * screen the way a real model reference arrives. + */ + +/** 120 characters, no break opportunity anywhere in it. */ +const LONG_MODEL_REF = `IDEA-Research/grounding-dino-tiny@${"0123456789abcdef".repeat(6).slice(0, 86)}`; + +/** A job whose every write refuses, carrying `message` back verbatim. */ +async function openRefusingSave(page: Page, sent: Request[], message: string): Promise { + await openJob(page, sent, undefined, { + ...openedWorld(), + refuseSave: { code: "STUB_UNMAPPED_REFUSAL", message }, + }); + await drawOneUnsavedBox(page); + await page.getByTestId("save-and-stay").click(); + await expect(page.getByTestId("save-refusal")).toBeVisible(); +} + +test("an in-editor notice anchors top-right of the stage, clear of both corners", async ({ + page, +}) => { + const sent: Request[] = []; + await openRefusingSave(page, sent, "the kernel's own wording"); + + const stage = (await page.getByTestId("canvas-stage").boundingBox())!; + const card = (await page.getByTestId("save-refusal").boundingBox())!; + // The stage's hairline, subtracted rather than absorbed into the number: an + // absolutely positioned child is offset from the padding edge, so a bare + // `boundingBox` comparison reads 17 and would have to be explained as 16 + 1 + // every time somebody changed the border. + const border = await page + .getByTestId("canvas-stage") + .evaluate((element) => Number.parseFloat(getComputedStyle(element).borderTopWidth)); + + // 16px — the `md` step — in from the stage's top and right edges. Measured + // against the *stage*, which is the surface the notice floats over; the top bar + // is above it and is not what the inset is relative to. + expect(Math.round(card.y - stage.y - border)).toBe(16); + expect(Math.round(stage.x + stage.width - border - (card.x + card.width))).toBe(16); + + // The two occupied corners, and the whole reason this one was chosen. The + // suggest card used to clear the zoom widget with a hard-coded `bottom-16`, + // and before that sat *under* it, where the widget's subtree swallowed the + // presses meant for the card's own buttons. + const strip = (await page.getByTestId("tool-palette").boundingBox())!; + expect(card.x).toBeGreaterThan(strip.x + strip.width); + const zoom = (await page.getByTestId("zoom-widget").boundingBox())!; + expect(card.y + card.height).toBeLessThan(zoom.y); +}); + +test("a notice wraps a model reference no fixed width could have fitted", async ({ page }) => { + const sent: Request[] = []; + await openRefusingSave(page, sent, `Could not reach ${LONG_MODEL_REF} on this machine.`); + + const notice = page.getByTestId("save-refusal"); + // The whole token is on screen, not an elided prefix. + await expect(notice).toContainText(LONG_MODEL_REF); + + // Nothing to scroll sideways: the token broke mid-word rather than pushing the + // card's content past its own edge. Widening alone would not have done this — + // `wrap-anywhere` is the invariant and the width is comfort. + const overflow = await notice.evaluate((element) => element.scrollWidth - element.clientWidth); + expect(overflow).toBeLessThanOrEqual(0); + + // And the card itself did not grow out of the stage to make room. + const stage = (await page.getByTestId("canvas-stage").boundingBox())!; + const card = (await notice.boundingBox())!; + expect(card.x).toBeGreaterThanOrEqual(stage.x); + expect(card.x + card.width).toBeLessThanOrEqual(stage.x + stage.width); + // The page never scrolls sideways for a message. + expect( + await page.evaluate(() => document.documentElement.scrollWidth <= window.innerWidth), + ).toBe(true); +}); + +/** + * The chip's replacement, and it is only provable here. + * + * Radix opens a tooltip on hover and on focus, and jsdom runs neither for real — + * so "the chord is still taught" is a claim about a real pointer over a real + * button, on the one control that used to print `⌘S` inside itself. + */ +test("Save and stay teaches its chord in a tooltip now the keycap is gone", async ({ page }) => { + const sent: Request[] = []; + await openJob(page, sent); + await drawOneUnsavedBox(page); + + const stay = page.getByTestId("save-and-stay"); + await expect(stay.locator("kbd")).toHaveCount(0); + + await stay.hover(); + + const tip = page.getByTestId("save-and-stay-shortcut").first(); + await expect(tip).toBeVisible(); + // `modKey()` spells the platform's own modifier, so the assertion is on the + // half that does not move. + await expect(tip).toContainText(/Save and stay \((⌘|Ctrl)S\)/); +}); diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index ad5cfd8a..bd4c0291 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -171,6 +171,7 @@ import { import { Eye } from "lucide-react"; import { AnnotatorPanel } from "./AnnotatorPanel"; import { CanvasReassign } from "./CanvasReassign"; +import { EditorNotice, EditorNotices } from "./EditorNotice"; import { ShortcutSheet, modKey } from "./ShortcutSheet"; import { ToolPalette } from "./ToolPalette"; import { ZoomWidget } from "./ZoomWidget"; @@ -1639,11 +1640,10 @@ function Workspace({ is now on screen rather than in a tooltip: **status is never colour alone** (`DESIGN.md`), and prose is the strongest form of that. */} - - + @@ -1949,6 +1949,59 @@ function Workspace({ )} + + {/* + The other half of the forward gesture, and the reason it is here + rather than a zone away on the right. + + *Advance* and *persist in place* are one decision read two ways — + having finished with this frame, do you move on or stay on it — and + they were a bar apart, the second of them a ghost behind a `⌘S` chip + in the zone that also holds the progress readout and the overflow. + Adjacency is what says they are alternatives; a ghost at the far end + said the quieter thing, that saving without moving is a convenience. + + **Filled `success`, which is the recorded exception to one filled + button per view** (`DESIGN.md`). Two filled controls compete when + they are two answers to *what do I do next*; these are two halves of + one answer, and colour is what separates their intent where a second + near-black would simply contend with the primary. + + **No hotkey chip.** `Chip` is a muted box on a bordered ground, + which inverts into a smudge inside a filled control — the same + finding that keeps one off the flow verb. The chord is unchanged and + is taught by the tooltip instead, which is the tool strip's own + pattern (`Box (B)`). Native `disabled` rather than `aria-disabled`, + so the tooltip opens exactly while there is something to save — the + moment the chord is worth learning — and the shortcut sheet, which + derives its rows from the live registry, carries it unconditionally. + + It keeps the frame verbs' lifetime rather than `readOnly`'s: a + closed batch or a finished job has nothing to save on any frame, and + inside a working job the slot holds so the cluster does not change + width as somebody walks a mixed job. Reabsorption is unchanged — it + is still the first control the bar gives up below `xl`. + */} + {frameVerbs && ( + + + + + + Save and stay ({modKey()}S) + + + )} @@ -1973,41 +2026,6 @@ function Workspace({ : `${Math.max(0, counts.total - counts.unannotated)} / ${counts.total} annotated`} - {/* - The explicit save, on the bar. - - Removing it on the grounds that it duplicates an automatic - behaviour misses this: ⌘S is - invisible, and the overflow put the one press meaning *store this now, - without going anywhere* two clicks from the work. It is a **ghost**, - which is the honest weight — most people never need it, because - navigating and settling both save. - - First to be reabsorbed when the bar runs out of room (decision 4): it - is the one control on the right whose job the keyboard and every other - exit already do, and the overflow carries it below `xl`. - - Reabsorbed at `xl`. - That move was what the centred cluster cost while the class field - still held 192px in the middle: the grid hands each side exactly half - of what is left, and the right zone is the heavier of the two. With - the field in the side panel the halves cover the demand again, so the - patch is reverted rather than kept — measured, not assumed, in - `e2e/annotate.spec.ts`. - */} - - {/* The review move, when the frame declares one — outline, because the filled slot belongs to the flow verb. Rendered rather than @@ -2071,16 +2089,21 @@ function Workspace({ {/* `Save and stay`, reabsorbed — `xl:hidden` is the exact inverse of the button's `hidden xl:inline-flex`, so the control exists once - at every width. */} - attempt()} - > - - Save and stay - + at every width. Gated on `frameVerbs` for the same reason the + button is: a closed batch or a finished job has nothing to save + on any frame, and a reabsorbed copy that outlived its button + would be the control existing in two states rather than one. */} + {frameVerbs && ( + attempt()} + > + + Save and stay + + )} {/* The review move, reabsorbed one breakpoint later. */} {reviewAction !== undefined && ( - {/* - Why the counter did not move, said where the work is happening. - Rendered whenever the asset is skipped rather than only after a save: the - user who is about to draw deserves it more than the one who already has. - */} - {/* - The refusals that had nowhere to go (audit F3 and F4). - - `setProgress.isError` and `finishJob.isError` were read **nowhere in this - file**: pressing Skip, Un-skip, Accept or Finish job against a refusal did - nothing at all and said nothing about it — the button came back enabled, - the badge did not move, and the page looked like it had ignored the click. - Three of those four are one-press actions with no other feedback surface, - which is what made this the quietest failure in the product. - - One line rather than four, because they are mutually exclusive in - practice — each is a single press and react-query clears the error on the - next attempt — and because a toolbar with four empty error slots in it is - a toolbar nobody can read. It sits with the banners rather than in the bar - for the same reason: the bar is full, and a refusal is a sentence. - */} - {actionRefusal !== null && ( -

-

- )} - {/* Read-only, said out loud and at the top (F2). The `ui-capabilities` rule is that read-only is a *mode*, not an accident: "open it and let the saves @@ -2345,30 +2336,108 @@ function Workspace({ )} {/* - The suggest tool's own voice — a sibling of the canvas for - `ToolPalette`'s reason, and in the one corner the editor does not - already occupy. - - Rendered for the whole session rather than only for its refusals: the - asking state, the found-nothing state and the accept affordance are - the same question answered differently, and scattering them would - leave a person assembling one answer from three places. + Everything the editor floats over the picture, in one column. + + The order is *how much it stops you*, read downwards. An opening + refusal says nothing on this page can be written at all; a save + refusal says this frame's work did not land; an action refusal is one + button that did not fire; the suggest session is the tool talking + about itself. On the ordinary path only the last of the four is ever + present, so the ordering only decides what happens on the rare frame + where two things are true at once — and there, the one that is not + recoverable goes first. + + All four used to live somewhere else: two as badges inside the top + bar's microtext, one as a full-bleed strip under the header, and the + suggest card bottom-right over the zoom cluster. Four placements for + one class of message meant *where* a sentence appeared depended on + which mutation produced it. */} - {suggesting !== null && ( - - )} + + {openingRefusal !== null && openingRefusal !== undefined && ( + } + title={asApiError(openingRefusal).code} + > + {refusalProse(openingRefusal)} + + )} + + {/* + Why the save did not happen. It left the top bar's `● annotated · + Saved` microtext with the rest of the refusals — that readout says + *where the work is*, and after a failed save the honest answer there + is `unsaved`, which is what it now shows. The reason is a sentence + and a sentence needs room; a destructive badge in a 44px row had + neither. + */} + {save.isError && ( + } + title={asApiError(save.error).code} + > +

This frame could not be saved

+

{refusalProse(save.error)}

+

+ Your work is still here — nothing has been discarded. +

+ + )} + + {/* + The refusals that had nowhere to go (audit F3 and F4). + + `setProgress.isError` and `finishJob.isError` were read **nowhere in + this file**: pressing Skip, Un-skip, Accept or Finish job against a + refusal did nothing at all and said nothing about it — the button + came back enabled, the badge did not move, and the page looked like + it had ignored the click. Three of those four are one-press actions + with no other feedback surface, which is what made this the quietest + failure in the product. + + One notice rather than four, because they are mutually exclusive in + practice: each is a single press, and react-query clears the error + on the next attempt. + */} + {actionRefusal !== null && ( + } + title={asApiError(actionRefusal).code} + > + {refusalProse(actionRefusal)} + + )} + + {/* + The suggest tool's own voice. + + Rendered for the whole session rather than only for its refusals: + the asking state, the found-nothing state and the accept affordance + are the same question answered differently, and scattering them + would leave a person assembling one answer from three places. + */} + {suggesting !== null && ( + + )} + {drawn} object{drawn === 1 ? "" : "s"} @@ -2684,39 +2753,22 @@ function AssetProgressDot({ progress }: { readonly progress: string }): JSX.Elem } /** - * Why the page could not open the batch or the job it was asked to open. - * - * Nothing at all when there is nothing to say — the common case by far. It renders beside the - * save state rather than inside it: the two answer different questions, and the - * one that outlives every save must not be overwritten by the next one, nor - * overwrite it. - */ -function OpeningRefusal({ error }: { readonly error: unknown }): JSX.Element | null { - if (error === null || error === undefined) return null; - return ( - - {refusalProse(error)} - - ); -} - -/** - * `DESIGN.md`'s save-state indicator: saving, saved, or why it did not. + * `DESIGN.md`'s save-state indicator: saving, saved, or not yet stored. * - * It must not render the raw kernel `code` — `BATCH_NOT_IN_ANNOTATION` as a - * destructive badge. A kernel identifier is what a bug report should - * quote, not what a person should read, so the badge carries the sentence and the - * code goes in the `title` where somebody filing that report can still find it. + * **Three readings, not four.** It used to carry a fourth — the refusal itself, + * as a destructive badge — and that reading has moved to the stage's notice + * column, where a sentence has room. What is left is the question this readout + * actually answers, *where is the work*, and after a refused save the honest + * answer is the one it now gives: `unsaved`. The two are not the same fact, and a + * readout that swapped one for the other left the person who had just been + * refused with no statement at all about whether their boxes still existed. */ function SaveState({ dirty, pending, - error, }: { readonly dirty: boolean; readonly pending: boolean; - /** The refusal itself, or `null`. Prose is derived; the code is not the message. */ - readonly error: unknown; }): JSX.Element { if (pending) { return ( @@ -2725,13 +2777,6 @@ function SaveState({ ); } - if (error !== null && error !== undefined) { - return ( - - {refusalProse(error)} - - ); - } if (dirty) { return ( diff --git a/frontend/ui-core/src/annotator/EditorNotice.tsx b/frontend/ui-core/src/annotator/EditorNotice.tsx new file mode 100644 index 00000000..01ce3a41 --- /dev/null +++ b/frontend/ui-core/src/annotator/EditorNotice.tsx @@ -0,0 +1,132 @@ +/** + * The annotation editor's one notice surface: everything the editor has to say + * about itself, in one corner, in one treatment. + * + * ## Why one surface at all + * + * The editor used to answer in four places. A suggest refusal was a card + * bottom-right, a save refusal was a destructive badge inside the top bar's + * `● annotated · Saved` microtext, an opening refusal was a second badge beside + * it, and a refused Skip / Un-skip / Accept / Finish job was a full-bleed strip + * under the header. Four placements and three treatments for one class of + * message, so *where* a sentence appeared depended on which mutation produced + * it — which is a fact about this file's history and not about anything a person + * could learn. + * + * ## Top-right, and the bottom-right corner is why + * + * `ZoomWidget` sits at `bottom-3 right-3` and the suggest card cleared it with an + * explicit `bottom-16` — one component holding a constant about another, and a + * card that had already been observed *under* the widget, where the widget's + * subtree swallowed the presses meant for the card's own buttons. Top-right is + * unoccupied: the tool strip is top-left, the object counter bottom-left, the + * zoom cluster bottom-right. + * + * The inset is `4` (16px, the `md` step) from the stage's top and right edges. + * The stage already begins below the 44px top bar, so this is measured against + * the surface the notice floats over rather than against the header. + * + * ## A stack, not a slot + * + * More than one thing can be true at once — a suggest session is live and the + * save just refused — so the surface is a column, most-blocking first. A single + * slot would have to choose, and choosing means hiding a refusal behind a + * spinner. + * + * `pointer-events-none` on the column with `pointer-events-auto` on each card: + * the column spans a fixed width down the side of the picture, and an invisible + * strip that ate drags along the right edge of the canvas would be a worse defect + * than the one this replaces. + * + * ## Wrapping is the invariant; width is comfort + * + * A model reference is a single unbroken token — + * `IDEA-Research/grounding-dino-tiny@<40 hex>` — and **no fixed width guarantees + * the next one fits**, so the body wraps mid-token (`wrap-anywhere`) and the + * width is chosen so the common case never has to. `max-w-md` (448px) leaves the + * `w-12` tool strip roughly 190px of clearance at the narrowest supported + * desktop; see `MAX_WIDTH` below. + */ + +import type { JSX, ReactNode } from "react"; + +/** + * The surface's width, stated here because the number is a measurement rather + * than a taste. + * + * At a 1280px viewport the rail takes 240px, the editor's row spends 24px of + * padding and a 12px gap, and the side panel is a fixed `w-72` (288px) — so the + * stage is 716px. Inset 16px from its right edge, a 448px card's left edge lands + * at 252px, and the tool strip ends at 60px. Roughly 190px of clearance, which + * is what "clear of the tool strip" has to mean for a surface that may grow a + * line. + * + * 448px also fits a full `owner/model@revision` reference on one line at + * `text-meta`, which is the width the previous 320px could not manage. + */ +const MAX_WIDTH = "max-w-md"; + +/** + * The stage's notice column. Rendered once, whether or not it holds anything. + * + * `z-20` puts it over the tool strip and the zoom widget — both of which sit in + * the stage's own stacking order with no `z` of their own — and leaves the + * header's `z-50` popover untouched. + */ +export function EditorNotices({ children }: { readonly children: ReactNode }): JSX.Element { + return ( +
+ {children} +
+ ); +} + +export interface EditorNoticeProps { + /** The card's own handle, so a test names the message and not the column. */ + readonly testId: string; + /** + * `warn` is the destructive-alert skin — a `destructive` hairline over a 5% + * fill, the treatment the suggest card shipped with. `calm` is for the states + * where nothing is wrong and a red card would teach somebody to distrust a + * working tool. + */ + readonly tone: "calm" | "warn"; + readonly icon: ReactNode; + readonly children: ReactNode; + /** The kernel's identifier, where a bug report can quote it. Never the message. */ + readonly title?: string; +} + +export function EditorNotice({ + testId, + tone, + icon, + children, + title, +}: EditorNoticeProps): JSX.Element { + return ( +
+ + {/* `min-w-0` lets the column shrink below its content's intrinsic width, + which is the half of the wrap rule flexbox owns — without it a long + token widens the flex item instead of breaking. */} +
{children}
+
+ ); +} diff --git a/frontend/ui-core/src/annotator/SuggestPanel.tsx b/frontend/ui-core/src/annotator/SuggestPanel.tsx index a6499db5..fa90c1dd 100644 --- a/frontend/ui-core/src/annotator/SuggestPanel.tsx +++ b/frontend/ui-core/src/annotator/SuggestPanel.tsx @@ -11,6 +11,10 @@ * not a redirect (which loses the frame), and not a modal (which stops the * gesture the page exists for). * + * **Where that card sits is `EditorNotice`'s business and not this component's.** + * Every message the editor floats over the stage goes to one top-right column, so + * this file chooses the sentence and the tone and nothing about the geometry. + * * ## One panel for six states, because they are one question * * "What is the suggest tool doing" has six honest answers, and each of them is a @@ -51,6 +55,7 @@ import { import { Check, Loader2, Sparkles, TriangleAlert, X } from "lucide-react"; import type { JSX, ReactNode } from "react"; +import { EditorNotice } from "./EditorNotice"; import { Button } from "../primitives/Button"; import type { SuggestBlocker } from "../data/inferenceQueries"; @@ -145,7 +150,7 @@ export function SuggestPanel({ */ if (isParked(session)) { return ( - }> + }>

{heldClass === null ? "Nothing selected to suggest for" @@ -172,7 +177,7 @@ export function SuggestPanel({

That suggestion could not be made

{/* The server's sentence, verbatim. It is the one that carries the install command when the cause is a missing extra. */} @@ -225,13 +230,13 @@ export function SuggestPanel({

Your clicks are still here — press Esc to clear them, or click again to retry.

- + ); } if (session.status === "asking") { return ( - }> + }>

Looking at that…

@@ -240,13 +245,13 @@ export function SuggestPanel({

The first click on a frame is the slow one — refining after it is quick.

- + ); } if (session.status === "none") { return ( - }> + }>

Nothing to suggest there

@@ -255,13 +260,13 @@ export function SuggestPanel({ something that is not part of it.

- + ); } if (isAcceptable(session)) { return ( - }> + }>

A shape for “{session.labelClass}”

@@ -279,12 +284,12 @@ export function SuggestPanel({ Esc - + ); } return ( - }> + }>

Click the thing you want

@@ -293,7 +298,7 @@ export function SuggestPanel({ that is not part of it.

{hasPending(session) && } - + ); } @@ -322,55 +327,3 @@ function Chip({ children }: { readonly children: ReactNode }): JSX.Element { ); } - -/** - * The card itself: bottom-right of the stage, clear of the tool strip. - * - * Bottom-**right** rather than beside the strip, because the strip is top-left - * and the object counter is bottom-left: this is the one corner the editor does - * not already occupy, and a panel that covered the tools would hide the button - * that arms it. - */ -function Card({ - testId, - tone, - icon, - children, -}: { - readonly testId: string; - readonly tone: "calm" | "warn"; - readonly icon: ReactNode; - readonly children: ReactNode; -}): JSX.Element { - return ( -
- -
{children}
-
- ); -} diff --git a/frontend/ui-core/src/annotator/editorNotice.test.tsx b/frontend/ui-core/src/annotator/editorNotice.test.tsx new file mode 100644 index 00000000..ab4e5a74 --- /dev/null +++ b/frontend/ui-core/src/annotator/editorNotice.test.tsx @@ -0,0 +1,233 @@ +/** + * The editor's one notice surface, asserted as a *unification* rather than as a + * card. + * + * The claim is not "a refusal renders" — three suites already say that, and they + * said it while the refusals were scattered across four placements in three + * treatments. The claim is that every sentence the editor floats over the stage + * comes out of one anchor, so a message's position stops depending on which + * mutation produced it. That is a containment check, which is why it is written + * here and not inside whichever component happens to own a given error. + * + * Geometry is deliberately **not** asserted in jsdom: `getBoundingClientRect` + * answers all zeros there, so "top-right, clear of the tool strip, wrapping a + * 120-character token" is `e2e/annotate.spec.ts`'s to prove in a real browser. + * What this file can prove is *which element contains which*, and that survives + * a layout engine's absence. + */ + +import { QueryClient } from "@tanstack/react-query"; +import { render, screen, waitFor } from "@testing-library/react"; +import { userEvent } from "@testing-library/user-event"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { JSX, ReactNode } from "react"; + +import { ApiProvider } from "../data/ApiProvider"; +import { writeToken } from "../data/session"; +import { AnnotationPage } from "./AnnotationPage"; +import { TooltipProvider } from "../primitives/Menu"; +import { assetActions, batchActions, jobActions } from "../testing/wire.fixtures.js"; + +const API = "http://visionset.test"; +const PROJECT = "11111111-1111-4111-8111-111111111111"; +const BATCH = "22222222-2222-4222-8222-222222222222"; +const JOB = "33333333-3333-4333-8333-333333333333"; +const ASSET = "44444444-4444-4444-8444-444444444444"; + +const SCHEMA = { + project_id: PROJECT, + version: 1, + description: null, + created_at: null, + provenance: "curated", + classes: [{ name: "vehicle", geometry: "bbox", color: "#3355ff", attributes: [] }], +}; + +/** The batch's state, which decides whether the page tries to open it. */ +let batchState: "approved" | "in_annotation" = "in_annotation"; +/** The code every write is refused with, or `null` for a wire that accepts. */ +let refuseWith: string | null = null; + +function answer(path: string): unknown { + if (path === `/jobs/${JOB}`) { + return { + id: JOB, + batch_id: BATCH, + state: "in_progress", + asset_count: 1, + allowed_actions: jobActions("in_progress", { settled: false }), + }; + } + if (path === `/batches/${BATCH}`) { + return { + id: BATCH, + project_id: PROJECT, + name: "drive-01", + state: batchState, + schema_version: 1, + asset_count: 1, + allowed_actions: batchActions(batchState), + promoted_asset_count: 0, + parent_batch_id: null, + progress: { + unannotated: 1, + annotated: 0, + skipped: 0, + review_pending: 0, + accepted: 0, + total: 1, + }, + }; + } + if (path.endsWith("/schema/versions/1") || path.endsWith("/schema")) return SCHEMA; + if (path.endsWith("/assets")) { + return { + items: [ + { + id: ASSET, + project_id: PROJECT, + modality: "image", + content_hash: "abcdef00".padEnd(64, "0"), + width: 640, + height: 480, + format: "png", + thumbnail_hash: null, + frame_index: null, + frame_timestamp: null, + source_id: null, + ingested_at: null, + job_id: JOB, + progress: "unannotated", + allowed_actions: assetActions("unannotated", { batchState: "in_annotation" }), + }, + ], + total: 1, + }; + } + return { items: [], total: 0 }; +} + +beforeEach(() => { + batchState = "in_annotation"; + refuseWith = null; + writeToken("a-token"); + vi.stubGlobal("matchMedia", (query: string) => ({ + media: query, + matches: true, + addEventListener: () => {}, + removeEventListener: () => {}, + })); + vi.stubGlobal("fetch", async (request: Request) => { + const path = new URL(request.url).pathname; + if (request.method !== "GET") { + if (refuseWith !== null) { + return new Response(JSON.stringify({ code: refuseWith, message: "refused" }), { + status: 409, + headers: { "content-type": "application/json" }, + }); + } + return new Response(JSON.stringify({}), { + status: 200, + headers: { "content-type": "application/json" }, + }); + } + return new Response(JSON.stringify(answer(path)), { + status: 200, + headers: { "content-type": "application/json" }, + }); + }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); + globalThis.sessionStorage.clear(); +}); + +function mount(node: ReactNode): JSX.Element { + return ( + + {node} + + ); +} + +async function open(): Promise { + render(mount()); + await screen.findByTestId("annotation-page"); +} + +describe("the notice column", () => { + it("floats over the stage rather than over the bar or the side panel", async () => { + // The anchor is the stage, which is what makes "over the picture" true and + // what keeps a notice off the tool strip and the object list. It exists + // whether or not it is holding anything: an anchor that appeared with its + // first message would be a second thing that can be absent. + await open(); + + const column = screen.getByTestId("editor-notices"); + expect(screen.getByTestId("canvas-stage").contains(column)).toBe(true); + expect(screen.getByTestId("annotation-page").querySelector("header")?.contains(column)).toBe( + false, + ); + }); + + it("is where a refused progress move lands, not a strip under the header", async () => { + // `Skip` against a refusal. It used to be a full-bleed `

` between the + // header and the workspace — a fourth placement for the same class of + // message. + refuseWith = "ASSET_NOT_WRITABLE"; + await open(); + + await userEvent.click(screen.getByTestId("skip")); + + const said = await screen.findByTestId("action-refusal"); + expect(screen.getByTestId("editor-notices").contains(said)).toBe(true); + // Prose, with the kernel's identifier kept where a bug report can quote it. + expect(said.textContent).toContain("labeling is settled"); + expect(said.textContent).not.toContain("ASSET_NOT_WRITABLE"); + expect(said.getAttribute("title")).toBe("ASSET_NOT_WRITABLE"); + }); + + it("is where a refused opening lands, not a badge in the top bar", async () => { + // An `approved` batch declares `start`, so the page sends one — and a + // refusal that is not `INVALID_TRANSITION` is a real one and is surfaced. + batchState = "approved"; + refuseWith = "BATCH_NOT_IN_ANNOTATION"; + await open(); + + const said = await screen.findByTestId("opening-refusal"); + expect(screen.getByTestId("editor-notices").contains(said)).toBe(true); + expect(said.textContent).toContain("not open for annotation"); + expect(said.getAttribute("title")).toBe("BATCH_NOT_IN_ANNOTATION"); + }); + + it("is where the suggest tool speaks, whatever it has to say", async () => { + // The card that moved. It is the whole session's voice — an asking state and + // a refusal are the same question answered differently — so the surface has + // to hold the calm readings as well, or arming the tool would make a panel + // jump corners as its state changed. + await open(); + await userEvent.click(screen.getByTestId("class-row-vehicle")); + await userEvent.click(screen.getByTestId("tool-suggest")); + + const panel = await screen.findByTestId("suggest-panel"); + expect(screen.getByTestId("editor-notices").contains(panel)).toBe(true); + }); + + it("leaves the top bar's microtext saying where the work is, not why it refused", async () => { + // The save state used to carry a fourth reading — the refusal itself, as a + // destructive badge in a 44px row. It answers *where is the work*, and after + // a refused save the honest answer there is `unsaved`. + await open(); + + await waitFor(() => expect(screen.getByTestId("save-state")).toBeDefined()); + expect(screen.getByTestId("save-state").textContent).toBe("Saved"); + // And there is nowhere else on the bar a refusal could still be hiding. + const header = screen.getByTestId("annotation-page").querySelector("header"); + expect(header?.querySelector("[data-testid='opening-refusal']")).toBeNull(); + expect(header?.querySelector("[data-testid='save-refusal']")).toBeNull(); + }); +}); diff --git a/frontend/ui-core/src/annotator/topBar.test.tsx b/frontend/ui-core/src/annotator/topBar.test.tsx index c1fdb352..a1510272 100644 --- a/frontend/ui-core/src/annotator/topBar.test.tsx +++ b/frontend/ui-core/src/annotator/topBar.test.tsx @@ -76,6 +76,12 @@ let jobCounts: { /** Whether the frames arrive carrying a box — what `drawn > 0` reads. */ let annotated = false; +/** + * Whether the batch has closed — the dimension that withholds every move on + * every frame at once, which is what the frame verbs' own lifetime keys on. + */ +let closedBatch = false; + /** The nth asset's id, distinct enough to read in a failure message. */ function assetId(index: number): string { return `4444444${index}-4444-4444-8444-444444444444`; @@ -101,7 +107,10 @@ function answer(path: string): unknown { batch_id: BATCH, state: "in_progress", asset_count: 1, - allowed_actions: jobActions("in_progress", { settled: jobSettled }), + allowed_actions: jobActions("in_progress", { + batchState: closedBatch ? "completed" : "in_annotation", + settled: jobSettled, + }), }; } if (path === `/batches/${BATCH}`) { @@ -109,10 +118,10 @@ function answer(path: string): unknown { id: BATCH, project_id: PROJECT, name: "drive-01", - state: "in_annotation", + state: closedBatch ? "completed" : "in_annotation", schema_version: 1, asset_count: 1, - allowed_actions: batchActions("in_annotation"), + allowed_actions: batchActions(closedBatch ? "completed" : "in_annotation"), promoted_asset_count: 0, parent_batch_id: null, progress: { @@ -142,7 +151,9 @@ function answer(path: string): unknown { ingested_at: null, job_id: JOB, progress, - allowed_actions: assetActions(progress, { batchState: "in_annotation" }), + allowed_actions: assetActions(progress, { + batchState: closedBatch ? "completed" : "in_annotation", + }), })); return { items, total: items.length }; } @@ -175,6 +186,7 @@ beforeEach(() => { jobSettled = false; assetCount = 1; annotated = false; + closedBatch = false; writeToken("a-token"); vi.stubGlobal("matchMedia", (query: string) => ({ media: query, @@ -623,15 +635,15 @@ describe("the flow verb", () => { ); }); - it("carries no hotkey chip, unlike its two neighbours on the bar", async () => { - // `Chip` is a muted box on a bordered ground — right on the ghost and - // the outline controls, a smudge on the only filled one. The chord is - // unchanged, which is what the next test asserts; this is about the pixels. + it("carries no hotkey chip, and neither does the filled control beside it", async () => { + // `Chip` is a muted box on a bordered ground — right on the outline `Skip`, + // a smudge inside either fill. Both chords are unchanged, which is what the + // next test and the tooltip assert; this is about the pixels. assetCount = 2; await open(); expect(screen.getByTestId("save-and-next").querySelector("kbd")).toBeNull(); - expect(screen.getByTestId("save-and-stay").querySelector("kbd")?.textContent).toContain("S"); + expect(screen.getByTestId("save-and-stay").querySelector("kbd")).toBeNull(); expect(screen.getByTestId("skip").querySelector("kbd")?.textContent).toBe("X"); }); @@ -687,6 +699,80 @@ describe("the flow verb", () => { }); }); +/** + * The forward pair: two filled controls, and the exception that lets them be two. + * + * `DESIGN.md`'s *one filled button per view* is a count and is tested as one, so + * the recorded exception has to be a count too — otherwise "two fills are allowed + * here" degrades into "any number of fills are allowed here", which is the rule + * with nothing left of it. The sweep below is `filled()`'s twin over `bg-success`, + * and both are asserted as whole sets rather than as memberships. + */ +describe("the forward-action pair", () => { + /** Every `success`-filled control on the bar. `filled()`'s counterpart. */ + function successFilled(): HTMLElement[] { + // `classList.contains`, for `filled()`'s reason: the substring form also + // matches `hover:bg-success-hover`. + return [...document.querySelectorAll("header button")].filter((button) => + button.classList.contains("bg-success"), + ); + } + + it("puts Save and stay in the resolve group, immediately after the primary", async () => { + // Adjacency is the whole claim: *advance* and *persist in place* are one + // decision read two ways, and they used to be a zone apart. Document order + // inside the cluster is what a reader gets. + assetCount = 2; + await open(); + + const cluster = screen.getByTestId("frame-navigation"); + const order = [...cluster.querySelectorAll("button[data-testid]")] + .map((button) => button.getAttribute("data-testid")) + .filter((id) => id === "skip" || id === "save-and-next" || id === "save-and-stay"); + expect(order).toEqual(["skip", "save-and-next", "save-and-stay"]); + }); + + it("leaves exactly one primary fill and exactly one success fill", async () => { + assetCount = 2; + await open(); + + expect(filled().map((button) => button.getAttribute("data-testid"))).toEqual(["save-and-next"]); + expect(successFilled().map((button) => button.getAttribute("data-testid"))).toEqual([ + "save-and-stay", + ]); + }); + + it("keeps the pair a pair on the last frame, where Finish job holds the primary", async () => { + // The filled slot is contended by arithmetic rather than by a declaration, + // and the success half is not part of that contention: you can still save + // without leaving the frame you are finishing on. + assetCount = 1; + jobSettled = true; + progress = "annotated"; + await open(); + + expect(filled().map((button) => button.getAttribute("data-testid"))).toEqual(["finish-job"]); + expect(successFilled().map((button) => button.getAttribute("data-testid"))).toEqual([ + "save-and-stay", + ]); + }); + + it("leaves with the frame verbs once the job is closed, in both its places", async () => { + // A closed batch has nothing to save on any frame, so a filled control that + // could never fire would be a fill with nothing behind it — principle 9 at + // the loudest weight the bar has. It goes with Skip and the flow verb, and + // the overflow copy goes with it, or the control would exist in two states + // rather than one. + closedBatch = true; + assetCount = 2; + await open(); + + expect(screen.queryByTestId("save-and-stay")).toBeNull(); + expect(screen.queryByTestId("menu-save")).toBeNull(); + expect(successFilled()).toEqual([]); + }); +}); + describe("the frame's state, in prose", () => { it("says the word beside the dot rather than keeping it in a tooltip", async () => { progress = "annotated"; diff --git a/frontend/ui-core/src/primitives/Button.tsx b/frontend/ui-core/src/primitives/Button.tsx index 661096b4..1568a158 100644 --- a/frontend/ui-core/src/primitives/Button.tsx +++ b/frontend/ui-core/src/primitives/Button.tsx @@ -41,6 +41,19 @@ export const buttonVariants = cva( variant: { primary: `bg-primary text-primary-foreground hover:bg-primary-hover ${DISABLED_FILLED}`, secondary: `border border-input bg-card text-foreground hover:bg-muted ${DISABLED_FILLED}`, + /** + * The second filled weight, and it has exactly one sanctioned caller: + * the annotation editor's `Save and stay` (`DESIGN.md`, *One filled + * button per view*). Colour is what separates *advance* from + * *persist in place* where a second near-black would compete with the + * primary rather than pair with it. + * + * Not a general "confirm" variant. A view whose forward action is a + * success is still `primary` — this exists because two halves of one + * gesture sit side by side, which is a shape the rest of the product + * does not have. + */ + success: `bg-success text-success-foreground hover:bg-success-hover ${DISABLED_FILLED}`, ghost: "text-muted-foreground hover:bg-muted hover:text-foreground " + // No fill: a ghost has none to grey out, and giving it one on the way diff --git a/frontend/ui-core/src/styles.css b/frontend/ui-core/src/styles.css index c8893ecc..44c5f126 100644 --- a/frontend/ui-core/src/styles.css +++ b/frontend/ui-core/src/styles.css @@ -109,9 +109,17 @@ /* * Status, desaturated to sit on a near-monochrome page. Each is used as ink - * and as a 10%-alpha fill behind its own border, never as a saturated block. + * and as a 10%-alpha fill behind its own border, never as a saturated block — + * with one exception, the annotation editor's `Save and stay`, which is a + * filled `success` control. Its two companions exist for that fill and for + * nothing else: `success-foreground` is the ink on it, and `success-hover` + * lifts the fill by the same channel deltas `primary-hover` lifts `primary` + * (+12, +12, +16), so the two filled variants brighten by the same amount + * under a pointer instead of each inventing a feel. */ --color-success: #2e7d5b; + --color-success-hover: #3a896b; + --color-success-foreground: #ffffff; --color-warning: #b98217; --color-destructive: #c93b3b; --color-destructive-foreground: #ffffff; diff --git a/frontend/ui-core/src/tokens.ts b/frontend/ui-core/src/tokens.ts index f74dde5e..d24f5c9a 100644 --- a/frontend/ui-core/src/tokens.ts +++ b/frontend/ui-core/src/tokens.ts @@ -62,7 +62,12 @@ export const COLOR = { // `tokens.test.ts` — a solid near-black ring on a near-black button is a smudge. ring: "rgba(30, 33, 48, 0.35)", + // The one status colour that is also a fill: the annotation editor's + // `Save and stay`. `success-hover` lifts it by `primary-hover`'s own channel + // deltas (+12, +12, +16), so the bar's two filled controls brighten alike. success: "#2e7d5b", + "success-hover": "#3a896b", + "success-foreground": "#ffffff", warning: "#b98217", destructive: "#c93b3b", "destructive-foreground": "#ffffff",