feat(graph): add mousewheel factorByDelta for smooth trackpad zoom - #5082
feat(graph): add mousewheel factorByDelta for smooth trackpad zoom#5082xianjianlf2 wants to merge 2 commits into
factorByDelta for smooth trackpad zoom#5082Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new factorByDelta option to the mouse wheel zoom functionality, enabling smooth, trackpad-friendly zooming by scaling the zoom step proportionally to the wheel-delta magnitude. It also includes corresponding documentation updates and unit tests. The feedback suggests adding safety checks to ensure that the zoom factor calculation does not result in NaN or invalid values if the delta is non-finite or the factor is negative.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const d = deltaY != null ? deltaY : delta | ||
| this.cumulatedFactor = factor ** (-d / 100) |
There was a problem hiding this comment.
If deltaY and delta are both undefined or NaN (e.g., due to a custom or malformed event), or if factor is configured as a negative number, factor ** (-d / 100) will evaluate to NaN or an invalid value. This can propagate to targetScale and cause rendering errors or exceptions when calling this.graph.zoom(NaN).
To prevent this, we should ensure d is a finite number and factor is positive before performing the exponentiation.
| const d = deltaY != null ? deltaY : delta | |
| this.cumulatedFactor = factor ** (-d / 100) | |
| const d = deltaY != null ? deltaY : delta | |
| const f = factor > 0 ? factor : 1.2 | |
| this.cumulatedFactor = Number.isFinite(d) ? f ** (-d / 100) : 1 |
|
Thanks for the review. Applied in d21661d: on a non-finite |
The mousewheel handler ignores the wheel-delta magnitude and clamps every event to a fixed step (>= 5%, floored at 1.05). A Mac trackpad pinch emits many small high-frequency wheel events, so each one applies a full step and the canvas zooms far too fast; tuning `factor` below 1.05 cannot help because of the floor. Add an opt-in `factorByDelta` option. When enabled, the zoom step is proportional to the wheel delta (`factor ** (-delta / 100)`): a standard ~100px notch keeps the classic mouse-wheel feel, while small trackpad deltas produce smooth, proportionally small steps. It also consumes the per-frame accumulated delta that `MouseWheelHandle` already computes and passes to the callback but the handler previously discarded. Defaults to `false`, so existing behavior is unchanged. Closes antvis#1725
… factor Address review feedback: a malformed event (non-finite delta) or a misconfigured negative `factor` would make `factor ** (-d / 100)` evaluate to NaN and reach `graph.zoom(NaN)`. Fall back to no zoom on a non-finite delta and to the default factor when it is not positive, bringing the new path to parity with the NaN-safe quantized path.
d21661d to
7c049ad
Compare
Need
Fixes #1725 (and the trackpad half of #1090). On a Mac trackpad, pinch-to-zoom on the canvas is far too fast/jumpy.
Updating Reason
MouseWheel.onMouseWheelignores the wheel-delta magnitude — it only looks atdeltaY's sign and clamps every event to a fixed step (5% above 15% scale, with a hard floor of1.05):A mouse wheel emits one event per notch, so a fixed 5% step feels right. But a trackpad pinch emits many small high-frequency wheel events, and each one applies a full step → the canvas rockets in/out. Lowering
factorcannot fix it because values below1.05are floored.This PR adds an opt-in
factorByDeltaoption. When enabled, the zoom step is proportional to the wheel delta:A standard ~100px notch equals one
factormultiplication (classic mouse-wheel feel preserved), while small trackpad deltas produce smooth, proportionally small steps. It consumes the per-frame accumulated delta thatMouseWheelHandlealready computes viarequestAnimationFrameand passes to the callback but the handler previously discarded.The default is
false, so existing behavior is unchanged — the legacy quantized path is byte-for-byte identical.Related Testing
Extended
__tests__/graph/mousewheel.spec.tswith afactorByDeltasuite:1.2 ** (-delta/100))deltaYbatched byMouseWheelHandleAll 12 tests pass;
tsc --noEmitis clean; the change adds no new lint findings tosrc/graph/mousewheel.ts.User Tips
New optional
MouseWheelOptions.factorByDelta?: boolean(defaultfalse). Opt in for trackpad-friendly, proportional zooming:Docs updated in
site/docs/api/graph/mousewheel.{en,zh}.md.