Skip to content

fix(annotator): the hand is one of the tools, not a mode lit beside them - #581

Merged
JArmandoAnaya merged 3 commits into
mainfrom
fix/hand-tool-exclusive
Aug 14, 2026
Merged

fix(annotator): the hand is one of the tools, not a mode lit beside them#581
JArmandoAnaya merged 3 commits into
mainfrom
fix/hand-tool-exclusive

Conversation

@JArmandoAnaya

Copy link
Copy Markdown
Contributor

What was wrong

The hand and whichever tool the active class derived were lit at the same time, and the hand
sat above the whole strip rather than in it.

Both come from one thing: core/interaction/tool.ts derives the tool from the active class and
stores nothing, while the hand is a mode held beside that class (#578's panTool). Nothing ever
made the two exclusive, because they are not the same kind of value.

Why exclusivity is the honest answer, not a tidy-up

They are already exclusive in the canvas. AnnotatorCanvas's pointer-down reads

if (button !== "primary" || handNow()) { beginPan(event); return; }

and that branch sits before the suggest branch and before the machine dispatch. So while the
hand is on, no tool row and no suggest click can act at all — a second lit button was describing a
tool that does nothing.

So the strip lights one button at a time and the hand is in that rotation, and reaching for a
drawing class puts the hand away.

The second half is one funnel, not four call sites

AnnotationPage's activateClass already documents itself as the single route — "the panel's
list, the tool strip, a digit hotkey and the canvas's own activate-class"
— so setHandTool(false)
is wrapped around it once. The prop binding is renamed to armClass, which leaves all four existing
call sites untouched.

toggleSuggest arms through that funnel, so the sparkle puts the hand away without knowing that
it has to.

Only that direction is automatic. Raising the hand leaves the class where it was: it is a way of
looking at the picture, and somebody who pans and puts the hand down wants the class they were
drawing with, not select. Suggest and a class stay lit together — the one pair here that is
legitimately both on, since suggest is a mode over the class it borrows.

The move

The hand button goes to the end of the tool block, below the + and above the history divider.
It was on top as the one control that does not draw, which read as a heading over the tools rather
than as one of them.

Existing assertions are count/label/active-based rather than order-based, so nothing broke; a new
order assertion covers the half a getByTestId cannot see.

Tests

Four new cases in toolPalette.test.tsx — the tool row and the sparkle each losing the lit state to
the hand, and the order. Both display rules mutation-verified: dropping either !hand.active
turns exactly one named test red.

The funnel is a page-level wiring fact, so it is asserted where the page really runs:
e2e/annotate.spec.ts's hand scenario now presses h, then a digit, and asserts the hand goes off
and the box tool lights. Mutation-verified too — deleting setHandTool(false) fails that
scenario at exactly the new line (data-active stayed "true"), rather than passing quietly.

Checks

Rebased onto 0b56ac0 and re-verified on the rebased head: scripts/check.sh frontend PASSED
(annotator 1027, ui-core 980), scripts/check.sh browser PASSED (270 e2e + 1 cycle),
scripts/check.sh docs generated PASSED. No Python touched, no CI job added or renamed, so the
main ruleset is unchanged.

DESIGN.md's tool-strip spec carries the new order and the one-lit-button rule.

The hand and whichever tool the active class derived were lit at the same
time, and the hand sat above the whole strip rather than in it. Both come from
the same thing: the tool is derived from the active class and the hand is a
mode held beside it, so nothing ever made the two exclusive.

They are exclusive in the canvas already, and that is what settles it.
`AnnotatorCanvas` answers a primary press with a pan *before* the suggest
branch and before the machine dispatch, so while the hand is on no tool row and
no suggest click can act — a second lit button was describing a tool that does
nothing.

So the strip lights one button at a time and the hand is in that rotation, and
reaching for a drawing class puts the hand away. The second half is wrapped
around `activateClass`, the funnel its own docstring already names — the
panel's list, the tool strip, a digit hotkey and the canvas's `activate-class`
all arrive through it, so the rule is written once rather than at each button.
`toggleSuggest` arms through the same funnel, so the sparkle puts the hand away
without knowing that it has to.

Only that direction is automatic. Raising the hand leaves the class where it
was: it is a way of looking at the picture, and somebody who pans and puts the
hand down wants the class they were drawing with. Suggest and a class stay lit
together, which is the one pair here that is legitimately both on: it is a mode
over the class it borrows.

The button moves to the end of the tool block, below the `+`. It was on top as
the one control that does not draw, which read as a heading over the tools
rather than as one of them.
…light

Two corrections to the commit before this, both found by using it.

**Select was unreachable from the hand.** A press whose tool has not moved is a
no-op — the rule that stops a second bbox class silently re-pointing the first
— and every frame opens sitting in `select`, so the Select button had nothing
to do and the mode stayed up. The only way down was to arm some other tool
first. Putting the hand down *is* a move, so that is the branch the press takes
when the tool itself does not: one `else if`, and the class still does not
shift.

**The suggest button is out of the exclusive group.** Dimming it under the hand
was behaviour nobody asked for and it made the button lie: armed, it reads as
off, and the press that looks like it turns it on is the one that turns it off.
It is a mode over the class it borrows and is legitimately lit beside a tool,
so it keeps its own state. The hand and the derived tool are the pair that are
one light.
@JArmandoAnaya

Copy link
Copy Markdown
Contributor Author

Two corrections pushed as 52bfab3, both found by using the branch:

Select was unreachable from the hand. A press whose tool has not moved is a no-op — the rule that stops a second bbox class silently re-pointing the first — and every frame opens sitting in select, so the Select button had nothing to do and the mode stayed up. Putting the hand down is a move, so that is the branch the press takes when the tool itself does not. One else if; the class still does not shift.

The suggest button is out of the exclusive group. Dimming it under the hand made the button lie: armed, it read as off, so the press that looked like it turned it on was the one that turned it off. It is a mode over the class it borrows and is legitimately lit beside a tool. The hand and the derived tool are the pair that are one light.

Mutation-verified: deleting the else if turns the new named case red. Locally check.sh frontend PASSED (ui-core 981), check.sh browser had one failure — selecting on the canvas scrolls the object's row into view, which is #550 verbatim (reproduced on unmodified main, passes alone in 14.2 s, passed in the previous full run on this branch).

An armed tool kept drawing its crosshair across the picture while the hand was
up, and kept lighting the grip under the pointer. Only a page sitting in
`select` looked right, which is what made it read as the hand having no state
of its own.

It did not. `hover` is one piece of state with two readers — the affordance and
the drawing guides — and the hand reached neither: it was applied to the
*cursor* alone, which makes it a cursor rather than a mode. Both readers were
therefore offering something the next press cannot keep, because
`handlePointerDown` answers that press with a pan before the machine or the
suggest branch hears it.

So the mode is spent once, on the state both readers derive from: `pointing` is
`hover` while the hand is down and `null` while it is up, and the affordance and
the crosshair both read it. A hand that had to be remembered at each render site
is one that would be forgotten at the next — this was the third such site and
there is no reason to think it was the last.

`hover` itself keeps tracking, so putting the hand down restores the guides and
the highlight where the pointer already is, with no move needed to wake them.
The crosshair group gains a test id, because "the hand put the tools away" is a
claim about the most visible thing an armed tool draws, and a browser had no way
to name it.
@JArmandoAnaya

Copy link
Copy Markdown
Contributor Author

Third commit, a21252e — and this one is the root cause the first two were circling.

An armed tool kept drawing its crosshair while the hand was up, and kept lighting the grip under the pointer; only a page sitting in select looked right, which is exactly what "the hand has no strong state" describes.

It did not have one. hover is a single piece of state with two readers — the affordance and the drawing guides — and the hand reached neither: it was applied to the cursor alone, which makes it a cursor rather than a mode. Both readers were offering what the next press cannot keep, because handlePointerDown answers that press with a pan before the machine or the suggest branch hears it.

So the mode is now spent once, on the state both readers derive from:

const pointing = hand ? null : hover;

and the affordance and the crosshair both read pointing. A hand that has to be remembered at each render site is one that gets forgotten at the next — this was the third such site. hover itself keeps tracking, so putting the hand down restores the guides where the pointer already is, with no move needed.

New browser scenario, and it is mutation-verified against the reported defect: with const pointing = hover it fails with toHaveCount expected 0, received 1 — the crosshair on screen under a raised hand, which is the screenshot. The crosshair group gains a test id so a browser can name it at all.

Gates on this head: check.sh frontend PASSED (annotator 1027, ui-core 981), check.sh browser docs generated PASSED (271 e2e + 1 cycle; the #550 flake did not recur).

@JArmandoAnaya
JArmandoAnaya merged commit 7b3a39d into main Aug 14, 2026
15 checks passed
@JArmandoAnaya
JArmandoAnaya deleted the fix/hand-tool-exclusive branch August 14, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant