feat(annotator): one notice surface top-right, and the forward actions become a pair - #484
Merged
Merged
Conversation
…s become a pair 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`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #481.
What changed
One notice surface.
frontend/ui-core/src/annotator/EditorNotice.tsxis a newpositioned column at the top-right of the stage, inset 16px (the
mdstep) from its topand right edges, over the picture at
z-20. Everything the editor floats over the stagenow comes out of it:
bottom-16 right-3,max-w-80save-state, a destructive badge inside the top bar's microtextsave-refusalopening-refusal, a second badge beside itopening-refusal, in the columnaction-refusal, a full-bleed strip under the headeraction-refusal, in the columnThe column is a stack, most-blocking first, because more than one can be true at
once — a suggest session is live and the save just refused. A single slot would have to
choose, and choosing means hiding a refusal behind a spinner.
pointer-events-noneonthe column with
pointer-events-autoon each card, so an empty column is not aninvisible strip eating drags down the right edge of the canvas.
Every
data-testidand everytitle(the kernel code, where a bug report can quote it)is unchanged, so the existing refusal assertions in
annotate.spec.tsstill address thesame messages.
Width and wrapping.
max-w-md(448px) andwrap-anywhereon the body. Width isstated as a measurement in the module: 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 288px,
so the stage is 716px; inset 16px, a 448px card's left edge lands at 252px against a tool
strip that 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@revisionon one line attext-meta, which 320px could not.The wrap is the part that actually fixes it. A model reference is one unbroken token and
no fixed width guarantees the next one fits, so wrapping is the invariant and the
width is comfort. Asserted with a 120-character token in chromium.
The forward-action pair. The resolve group reads
Skip · [primary next-action] · Save and stay. Save and stay is filledsuccess(bg-success/text-success-foreground/
hover:bg-success-hover, all tokens — no literal), it has lost the⌘Skeycap chip,and the chord is taught by its tooltip instead. Its reabsorption is unchanged: still the
first control the bar gives up below
xl, still with the exact inverse breakpoint on theoverflow row. It now leaves with the frame verbs once the job is closed — a filled
control that could never fire is a fill with nothing behind it — and the overflow copy
leaves with it so the control exists in one state, not two.
Tokens.
success-hover(#3a896b) andsuccess-foreground(#ffffff) joinstyles.cssandtokens.ts. The hover is derived rather than picked:primary-hoverlifts
primaryby (+12, +12, +16) per channel, and the same deltas applied tosuccessgive this, so the bar's two filled controls brighten by the same amount under a pointer.
tokens.test.ts's both-directions parity is green.DESIGN.md. One filled button per view gains the recorded exception with its
reasoning; the count test is restated as two counts rather than relaxed into none. The
annotation-workspace top-bar prose is rewritten for the shipped bar (zone table, the
Save-and-stay paragraph, the hotkey-chip rule, the notice surface, the three-reading save
state).
Step 0.3 — does Next persist before advancing?
Yes, and the label is already the honest one, so A3's relabel is a no-op:
AnnotationPage's flow verb isgo(1)→goTo→attempt→commit→save.mutateAsync(plan)when dirty, which is the same save-first path the navigator's‹›, the gallery tiles and the back arrow all use. The label is already derived —!readOnly && (dirty || drawn > 0) ? "Save and next" : "Next"— which is a strongerversion of what A3 asks for: the button never promises a save it will not perform, so it
reads
Nexton an untouched frame rather than claiming a save that would not happen.No change was made to it.
Stop-and-flag inventory: sonner
The condition did not fire. The editor renders no error through a toast: the only two
toastcalls onAnnotationPagearetoast.success(a class added, a job finished), andgrepfinds notoast.erroranywhere inui-coreorapp. Nothing had to be moved offa disappearing surface, and no design call about which surface owns which error class was
needed.
Tests
New:
frontend/ui-core/src/annotator/editorNotice.test.tsx— the unification as acontainment check: the column is inside
canvas-stageand not inside the header, and arefused progress move, a refused opening and the suggest session all land in it. Also
that the bar's microtext says
unsavedrather than a refusal, and that no refusaltestid survives inside
<header>.topBar.test.tsx→the forward-action pair— document order inside the cluster isskip, save-and-next, save-and-stay; exactly onebg-primaryand exactly onebg-successon the bar, on a middle frame and on the last frame; both copies leavewhen the job closes. The harness gained a
closedBatchtoggle for the last of those.annotate.spec.ts— three chromium scenarios: the 16px inset measured against thestage's inner edge and clear of both occupied corners; a 120-character model reference
wrapping with
scrollWidth <= clientWidth, the card inside the stage and the page notscrolling sideways; the tooltip carrying
Save and stay (⌘S | CtrlS)on hover with nokbdleft in the button. The stub gained arefuseSavelifecycle option — a code withno
REFUSAL_PROSEentry falls through to the server's message, which is the only way toput an arbitrary token on screen.
Changed:
topBar.test.tsx's chip assertion now says neither fill carries one.Geometry is deliberately not asserted in jsdom —
getBoundingClientRect,scrollWidthand
clientWidthall answer zero there, so a component test for "the notice does notoverflow" would pass with the wrap rule deleted and the anchor moved back.
Mutation verification
Each new rule was broken and the guard that names it went red. Anchors asserted unique
before patching and present after; each reverted by its own recorded diff (
git apply -R),never by checking out a path. The work was committed first, so a revert could not take the
implementation with it.
bottom-16 right-3anchors top-right of the stage(chromium)wrap-anywhereremoved from the bodywraps a model reference no fixed width could have fittedvariant="success"→variant="ghost"leaves exactly one primary fill and exactly one success fill⌘Skeycap chip re-addedcarries no hotkey chip, and neither does the filled control beside itaction-refusalrenamed out of the notice columnis where a refused progress move landsThe chip mutation aborted on its first spelling — the harness found three occurrences of
the anchor and refused to patch ambiguously rather than silently patching the wrong one.
Re-run with a unique three-line anchor; red as above.
Local gate
Staged against this box's ~10-minute command ceiling, pytest split by directory derived
from
ls tests/at run time. Every stage's exit code:pytest tests/architecture0pytest tests/cli0pytest tests/examples0pytest tests/fixtures0pytest tests/formats0pytest tests/inference0pytest tests/jobs0pytest tests/kernel0pytest tests/mcp0pytest tests/packaging0pytest tests/scripts5pytest tests/server0pytest tests/test_versioning.py0ruff check .0ruff format --check .0mypy src/visionset/kernel0lint-imports0check.sh frontend generated0check.sh browser0— 237 e2e passed (was 234), 1 cycle passedtests/scriptsexits 5 by design — nothing pytest-shaped lives there; it isnode --testand runs under
check.sh generated.Found, not fixed
mechanism is
AnnotatorCanvas's initial-fit layout effect re-firing on a documentobject the save replaces. Untouched here.