diff --git a/DESIGN.md b/DESIGN.md index f440c4f..9497924 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1079,9 +1079,10 @@ What a viewer is (decisions of 2026-08-07, #426, and 2026-08-08, #439): viewer can ask. - **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 `move`, no - resize keywords, because no such gesture exists. The tool strip is not - rendered at all, for the same reason it never was. + 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. - **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 @@ -1163,6 +1164,13 @@ gallery badges (#55) — and it **already exists, shipped and unit-tested**: on the picture made it a control somebody moved blind. Its outline is dashed and an accepted annotation's is solid, which is what tells proposed from committed at a glance. +**The cursor promises the common outcome, not the rare one** (#567). In Select mode, +hovering a shape — its body, its edge band, or a vertex — is the **plain arrow**: a press +there *selects*, and only becomes a move if the pointer then travels. What reports which +shape a press would take is the hover **highlight**, not the cursor. The four-arrow `move` +appears only while a drag is actually in flight, and the directional resize keywords only on +a selected box's grips, where they name an axis the arrow cannot. + **The canvas label is part of what selection looks like.** A frame carrying forty boxes drew forty class names over the picture at all times, which hides the asset behind the annotations of it. The panel is the full inventory; the canvas answers *what is this one* for the shape diff --git a/frontend/annotator/src/adapters/react/TransientLayer.tsx b/frontend/annotator/src/adapters/react/TransientLayer.tsx index 2726d5f..cc459ea 100644 --- a/frontend/annotator/src/adapters/react/TransientLayer.tsx +++ b/frontend/annotator/src/adapters/react/TransientLayer.tsx @@ -377,9 +377,8 @@ function PendingPolygonShape({ points, cursor, color, zoom, closeRing }: { * * `body` and `empty` draw nothing: a hot body is the committed layer's to fill, * and empty canvas has nothing to promise. An `edge` shows where a double-click - * would insert a vertex, which is the one affordance with no shape of its own — - * `affordance.ts` notes that it answers a `move` cursor and leaves the hint to - * whoever renders `hot`. + * would insert a vertex — the one affordance with no shape of its own, and since + * #567 no cursor of its own either, so this is the whole of the hint. */ function HotTarget({ hot, zoom }: { readonly hot: Target; readonly zoom: number }): JSX.Element | null { if (hot.kind === "handle") { diff --git a/frontend/annotator/src/core/interaction/affordance.test.ts b/frontend/annotator/src/core/interaction/affordance.test.ts index 402d476..3bd001a 100644 --- a/frontend/annotator/src/core/interaction/affordance.test.ts +++ b/frontend/annotator/src/core/interaction/affordance.test.ts @@ -129,30 +129,29 @@ describe("hovering, with nothing in flight", () => { } }); - it("offers a move over a body, and names the body it would move", () => { + it("answers default over a body, and names the body a press would take", () => { expect(at(IDLE, BOX_BODY)).toEqual({ - cursor: "move", + cursor: "default", hot: { kind: "body", id: BOX_ID }, }); }); - it("offers a move over a vertex of the picked polygon", () => { + it("answers default over a vertex of the picked polygon", () => { const answer = at(IDLE, POLY_VERTEX, "select", scene(selectOnly(POLY_ID))); - expect(answer.cursor).toBe("move"); + expect(answer.cursor).toBe("default"); expect(answer.hot).toEqual({ kind: "vertex", id: POLY_ID, index: 0, point: POLY_VERTEX }); }); - it("offers a move over an edge, because a press on one starts a move", () => { - // `IDLE_ROW` groups edge with body — `if (kind === "body" || kind === "edge") - // return pressOnShape(...)`. Showing `default` here would be the drawing-tool - // lie inverted: under-promising, and still a disagreement with the table. The - // press is asserted in the same test so the two cannot drift apart. + it("answers default over an edge, where the press still starts a move", () => { + // The cursor stopped advertising the move (#567), so the press is the half + // that matters here: asserted below, in the same test, so the two cannot + // drift apart. // Seven pixels outside the polygon's top edge: past the 4-px shape tolerance // that would make it a body hit, inside the 15-px edge band. const point: Point = [350, 293]; const where = scene(selectOnly(POLY_ID)); const answer = affordanceAt(IDLE, where, "select", point); - expect(answer.cursor).toBe("move"); + expect(answer.cursor).toBe("default"); expect(answer.hot.kind).toBe("edge"); const pressed = transition(IDLE, down(point), { @@ -174,7 +173,7 @@ describe("hovering, with nothing in flight", () => { // `resolveTarget` ranks grips for selected boxes only, and the body underneath // is what a press would actually take. const answer = at(IDLE, BOX_NW, "select", scene()); - expect(answer.cursor).toBe("move"); + expect(answer.cursor).toBe("default"); expect(answer.hot).toEqual({ kind: "body", id: BOX_ID }); }); @@ -405,12 +404,15 @@ describe("the cursor table", () => { it("can actually produce every cursor the union declares", () => { // The union is vocabulary, and vocabulary nobody speaks is dead weight that // reads as capability. Every member has to come out of a real call: the four - // resize keywords from the grips, `move` from a body, `crosshair` from a - // drawing tool, `default` from empty canvas, `pointer` from the first vertex of - // a polygon long enough to close. + // resize keywords from the grips, `move` from a drag in flight — its only + // source since a hover stopped offering it (#567) — `crosshair` from a drawing + // tool, `default` from empty canvas, `pointer` from the first vertex of a + // polygon long enough to close. const closeable = drawing(...PENDING); + const dragging = worldIn("moving"); const produced = new Set([ ...BBOX_HANDLES.map((handle) => at(IDLE, GRIP_POSITIONS[handle]).cursor), + affordanceAt(dragging.state, sceneOfWorld(dragging), "select", EMPTY_POINT).cursor, at(IDLE, BOX_BODY).cursor, at(IDLE, EMPTY_POINT, "bbox").cursor, at(IDLE, EMPTY_POINT).cursor, @@ -459,10 +461,13 @@ describe("the viewer's affordance (#426)", () => { expect(affordance.hot).toEqual({ kind: "body", id: BOX_ID }); }); - it("answers default over a body, where the editor's select tool says move", () => { - expect(affordanceAt(IDLE, viewerScene(), "select", BOX_BODY).cursor).toBe("move"); - expect(viewerAffordanceAt(viewerScene(), BOX_BODY).cursor).toBe("default"); - expect(viewerAffordanceAt(viewerScene(), BOX_BODY).hot).toEqual({ kind: "body", id: BOX_ID }); + it("resolves no grip at all, which is what the editor still does differently", () => { + // Both modes answer `default` over a body since #567, so the cursor no longer + // tells them apart. What does: a viewer never resolves a grip or a vertex, so + // no resize keyword can appear anywhere in it. + expect(viewerAffordanceAt(viewerScene(), BOX_NW).cursor).toBe("default"); + expect(viewerAffordanceAt(viewerScene(), BOX_NW).hot).toEqual({ kind: "body", id: BOX_ID }); + expect(at(IDLE, BOX_NW, "select", scene(selectOnly(BOX_ID))).hot.kind).toBe("handle"); }); it("answers default and no target over empty canvas", () => { diff --git a/frontend/annotator/src/core/interaction/affordance.ts b/frontend/annotator/src/core/interaction/affordance.ts index 0fe84cc..ca0b1a4 100644 --- a/frontend/annotator/src/core/interaction/affordance.ts +++ b/frontend/annotator/src/core/interaction/affordance.ts @@ -194,15 +194,13 @@ function hovering(scene: Scene, tool: Tool, point: Point): Affordance { case "vertex": case "body": case "edge": - // `edge` is grouped with `body` because `IDLE_ROW` groups them: - // `if (target.kind === "body" || target.kind === "edge") return pressOnShape(...)`. - // A press in the 15-px band around a selected polygon picks it and starts a - // move, so `move` is what the cursor owes. Showing `default` there would be - // the drawing-tool lie inverted — under-promising rather than over — and - // still a disagreement with the table. The double-click that inserts a - // vertex is a second meaning for the same band, not the only one; a renderer - // that wants to hint at it has the `edge` target in `hot`. - return { cursor: "move", hot: target }; + // `edge` is grouped with `body` because `IDLE_ROW` groups them. + // + // `default`, reversing an earlier `move` (#567): a press here *selects*, + // and only becomes a move if the pointer travels — so `move` advertised the + // rarer outcome. `hot` is unchanged, so the shape still highlights, and a + // drag in flight still answers `move` below. + return { cursor: "default", hot: target }; case "empty": return { cursor: "default", hot: NO_TARGET }; } @@ -257,11 +255,12 @@ export function affordanceAt( /** * The affordance a **viewer** answers, where selection is the only gesture. * - * The cursor is `default` everywhere: a read-only page never - * shows `move`, because no move exists to promise. What survives is the hot - * body, so hovering still says *this is the shape a press would pick*: a - * highlight aids selection, which is a read, where a cursor change advertises - * an edit. + * The cursor is `default` everywhere: a read-only page never shows a resize + * keyword, because no such gesture exists. What survives is the hot body, so + * hovering still says *this is the shape a press would pick*. + * + * The editor now answers `default` over a shape too (#567); what still separates + * the modes is below — a viewer resolves no grip and no vertex at all. * * It deliberately does not call `resolveTarget`: that resolver offers grips and * vertices on the selected shape, and a viewer draws none (the same mirror as diff --git a/frontend/app/e2e/annotate.spec.ts b/frontend/app/e2e/annotate.spec.ts index 1ebd9c9..7524cc0 100644 --- a/frontend/app/e2e/annotate.spec.ts +++ b/frontend/app/e2e/annotate.spec.ts @@ -1421,12 +1421,12 @@ function storedBox(assetId: string): Record { } /** - * Read-only selection highlights — stroke and - * label — and advertises nothing. No move cursor anywhere, no grips or vertex - * dots on the selected shape. The editor is asserted beside it, so the claim is - * about the mode and not about the fixture. + * Read-only selection highlights and advertises nothing: no grips, no vertex + * dots. The editor is asserted beside it, so the claim is about the mode and not + * about the fixture. The cursor no longer separates them (#567) and is not + * compared here. */ -test("read-only selection shows no move cursor and no handles; the editor shows both", async ({ +test("read-only selection grows no handles, where the editor's does", async ({ page, }) => { const sent: Request[] = []; @@ -1447,16 +1447,9 @@ test("read-only selection shows no move cursor and no handles; the editor shows await expect(page.locator("[data-handle]")).toHaveCount(0); await expect(page.locator("[data-vertex]")).toHaveCount(0); - // (b) …and hovering the body promises nothing: the pane's cursor is the - // default arrow, not `move`. - await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); - const viewing = await page - .getByTestId("annotator-pane") - .evaluate((node) => getComputedStyle(node).cursor); - expect(viewing).toBe("default"); }); -test("the editor still offers what the viewer withholds — move cursor and grips", async ({ +test("the editor grows grips on selection, and hovering a shape stays a plain arrow", async ({ page, }) => { const sent: Request[] = []; @@ -1469,11 +1462,14 @@ test("the editor still offers what the viewer withholds — move cursor and grip await expect(page.getByTestId("object-row-0")).toHaveAttribute("data-selected", "true"); await expect(page.locator("[data-handle]").first()).toBeVisible(); + + // Hovering the body is a plain arrow, not the four-arrow `move` (#567). Only a + // browser has a computed cursor at all. await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); const editing = await page .getByTestId("annotator-pane") .evaluate((node) => getComputedStyle(node).cursor); - expect(editing).toBe("move"); + expect(editing).toBe("default"); }); /** @@ -3235,6 +3231,59 @@ test("the preview draws its vertices, and a committed shape does not", async ({ await expect(committed.locator("polygon")).not.toHaveAttribute("stroke-dasharray", "10 6"); }); +test("the detail slider moves under the pointer, and hands the keyboard back", async ({ + page, +}) => { + const sent: Request[] = []; + await openJob(page, sent, undefined, undefined, undefined, undefined, true); + await servePolygonSuggestion(page); + + await page.getByTestId("tool-suggest").click(); + const picture = (await page.getByTestId("annotator-canvas").boundingBox())!; + await page.mouse.click(picture.x + picture.width / 2, picture.y + picture.height / 2); + await expect(page.getByTestId("suggestion-shape")).toBeVisible(); + await page.getByTestId("suggest-adjust-open").click(); + + const slider = page.getByTestId("suggest-detail"); + await expect(slider).toHaveValue("1"); + const before = asks(sent); + + // A real drag: press the thumb, travel, release. `fill()` and `click()` both + // set the value without ever exercising the default action, which is exactly + // the gap that let a slider ship unmovable — `preventDefault` on the press + // cancelled the drag and every jsdom assertion still passed (#563). + const track = (await slider.boundingBox())!; + await page.mouse.move(track.x + track.width / 2, track.y + track.height / 2); + await page.mouse.down(); + await page.mouse.move(track.x + track.width - 1, track.y + track.height / 2, { steps: 8 }); + await page.mouse.up(); + + await expect(slider).toHaveValue("2"); + await expect(page.getByTestId("suggest-detail-label")).toContainText("Fine"); + const fine = await drawnVertices(page); + // Still no round trip: the drag is arithmetic, like the brackets. + expect(asks(sent)).toBe(before); + + // Dragging the other way, to the coarsest stop. Two *client* simplifications + // compared against each other — the answer's own geometry arrives already + // reduced by the server and is not one of the three steps. + await page.mouse.move(track.x + track.width / 2, track.y + track.height / 2); + await page.mouse.down(); + await page.mouse.move(track.x + 1, track.y + track.height / 2, { steps: 8 }); + await page.mouse.up(); + await expect(slider).toHaveValue("0"); + await expect(page.getByTestId("suggest-detail-label")).toContainText("Coarse"); + expect(fine).toBeGreaterThan(await drawnVertices(page)); + + // And the canvas has its keyboard back the moment the drag ended — without + // this the brackets, Esc and Enter are all dead and nothing says why. + await page.keyboard.press("]"); + await expect(slider).toHaveValue("1"); + await page.keyboard.press("Escape"); + await expect(page.getByTestId("suggest-adjustments")).toHaveCount(0); + await expect(page.getByTestId("suggestion-shape")).toBeVisible(); +}); + test("a press on the suggest panel never reaches the picture underneath", async ({ page }) => { const sent: Request[] = []; await openJob(page, sent, undefined, undefined, undefined, undefined, true); diff --git a/frontend/ui-core/src/annotator/SuggestPanel.tsx b/frontend/ui-core/src/annotator/SuggestPanel.tsx index 986ae1c..a934c28 100644 --- a/frontend/ui-core/src/annotator/SuggestPanel.tsx +++ b/frontend/ui-core/src/annotator/SuggestPanel.tsx @@ -508,16 +508,33 @@ function Chip({ children }: { readonly children: ReactNode }): JSX.Element { * would stop stepping, and `Esc` would stop being the preview's undo, both with * nothing on screen to say why. Found in a browser: jsdom has no focus to move. * - * On every control here, the slider included: all of them act on the canvas, so - * none of them has any business holding focus. `mousedown` rather than the whole - * control, so Tab still reaches the slider for somebody who wants to drive it - * from the keyboard — that is a deliberate arrival, not a side effect of pointing - * at it. + * On the buttons only. **A range input drags on the default action**, so + * cancelling its `mousedown` leaves a slider that looks alive and cannot be moved + * by hand at all — which is what shipped, and what nothing caught, because the + * test asserted the guard fired rather than that the thumb followed the pointer. + * The slider uses {@link returnFocusToCanvas} instead: it takes focus for the + * duration of the drag, like any form control, and hands it back on release. */ function keepFocusOnCanvas(event: { preventDefault: () => void }): void { event.preventDefault(); } +/** + * Give the canvas its keyboard back once a pointer gesture on a control is over. + * + * The other half of the same rule, for a control whose default action is the + * whole point of it. `FrameGallery` already returns focus this way after its + * overlay closes, and by the same route — `ui-core` holds no ref to the + * annotator's root, and threading one down for this would be a prop on every + * layer between here and there. + * + * On release rather than on change: a drag emits a change per step, and pulling + * focus mid-drag would end the gesture under the pointer. + */ +function returnFocusToCanvas(): void { + document.querySelector('[data-testid="annotator-root"]')?.focus(); +} + /** * The settings, inside the card that is already on screen. * @@ -593,11 +610,12 @@ function Adjustments({ aria-label="Detail" aria-valuetext={`${labelFor(detail)}, ${vertexCount(session)} points`} data-testid="suggest-detail" - // Dragging this must not take focus off the canvas: every chord in - // the editor is a keydown on the annotator's own root, so a control - // that took focus would switch `[`, `]`, Esc and Enter off with - // nothing on screen to say why. Tab still reaches it deliberately. - onMouseDown={keepFocusOnCanvas} + // No `preventDefault` here: a range input *drags* on its default + // action, so cancelling the press is what made this unmovable by + // hand (#563). Focus goes back to the canvas on release instead, so + // `[`, `]`, Esc and Enter are live again the moment the drag ends. + onMouseUp={returnFocusToCanvas} + onTouchEnd={returnFocusToCanvas} onChange={(event) => onDetail(DETAIL_STEPS[Number(event.target.value)] ?? detail)} /> {/* diff --git a/frontend/ui-core/src/annotator/suggestPanel.test.tsx b/frontend/ui-core/src/annotator/suggestPanel.test.tsx index 1c75687..84e3646 100644 --- a/frontend/ui-core/src/annotator/suggestPanel.test.tsx +++ b/frontend/ui-core/src/annotator/suggestPanel.test.tsx @@ -524,14 +524,35 @@ describe("the adjustments, which are a section and never a popup", () => { expect(onDetail).toHaveBeenCalledWith("fine"); }); - it("does not let a press on the slider take focus off the canvas", () => { - // Every chord in the editor is a keydown on the annotator's own root, so a - // control that took focus would switch `[`, `]`, Esc and Enter off with - // nothing on screen to say why (#557). + it("lets a press on the slider through, because that press is the drag", () => { + // The defect this replaces: `preventDefault` on the press cancelled a range + // input's own drag, leaving a control that looked alive and could only be + // moved with the brackets. The old test asserted the guard *fired*, which is + // exactly the assertion a dead control passes (#563). render(mount({ session: showingPolygon(), adjusting: true, onAdjusting: vi.fn(), onDetail: vi.fn() })); const press = fireEvent.mouseDown(screen.getByTestId("suggest-detail")); - expect(press).toBe(false); + expect(press).toBe(true); + }); + + it("hands the keyboard back to the canvas when the drag ends", () => { + // The other half: the slider may hold focus while it is being dragged, but + // not after, or `[`, `]`, Esc and Enter stay dead with nothing to say why. + const root = document.createElement("div"); + root.setAttribute("data-testid", "annotator-root"); + root.tabIndex = 0; + document.body.appendChild(root); + + render(mount({ session: showingPolygon(), adjusting: true, onAdjusting: vi.fn(), + onDetail: vi.fn() })); + const slider = screen.getByTestId("suggest-detail"); + slider.focus(); + expect(document.activeElement).toBe(slider); + + fireEvent.mouseUp(slider); + expect(document.activeElement).toBe(root); + + root.remove(); }); it("keeps the controls operable on an answer with nothing in it", () => {