Justin-ZS commented on code in PR #21588:
URL: https://github.com/apache/echarts/pull/21588#discussion_r3145334917
##########
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:
I’d decide this based on whether the “no restriction” state needs any
distinct runtime behavior.
If `null` / `undefined` only means “clear this option and use the default
no-restriction behavior”, then `NullUndefined` feels like the better fit. The
option type can accept it, and the effective runtime/model value can still be
normalized to `WheelAxisType | undefined`, where `undefined` means no
restriction. In that case, `null` does not need to add any special logic beyond
normalization.
If we need to distinguish an explicit “all axes” state from an
omitted/cleared option in runtime logic, merging, serialization, or docs, then
a concrete literal such as `'all'` would be more appropriate, because it
represents a real semantic state rather than just absence/clearing.
For this PR, I don’t see a separate behavior for explicit “all” yet, so I’d
lean toward allowing `NullUndefined` if we need a field-present no-restriction
shape, but not adding a new public `'all'` literal for now.
--
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]
