From 858147b4d2a43517de6271f8d3aa5d4ad5dbafb5 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Thu, 13 Aug 2026 13:14:52 -0700 Subject: [PATCH 1/7] feat(annotator): the wheel pans, the modifier zooms, and the hand is a mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the pure gesture math to `adapters/viewport.ts` — wheel normalisation over both axes, a zoom factor whose softness tells a mouse notch from a pinch, and a two-finger pinch as a scale about a travelling centroid — and wires it through the React adapter. Plain wheel now pans, both axes, which is what gives a trackpad a pan at all; `ctrl`/`cmd`+wheel zooms, and that one branch serves a macOS pinch, a Windows precision-touchpad pinch and a mouse alike. The hand joins as a mode over the top, `h` through the registry and `Space` held as an adapter substitution, and two touch pointers are a gesture regardless of tool. cf. #576 --- .../src/adapters/react/AnnotatorCanvas.tsx | 355 ++++++++++++++++-- .../annotator/src/adapters/viewport.test.ts | 196 ++++++++++ frontend/annotator/src/adapters/viewport.ts | 121 ++++++ frontend/annotator/src/core/input/actions.ts | 22 ++ .../annotator/src/core/input/bindings.test.ts | 14 + frontend/annotator/src/core/input/bindings.ts | 5 + frontend/annotator/src/core/input/index.ts | 1 + frontend/annotator/src/index.ts | 5 + 8 files changed, 683 insertions(+), 36 deletions(-) diff --git a/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx b/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx index 9764b706..41fcbd9a 100644 --- a/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx +++ b/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx @@ -170,8 +170,11 @@ import { IDENTITY_VIEWPORT, fitToViewport, imageRenderingAt, + normalizedWheel, panBy, + pinchBetween, screenToImage, + wheelZoomFactor, zoomAbout, } from "../viewport"; import type { Viewport } from "../viewport"; @@ -196,14 +199,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 +405,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 +457,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 +481,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 +640,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 +766,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 +899,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 +932,49 @@ 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. + * + * `gestureNow` outlives the fingers that started it: it is cleared when the + * *last* one lifts, not when one does. That is what makes the exit + * jump-free — with one finger left over, the gesture is inert and the + * survivor's moves and its lift are swallowed rather than being promoted + * into a drag the person did not ask for. + */ + 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 +984,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 +997,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 +1034,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 +1107,26 @@ export function AnnotatorCanvas({ dispatch({ type: "pointer-move", point }); } + /** + * A finger left the glass, and the gesture ends when the *last* one does. + * + * Answers whether this lift belongs to a gesture and should therefore reach + * nothing else — true for every touch lift while one is running, the one that + * ends it included. + */ + 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 === 0) 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 +1136,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 +1251,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. + 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 pinch = pinchBetween(before, after); + const beforeCentroid: readonly [number, number] = [ + (before[0][0] + before[1][0]) / 2, + (before[0][1] + before[1][1]) / 2, + ]; + const held = screenToImage(viewport, beforeCentroid[0], beforeCentroid[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(pinch.centroidX, 8); + expect(imageToScreen(zoomed, held)[1]).toBeCloseTo(pinch.centroidY, 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"; From f6abeea5e328845306f8873d91f4c3f9e7b77c0f Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Thu, 13 Aug 2026 13:20:07 -0700 Subject: [PATCH 2/7] feat(ui): the hand joins the tool strip, and the help sheet documents the gestures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The strip is no longer absent for a viewer: it renders with the hand and the shortcut sheet and nothing else, which retires the exception it used to carry — 'every control on the palette picks a drawing tool' stopped being true the moment one of them was navigation. The sheet keeps its registry-derived rows and gains a written Navigate section, because a two-finger scroll and a pinch have no chord to be read off. Anything that does have one stays in the derived half. cf. #576 --- .../ui-core/src/annotator/AnnotationPage.tsx | 114 ++++++++++------- .../ui-core/src/annotator/ShortcutSheet.tsx | 116 +++++++++++++++++- .../ui-core/src/annotator/ToolPalette.tsx | 90 ++++++++++---- .../src/annotator/toolPalette.test.tsx | 73 +++++++++++ 4 files changed, 323 insertions(+), 70 deletions(-) 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(); + } + }); +}); From 32aac7b0936704ef6c289614c75acfe211424337 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Thu, 13 Aug 2026 13:30:36 -0700 Subject: [PATCH 3/7] docs: the navigation model, in DESIGN.md and docs/annotations.md cf. #576 --- DESIGN.md | 27 +++- docs/annotations.md | 74 ++++++++++- frontend/app/bench/_gestures.ts | 5 + frontend/app/e2e/_frame.ts | 28 ++++ frontend/app/e2e/annotate.spec.ts | 97 ++++++++++++-- frontend/app/e2e/demo.spec.ts | 89 ++++++++++++- frontend/app/e2e/perf.spec.ts | 9 +- frontend/app/e2e/showcase.spec.ts | 12 +- frontend/app/e2e/touch.spec.ts | 209 ++++++++++++++++++++++++++++++ 9 files changed, 520 insertions(+), 30 deletions(-) create mode 100644 frontend/app/e2e/touch.spec.ts 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/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/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..8db3950d 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); @@ -2052,6 +2064,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 +3032,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..1dc2505f --- /dev/null +++ b/frontend/app/e2e/touch.spec.ts @@ -0,0 +1,209 @@ +/** + * 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. + * + * ## `touchPoints` is the set that is still down, never the set that changed + * + * The protocol's own wording is "active touch points on the touch device", and + * Chromium derives press, move and release by comparing one event's list with + * the last one's. So a two-finger lift is `touchEnd` with an **empty** list, and + * lifting one of two is `touchEnd` naming the finger that stayed. Getting this + * backwards does not error — it produces a gesture that never ends, which is a + * scenario that passes for the wrong reason. + * + * ## 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; +} + +/** No fingers left. `touchEnd`'s spelling for "all of them came up". */ +const NONE: readonly Contact[] = []; + +/** + * 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", NONE); + + 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", NONE); + + 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", NONE); + + 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", NONE); + + 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 without a jump, and the survivor is inert. + * + * This is what `gestureNow` outliving its two contacts buys. Fingers never leave + * a screen together, so every pinch ends with one still down — and if that one + * were promoted into a drag pan, or forwarded to the machine, every pinch would + * finish by sliding the picture sideways or drawing a box nobody asked for. + */ +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 up, one still down: the list is what stayed. + const survivor = pair(centre, 140)[0]!; + await touch(client, "touchEnd", [survivor]); + const settled = await canvasOrigin(page); + const zoomed = (await frameOf(page)).zoom; + + // It 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", NONE); + + 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"); +}); From cf534697133dcf0f3f2e89a16e8119439e4a9006 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Thu, 13 Aug 2026 13:32:11 -0700 Subject: [PATCH 4/7] test(annotator): the pinch invariant asserts against a midpoint it computed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It asserted the held pixel lands on `pinch.centroidX`, which a mutation taking the centroid from before the move satisfies — `dx` comes off the same field, so the expectation slid exactly as far as the answer did. Both midpoints are now computed in the test. cf. #576 --- .../annotator/src/adapters/viewport.test.ts | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/frontend/annotator/src/adapters/viewport.test.ts b/frontend/annotator/src/adapters/viewport.test.ts index bea9ad50..400e61a8 100644 --- a/frontend/annotator/src/adapters/viewport.test.ts +++ b/frontend/annotator/src/adapters/viewport.test.ts @@ -403,6 +403,13 @@ describe("two fingers are one gesture: a scale about a point, and that point's t 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], @@ -412,18 +419,25 @@ describe("two fingers are one gesture: a scale about a point, and that point's t [60, 140], [420, 260], ] as const; - const pinch = pinchBetween(before, after); - const beforeCentroid: readonly [number, number] = [ - (before[0][0] + before[1][0]) / 2, - (before[0][1] + before[1][1]) / 2, + const midpoint = (pair: typeof before): readonly [number, number] => [ + (pair[0][0] + pair[1][0]) / 2, + (pair[0][1] + pair[1][1]) / 2, ]; - const held = screenToImage(viewport, beforeCentroid[0], beforeCentroid[1]); + 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(pinch.centroidX, 8); - expect(imageToScreen(zoomed, held)[1]).toBeCloseTo(pinch.centroidY, 8); + 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", () => { From d85718abf594e8679bd3e4e10a1ea52235109cb7 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Thu, 13 Aug 2026 13:39:01 -0700 Subject: [PATCH 5/7] test: the browser contract for the new gestures, and two findings from breaking it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `e2e/touch.spec.ts` — six scenarios over Chromium's own touch input, driven through CDP because Playwright has one touch verb and one contact. Existing wheel-zoom scenarios now hold the modifier through `zoomWheel`; a spec that forgot it would still pass its 'the picture moved' assertions while measuring the wrong gesture. Two things came out of mutating the rules rather than writing them: **The gesture lifetime was wrong.** Clearing it when the *last* finger lifts left a pinch inert until the whole hand came off the glass, so lifting one finger and putting another down did nothing. Below two contacts there is no gesture, and two down again is a new one, re-seeded from where the fingers actually are. The survivor needs no swallowing either — `IDLE_ROW` has a `pointer-down` handler and nothing else, so its stray events are silence. **CDP's `touchEnd` lists the points that LEFT**, not the ones remaining, which is the opposite of the protocol's own wording. Following the wording produces a suite that passes and measures nothing: the survivor a scenario then moves is a contact the browser thinks is gone, so every 'nothing moved' assertion holds for the wrong reason. cf. #576 --- .../src/adapters/react/AnnotatorCanvas.tsx | 48 +++++++-- frontend/app/e2e/touch.spec.ts | 100 ++++++++++++++---- 2 files changed, 115 insertions(+), 33 deletions(-) diff --git a/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx b/frontend/annotator/src/adapters/react/AnnotatorCanvas.tsx index 41fcbd9a..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 @@ -941,11 +964,16 @@ export function AnnotatorCanvas({ * Both fingers report `button: 0`, so nothing but the map distinguishes the * second press from a fresh first one. * - * `gestureNow` outlives the fingers that started it: it is cleared when the - * *last* one lifts, not when one does. That is what makes the exit - * jump-free — with one finger left over, the gesture is inert and the - * survivor's moves and its lift are swallowed rather than being promoted - * into a drag the person did not ask for. + * 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 }); @@ -1108,17 +1136,17 @@ export function AnnotatorCanvas({ } /** - * A finger left the glass, and the gesture ends when the *last* one does. + * A finger left the glass. Below two contacts there is no gesture. * - * Answers whether this lift belongs to a gesture and should therefore reach - * nothing else — true for every touch lift while one is running, the one that - * ends it included. + * 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 === 0) gestureNow.current = null; + if (touchesNow.current.size < 2) gestureNow.current = null; return true; } diff --git a/frontend/app/e2e/touch.spec.ts b/frontend/app/e2e/touch.spec.ts index 1dc2505f..e16495e7 100644 --- a/frontend/app/e2e/touch.spec.ts +++ b/frontend/app/e2e/touch.spec.ts @@ -17,14 +17,20 @@ * would emulate touch for every scenario in whatever file it sat in, and the * other specs are about a mouse. * - * ## `touchPoints` is the set that is still down, never the set that changed + * ## On `touchEnd`, `touchPoints` is the set that LEFT * - * The protocol's own wording is "active touch points on the touch device", and - * Chromium derives press, move and release by comparing one event's list with - * the last one's. So a two-finger lift is `touchEnd` with an **empty** list, and - * lifting one of two is `touchEnd` naming the finger that stayed. Getting this - * backwards does not error — it produces a gesture that never ends, which is a - * scenario that passes for the wrong reason. + * 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 * @@ -46,8 +52,10 @@ interface Contact extends Point { readonly id: number; } -/** No fingers left. `touchEnd`'s spelling for "all of them came up". */ -const NONE: readonly Contact[] = []; +/** 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. @@ -104,7 +112,7 @@ test("two fingers moving apart zoom the stage", async ({ page }) => { await touch(client, "touchStart", pair(centre, 60)); await touch(client, "touchMove", pair(centre, 120)); - await touch(client, "touchEnd", NONE); + await touch(client, "touchEnd", both(centre, 120)); await expect.poll(async () => (await frameOf(page)).zoom).toBeGreaterThan(frame.zoom * 1.8); }); @@ -116,7 +124,7 @@ test("two fingers moving together zoom out", async ({ page }) => { await touch(client, "touchStart", pair(centre, 160)); await touch(client, "touchMove", pair(centre, 80)); - await touch(client, "touchEnd", NONE); + await touch(client, "touchEnd", both(centre, 80)); await expect.poll(async () => (await frameOf(page)).zoom).toBeLessThan(frame.zoom * 0.7); }); @@ -137,7 +145,7 @@ test("two fingers travelling together pan without zooming", async ({ page }) => await touch(client, "touchStart", pair(centre, 100)); await touch(client, "touchMove", pair(centre, 100, -130, 70)); - await touch(client, "touchEnd", NONE); + await touch(client, "touchEnd", both(centre, 100, -130, 70)); await expect .poll(async () => Math.round((await canvasOrigin(page)).x)) @@ -163,7 +171,7 @@ test("a pinch scales about the point between the fingers", async ({ page }) => { await touch(client, "touchStart", pair(centre, 70)); await touch(client, "touchMove", pair(centre, 150)); - await touch(client, "touchEnd", NONE); + await touch(client, "touchEnd", both(centre, 150)); await expect.poll(async () => (await frameOf(page)).zoom).toBeGreaterThan(before.zoom * 1.5); @@ -173,12 +181,18 @@ test("a pinch scales about the point between the fingers", async ({ page }) => { }); /** - * Lifting one finger ends the pinch without a jump, and the survivor is inert. + * 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. * - * This is what `gestureNow` outliving its two contacts buys. Fingers never leave - * a screen together, so every pinch ends with one still down — and if that one - * were promoted into a drag pan, or forwarded to the machine, every pinch would - * finish by sliding the picture sideways or drawing a box nobody asked for. + * 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); @@ -189,16 +203,17 @@ test("lifting one finger ends the pinch, and the other one does nothing", async await touch(client, "touchMove", pair(centre, 140)); await expect.poll(async () => (await frameOf(page)).zoom).toBeGreaterThan(frame.zoom * 1.5); - // One up, one still down: the list is what stayed. - const survivor = pair(centre, 140)[0]!; - await touch(client, "touchEnd", [survivor]); + // 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; - // It travels a long way. Nothing may move. + // 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", NONE); + await touch(client, "touchEnd", [travelled]); const after = await canvasOrigin(page); expect(Math.round(after.x)).toBe(Math.round(settled.x)); @@ -207,3 +222,42 @@ test("lifting one finger ends the pinch, and the other one does nothing", async // 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]); +}); From 70e497b09c9d7fce8cf497ab8749023572c46056 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Thu, 13 Aug 2026 13:43:03 -0700 Subject: [PATCH 6/7] fix(annotator): the midpoint helper takes a pair, not the literal it was inferred from cf. #576 --- frontend/annotator/src/adapters/viewport.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/annotator/src/adapters/viewport.test.ts b/frontend/annotator/src/adapters/viewport.test.ts index 400e61a8..504691e1 100644 --- a/frontend/annotator/src/adapters/viewport.test.ts +++ b/frontend/annotator/src/adapters/viewport.test.ts @@ -419,7 +419,9 @@ describe("two fingers are one gesture: a scale about a point, and that point's t [60, 140], [420, 260], ] as const; - const midpoint = (pair: typeof before): readonly [number, number] => [ + 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, ]; From ee78c30f7ef74888fddc65cbadf876d935f5e4d1 Mon Sep 17 00:00:00 2001 From: Armando Anaya Date: Thu, 13 Aug 2026 13:46:44 -0700 Subject: [PATCH 7/7] test(e2e): the read-only strip assertions the first pass missed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three in the cycle suite and a second in `annotate.spec.ts` — the scenario asserts it once per frame and a single-site edit caught only the first. `viewport.spec.ts`'s is left alone: that one is the too-narrow viewport, where the whole editor is unmounted. cf. #576 --- frontend/app/cycle/cycle.spec.ts | 11 ++++++++--- frontend/app/e2e/annotate.spec.ts | 3 ++- 2 files changed, 10 insertions(+), 4 deletions(-) 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/annotate.spec.ts b/frontend/app/e2e/annotate.spec.ts index 8db3950d..2aafe2ea 100644 --- a/frontend/app/e2e/annotate.spec.ts +++ b/frontend/app/e2e/annotate.spec.ts @@ -1620,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 —