Justin-ZS commented on code in PR #21588:
URL: https://github.com/apache/echarts/pull/21588#discussion_r3165255000
##########
src/component/helper/RoamController.ts:
##########
@@ -362,43 +392,43 @@ class RoamController extends
Eventful<RoamEventDefinition> {
const shouldZoom = isAvailableBehavior('zoomOnMouseWheel', e,
this._opt);
const shouldMove = isAvailableBehavior('moveOnMouseWheel', e,
this._opt);
const wheelDelta = e.wheelDelta;
- const absWheelDeltaDelta = Math.abs(wheelDelta);
- const originX = e.offsetX;
- const originY = e.offsetY;
-
// wheelDelta maybe -0 in chrome mac.
if (wheelDelta === 0 || (!shouldZoom && !shouldMove)) {
return;
}
- // If both `shouldZoom` and `shouldMove` is true, trigger
- // their event both, and the final behavior is determined
- // by event listener themselves.
+ // Reach through to the native WheelEvent for per-axis deltas;
+ // zrender's `wheelDelta` collapses the two axes into a single scalar.
+ // Pre-2013 IE has no `deltaY`; the axis helpers fall back to the
+ // passed default in that case.
+ const nativeEvent = (e.event as unknown as WheelEvent) || null;
+ const originX = e.offsetX;
+ const originY = e.offsetY;
if (shouldZoom) {
- // Convenience:
- // Mac and VM Windows on Mac: scroll up: zoom out.
- // Windows: scroll up: zoom in.
-
- // FIXME: Should do more test in different environment.
- // wheelDelta is too complicated in difference nvironment
- //
(https://developer.mozilla.org/en-US/docs/Web/Events/mousewheel),
- // although it has been normallized by zrender.
- // wheelDelta of mouse wheel is bigger than touch pad.
- const factor = absWheelDeltaDelta > 3 ? 1.4 : absWheelDeltaDelta >
1 ? 1.2 : 1.1;
- const scale = wheelDelta > 0 ? factor : 1 / factor;
+ // `scaleY` defaults to `scale` on the IE fallback so the single
+ // scalar still drives zoom; `scaleX` defaults to identity so
+ // `zoomOnMouseWheelAxis: 'horizontal'` becomes a safe no-op.
+ const scale = ladderZoomScale(wheelDelta);
this._checkTriggerMoveZoom(this, 'zoom', 'zoomOnMouseWheel', e, {
- scale: scale, originX: originX, originY: originY,
isAvailableBehavior: null
+ scale: scale,
+ scaleX: axisZoomScale(nativeEvent, 'horizontal', 1),
Review Comment:
Thanks, `5c92148` addresses the aggregation edge case I was worried about.
Scoping each axis merge to dataZooms where the matching wheel behavior is
active keeps the controller-level guard aligned with the actual listeners.
The API shape also looks good to me as-is. I ran `npm run checktype` locally
and it passes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
