diff --git a/DESIGN.md b/DESIGN.md index 76121cbf..cf92db0c 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -975,6 +975,11 @@ The page the reference design shows (#56), with measurements verified in v1's so variant** (the near-black), inactive = ghost; a `h-px w-6` divider; help at the bottom. Tooltips open right with the shortcut ("Select (V)", "Box (B)", "Polygon (P)"). Icons: MousePointer2 / Square / Spline; only tools the schema's geometries allow. + **Above them all, the hand** (#576, `Hand`, `H`) — and it is the one button here that + the schema does not gate, because it answers a question about the *device* rather than + about the project: a pan had exactly one spelling, a middle- or secondary-button drag, + and a trackpad, a tablet and a pen have no second button to offer. Cursor `grab`, and + `grabbing` while a drag is under way. Below a second divider, **undo and redo** (#368): the chords have worked since #46 and had no representation on screen at all, so the annotator's headline capability over v1 was invisible to anybody who did not already know it. Disabled *with the reason* @@ -1064,6 +1069,16 @@ The page the reference design shows (#56), with measurements verified in v1's so interpolated blur. Both bounds are **disabled with the reason** per principle 9 — the `−`/`+` carry `aria-disabled` and a tooltip naming the limit, never a press that silently does nothing. `docs/annotations.md` carries the argument. +- **Navigating the picture is one model across every device** (#576). Pan: a two-finger + trackpad scroll, a mouse wheel, a middle- or right-button drag, `Space` held with any + drag, or the hand tool. Zoom: a trackpad pinch, `Ctrl`/`Cmd` with a scroll, the widget's + `−`/`+`, and `mod+0` to fit. On a touchscreen one finger draws — or pans while the hand + is on — and two fingers pinch and drag together. **A bare wheel pans; it used to zoom**, + which is deliberate and is the whole of what made a trackpad workable: a two-finger + scroll is how anybody moves around a canvas, and while it zoomed there was no gesture on + a trackpad that moved the picture at all. Mouse zoom is the modifier and the buttons. + The shortcut sheet's **Navigate** section is the one place this is written for a user, + and it is hand-written rather than derived — a gesture has no chord to be read off. ### The read-only mode @@ -1106,9 +1121,15 @@ What a viewer is (decisions of 2026-08-07, #426, and 2026-08-08, #439): - **Selection highlights; it does not advertise.** A selected shape renders the selected treatment — stroke 3, the label — with **no grips and no vertex dots**, and the cursor is the **default arrow everywhere**: no resize keywords, - because no such gesture exists. The tool strip is not rendered at all, for the - same reason it never was. Since #567 the editor also shows the plain arrow over - a shape, so what separates the modes is the grips rather than the cursor. + because no such gesture exists. Since #567 the editor also shows the plain arrow + over a shape, so what separates the modes is the grips rather than the cursor. +- **The tool strip renders, carrying navigation and nothing else** (#576). It used + to be absent entirely, and the reason held while it was true: every control on it + picked a *drawing* tool, and a tool palette over a canvas that cannot be drawn on + explains nothing. The hand is not a drawing tool — it is what a person reaches for + when the picture is in the wrong place — and navigating a batch nobody may edit is + most of what a viewer does, so the strip keeps the hand and the shortcut sheet and + loses every other button. The banner still carries the reason, once. - **Selection is one state, reflected everywhere.** A press on a shape selects it — the one pointer gesture a viewer keeps, resolved by the same hit rule the right-click menu uses — and the objects panel's row highlights and scrolls diff --git a/docs/annotations.md b/docs/annotations.md index 48cee9e0..5bc22564 100644 --- a/docs/annotations.md +++ b/docs/annotations.md @@ -296,6 +296,7 @@ thing that turns one into a store call. | `mod+v` | paste it onto this frame, offset and selected | v1 | | `mod+0` | ask the host to zoom to 100% | v1 | | `?` | ask the host for the shortcut sheet | v1 | +| `h` | turn the hand on or off - with it on, any drag pans | **#576** | | `v` | select mode - no active class | v1 | | `1`-`9` | the schema's first nine classes, in authored order | **new** | @@ -313,6 +314,10 @@ and losing a keystroke is better than losing the session. Not bound, each for a reason: `b`/`p`/`k`/`l`, because the tool is derived from the class here, so a tool key *is* a class key; and the lane-attribute hotkeys, because attributes belong to a panel. +**`Space` is not here either**, and that one is structural rather than a choice: it is the hand's +transient spelling, held rather than pressed, and a `Keystroke` has no shape for a release. A row +here could turn the hand on and never off again. It is an adapter substitution instead, the class +`enter` and `escape` already belong to. ### Copy and paste, and where a clipboard lives @@ -422,10 +427,64 @@ path converts anything. The corollary is the trap: a 2-pixel stroke written as ` pixels - a hair at 8× and a slab at 10% - so every thickness, radius and font size goes through `screenPx(px, zoom)`. It is #41's tolerance finding pointed at drawing instead of at hit-testing. -Zoom is the wheel, and `ctrlKey` on a wheel event **is** how a browser reports a trackpad pinch. -Pan is a middle- or secondary-button drag. `mod+0` refits, and it is intercepted by the adapter -rather than forwarded, because the zoom is the adapter's - it is the one row of the `InputHost` -port that is not a pass-through. +`mod+0` refits, and it is intercepted by the adapter rather than forwarded, because the zoom is +the adapter's - it is the one row of the `InputHost` port that is not a pass-through. + +### One input model, and most of it is a wheel branch two lines long + +A pan used to have exactly one spelling, a middle- or secondary-button drag. A trackpad has no +second button, a pen has none, and a finger has none - so on a laptop there was no gesture that +moved the picture at all, while the gesture people actually make, a two-finger scroll, zoomed. + +The whole of that fix is which side of one branch a wheel event falls on: + +| Held | What a wheel event means | +| --- | --- | +| `ctrlKey` or `metaKey` | zoom, anchored at the cursor | +| nothing | pan, both axes | + +That single branch serves four devices, because **`ctrlKey` on a wheel event is how a browser +reports a trackpad pinch** - on macOS and on a Windows precision touchpad alike, with no gesture +API involved - and `Ctrl`/`Cmd`+wheel is the convention for zooming with a mouse. The two arrive +identically, so they are answered identically. + +**A bare wheel now pans where it used to zoom.** That is the deliberate half, and it is what makes +a trackpad workable rather than a nicety on top. Mouse zoom did not become unreachable: it is the +modifier, the widget's `-`/`+`, and `mod+0`. + +What `ctrlKey` can no longer do is tell a pinch from a mouse wheel, since it is now set by both. +`wheelZoomFactor` tells them apart by **magnitude** instead: a notch is a large quantised value - +120 pixels, three lines, one page - and a pinch is a stream of small continuous ones, so a +threshold at 40 sits in a gap rather than in a distribution. Being wrong about it costs a gesture +that zooms too briskly, never a wrong answer. The softness on the wheel side is derived rather +than picked: `120 / ln(1.25)` is about 538, which makes one notch worth exactly one press of the +`+` button. + +The rest of the model is more spellings of the same two verbs. + +- **`Space` held** is the hand for as long as it is down. It cannot be a registry row - a + keystroke is a press and this is a hold - so it is an adapter substitution, and it is cleared on + blur as well as on keyup, because its release lands in whatever took the focus and never here. +- **The hand tool**, `h`, is the persistent one. Not a fifth `Tool`: `tool.ts` derives the tool + from the active class and stores nothing, so the mode is the host's and arrives as `panTool`, + which is the arrangement the suggest tool already established. +- **Two touch pointers** are a gesture whatever tool is armed. `pinchBetween` answers a scale + about a travelling centroid - one gesture and not two, because a pinch that also drifts is one + thing and answering it as a zoom followed by a pan makes the picture jump between them. The + gesture outlives its two contacts and is cleared when the **last** finger lifts, which is what + stops the survivor of every pinch from being promoted into a drag nobody asked for. + +**The non-primary pan is untouched and still unconditional**, for the reason argued below: a +conditional pan is unpredictable, and on macOS ctrl-click *is* a secondary press. The hand is a +second branch beside it, taken on a primary press, and it sits **before** the read-only select - +somebody navigating a batch they may not edit is who most needs to pan. + +All of the arithmetic is in `adapters/viewport.ts` beside the transform, so it is unit-tested +without a browser: `normalizedWheel` folds `deltaMode`, `wheelZoomFactor` carries the softness +split, `pinchBetween` answers the two-finger case, and `zoomAbout` and `panBy` were already there. +`AnnotatorCanvas` holds only the branches. What no unit test can reach - that a browser really +delivers these events - is `e2e/touch.spec.ts` and the wheel scenarios, which drive Chromium's own +input rather than constructing events. ### Dragging repaints one layer @@ -557,6 +616,13 @@ secondary press**, so routing it would make one ctrl-click raise both spellings the vertex delete - v1's own bug, which #44 closed deliberately and `machine.test.ts` still guards. +#576 did not weaken this. The hand is a *second* branch beside the unconditional one, +taken on a primary press while the mode is on, so both of them forward nothing and +neither is conditional on what the machine would have done. What it changes is only +that a press can now be swallowed for a reason the person chose - which is the +difference between a gesture that works everywhere and one that works where the +vertices are not. + What each capability costs then differs: - The vertex delete costs **nothing**. The toggle modifier reaches the same call; diff --git a/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx b/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx index 9764b706..e7246d88 100644 --- a/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx +++ b/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx @@ -83,6 +83,29 @@ * the transform, `setPointerCapture`, and the browser's own `dblclick` * forwarded — the four pointer handlers. * + * ## One input model, and where each half of it lives + * + * A pan had exactly one spelling — a middle- or secondary-button drag — and a + * trackpad, a pen and a finger have no second button, so on a laptop there was no + * gesture that moved the picture. Four things fixed that, and only the first is a + * change to something that already worked: + * + * 1. **The wheel branches on `ctrlKey || metaKey`**: held, it zooms about the + * cursor; bare, it pans both axes. One branch for four devices, because that + * flag is how a browser reports a trackpad pinch *and* how a mouse asks to + * zoom. A bare wheel used to zoom; it pans now, deliberately. + * 2. **`Space` held** is the hand while it is down — a substitution rather than a + * registry row, because a keystroke is a press and this needs a release. + * 3. **`panTool`** is the persistent hand, the host's to own for `suggestion`'s + * reason. + * 4. **Two touch pointers** are a gesture whatever tool is armed, tracked in + * `touchesNow` because both fingers report `button: 0` and nothing else tells + * the second press from a fresh first one. + * + * All four are wiring. The arithmetic — `normalizedWheel`, `wheelZoomFactor`, + * `pinchBetween`, and the `zoomAbout`/`panBy` that were already there — is in + * `adapters/viewport.ts`, where it is unit-tested without a browser. + * * ## Capture is taken after the dispatch, never before * * `state.ts` records that v1 carried a `captured` boolean because acquiring @@ -170,8 +193,11 @@ import { IDENTITY_VIEWPORT, fitToViewport, imageRenderingAt, + normalizedWheel, panBy, + pinchBetween, screenToImage, + wheelZoomFactor, zoomAbout, } from "../viewport"; import type { Viewport } from "../viewport"; @@ -196,14 +222,14 @@ const DRAG_STATES: ReadonlySet = new Set([ /** Breathing room around a fitted asset, in screen pixels. */ const FIT_PADDING_PX = 16; -/** How much wheel travel doubles the zoom. Larger is gentler. */ -const WHEEL_SOFTNESS = 400; - -/** The same for a trackpad pinch, whose deltas are an order of magnitude smaller. */ -const PINCH_SOFTNESS = 100; - -/** `WheelEvent.deltaMode`: 0 pixels, 1 lines (Firefox), 2 pages. */ -const DELTA_SCALE: Readonly> = { 0: 1, 1: 16, 2: 400 }; +/** + * The pointer types that can put a second contact on the glass. + * + * A pen reports pressure and tilt and is still one pointer; a mouse is one + * pointer with buttons. Only touch can be two at once, which is what makes a + * pinch a touch-only shape here. + */ +const MULTI_TOUCH = "touch"; export interface AnnotatorCanvasProps { /** @@ -402,6 +428,26 @@ export interface AnnotatorCanvasProps { * which would draw a shape somebody was trying to point at. */ readonly onSuggestPoint?: (point: Point, polarity: Polarity) => void; + /** + * The hand: while it is on, a **primary** drag pans instead of drawing. + * + * The host holds it for the reason it holds `suggestion` — the palette lights + * a button for it, and a mode the canvas kept to itself could not be drawn. + * `h` reaches it as `TOGGLE_HAND` through `onHostAction`, and the two doors + * are one state. + * + * The pan contract's **third** occupant, honoured in the same place as the + * other two: `handlePointerDown` diverts, having first cancelled anything the + * pointer had in flight. It does not need the arming effect the suggest tool + * has, because the divert is unconditional and there is a second, transient + * spelling — holding `Space` — which arms mid-gesture routinely. + * + * It exists because a pan had exactly one spelling, a middle- or + * secondary-button drag, and a trackpad, a tablet and a pen have no second + * button to offer. The wheel and the pinch cover a trackpad; this covers the + * devices that have neither. + */ + readonly panTool?: boolean; } /** What a host can do to the stage. Read the position through `onViewChange`. */ @@ -434,6 +480,7 @@ export function AnnotatorCanvas({ readOnly = false, suggestion = null, onSuggestPoint, + panTool = false, }: AnnotatorCanvasProps): JSX.Element { const snapshot = useAnnotatorSnapshot(store); const { asset, schema } = snapshot.document; @@ -457,6 +504,58 @@ export function AnnotatorCanvas({ hiddenNow.current = hiddenIds; const panNow = useRef<{ readonly x: number; readonly y: number } | null>(null); + /** + * The hand's transient spelling: `Space`, while it is held. + * + * A ref and a piece of state, because both readers need it and they need it + * at different times — `handlePointerDown` reads the ref inside an event, the + * cursor reads the state at render. It is set from a keydown and cleared from + * a keyup *and* from the blur handler, which is what stops a window switch + * mid-hold from leaving the hand on with no way to notice. + */ + const [spaceHeld, setSpaceHeld] = useState(false); + const spaceHeldNow = useRef(spaceHeld); + const setSpaceHold = useCallback((held: boolean) => { + if (spaceHeldNow.current === held) return; + spaceHeldNow.current = held; + setSpaceHeld(held); + }, []); + + const panToolNow = useRef(panTool); + panToolNow.current = panTool; + /** Either spelling of the hand. `handNow` is the event-time read of the same pair. */ + const hand = panTool || spaceHeld; + const handNow = (): boolean => panToolNow.current || spaceHeldNow.current; + + /** + * Whether a drag pan is under way, for the cursor and for nothing else. + * + * State rather than the `panNow` ref because only a render can change a + * cursor, and it is set **only while the hand is on** — that is the one mode + * where `grab` and `grabbing` differ, so a middle-button drag pays no + * re-render for a cursor nobody is looking at. Clearing is unconditional and + * free: `useState` bails on an unchanged value. + */ + const [panning, setPanning] = useState(false); + + /** + * Every touch pointer currently down, by id, at its last known position. + * + * Touch is the one input where the adapter has to count. A mouse press is one + * pointer and its `button` says which; two fingers are two `pointerdown`s that + * both report `button: 0`, and nothing in the event distinguishes the second + * from a fresh press of the first. So the map is what makes a pinch nameable + * at all — and it is scoped to `pointerType === "touch"`, because a pen and a + * mouse cannot produce a second contact and counting them would only add a way + * to be wrong. + */ + const touchesNow = useRef(new Map()); + /** The two ids a gesture is between, and where they were on the last move. */ + const gestureNow = useRef<{ + readonly ids: readonly [number, number]; + readonly at: readonly [readonly [number, number], readonly [number, number]]; + } | null>(null); + const applyViewport = useCallback((next: Viewport) => { viewNow.current = next; setView(next); @@ -564,19 +663,41 @@ export function AnnotatorCanvas({ if (pane === null) return; const onWheel = (event: WheelEvent): void => { event.preventDefault(); - const rect = pane.getBoundingClientRect(); - // `ctrlKey` on a wheel event IS how a browser reports a trackpad pinch; - // no separate gesture API is involved, and its deltas are much smaller. - const delta = event.deltaY * (DELTA_SCALE[event.deltaMode] ?? 1); - const softness = event.ctrlKey ? PINCH_SOFTNESS : WHEEL_SOFTNESS; - applyViewport( - zoomAbout( - viewNow.current, - Math.exp(-delta / softness), - event.clientX - rect.left, - event.clientY - rect.top, - ), - ); + const [dx, dy] = normalizedWheel(event.deltaX, event.deltaY, event.deltaMode); + /** + * **One branch, and it serves four devices.** + * + * `ctrlKey` on a wheel event is how a browser reports a trackpad pinch — + * on macOS and on a Windows precision touchpad alike, with no gesture API + * involved — and `ctrl`/`cmd` + wheel is the convention for zooming with + * a mouse. Those are the same flag, so they are the same branch, and + * everything else is a pan. + * + * The half that changed is what "everything else" now covers. A plain + * wheel used to zoom, which made a two-finger trackpad scroll — the + * ordinary way anyone moves around a canvas — zoom instead of scroll, and + * left a trackpad with no pan at all. Now it pans, `deltaX` included, and + * a mouse wheel pans vertically for the same reason it scrolls a page + * vertically. Zoom did not become unreachable: it is the modifier, the + * pinch, `mod+0` and the two buttons in the corner. + * + * The sign is inverted because a scroll reports how far the *content* + * should travel against the gesture, and `panBy` moves the content with + * it: scrolling down looks at what is below, so the picture goes up. + */ + if (event.ctrlKey || event.metaKey) { + const rect = pane.getBoundingClientRect(); + applyViewport( + zoomAbout( + viewNow.current, + wheelZoomFactor(dy), + event.clientX - rect.left, + event.clientY - rect.top, + ), + ); + return; + } + applyViewport(panBy(viewNow.current, -dx, -dy)); }; pane.addEventListener("wheel", onWheel, { passive: false }); return () => pane.removeEventListener("wheel", onWheel); @@ -668,6 +789,29 @@ export function AnnotatorCanvas({ function handleKeyDown(event: ReactKeyboardEvent): void { // (5) The browser is still deciding what was typed. if (isComposing({ isComposing: event.nativeEvent.isComposing, keyCode: event.keyCode })) return; + + /** + * `Space` held is the hand, for as long as it is held. + * + * Read before `resolve`, and it is the only chord that is. It cannot be a + * registry row for the reason `TOGGLE_HAND` states — a keystroke is a press + * and this needs a release — so it is a substitution, the class `enter` and + * `escape` already belong to, and it is placed *first* because it is a hold + * rather than a decision: nothing about the document depends on it. + * + * `repeat` is dropped rather than ignored. A held key autorepeats, and every + * repeat would re-enter the mode that is already on; the press that turns it + * on is the first one. + * + * `preventDefault` unconditionally, so the page underneath does not scroll + * — which is the one thing `Space` means to a browser by default. + */ + if (event.key === " " && !isTextEntry(event.target instanceof HTMLElement ? event.target : null)) { + event.preventDefault(); + if (!event.repeat) setSpaceHold(true); + return; + } + const keystroke = keystrokeOf({ // (6) The digit row is a row of positions, not of characters. key: digitFromCode(event.code) ?? event.key, @@ -778,6 +922,31 @@ export function AnnotatorCanvas({ } } + /** + * The other half of the hold. Only `Space`, and only ever a release. + * + * A drag in progress when the key comes up finishes as a pan: `panNow` is + * already set and `handlePointerMove` reads it rather than the mode, so + * letting go of the key mid-gesture does not strand the picture halfway. + */ + function handleKeyUp(event: ReactKeyboardEvent): void { + if (event.key !== " ") return; + setSpaceHold(false); + } + + /** + * Start a drag pan. `state.ts`'s written contract, in one place for the three + * gestures that reach it: a non-primary press, the hand, and either of them + * over a shape mid-draw. While panning the adapter forwards nothing, and if a + * gesture was in flight when the pan began it cancels it first. + */ + function beginPan(event: ReactPointerEvent): void { + if (interactionNow.current.type !== "idle") dispatch({ type: "pointer-cancel" }); + panNow.current = { x: event.clientX, y: event.clientY }; + if (handNow()) setPanning(true); + event.currentTarget.setPointerCapture(event.pointerId); + } + function handlePointerDown(event: ReactPointerEvent): void { // (7) Named or nothing: a side button forwards no event at all. const button = pointerButton(event.button); @@ -786,6 +955,54 @@ export function AnnotatorCanvas({ // was clicked. Pressing on it is the click. rootRef.current?.focus({ preventScroll: true }); + /** + * Two fingers are a gesture, whatever tool is armed. + * + * The count is the whole rule: one finger is the pointer for whatever the + * class derives, two are a pinch and a pan together, and a third joins + * nothing — it lands while a gesture is running and is swallowed with it. + * Both fingers report `button: 0`, so nothing but the map distinguishes the + * second press from a fresh first one. + * + * A gesture lasts exactly as long as two fingers are down, and re-forms + * when two are down again — so lifting one and putting another back + * continues the pinch instead of leaving it dead until the hand comes off + * the glass. It re-forms from the contacts' *current* positions, which is + * what makes that free of a jump. + * + * The survivor of a lift needs no swallowing, and that was measured rather + * than assumed: `IDLE_ROW` has a `pointer-down` handler and nothing else, + * so the stray moves and the stray lift reach an idle machine and are + * silence. Its press happened before the gesture and cannot happen again. + */ + if (event.pointerType === MULTI_TOUCH) { + touchesNow.current.set(event.pointerId, { x: event.clientX, y: event.clientY }); + if (gestureNow.current !== null) return; + if (touchesNow.current.size >= 2) { + beginGesture(); + return; + } + } + + /** + * Pan, in both its spellings, and they differ only in the first line. + * + * A non-primary press has always panned and still does — unconditionally, + * because a conditional pan is unpredictable (`docs/annotations.md` argues + * it at length: right-drag would pan on empty canvas and not over a vertex). + * What joins it is the hand, which is what gives a trackpad, a pen and a + * finger the gesture a second mouse button used to be required for. + * + * **Before the read-only branch**, and that is the point of its position: a + * viewer navigating a batch they may not edit is exactly who most needs to + * pan, and a hand that only worked in edit mode would be a control that + * disappears when the page goes quiet. + */ + if (button !== "primary" || handNow()) { + beginPan(event); + return; + } + // Read-only: a primary press *selects* and does nothing else. It // never reaches the machine, so no drag state — a draw, a move, a resize, a // vertex drag — is reachable at all, which is a stronger guarantee than @@ -795,9 +1012,8 @@ export function AnnotatorCanvas({ // `topmostAnnotationAt` with the body tolerance over the hidden-filtered // document — the same rule `viewerAffordanceAt` highlights with and the // same one the right-click menu resolves, so the highlight, the press and - // the menu cannot disagree about what is "under" a point. Non-primary is a - // pan, which changes nothing, and falls through to the branch below. - if (readOnly && button === "primary") { + // the menu cannot disagree about what is "under" a point. + if (readOnly) { const point = imagePoint(event); if (point === null) return; const hit = topmostAnnotationAt( @@ -809,15 +1025,6 @@ export function AnnotatorCanvas({ return; } - if (button !== "primary") { - // `state.ts`'s written contract: while panning the adapter forwards nothing, - // and if a gesture was in flight when the pan began it cancels it first. - if (interactionNow.current.type !== "idle") dispatch({ type: "pointer-cancel" }); - panNow.current = { x: event.clientX, y: event.clientY }; - event.currentTarget.setPointerCapture(event.pointerId); - return; - } - const point = imagePoint(event); if (point === null) return; @@ -855,7 +1062,67 @@ export function AnnotatorCanvas({ } } + /** The first two fingers down become the gesture, and a drag pan yields to it. */ + function beginGesture(): void { + const down = [...touchesNow.current.entries()]; + const first = down[0]; + const second = down[1]; + if (first === undefined || second === undefined) return; + if (interactionNow.current.type !== "idle") dispatch({ type: "pointer-cancel" }); + panNow.current = null; + gestureNow.current = { + ids: [first[0], second[0]], + at: [ + [first[1].x, first[1].y], + [second[1].x, second[1].y], + ], + }; + } + + /** + * One frame of a two-finger gesture: the scale and the drift, applied together. + * + * The order is the one `pinchBetween` documents — translate by the centroid's + * travel, then scale about where the centroid ended — and it is what keeps + * whatever was between the fingers between the fingers. The other way round + * scales about a point that has not moved yet, and the picture slides out from + * under the gesture. + * + * A gesture whose two fingers are not both still down does nothing, and still + * swallows the event — `handlePointerDown` says why it outlives them. + */ + function moveGesture(): void { + const gesture = gestureNow.current; + const pane = paneRef.current; + if (gesture === null || pane === null) return; + const a = touchesNow.current.get(gesture.ids[0]); + const b = touchesNow.current.get(gesture.ids[1]); + if (a === undefined || b === undefined) return; + const at = [ + [a.x, a.y], + [b.x, b.y], + ] as const; + const pinch = pinchBetween(gesture.at, at); + gestureNow.current = { ids: gesture.ids, at }; + const rect = pane.getBoundingClientRect(); + applyViewport( + zoomAbout( + panBy(viewNow.current, pinch.dx, pinch.dy), + pinch.factor, + pinch.centroidX - rect.left, + pinch.centroidY - rect.top, + ), + ); + } + function handlePointerMove(event: ReactPointerEvent): void { + if (event.pointerType === MULTI_TOUCH && touchesNow.current.has(event.pointerId)) { + touchesNow.current.set(event.pointerId, { x: event.clientX, y: event.clientY }); + } + if (gestureNow.current !== null) { + moveGesture(); + return; + } const panning = panNow.current; if (panning !== null) { applyViewport(panBy(viewNow.current, event.clientX - panning.x, event.clientY - panning.y)); @@ -868,9 +1135,26 @@ export function AnnotatorCanvas({ dispatch({ type: "pointer-move", point }); } + /** + * A finger left the glass. Below two contacts there is no gesture. + * + * Answers whether this lift belonged to one and should therefore reach + * nothing else — the lift that ends it included, because the alternative is + * dispatching a `pointer-up` for a press the machine was told to cancel. + */ + function releaseTouch(event: ReactPointerEvent): boolean { + if (event.pointerType !== MULTI_TOUCH) return false; + touchesNow.current.delete(event.pointerId); + if (gestureNow.current === null) return false; + if (touchesNow.current.size < 2) gestureNow.current = null; + return true; + } + function handlePointerUp(event: ReactPointerEvent): void { + if (releaseTouch(event)) return; if (panNow.current !== null) { panNow.current = null; + setPanning(false); return; } const button = pointerButton(event.button); @@ -880,11 +1164,28 @@ export function AnnotatorCanvas({ dispatch({ type: "pointer-up", point, button, modifiers: modifiersOf(event) }); } - function handlePointerCancel(): void { + function handlePointerCancel(event?: ReactPointerEvent): void { + if (event !== undefined) releaseTouch(event); panNow.current = null; + setPanning(false); dispatch({ type: "pointer-cancel" }); } + /** + * The root lost the focus: everything the pointer and the keyboard were + * holding is let go of. + * + * A held `Space` most of all. Its keyup lands in whatever took the focus and + * never here, so without this the hand survives a window switch with nothing + * on screen to say why the canvas has stopped drawing. + */ + function handleBlur(): void { + touchesNow.current.clear(); + gestureNow.current = null; + setSpaceHold(false); + handlePointerCancel(); + } + function handleContextMenu(event: ReactMouseEvent): void { // A secondary-button drag pans; without this it also opens the browser's menu. event.preventDefault(); @@ -978,10 +1279,11 @@ export function AnnotatorCanvas({ tabIndex={0} aria-keyshortcuts={ariaKeyshortcuts(registry.keys())} onKeyDown={handleKeyDown} + onKeyUp={handleKeyUp} // A window blur or a click on host chrome interrupts a *drag*; a // click-by-click polygon session survives it, which is `machine.ts`'s // deliberate `pointer-cancel` asymmetry doing real work here. - onBlur={handlePointerCancel} + onBlur={handleBlur} style={{ position: "relative", width: "100%", height: "100%", outline: "none" }} >
{ expect(fitToViewport({ id: "d", width: 0, height: 0 }, 800, 600)).toBe(IDENTITY_VIEWPORT); }); }); + +describe("a wheel event's travel is read in screen pixels whatever it was reported in", () => { + it("passes pixels through", () => { + expect(normalizedWheel(3, -120, 0)).toEqual([3, -120]); + }); + + it("reads Firefox's lines as pixels", () => { + expect(normalizedWheel(0, 3, 1)).toEqual([0, 48]); + }); + + it("reads a page as pixels", () => { + expect(normalizedWheel(0, 1, 2)).toEqual([0, 400]); + }); + + it("carries the horizontal axis, which is the one a two-finger scroll needs", () => { + expect(normalizedWheel(-2, 0, 1)).toEqual([-32, 0]); + }); + + it("reads an unrecognised delta mode as pixels rather than refusing it", () => { + expect(normalizedWheel(5, 7, 99)).toEqual([5, 7]); + }); +}); + +describe("the wheel's zoom factor", () => { + it("makes one mouse notch worth one press of a 1.25 step button", () => { + expect(wheelZoomFactor(-120)).toBeCloseTo(1.25, 3); + }); + + it("is the exact inverse in the other direction, so a notch back undoes a notch out", () => { + expect(wheelZoomFactor(-120) * wheelZoomFactor(120)).toBeCloseTo(1, 10); + }); + + it("reads a small continuous delta as a pinch and scales it far more steeply", () => { + // The same 20 pixels of travel: as a pinch it is a fifth of the way to + // doubling, as a wheel notch it would barely move. + expect(wheelZoomFactor(-20)).toBeCloseTo(Math.exp(20 / 100), 10); + }); + + it("reads travel at the notch threshold as a wheel", () => { + expect(wheelZoomFactor(-40)).toBeCloseTo(Math.exp(40 / 538), 10); + }); + + it("answers no change for a delta that is not a number", () => { + expect(wheelZoomFactor(Number.NaN)).toBe(1); + expect(wheelZoomFactor(Number.POSITIVE_INFINITY)).toBe(1); + }); +}); + +describe("two fingers are one gesture: a scale about a point, and that point's travel", () => { + it("reads fingers moving apart as a zoom in", () => { + const pinch = pinchBetween( + [ + [100, 100], + [200, 100], + ], + [ + [50, 100], + [250, 100], + ], + ); + expect(pinch.factor).toBeCloseTo(2, 10); + }); + + it("reads fingers moving together as a zoom out", () => { + const pinch = pinchBetween( + [ + [0, 0], + [0, 100], + ], + [ + [0, 25], + [0, 75], + ], + ); + expect(pinch.factor).toBeCloseTo(0.5, 10); + }); + + it("reports the midpoint the gesture ended on", () => { + const pinch = pinchBetween( + [ + [0, 0], + [100, 40], + ], + [ + [20, 10], + [140, 70], + ], + ); + expect(pinch.centroidX).toBeCloseTo(80, 10); + expect(pinch.centroidY).toBeCloseTo(40, 10); + }); + + it("reports how far that midpoint travelled", () => { + const pinch = pinchBetween( + [ + [0, 0], + [100, 40], + ], + [ + [20, 10], + [140, 70], + ], + ); + expect(pinch.dx).toBeCloseTo(30, 10); + expect(pinch.dy).toBeCloseTo(20, 10); + }); + + it("separates a drift from a scale: two fingers moving together only translate", () => { + const pinch = pinchBetween( + [ + [100, 100], + [200, 100], + ], + [ + [130, 160], + [230, 160], + ], + ); + expect(pinch.factor).toBeCloseTo(1, 10); + expect(pinch.dx).toBeCloseTo(30, 10); + expect(pinch.dy).toBeCloseTo(60, 10); + }); + + it("keeps whatever is under the midpoint under the midpoint, drift and scale together", () => { + // The invariant the whole gesture is judged by, and the reason the caller + // applies the translation first and then zooms about the centroid. + // + // **Both midpoints are computed here rather than read off the result**, and + // that is the difference between this assertion and a tautology. Asserting + // the held pixel lands on `pinch.centroidX` passes under a mutation that + // takes the centroid from *before* the move — because `dx` is derived from + // the same field, so the expectation slides exactly as far as the answer + // does. Mutation-verified: taking the centroid from `before` reddens this. + const viewport: Viewport = { zoom: 0.8, panX: 37, panY: -12 }; + const before = [ + [100, 100], + [300, 200], + ] as const; + const after = [ + [60, 140], + [420, 260], + ] as const; + const midpoint = ( + pair: readonly [readonly [number, number], readonly [number, number]], + ): readonly [number, number] => [ + (pair[0][0] + pair[1][0]) / 2, + (pair[0][1] + pair[1][1]) / 2, + ]; + const from = midpoint(before); + const to = midpoint(after); + + const pinch = pinchBetween(before, after); + expect(pinch.centroidX).toBeCloseTo(to[0], 10); + expect(pinch.centroidY).toBeCloseTo(to[1], 10); + expect(pinch.dx).toBeCloseTo(to[0] - from[0], 10); + expect(pinch.dy).toBeCloseTo(to[1] - from[1], 10); + + const held = screenToImage(viewport, from[0], from[1]); + const panned = panBy(viewport, pinch.dx, pinch.dy); + const zoomed = zoomAbout(panned, pinch.factor, pinch.centroidX, pinch.centroidY); + + expect(imageToScreen(zoomed, held)[0]).toBeCloseTo(to[0], 8); + expect(imageToScreen(zoomed, held)[1]).toBeCloseTo(to[1], 8); + }); + + it("answers the identity for two pointers in the same place, rather than dividing by zero", () => { + const pinch = pinchBetween( + [ + [50, 50], + [50, 50], + ], + [ + [50, 50], + [90, 90], + ], + ); + expect(pinch).toEqual({ factor: 1, centroidX: 0, centroidY: 0, dx: 0, dy: 0 }); + }); + + it("answers the identity for a coordinate that is not a number", () => { + const pinch = pinchBetween( + [ + [Number.NaN, 0], + [100, 0], + ], + [ + [0, 0], + [200, 0], + ], + ); + expect(pinch.factor).toBe(1); + }); + + it("leaves the viewport untouched when the gesture was degenerate", () => { + const viewport: Viewport = { zoom: 3, panX: 10, panY: 20 }; + const pinch = pinchBetween( + [ + [50, 50], + [50, 50], + ], + [ + [10, 10], + [10, 10], + ], + ); + const panned = panBy(viewport, pinch.dx, pinch.dy); + expect(zoomAbout(panned, pinch.factor, pinch.centroidX, pinch.centroidY)).toBe(viewport); + }); +}); diff --git a/frontend/annotator/src/adapters/viewport.ts b/frontend/annotator/src/adapters/viewport.ts index b73d57c4..f0088ffb 100644 --- a/frontend/annotator/src/adapters/viewport.ts +++ b/frontend/annotator/src/adapters/viewport.ts @@ -237,3 +237,124 @@ export function fitToViewport( panY: (viewportHeight - asset.height * zoom) / 2, }; } + +/** + * `WheelEvent.deltaMode`: 0 pixels, 1 lines (Firefox), 2 pages. + * + * The same physical notch is reported as `120`, as `3` or as `1` depending on + * the browser, so a handler reading `deltaY` raw is three orders of magnitude + * out on two of the three. + */ +const DELTA_SCALE: Readonly> = { 0: 1, 1: 16, 2: 400 }; + +/** + * A wheel event's travel in screen pixels, whatever unit it was reported in. + * + * **Both axes**, where the zoom path only ever read the second: a two-finger + * trackpad scroll is a pan, and a pan goes sideways. An unrecognised + * `deltaMode` is read as pixels rather than refused, `clampZoom`'s rule for + * `clampZoom`'s reason — a view that moves the wrong distance is corrected by + * the next notch, and nothing about it reaches the document. + */ +export function normalizedWheel( + deltaX: number, + deltaY: number, + deltaMode: number, +): readonly [number, number] { + const scale = DELTA_SCALE[deltaMode] ?? 1; + return [deltaX * scale, deltaY * scale]; +} + +/** + * How much wheel travel doubles the zoom. Larger is gentler. + * + * Derived rather than picked. One notch of a mouse wheel is 120 pixels of + * travel in every browser that reports pixels, and `120 / ln(1.25) ≈ 538` is + * what makes that notch worth exactly one press of a host's `+` button, whose + * step is 1.25. Two doors onto one behaviour, the way `mod+0` and a fit button + * are — a person who reaches for the wheel and a person who reaches for the + * button move the picture by the same amount. It was 400, which is 1.35 a + * notch and agreed with nothing. + */ +const WHEEL_SOFTNESS = 538; + +/** The same for a trackpad pinch, whose deltas are an order of magnitude smaller. */ +const PINCH_SOFTNESS = 100; + +/** + * Above this much travel in one event, the gesture is a wheel and not a pinch. + * + * `ctrlKey` used to tell the two apart, and it was exact: a browser sets it for + * a trackpad pinch and for nothing else a wheel does. It cannot any more — + * `ctrl`/`cmd` + wheel is the mouse's own zoom now, so both gestures arrive + * with the flag set and the number is all that is left. It is enough of a + * boundary to be worth drawing: a notch is a large quantised value — 120 + * pixels, three lines, one page — and a pinch is a stream of small continuous + * ones, so 40 sits in a gap rather than in a distribution. Being wrong costs a + * gesture that zooms too briskly or too slowly, never a wrong answer. + */ +const MOUSE_NOTCH_PX = 40; + +/** + * The multiplicative zoom a wheel event asks for, sign and softness included. + * + * Multiplicative because zoom is: two notches out and two notches back land + * exactly where they started, which additive steps do not. + */ +export function wheelZoomFactor(delta: number): number { + if (!Number.isFinite(delta)) return 1; + const softness = Math.abs(delta) >= MOUSE_NOTCH_PX ? WHEEL_SOFTNESS : PINCH_SOFTNESS; + return Math.exp(-delta / softness); +} + +/** What two pointers did to the picture between one move and the next. */ +export interface Pinch { + /** The scale they asked for. 1 when the distance between them did not change. */ + readonly factor: number; + /** Their midpoint after the move, in the viewport element's own pixels. */ + readonly centroidX: number; + readonly centroidY: number; + /** How far that midpoint travelled, in the same pixels. */ + readonly dx: number; + readonly dy: number; +} + +/** Neither scaled nor moved: what a degenerate gesture answers. */ +const NO_PINCH: Pinch = { factor: 1, centroidX: 0, centroidY: 0, dx: 0, dy: 0 }; + +/** + * A two-finger gesture, as a scale about a point together with that point's + * travel. Screen positions in, both relative to the viewport element's rect. + * + * Both halves at once, because that is what two fingers do. A pinch that also + * drifts is one gesture, and answering it as a zoom event and then a pan event + * would make the picture jump between them; the caller applies what comes back + * in one step — `panBy` the travel, then `zoomAbout` the centroid — and the + * thing under the midpoint stays under the midpoint. + * + * A degenerate gesture answers the identity: two pointers in one place, or a + * non-finite coordinate. That is not defensive tidying. A zero distance divides + * by zero, the NaN reaches `zoomAbout`, `clampZoom` answers 1, and the picture + * snaps to native scale in the middle of somebody's pinch — a jump caused + * precisely by the arithmetic that was meant to prevent one. + */ +export function pinchBetween( + before: readonly [readonly [number, number], readonly [number, number]], + after: readonly [readonly [number, number], readonly [number, number]], +): Pinch { + const [[ax0, ay0], [bx0, by0]] = before; + const [[ax1, ay1], [bx1, by1]] = after; + if (![ax0, ay0, bx0, by0, ax1, ay1, bx1, by1].every(Number.isFinite)) return NO_PINCH; + const spread = Math.hypot(bx0 - ax0, by0 - ay0); + const spreadAfter = Math.hypot(bx1 - ax1, by1 - ay1); + if (spread === 0 || spreadAfter === 0) return NO_PINCH; + const centroidX = (ax1 + bx1) / 2; + const centroidY = (ay1 + by1) / 2; + return { + factor: spreadAfter / spread, + centroidX, + centroidY, + dx: centroidX - (ax0 + bx0) / 2, + dy: centroidY - (ay0 + by0) / 2, + }; +} diff --git a/frontend/annotator/src/core/input/actions.ts b/frontend/annotator/src/core/input/actions.ts index 45337e22..4fdfc741 100644 --- a/frontend/annotator/src/core/input/actions.ts +++ b/frontend/annotator/src/core/input/actions.ts @@ -276,3 +276,25 @@ export const COARSER_SUGGESTION = "coarser-suggestion"; /** The other bracket. See {@link COARSER_SUGGESTION}. */ export const FINER_SUGGESTION = "finer-suggestion"; + +/** + * Turn the hand on or off — `h`, the persistent pan tool. + * + * A host row, and for `TOGGLE_SUGGEST`'s reason: it is a *mode over the top* of + * whatever tool the active class derives, not a fifth `Tool`. `tool.ts` derives + * and stores nothing, so a hand there would be a stored mode wearing a derived + * one's name with nowhere to be derived from. + * + * The mode exists because a pan had exactly one spelling — a middle- or + * secondary-button drag — and a trackpad, a tablet and a pen have no second + * button to offer. It reaches every pointer a device has. + * + * Its *transient* twin, holding `Space`, is deliberately **not** a row here. + * A registry entry is resolved from a keystroke and a keystroke is a press; + * a held key is a press and a release, and `keys.ts` has no shape for the + * second. So `Space` is an adapter substitution, the class `ACCEPT_SUGGESTION` + * and `DISCARD_SUGGESTION` already belong to — and the same rule applies for + * the same reason: the layer that can see the extra state is the one that + * decides. + */ +export const TOGGLE_HAND = "toggle-hand"; diff --git a/frontend/annotator/src/core/input/bindings.test.ts b/frontend/annotator/src/core/input/bindings.test.ts index e2993ca9..86fbe56c 100644 --- a/frontend/annotator/src/core/input/bindings.test.ts +++ b/frontend/annotator/src/core/input/bindings.test.ts @@ -33,6 +33,7 @@ import { TOGGLE_HELP, COARSER_SUGGESTION, FINER_SUGGESTION, + TOGGLE_HAND, TOGGLE_SUGGEST, } from "./actions"; import type { Action } from "./actions"; @@ -101,6 +102,9 @@ const DISPATCH: readonly DispatchRow[] = [ { chord: "s", key: "s", action: { kind: "host", name: TOGGLE_SUGGEST } }, { chord: "[", key: "[", action: { kind: "host", name: COARSER_SUGGESTION } }, { chord: "]", key: "]", action: { kind: "host", name: FINER_SUGGESTION } }, + // `space` is absent from this pair for the reason `enter` is absent above it: + // the hand's transient twin is a *held* key, and a keystroke is a press. + { chord: "h", key: "h", action: { kind: "host", name: TOGGLE_HAND } }, { chord: "v", key: "v", action: { kind: "activate-class", labelClass: null } }, ]; @@ -175,6 +179,16 @@ describe("the default shortcut table", () => { expect(resolve(DEFAULTS, keystroke("s", MOD))).toEqual({ kind: "host", name: SAVE }); }); + it("claims `h` for the hand and leaves the space bar to the adapter (#576)", () => { + // The hand's two spellings are split across two layers on purpose. The + // persistent one is a chord and lives here, so the shortcut sheet lists it + // and an override can move it. The transient one is `Space` *held*, which is + // a press and a release — and `keys.ts` has a shape for the press only, so a + // row here could turn the hand on and never off again. + expect(resolve(DEFAULTS, keystroke("h"))).toEqual({ kind: "host", name: TOGGLE_HAND }); + expect(resolve(DEFAULTS, keystroke(" "))).toBeNull(); + }); + it("answers null for a chord nobody bound", () => { expect(resolve(DEFAULTS, keystroke("q"))).toBeNull(); expect(resolve(DEFAULTS, keystroke("Escape", { altKey: true }))).toBeNull(); diff --git a/frontend/annotator/src/core/input/bindings.ts b/frontend/annotator/src/core/input/bindings.ts index 291cf22a..36de632d 100644 --- a/frontend/annotator/src/core/input/bindings.ts +++ b/frontend/annotator/src/core/input/bindings.ts @@ -131,6 +131,7 @@ import { TOGGLE_HELP, COARSER_SUGGESTION, FINER_SUGGESTION, + TOGGLE_HAND, TOGGLE_SUGGEST, } from "./actions"; import type { Action } from "./actions"; @@ -197,6 +198,10 @@ export const DEFAULT_BINDINGS: readonly Binding[] = [ // answers `false` so the chord falls through to the browser untouched. { chord: "[", action: { kind: "host", name: COARSER_SUGGESTION } }, { chord: "]", action: { kind: "host", name: FINER_SUGGESTION } }, + // `h`, the hand. Bare on `s`'s terms, and the last letter this table claims. + // Not `activate-class`, because navigation derives from no class — see + // `TOGGLE_HAND`, which also says why holding `Space` cannot be a row beside it. + { chord: "h", action: { kind: "host", name: TOGGLE_HAND } }, { chord: "v", action: { kind: "activate-class", labelClass: null } }, ]; diff --git a/frontend/annotator/src/core/input/index.ts b/frontend/annotator/src/core/input/index.ts index 49d52b75..a430607b 100644 --- a/frontend/annotator/src/core/input/index.ts +++ b/frontend/annotator/src/core/input/index.ts @@ -89,6 +89,7 @@ export { TOGGLE_HELP, COARSER_SUGGESTION, FINER_SUGGESTION, + TOGGLE_HAND, TOGGLE_SUGGEST, type Action, type ActionKind, diff --git a/frontend/annotator/src/index.ts b/frontend/annotator/src/index.ts index 1187b3b3..a25d2224 100644 --- a/frontend/annotator/src/index.ts +++ b/frontend/annotator/src/index.ts @@ -235,6 +235,7 @@ export { SKIP_FRAME, COARSER_SUGGESTION, FINER_SUGGESTION, + TOGGLE_HAND, TOGGLE_HELP, TOGGLE_SUGGEST, chordOf, @@ -279,9 +280,13 @@ export { fitToViewport, imageRenderingAt, imageToScreen, + normalizedWheel, panBy, + pinchBetween, screenToImage, + wheelZoomFactor, zoomAbout, + type Pinch, type Viewport, } from "./adapters/viewport"; diff --git a/frontend/app/bench/_gestures.ts b/frontend/app/bench/_gestures.ts index a6a9588c..aa774e6e 100644 --- a/frontend/app/bench/_gestures.ts +++ b/frontend/app/bench/_gestures.ts @@ -53,9 +53,14 @@ export async function pacedWheel( runLength = 10, ): Promise { await page.mouse.move(at.x, at.y); + // Held for the whole run rather than per notch: a bare wheel pans now (#576), + // and the modifier is what makes this a zoom. One down/up pair is also one + // fewer pair of input events between the notches being measured. + await page.keyboard.down("Control"); for (let step = 0; step < frames; step += 1) { const inward = Math.floor(step / runLength) % 2 === 0; await page.mouse.wheel(0, inward ? -120 : 120); await nextFrame(page); } + await page.keyboard.up("Control"); } diff --git a/frontend/app/cycle/cycle.spec.ts b/frontend/app/cycle/cycle.spec.ts index b17df500..436d3504 100644 --- a/frontend/app/cycle/cycle.spec.ts +++ b/frontend/app/cycle/cycle.spec.ts @@ -596,7 +596,10 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa * `complete` is the job's declaration, not the frame's. */ await expect(page.getByTestId("readonly-banner")).toContainText(/this job is finished/i); - await expect(page.getByTestId("tool-palette")).toHaveCount(0); + // The strip stays and carries navigation only (#576): the hand is not a + // drawing tool, and a viewer moving around a finished job still needs it. + await expect(page.getByTestId("tool-select")).toHaveCount(0); + await expect(page.getByTestId("tool-hand")).toHaveCount(1); await expect(page.getByTestId("class-region")).toHaveCount(0); await expect(page.getByTestId("save-and-next")).toHaveCount(0); @@ -605,7 +608,8 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa await page.getByTestId("prev-asset").click(); await expect(page.getByTestId("asset-position")).toHaveText("2/3"); await expect(page.getByTestId("readonly-banner")).toContainText(/this job is finished/i); - await expect(page.getByTestId("tool-palette")).toHaveCount(0); + await expect(page.getByTestId("tool-select")).toHaveCount(0); + await expect(page.getByTestId("tool-hand")).toHaveCount(1); }); await test.step("complete the batch", async () => { @@ -635,7 +639,8 @@ test("the whole cycle, from opening the app to a downloaded export", async ({ pa await expect(page.getByTestId("readonly-banner")).toContainText(/viewing only/i); await expect(page.getByTestId("banner-create-correction")).toBeVisible(); - await expect(page.getByTestId("tool-palette")).toHaveCount(0); + await expect(page.getByTestId("tool-select")).toHaveCount(0); + await expect(page.getByTestId("tool-hand")).toHaveCount(1); // The classes region leaves the viewer entirely — and the add-a-class doors // go with it rather than being disabled. await expect(page.getByTestId("class-region")).toHaveCount(0); diff --git a/frontend/app/e2e/_frame.ts b/frontend/app/e2e/_frame.ts index 1a2afaf5..0bdf4489 100644 --- a/frontend/app/e2e/_frame.ts +++ b/frontend/app/e2e/_frame.ts @@ -297,3 +297,31 @@ export async function expectProgress(page: Page, progress: string): Promise { await page.getByTestId("more-actions").click(); } + +/** + * A zoom notch over `at`, which is a wheel **with the modifier held**. + * + * A bare wheel pans now (#576), and every scenario that used to zoom with one is + * routed through here rather than holding the key inline — a spec that forgot it + * would still pass its "the picture moved" assertions and be measuring the wrong + * gesture entirely. + * + * `Control` and not `Meta`: both work in the product, and Playwright's + * `mouse.wheel` reads the keyboard's live modifier state, so the down/up pair is + * what puts `ctrlKey` on the event. The cursor is moved first because a + * `mouse.wheel` lands wherever the last press left the pointer, which after a + * button click is over the chrome and not the canvas. + */ +export async function zoomWheel(page: Page, at: Point, delta: number): Promise { + await page.mouse.move(at.x, at.y); + await page.keyboard.down("Control"); + await page.mouse.wheel(0, delta); + await page.keyboard.up("Control"); +} + +/** The pane's centre, which is where a scenario zooms when it does not care where. */ +export async function paneCentre(page: Page): Promise { + const box = await page.getByTestId("annotator-pane").boundingBox(); + if (box === null) throw new Error("annotator-pane has no bounding box"); + return { x: box.x + box.width / 2, y: box.y + box.height / 2 }; +} diff --git a/frontend/app/e2e/annotate.spec.ts b/frontend/app/e2e/annotate.spec.ts index 7524cc08..2aafe2ea 100644 --- a/frontend/app/e2e/annotate.spec.ts +++ b/frontend/app/e2e/annotate.spec.ts @@ -13,7 +13,7 @@ import { expect, test, type Page, type Request } from "@playwright/test"; import { assetActions, batchActions, jobActions } from "./_wire"; -import { expectNothingToSave, expectProgress, openOverflow, saveNow } from "./_frame"; +import { expectNothingToSave, expectProgress, openOverflow, saveNow, zoomWheel } from "./_frame"; const PROJECT = "11111111-1111-4111-8111-111111111111"; const BATCH = "22222222-2222-4222-8222-222222222222"; @@ -1077,8 +1077,9 @@ test("the zoom stops at 8x, and the control says so rather than going quiet", as const wheelOverCanvas = async (delta: number): Promise => { const box = await page.getByTestId("annotator-root").boundingBox(); if (box === null) throw new Error("annotator-root has no bounding box"); - await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); - await page.mouse.wheel(0, delta); + // Held, because a bare wheel pans (#576). Without it this scenario would + // still move the picture and would assert nothing at all about the zoom. + await zoomWheel(page, { x: box.x + box.width / 2, y: box.y + box.height / 2 }, delta); }; await wheelOverCanvas(-4000); @@ -1336,15 +1337,23 @@ test("a completed batch opens as a viewer, and says so", async ({ page }) => { await expect(banner).toContainText(/viewing only/i); await expect(banner).toContainText(/correction batch/i); - // Every control that writes is out, and the palette is gone entirely — a tool - // palette over a canvas that cannot be drawn on explains nothing. `skip` is - // absent rather than disabled: the pair keeps its slot inside a - // working job and loses it once the job is closed, which a completed batch's - // is. + // Every control that writes is out. `skip` is absent rather than disabled: the + // pair keeps its slot inside a working job and loses it once the job is + // closed, which a completed batch's is. await expectNothingToSave(page); await expect(page.getByTestId("skip")).toHaveCount(0); await expect(page.getByTestId("accept")).toHaveCount(0); - await expect(page.getByTestId("tool-palette")).toHaveCount(0); + + // The strip is present and carries navigation only (#576). It used to be + // absent entirely, and the reason was sound while every control on it picked a + // drawing tool — but navigating a batch nobody may edit is most of what a + // viewer does, and on a trackpad the hand is the only way to do it. + await expect(page.getByTestId("tool-palette")).toHaveCount(1); + await expect(page.getByTestId("tool-hand")).toHaveCount(1); + await expect(page.getByTestId("tool-help")).toHaveCount(1); + for (const drawing of ["tool-select", "tool-bbox", "tool-polygon", "tool-add-class", "tool-undo"]) { + await expect(page.getByTestId(drawing)).toHaveCount(0); + } }); test("a completed batch's canvas cannot be drawn on, however hard it is asked", async ({ page }) => { @@ -1576,8 +1585,11 @@ test("finishing the job turns the workspace into a viewer in place, on every fra // Visible success, in the vocabulary the add-a-class chain already uses. await expect(page.getByText(/job finished/i).first()).toBeVisible(); - // Everything that only ever performed an edit is **absent**, not disabled. - await expect(page.getByTestId("tool-palette")).toHaveCount(0); + // Everything that only ever performed an edit is **absent**, not disabled — + // the strip included, down to the one button on it that is navigation (#576). + await expect(page.getByTestId("tool-select")).toHaveCount(0); + await expect(page.getByTestId("tool-undo")).toHaveCount(0); + await expect(page.getByTestId("tool-hand")).toHaveCount(1); await expect(page.getByTestId("class-region")).toHaveCount(0); await expect(page.getByTestId("panel-split")).toHaveCount(0); await expect(page.getByTestId("skip")).toHaveCount(0); @@ -1608,7 +1620,8 @@ test("finishing the job turns the workspace into a viewer in place, on every fra await page.getByTestId("prev-asset").click(); await expect(page.getByTestId("asset-position")).toHaveText("1/2"); await expect(page.getByTestId("readonly-banner")).toBeVisible(); - await expect(page.getByTestId("tool-palette")).toHaveCount(0); + await expect(page.getByTestId("tool-select")).toHaveCount(0); + await expect(page.getByTestId("tool-hand")).toHaveCount(1); await expect(page.getByTestId("class-region")).toHaveCount(0); // The other half: the gallery still opens, and no save-first guard engages — @@ -2052,6 +2065,66 @@ test("the sheet lists the engine's own bindings, and the schema's class hotkeys" await expect(sheet.getByTestId("shortcut-text-fields")).toContainText( /typing in a field they are the browser/i, ); + + // `h` arrives as an ordinary derived row, which is the whole claim the sheet + // makes about itself: a binding was added and nobody edited this component. + await expect(sheet.locator('[data-chord="h"]')).toContainText(/hand/i); + + // The gestures are the half that cannot be derived — a two-finger scroll has + // no chord to be read off — so they are written, and this is what says they + // are on the sheet at all (#576). + await expect(sheet.getByTestId("shortcut-pan-rows")).toContainText(/two-finger scroll/i); + await expect(sheet.getByTestId("shortcut-pan-rows")).toContainText(/hold space/i); + await expect(sheet.getByTestId("shortcut-zoom-rows")).toContainText(/pinch/i); + await expect(sheet.getByTestId("shortcut-touch-rows")).toContainText(/two fingers/i); +}); + +/** + * The hand, both doors, and the proof that they are one state. + * + * `h` and the strip's button reach the same `handTool` on the page — the suggest + * tool's arrangement — so pressing one must light the other. A scenario driving + * only the button would pass with the chord unbound, which is exactly the half a + * trackpad user reaches for first. + */ +test("the hand turns a plain drag into a pan, from the key and from the button", async ({ + page, +}) => { + const sent: Request[] = []; + await openJob(page, sent); + + const canvas = page.getByTestId("annotator-canvas"); + const pane = (await page.getByTestId("annotator-pane").boundingBox())!; + const button = page.getByTestId("tool-hand"); + await expect(button).toHaveAttribute("data-active", "false"); + + await page.getByTestId("annotator-root").focus(); + await page.keyboard.press("h"); + await expect(button).toHaveAttribute("data-active", "true"); + + const before = (await canvas.boundingBox())!; + const from = { x: pane.x + pane.width * 0.5, y: pane.y + pane.height * 0.5 }; + await page.mouse.move(from.x, from.y); + await page.mouse.down(); + await page.mouse.move(from.x - 140, from.y - 60, { steps: 8 }); + await page.mouse.up(); + + await expect.poll(async () => Math.round((await canvas.boundingBox())!.x)).toBe( + Math.round(before.x - 140), + ); + // A pan is not an edit: the drag drew nothing and there is nothing to save. + await expectNothingToSave(page); + + // The button turns it back off, and the same drag draws again. + await button.click(); + await expect(button).toHaveAttribute("data-active", "false"); + await page.keyboard.press("1"); + const draw = { x: pane.x + pane.width * 0.4, y: pane.y + pane.height * 0.4 }; + await page.mouse.move(draw.x, draw.y); + await page.mouse.down(); + await page.mouse.move(draw.x + 90, draw.y + 70, { steps: 6 }); + await page.mouse.up(); + await expect(page.getByTestId("object-total")).toContainText("1 object"); }); /** @@ -2960,8 +3033,7 @@ test("saving leaves the viewport exactly where it was", async ({ page }) => { // Off the fitted view in both dimensions: a wheel notch over a point that is // not the pane's centre changes the zoom *and* the pan, and the secondary drag // after it moves the pan again on its own. - await page.mouse.move(pane.x + pane.width * 0.35, pane.y + pane.height * 0.35); - await page.mouse.wheel(0, -600); + await zoomWheel(page, { x: pane.x + pane.width * 0.35, y: pane.y + pane.height * 0.35 }, -600); await page.mouse.down({ button: "right" }); await page.mouse.move(pane.x + pane.width * 0.55, pane.y + pane.height * 0.5, { steps: 8 }); await page.mouse.up({ button: "right" }); diff --git a/frontend/app/e2e/demo.spec.ts b/frontend/app/e2e/demo.spec.ts index d800c8ca..be4d8588 100644 --- a/frontend/app/e2e/demo.spec.ts +++ b/frontend/app/e2e/demo.spec.ts @@ -23,7 +23,15 @@ import { expect, test } from "@playwright/test"; -import { canvasOrigin, drawBbox, expectCounts, focusCanvas, frameOf, SHOWCASE } from "./_frame"; +import { + canvasOrigin, + drawBbox, + expectCounts, + focusCanvas, + frameOf, + SHOWCASE, + zoomWheel, +} from "./_frame"; test.beforeEach(async ({ page }) => { await page.goto(SHOWCASE); @@ -107,8 +115,7 @@ test("the wheel zooms the stage, and mod+0 puts it back", async ({ page }) => { const frame = await frameOf(page); const fitted = frame.zoom; - await page.mouse.move(frame.at(640, 360).x, frame.at(640, 360).y); - await page.mouse.wheel(0, -240); + await zoomWheel(page, frame.at(640, 360), -240); const zoomed = await frameOf(page); expect(zoomed.zoom).toBeGreaterThan(fitted); @@ -147,3 +154,79 @@ test("mod+0 recentres a panned view, not just its scale", async ({ page }) => { ); expect(Math.round((await canvasOrigin(page)).y)).toBe(Math.round(origin.y)); }); + +/** + * A bare wheel pans, and it is the change that gives a trackpad a pan at all. + * + * Before this, every wheel event zoomed — so a two-finger scroll, which is how + * anybody moves around a canvas, zoomed instead of scrolling and there was no + * gesture on a trackpad that moved the picture. The modifier is what still + * zooms, and the scenario above asserts that half. + * + * Both assertions matter and neither alone would do: a zoom also moves the + * ``'s origin, so "it moved" is satisfied by the old behaviour. The zoom + * being *unchanged* is the half that says this was a pan. + */ +test("a bare wheel pans the stage and leaves the zoom alone", async ({ page }) => { + const frame = await frameOf(page); + const origin = await canvasOrigin(page); + + const at = frame.at(640, 360); + await page.mouse.move(at.x, at.y); + await page.mouse.wheel(0, 120); + + await expect.poll(async () => Math.round((await canvasOrigin(page)).y)).toBe( + Math.round(origin.y - 120), + ); + expect((await frameOf(page)).zoom).toBeCloseTo(frame.zoom, 3); +}); + +/** `deltaX` too: a trackpad scrolls sideways, and a pan that ignored it would be half a pan. */ +test("a bare wheel pans sideways as well", async ({ page }) => { + const frame = await frameOf(page); + const origin = await canvasOrigin(page); + + const at = frame.at(640, 360); + await page.mouse.move(at.x, at.y); + await page.mouse.wheel(-90, 0); + + await expect.poll(async () => Math.round((await canvasOrigin(page)).x)).toBe( + Math.round(origin.x + 90), + ); + expect(Math.round((await canvasOrigin(page)).y)).toBe(Math.round(origin.y)); +}); + +/** + * `Space` held is the hand, and it is the spelling that needs no host at all. + * + * It cannot be a registry row — a keystroke is a press and this is a hold — so it + * is an adapter substitution, and the release is as much a part of it as the + * press. The second drag is what proves the release: without it a scenario + * asserting only that the pan happened would pass with the mode stuck on + * forever, which is the failure a held key actually has. + */ +test("holding space turns a primary drag into a pan, and letting go gives it back", async ({ + page, +}) => { + const frame = await frameOf(page); + await focusCanvas(page); + const origin = await canvasOrigin(page); + + const from = frame.at(640, 360); + await page.keyboard.down(" "); + await page.mouse.move(from.x, from.y); + await page.mouse.down(); + await page.mouse.move(from.x - 120, from.y, { steps: 8 }); + await page.mouse.up(); + await page.keyboard.up(" "); + + await expect.poll(async () => Math.round((await canvasOrigin(page)).x)).toBe( + Math.round(origin.x - 120), + ); + // Nothing was drawn: the press never reached the machine. + await expectCounts(page, 0, 0); + + // And with the key up, the same drag draws again — the mode was transient. + await drawBbox(page, frame, { x: 100, y: 100 }, { x: 300, y: 260 }); + await expectCounts(page, 1, 1); +}); diff --git a/frontend/app/e2e/perf.spec.ts b/frontend/app/e2e/perf.spec.ts index ba9e6412..90f6c2b8 100644 --- a/frontend/app/e2e/perf.spec.ts +++ b/frontend/app/e2e/perf.spec.ts @@ -62,7 +62,7 @@ import { layerCounts, watchLayers, } from "./_bench"; -import { expectCounts, focusCanvas, frameOf } from "./_frame"; +import { expectCounts, focusCanvas, frameOf, zoomWheel } from "./_frame"; /** * The box every drag scenario grabs — column 3, row 1. @@ -204,9 +204,11 @@ test("one wheel notch writes the stage and touches no annotation at all", async const frame = await frameOf(page, BENCH_ASSET); const at = frame.at(1920, 1080); + // Onto the target first, so the counter starts with the pointer already there + // — an *approach* to a shape flips a `fill-opacity` and would be counted. await page.mouse.move(at.x, at.y); await watchLayers(page); - await page.mouse.wheel(0, -120); + await zoomWheel(page, at, -120); // Poll the **stage**, which is where a zoom now writes. A wheel event is not a // discrete React event, so the commit lands on a later task than the dispatch. @@ -261,8 +263,7 @@ test("a zoom still leaves a stroke two screen pixels wide, which is what it was expect(await strokeAtThisZoom()).toBeCloseTo(2, 1); const at = frame.at(1920, 1080); - await page.mouse.move(at.x, at.y); - await page.mouse.wheel(0, -600); + await zoomWheel(page, at, -600); await expect .poll(async () => (await page.getByTestId("annotator-canvas").boundingBox())?.width ?? 0) .toBeGreaterThan(frame.zoom * BENCH_ASSET.width * 1.5); diff --git a/frontend/app/e2e/showcase.spec.ts b/frontend/app/e2e/showcase.spec.ts index 69c7aa18..5714d2f7 100644 --- a/frontend/app/e2e/showcase.spec.ts +++ b/frontend/app/e2e/showcase.spec.ts @@ -18,7 +18,14 @@ import { expect, test, type Page } from "@playwright/test"; -import { expectCounts, expectFitted, focusCanvas, frameOf, SHOWCASE } from "./_frame"; +import { + expectCounts, + expectFitted, + focusCanvas, + frameOf, + SHOWCASE, + zoomWheel, +} from "./_frame"; test.beforeEach(async ({ page }) => { await page.goto(SHOWCASE); @@ -120,8 +127,7 @@ test("the zoom readout reports the fit, follows the wheel and comes back on mod+ expectFitted(frame); await expectReadout(page, frame.zoom); - await page.mouse.move(frame.at(640, 360).x, frame.at(640, 360).y); - await page.mouse.wheel(0, -240); + await zoomWheel(page, frame.at(640, 360), -240); const zoomed = await frameOf(page); expect(zoomed.zoom).toBeGreaterThan(frame.zoom); diff --git a/frontend/app/e2e/touch.spec.ts b/frontend/app/e2e/touch.spec.ts new file mode 100644 index 00000000..e16495e7 --- /dev/null +++ b/frontend/app/e2e/touch.spec.ts @@ -0,0 +1,263 @@ +/** + * Two fingers on the glass: pinch to zoom and drag to move, at the same time. + * + * ## Why this is its own file, and why it reaches for CDP + * + * Playwright's input API has one touch verb, `page.touchscreen.tap`, and one + * contact. A pinch needs two, and the only thing in the stack that can put two + * fingers down is the protocol underneath — `Input.dispatchTouchEvent`, which + * Chromium turns into real `pointerdown`/`pointermove`/`pointerup` carrying + * `pointerType: "touch"`, exactly as a touchscreen would. The events the adapter + * receives here are the browser's own, not a page-side forgery: nothing in this + * file constructs a DOM event or calls the method that would dispatch one, which + * `tests/scripts/annotator_boundary.test.mjs` forbids across all of `frontend/` + * and which would make the suite prove nothing about the adapter anyway. + * + * A file of its own because `hasTouch` is a **context** option: `test.use` here + * would emulate touch for every scenario in whatever file it sat in, and the + * other specs are about a mouse. + * + * ## On `touchEnd`, `touchPoints` is the set that LEFT + * + * The protocol's wording — "active touch points on the touch device" — reads the + * other way, and following it produces a suite that passes and measures nothing. + * **Measured rather than read**: with two fingers down, `touchEnd` naming the + * one that stayed leaves the *other* one active, so a scenario that then moves + * the survivor is moving a contact the browser thinks is gone. It does not + * error. It just quietly does nothing, and every "nothing moved" assertion + * passes for the wrong reason. + * + * The proof is `lifting one finger and putting another down continues the + * pinch`: it is the one scenario here whose expected outcome is a *change*, so + * it is the one the convention can be wrong about visibly. It failed under the + * other reading and passes under this one. + * + * ## The honest limit + * + * Chromium's touch emulation is close to what a real finger produces and is not + * the same thing: no contact area, no palm rejection, none of the jitter a hand + * actually has. What these scenarios pin is the arithmetic and the bookkeeping — + * the count that decides a gesture, the scale, the centroid, and the exit — + * which is the part that can be wrong in a way nobody notices. + */ + +import { expect, test, type CDPSession, type Page } from "@playwright/test"; + +import { canvasOrigin, frameOf, SHOWCASE, type Point } from "./_frame"; + +test.use({ hasTouch: true }); + +/** One contact, in the shape CDP wants. */ +interface Contact extends Point { + readonly id: number; +} + +/** Every contact a `pair` put down — `touchEnd`'s spelling for "hand off the glass". */ +function both(centre: Point, spread: number, dx = 0, dy = 0): readonly Contact[] { + return pair(centre, spread, dx, dy); +} + +/** + * A touch session over the page, held for the scenario. + * + * The session is the thing that has to be reused: each dispatch is a frame of + * one continuous gesture and Chromium tracks the contacts between them, so a + * fresh session per call would be four handshakes per pinch. + */ +async function touching(page: Page): Promise { + return await page.context().newCDPSession(page); +} + +async function touch( + client: CDPSession, + type: "touchStart" | "touchMove" | "touchEnd", + points: readonly Contact[], +): Promise { + await client.send("Input.dispatchTouchEvent", { + type, + touchPoints: points.map((point) => ({ x: point.x, y: point.y, id: point.id })), + }); +} + +/** Two fingers `spread` apart either side of `centre`, offset by `dx`/`dy`. */ +function pair(centre: Point, spread: number, dx = 0, dy = 0): readonly Contact[] { + return [ + { id: 1, x: centre.x - spread + dx, y: centre.y + dy }, + { id: 2, x: centre.x + spread + dx, y: centre.y + dy }, + ]; +} + +/** The asset pixel currently under a screen position. */ +async function assetPixelAt(page: Page, at: Point): Promise { + const frame = await frameOf(page); + const origin = await canvasOrigin(page); + return { x: (at.x - origin.x) / frame.zoom, y: (at.y - origin.y) / frame.zoom }; +} + +/** Where an asset pixel currently sits on screen. The inverse, for the same reason. */ +async function screenPositionOf(page: Page, pixel: Point): Promise { + const frame = await frameOf(page); + const origin = await canvasOrigin(page); + return { x: origin.x + pixel.x * frame.zoom, y: origin.y + pixel.y * frame.zoom }; +} + +test.beforeEach(async ({ page }) => { + await page.goto(SHOWCASE); +}); + +test("two fingers moving apart zoom the stage", async ({ page }) => { + const frame = await frameOf(page); + const client = await touching(page); + const centre = frame.at(640, 360); + + await touch(client, "touchStart", pair(centre, 60)); + await touch(client, "touchMove", pair(centre, 120)); + await touch(client, "touchEnd", both(centre, 120)); + + await expect.poll(async () => (await frameOf(page)).zoom).toBeGreaterThan(frame.zoom * 1.8); +}); + +test("two fingers moving together zoom out", async ({ page }) => { + const frame = await frameOf(page); + const client = await touching(page); + const centre = frame.at(640, 360); + + await touch(client, "touchStart", pair(centre, 160)); + await touch(client, "touchMove", pair(centre, 80)); + await touch(client, "touchEnd", both(centre, 80)); + + await expect.poll(async () => (await frameOf(page)).zoom).toBeLessThan(frame.zoom * 0.7); +}); + +/** + * A pinch that also drifts is **one** gesture, and this is the half that a + * separate zoom handler and pan handler would get wrong. + * + * The fingers keep their distance and travel together, so the scale is exactly 1 + * and the whole of the movement is the centroid's. Anything reading only the + * distance between them would answer "nothing happened". + */ +test("two fingers travelling together pan without zooming", async ({ page }) => { + const frame = await frameOf(page); + const origin = await canvasOrigin(page); + const client = await touching(page); + const centre = frame.at(640, 360); + + await touch(client, "touchStart", pair(centre, 100)); + await touch(client, "touchMove", pair(centre, 100, -130, 70)); + await touch(client, "touchEnd", both(centre, 100, -130, 70)); + + await expect + .poll(async () => Math.round((await canvasOrigin(page)).x)) + .toBe(Math.round(origin.x - 130)); + expect(Math.round((await canvasOrigin(page)).y)).toBe(Math.round(origin.y + 70)); + expect((await frameOf(page)).zoom).toBeCloseTo(frame.zoom, 3); +}); + +/** + * Whatever was between the fingers is still between the fingers. + * + * The invariant `pinchBetween` exists for, asserted where it is actually + * reachable: the asset pixel under the midpoint is read before and after, and a + * zoom that scaled about the wrong point moves it. Two screen pixels of slack, + * on `COORDINATE_SLACK`'s reasoning — the contacts are integers and the frame is + * measured rather than assumed. + */ +test("a pinch scales about the point between the fingers", async ({ page }) => { + const before = await frameOf(page); + const client = await touching(page); + const centre = before.at(640, 360); + const held = await assetPixelAt(page, centre); + + await touch(client, "touchStart", pair(centre, 70)); + await touch(client, "touchMove", pair(centre, 150)); + await touch(client, "touchEnd", both(centre, 150)); + + await expect.poll(async () => (await frameOf(page)).zoom).toBeGreaterThan(before.zoom * 1.5); + + const nowAt = await screenPositionOf(page, held); + expect(Math.abs(nowAt.x - centre.x)).toBeLessThanOrEqual(2); + expect(Math.abs(nowAt.y - centre.y)).toBeLessThanOrEqual(2); +}); + +/** + * Lifting one finger ends the pinch, and the survivor moves nothing. + * + * Fingers never leave a screen together, so every pinch ends with one still + * down, and if that one kept driving the gesture the picture would lurch on the + * way out of every pinch anybody makes. + * + * The survivor needing no special handling was **measured, not assumed**: + * `IDLE_ROW` carries a `pointer-down` handler and nothing else, so its stray + * moves and its stray lift reach an idle machine and are silence. That is why + * this scenario asserts the viewport rather than the swallowing — the swallowing + * has no observable consequence, and a test for it would be a test of a + * mechanism instead of a behaviour. + */ +test("lifting one finger ends the pinch, and the other one does nothing", async ({ page }) => { + const frame = await frameOf(page); + const client = await touching(page); + const centre = frame.at(640, 360); + + await touch(client, "touchStart", pair(centre, 70)); + await touch(client, "touchMove", pair(centre, 140)); + await expect.poll(async () => (await frameOf(page)).zoom).toBeGreaterThan(frame.zoom * 1.5); + + // One finger leaves. The list is what *left*, so the other one is still down. + const leaving = pair(centre, 140)[0]!; + const survivor = pair(centre, 140)[1]!; + await touch(client, "touchEnd", [leaving]); + const settled = await canvasOrigin(page); + const zoomed = (await frameOf(page)).zoom; + + // The survivor travels a long way. Nothing may move. + const travelled = { ...survivor, x: centre.x - 300, y: centre.y - 200 }; + await touch(client, "touchMove", [travelled]); + await touch(client, "touchEnd", [travelled]); + + const after = await canvasOrigin(page); + expect(Math.round(after.x)).toBe(Math.round(settled.x)); + expect(Math.round(after.y)).toBe(Math.round(settled.y)); + expect((await frameOf(page)).zoom).toBeCloseTo(zoomed, 3); + // And it drew nothing on the way out. + await expect(page.getByTestId("counts")).toHaveText("0 annotation(s), 0 selected"); +}); + +/** + * A finger swap continues the pinch, and this is the rule that decides how long + * a gesture lives. + * + * Two fingers is the whole condition — below that there is no gesture, and when + * a second contact arrives again there is one. **Mutation-verified**: clearing + * only when the *last* finger lifts, which is the obvious reading of "the + * gesture outlives its contacts", reddens this and nothing else. It also feels + * exactly like the bug it is: lift one finger and put it back, and the pinch is + * dead until you take your whole hand off the glass. + * + * Re-forming is jump-free because `beginGesture` reads the contacts' *current* + * positions, so the first frame after the swap compares a pair with itself. + */ +test("lifting one finger and putting another down continues the pinch", async ({ page }) => { + const frame = await frameOf(page); + const client = await touching(page); + const centre = frame.at(640, 360); + + await touch(client, "touchStart", pair(centre, 70)); + await touch(client, "touchMove", pair(centre, 130)); + await expect.poll(async () => (await frameOf(page)).zoom).toBeGreaterThan(frame.zoom * 1.4); + const afterFirst = (await frameOf(page)).zoom; + + // Finger 1 comes off; finger 2 stays exactly where it was. + const leaving = pair(centre, 130)[0]!; + const staying = pair(centre, 130)[1]!; + await touch(client, "touchEnd", [leaving]); + + // A different finger arrives, and the two of them pinch further apart. + const rejoined = { id: 3, x: centre.x - 130, y: centre.y }; + await touch(client, "touchStart", [staying, rejoined]); + const spreadFurther = { ...rejoined, x: centre.x - 260 }; + await touch(client, "touchMove", [staying, spreadFurther]); + + await expect.poll(async () => (await frameOf(page)).zoom).toBeGreaterThan(afterFirst * 1.2); + await touch(client, "touchEnd", [staying, spreadFurther]); +}); diff --git a/frontend/ui-core/src/annotator/AnnotationPage.tsx b/frontend/ui-core/src/annotator/AnnotationPage.tsx index c02c34a8..99646233 100644 --- a/frontend/ui-core/src/annotator/AnnotationPage.tsx +++ b/frontend/ui-core/src/annotator/AnnotationPage.tsx @@ -85,6 +85,7 @@ import { SAVE_AND_NEXT, SKIP_FRAME, TOGGLE_HELP, + TOGGLE_HAND, TOGGLE_SUGGEST, acceptedAnnotations, addAnnotationsCommand, @@ -782,6 +783,16 @@ function Workspace({ const [hiddenIds, setHiddenIds] = useState>(() => new Set()); const [view, setView] = useState(null); const [helpOpen, setHelpOpen] = useState(false); + /** + * The hand, held here rather than in the canvas so the palette can light it. + * + * The suggest tool's arrangement exactly: a mode the engine honours and the + * host owns, with `h` reaching it through `hostAction` and the strip's button + * reaching the same state. It survives `readOnly` — see `hostAction` — and it + * is deliberately **not** reset between frames: somebody navigating a batch + * with the hand on is navigating the batch, not this asset. + */ + const [handTool, setHandTool] = useState(false); const [galleryOpen, setGalleryOpen] = useState(false); /** * Which shape's class picker is open, if any. @@ -1124,6 +1135,14 @@ function Workspace({ toggleSuggest(); return true; } + // `h`. Claimed in every mode, unlike every other chord in this function: + // navigating a frame is the one thing a viewer does most of, and a hand that + // worked only while a batch was open would be a control that disappears + // exactly when it is most of what is left. + if (name === TOGGLE_HAND) { + setHandTool((on) => !on); + return true; + } // `↵` and `Esc`, substituted by the adapter only while a session is live, so // neither reaches here unless there is something to accept or take back. if (name === ACCEPT_SUGGESTION) { @@ -2408,6 +2427,7 @@ function Workspace({ // click meant for the model from drawing a box instead — so it is // `diverting`, which drops a parked session, and not the // whole of `suggesting`. + panTool={handTool} suggestion={diverting} onSuggestPoint={suggestAt} // The halo and the busy cursor. Keyed to `diverting` for the same @@ -2442,53 +2462,57 @@ function Workspace({ step. */} {/* - Fully hidden rather than disabled, which is the one place this page - departs from disabled-with-reason — every control on the palette picks - a *drawing* tool, and a tool palette over a canvas that cannot be drawn - on is not an explanation of anything. The banner above carries the - reason, once. + A viewer gets the strip, carrying the hand and the shortcut sheet and + nothing else. It used to get no strip at all, and the reason was sound + while it held: every control on it picked a *drawing* tool, and a tool + palette over a canvas that cannot be drawn on is not an explanation of + anything. The hand is not about drawing — it is what a person reaches + for when the picture is in the wrong place — so the sentence stopped + being true and the exception with it. The drawing half is still fully + hidden rather than disabled, and the banner above still carries that + reason once. */} - {!readOnly && ( - setHelpOpen((open) => !open)} - // Empty, unlike the class field's create row: `+` means "I want a - // class", not a particular one, and carrying the previous - // opening's name into it would be a prefill nobody asked for. - onAddClass={() => { - setNewClassName(""); - setAddingClass(true); - }} - // The chords work whether or not the page draws them; this is where - // it says so. `canUndo`/`canRedo` come off the snapshot, so the - // buttons and the keyboard read one command log. - history={{ - canUndo: snapshot.canUndo, - canRedo: snapshot.canRedo, - onUndo: () => store.undo(), - onRedo: () => store.redo(), - }} - // The strip hides it on a schema no class of which could - // hold the answer; this page offers it because it has an API - // behind it, which the showcase does not. - // - // `unavailable` is the parked reading: the schema can - // suggest, so the button is present, but the class the workspace is - // sitting on cannot hold one — which is a fact to state rather than - // a control that quietly stops working. Lit *and* dimmed, because - // both halves are true: the tool is still armed, and it cannot act. - suggest={{ - active: suggesting !== null, - onToggle: toggleSuggest, - unavailable: - suggesting !== null && isParked(suggesting) - ? `Suggest is on, but “${activeClass ?? ""}” cannot hold a suggested shape` - : null, - }} + setHandTool((on) => !on) }} + schema={store.document.schema} + tool={toolFor(store.document, activeClass)} + onActivateClass={activateClass} + onToggleHelp={() => setHelpOpen((open) => !open)} + // Empty, unlike the class field's create row: `+` means "I want a + // class", not a particular one, and carrying the previous + // opening's name into it would be a prefill nobody asked for. + onAddClass={() => { + setNewClassName(""); + setAddingClass(true); + }} + // The chords work whether or not the page draws them; this is where + // it says so. `canUndo`/`canRedo` come off the snapshot, so the + // buttons and the keyboard read one command log. + history={{ + canUndo: snapshot.canUndo, + canRedo: snapshot.canRedo, + onUndo: () => store.undo(), + onRedo: () => store.redo(), + }} + // The strip hides it on a schema no class of which could + // hold the answer; this page offers it because it has an API + // behind it, which the showcase does not. + // + // `unavailable` is the parked reading: the schema can + // suggest, so the button is present, but the class the workspace is + // sitting on cannot hold one — which is a fact to state rather than + // a control that quietly stops working. Lit *and* dimmed, because + // both halves are true: the tool is still armed, and it cannot act. + suggest={{ + active: suggesting !== null, + onToggle: toggleSuggest, + unavailable: + suggesting !== null && isParked(suggesting) + ? `Suggest is on, but “${activeClass ?? ""}” cannot hold a suggested shape` + : null, + }} /> - )} {/* Everything the editor floats over the picture, in one column. diff --git a/frontend/ui-core/src/annotator/ShortcutSheet.tsx b/frontend/ui-core/src/annotator/ShortcutSheet.tsx index 9611e918..b94f21a8 100644 --- a/frontend/ui-core/src/annotator/ShortcutSheet.tsx +++ b/frontend/ui-core/src/annotator/ShortcutSheet.tsx @@ -1,5 +1,20 @@ /** - * The keyboard shortcuts, read off the live registry. + * The shortcuts, read off the live registry, and the gestures, which cannot be. + * + * ## Half of this sheet is derived and half is written, and the split is not a + * compromise + * + * The keyboard half is the registry and nothing else — see below. The + * **Navigate** half is hand-written, and it has to be: a two-finger scroll, a + * pinch and a middle-drag have no chord, so there is no row anywhere to read + * them off. `bindings.ts` holds keystrokes; `AnnotatorCanvas`'s wheel listener + * and pointer handlers hold these, as branches rather than as data. + * + * That makes the written half the thing that can drift, so it is kept small and + * kept about *gestures only*. Anything with a chord — `h`, `mod+0` — appears in + * the derived rows above and is not repeated here, however tempting: a gesture + * list that also listed keys would be the hand-written table this file's whole + * argument is against, reintroduced one row at a time. * * ## The list is derived, never retyped * @@ -44,6 +59,7 @@ import { SAVE, SAVE_AND_NEXT, SKIP_FRAME, + TOGGLE_HAND, TOGGLE_HELP, type Action, type ActionKind, @@ -98,6 +114,7 @@ function hostPhrase(name: string): string { if (name === SAVE) return "Save now, and stay on this frame"; if (name === SAVE_AND_NEXT) return "Save and go to the next frame"; if (name === SKIP_FRAME) return "Skip this frame and go to the next"; + if (name === TOGGLE_HAND) return "Turn the hand on or off — with it on, any drag pans"; return name; } @@ -132,6 +149,53 @@ function capitalize(part: string): string { return part.charAt(0).toUpperCase() + part.slice(1); } +/** One gesture, and what it does. The left half is prose, not a chord. */ +interface GestureRow { + readonly gesture: string; + readonly means: string; +} + +/** + * How to move the picture, organised by what a person wants rather than by what + * they are holding. + * + * The length of the list is the point of it: a trackpad has no second mouse + * button and a pen has none either, so until this each of them had no pan at + * all. The middle-and-right drag is the row that always worked, and it is last + * rather than first because it is the one fewest readers can use. + */ +function panning(mod: string): readonly GestureRow[] { + return [ + { gesture: "Two-finger scroll", means: "Trackpad. Moves in both directions" }, + { gesture: "Scroll wheel", means: "Moves up and down" }, + // `Space` has no row above and cannot have one — a keystroke is a press, and + // this is a hold. The hand's *other* spelling, `h`, is in the derived rows + // and is deliberately not repeated here. + { gesture: "Hold Space and drag", means: "The hand, for as long as the key is down" }, + { gesture: "Middle-drag or right-drag", means: "Works whatever tool is active" }, + // The one thing about this model somebody has to be told outright, rather + // than being left to discover that a modifier changes what a scroll means. + { gesture: `${mod} is what changes it`, means: "Held, the same scroll zooms instead" }, + ]; +} + +function zooming(mod: string): readonly GestureRow[] { + return [ + { gesture: "Pinch", means: "Trackpad. Zooms about the pointer" }, + { gesture: `${mod} and scroll`, means: "The same, with a mouse" }, + // The buttons and the readout, which no chord reaches. Fitting does have one + // — `mod+0` — so the fit button is left to the derived row that names it. + { gesture: "The − and + buttons", means: "Bottom right of the picture. 5% to 800%" }, + ]; +} + +function touching(): readonly GestureRow[] { + return [ + { gesture: "One finger", means: "Draws, or pans while the hand is on" }, + { gesture: "Two fingers", means: "Pinch and drag together, about the point between them" }, + ]; +} + export interface ShortcutSheetProps { readonly open: boolean; readonly onOpenChange: (open: boolean) => void; @@ -166,9 +230,10 @@ export function ShortcutSheet({ open, onOpenChange, registry }: ShortcutSheetPro } }} > - Keyboard shortcuts + Shortcuts and gestures - Every chord the annotator claims, read from the live binding table. + Every chord the annotator claims, read from the live binding table, and the + gestures that move the picture. @@ -184,6 +249,16 @@ export function ShortcutSheet({ open, onOpenChange, registry }: ShortcutSheetPro )} +

Navigate

+

+ Nothing here is a chord, so none of it can come from the table above — a pointer + gesture has no row to be read off. Both modifiers work on every platform; the label + shows the one this machine presses. +

+ + + +

Inside a text field

{mod} + C and{" "} @@ -197,6 +272,41 @@ export function ShortcutSheet({ open, onOpenChange, registry }: ShortcutSheetPro ); } +/** + * A gesture list, shaped like `Rows` so the two read as one sheet. + * + * Deliberately a second component rather than a widened `Rows`: those rows come + * from the registry and carry an `Action`, these are written by hand and carry a + * sentence, and folding them together would make the derived half look + * hand-written — which is the exact confusion this file's docstring exists to + * prevent. + */ +function Gestures({ + testId, + caption, + rows, +}: { + readonly testId: string; + readonly caption: string; + readonly rows: readonly GestureRow[]; +}): JSX.Element { + return ( + + + + {rows.map((row) => ( + + + + + ))} + +
+ {caption} +
{row.gesture}{row.means}
+ ); +} + function Rows({ testId, rows, diff --git a/frontend/ui-core/src/annotator/ToolPalette.tsx b/frontend/ui-core/src/annotator/ToolPalette.tsx index fb093c6b..b9bbe035 100644 --- a/frontend/ui-core/src/annotator/ToolPalette.tsx +++ b/frontend/ui-core/src/annotator/ToolPalette.tsx @@ -93,6 +93,7 @@ import { } from "@visionset/annotator"; import { CircleHelp, + Hand, MousePointer2, Plus, Redo2, @@ -259,6 +260,37 @@ export interface ToolPaletteProps { readonly onUndo: () => void; readonly onRedo: () => void; }; + /** + * The hand, and it is the one button here that is **not schema-gated**. + * + * Every other control on this strip is a question about the schema: which + * geometries the declared classes can hold, whether any of them can hold a + * suggestion. The hand is a question about the *device* — it exists because a + * pan had exactly one spelling, a middle- or secondary-button drag, and a + * trackpad, a tablet and a pen have no second button to offer. No schema makes + * that more or less true, so `toolChoices` never sees it and it is never + * absent. + * + * Required rather than optional, unlike `suggest` and `history`: those are + * capabilities a host may not have behind it, and this is one every host + * already has — the canvas implements it, not the page. + */ + readonly hand: { + readonly active: boolean; + readonly onToggle: () => void; + }; + /** + * A viewer, who may navigate and may not draw. + * + * The strip used to be absent entirely in this mode, and the reason it gave + * was sound while it held: *"every control on the palette picks a drawing + * tool, and a tool palette over a canvas that cannot be drawn on is not an + * explanation of anything."* The hand is what retires it. Navigating a batch + * somebody may not edit is most of what a viewer does, so the one control that + * is not about drawing stays, with the shortcut sheet beside it, and every + * control that *is* about drawing goes. + */ + readonly readOnly?: boolean; } export function ToolPalette({ @@ -269,6 +301,8 @@ export function ToolPalette({ onAddClass, history, suggest, + hand, + readOnly = false, }: ToolPaletteProps): JSX.Element { /** * The canvas keeps the focus. @@ -288,30 +322,42 @@ export function ToolPalette({ data-testid="tool-palette" className="absolute left-3 top-3 flex w-12 flex-col items-center gap-1 rounded-xl border border-border bg-muted p-2 shadow-lg" > - {toolChoices(schema).map((choice) => ( - { - if (choice.unavailable !== null) return; - if (tool !== choice.tool) onActivateClass(choice.labelClass); - }} - > - - - ))} + {/* First, and above the tools rather than among them: it is the one + control here that does not draw, and the one a person reaches for when + the picture is in the wrong place rather than when it is wrong. */} + + + + + {!readOnly && + toolChoices(schema).map((choice) => ( + { + if (choice.unavailable !== null) return; + if (tool !== choice.tool) onActivateClass(choice.labelClass); + }} + > + + + ))} {/* After the drawing tools and before the `+`, because it is a way of drawing rather than a way of managing the schema — and a mode, so it is the one control here whose `active` is not `tool === choice.tool`. */} - {suggest !== undefined && schemaCanSuggest(schema) && ( + {!readOnly && suggest !== undefined && schemaCanSuggest(schema) && ( )} - {history !== undefined && ( + {!readOnly && history !== undefined && ( <>

{/* diff --git a/frontend/ui-core/src/annotator/toolPalette.test.tsx b/frontend/ui-core/src/annotator/toolPalette.test.tsx index 053979d2..c804f284 100644 --- a/frontend/ui-core/src/annotator/toolPalette.test.tsx +++ b/frontend/ui-core/src/annotator/toolPalette.test.tsx @@ -42,6 +42,7 @@ function mount( tool="select" onActivateClass={vi.fn()} onToggleHelp={vi.fn()} + hand={{ active: false, onToggle: vi.fn() }} {...overrides} /> @@ -347,3 +348,75 @@ describe("the suggest tool (#424)", () => { expect(screen.getByTestId("tool-suggest").getAttribute("aria-label")).toBe("Suggest (S)"); }); }); + +describe("the hand is the one button here that is not about the schema (#576)", () => { + it("is offered on a schema that declares nothing drawable at all", () => { + // Every other control on the strip answers a question about the schema. This + // one answers a question about the device — a trackpad, a pen and a finger + // have no second mouse button, which is the only spelling a pan used to have + // — so a tag-only project gets it exactly as a bbox project does. + const tagsOnly = { + ...SCHEMA, + classes: [SCHEMA.classes[3]], + } as unknown as Parameters[0]; + render(mount({ schema: tagsOnly })); + + expect(screen.getByTestId("tool-hand")).toBeTruthy(); + expect(screen.queryByTestId("tool-bbox")).toBeNull(); + expect(screen.queryByTestId("tool-polygon")).toBeNull(); + }); + + it("names its chord, and lights up when the host says it is on", () => { + const { rerender } = render(mount()); + expect(screen.getByTestId("tool-hand").getAttribute("aria-label")).toBe("Hand (H)"); + expect(screen.getByTestId("tool-hand").getAttribute("data-active")).toBe("false"); + + rerender(mount({ hand: { active: true, onToggle: vi.fn() } })); + expect(screen.getByTestId("tool-hand").getAttribute("data-active")).toBe("true"); + }); + + it("asks the host to toggle rather than holding the mode itself", () => { + const onToggle = vi.fn(); + render(mount({ hand: { active: false, onToggle } })); + + fireEvent.click(screen.getByTestId("tool-hand")); + + expect(onToggle).toHaveBeenCalledTimes(1); + }); +}); + +describe("a viewer gets the strip, carrying only what does not draw (#576)", () => { + it("keeps the hand and the shortcut sheet", () => { + render(mount({ readOnly: true })); + + expect(screen.getByTestId("tool-hand")).toBeTruthy(); + expect(screen.getByTestId("tool-help")).toBeTruthy(); + }); + + it("drops every control that draws, adds a class or steps the command log", () => { + render( + mount({ + readOnly: true, + onAddClass: vi.fn(), + suggest: { active: false, onToggle: vi.fn(), unavailable: null }, + history: { canUndo: true, canRedo: true, onUndo: vi.fn(), onRedo: vi.fn() }, + }), + ); + + // Every one of these is offered by the same mount without `readOnly`, which + // is what the tests above assert — so their absence here is the flag doing + // the work and not a prop nobody passed. + for (const testId of [ + "tool-select", + "tool-bbox", + "tool-polygon", + "tool-polyline", + "tool-suggest", + "tool-add-class", + "tool-undo", + "tool-redo", + ]) { + expect(screen.queryByTestId(testId)).toBeNull(); + } + }); +});