fix(annotator): a mouse wheel zooms again, and a trackpad scroll still pans - #579
Merged
Conversation
…l pans #576 gave the whole of a bare wheel event to the trackpad. That is what made a laptop workable — a two-finger scroll is how anybody moves around a canvas, and until then there was no gesture on a trackpad that moved the picture at all — but it took the mouse's zoom away with it: rolling the wheel scrolled the picture up and down. The two devices want opposite things from the same event, so the event is now answered by device. `isMouseWheel` decides, beside the rest of the wheel arithmetic in `adapters/viewport.ts`, and the listener asks the modifier first as it always did: ctrlKey || metaKey || isMouseWheel(...) -> zoom about the cursor otherwise -> pan, both axes It reads three signals and none of them is `deltaY`, which the operating system accelerates and which overlaps completely between the two devices: a `deltaMode` other than pixels is a discrete wheel, anything sideways is a scroll, and otherwise a whole number of 120-unit `wheelDelta` notches is a wheel. Every uncertain case answers trackpad — a trackpad that zooms when it was asked to scroll is the defect #576 fixed, while a mouse this declines still zooms with the modifier, the widget's buttons and `mod+0`. Nothing else in the navigation model moves: the softness split, the pinch, `Space`, the hand tool, the non-primary drag and the zoom widget are untouched, and `zoomWheel` in the e2e helpers still holds the modifier, so only the two bare-wheel scenarios in `demo.spec.ts` needed rewriting.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
#578 made every bare wheel event a pan. That is what made a laptop workable — a two-finger
scroll is how anybody moves around a canvas, and until then no gesture on a trackpad moved the
picture at all — but it took the mouse's zoom away with it: rolling the wheel scrolled the picture
up and down instead of zooming.
The two devices want opposite things from the same event, so a bare event is now answered by
device. The modifier is asked first, exactly as before:
ctrlKeyormetaKeyheldisMouseWheelis the new test, and it sits inadapters/viewport.tsbeside the rest of the wheelarithmetic, so it is unit-tested without a browser. It reads three signals and none of them is
deltaY, which the operating system accelerates and which overlaps completely between the twodevices:
deltaModeother than pixels is a discrete wheel, and nothing else reports lines or pages(this is Firefox's mouse);
wheelDeltanotches. Chrome quantises a discrete wheel tounits of 120 however much it accelerated
deltaY, and computes a precise device's as-3 * deltaY.Every uncertain case answers "trackpad." A trackpad that zooms when it was asked to scroll is
the defect #578 fixed; a mouse this declines — a Magic Mouse reports as a precise device — still
zooms with the modifier, with the widget's
−/+, and withmod+0to refit. The cost of beingwrong is one stray notch inside a hard flick, never a device with no gesture at all.
Nothing else in the navigation model moves:
wheelZoomFactorand its 40 px softness split,normalizedWheel, the touch pinch,Space, the hand tool, the non-primary drag and the zoomwidget are all untouched.
Findings
CDP's synthetic wheel reports
wheelDeltaYas ±120 whateverdeltaYsays. Measured with athrowaway probe before writing a single scenario:
page.mouse.wheel(0, 7),(0, 12)and(0, 40)all arrive as
-120, while(-90, 0)arrives as0. So a browser suite can spell a mouse notchand a sideways scroll, and has no spelling at all for a trackpad's vertical scroll — that half
of the predicate is unit-only, and
demo.spec.tsanddocs/annotations.mdsay so rather thanleaving a reader hunting for a scenario that cannot exist.
TypeScript 6 removed the deprecated
wheelDelta*members fromlib.dom.d.ts, soevent.wheelDeltaYdoes not typecheck. It is read through an explicit widening at the one callsite, and a browser that does not have the property lands on the pan.
Tests
Six new unit cases in
viewport.test.ts, one per rule plus the two shapes each device actuallyproduces. All four rules mutation-verified, each turning exactly one named test red: dropping
the
deltaModeclause fails the Firefox case, dropping thedeltaXclause fails the sideways case,120 → 3fails the trackpad-quantum case, and dropping the zero-delta clause fails theno-legacy-property case.
In
demo.spec.tsthe two bare-wheel scenarios were rewritten — a bare wheel now zooms, and thesideways scroll is the one gesture that is a pan on both sides of the device test.
zoomWheeline2e/_frame.tsstill holds the modifier, soannotate.spec.ts,showcase.spec.ts,perf.spec.ts,cycle.spec.tsand the bench needed no edit.Checks
Locally in the worktree:
scripts/check.sh frontendPASSED,scripts/check.sh browserPASSED (270 e2e + 1 cycle),
scripts/check.sh docs generatedPASSED. The python stage wasnot run — the diff touches zero
.pyfiles, and neitheropenapi.jsonnor the generated clientmoves. No CI job was added, removed or renamed, so the
mainruleset is untouched.Verified by hand on the hardware this is about — Chrome on macOS, mouse and trackpad both.
Docs
DESIGN.md's navigation-model entry anddocs/annotations.md's input-model section carry the newsplit and the heuristic's stated ceiling; the shortcut sheet's Navigate section moves the scroll
wheel from panning to zooming.