Copilot commented on code in PR #21655:
URL: https://github.com/apache/echarts/pull/21655#discussion_r3678884568
##########
src/component/helper/RoamController.ts:
##########
@@ -418,10 +435,52 @@ class RoamController extends
Eventful<RoamEventDefinition> {
) {
return;
}
- const scale = e.pinchScale > 1 ? 1.1 : 1 / 1.1;
- this._checkTriggerMoveZoom(this, 'zoom', null, e, {
- scale: scale, originX: e.pinchX, originY: e.pinchY,
isAvailableBehavior: null
- });
+ const originX = e.pinchX;
+ const originY = e.pinchY;
+
+ // Gate only the beginning of a pinch capture.
+ // Once captured, keep handling movement outside the roam area.
+ // Requiring a native touchstart for a new capture also
+ // prevents another RoamController from picking up an already-moving
pinch.
+ const isTouchStart = e.event.type === 'touchstart';
+ const isPinchStart = !this._pinching || isTouchStart;
+ if (isPinchStart) {
+ if (!isTouchStart || !this._checkPointer(e, originX, originY)) {
+ return;
+ }
+ }
Review Comment:
Pinch capture currently requires `e.event.type === 'touchstart'` on the
first emitted `pinch` event. In ZRender, `GestureMgr` emits the first `pinch`
gesture on `touchmove` (because it needs both a previous and current 2-touch
sample to compute `pinchScale`), so this early-return prevents pinch from ever
being captured/handled.
##########
src/component/helper/RoamController.ts:
##########
@@ -195,16 +205,20 @@ class RoamController extends
Eventful<RoamEventDefinition> {
this.disable();
this._enabled = true;
- if (controlType === true || (controlType === 'move' ||
controlType === 'pan')) {
+ if (moveEnabled) {
addRoamZrListener(zr, 'mousedown', mousedownHandler,
zInfoParsed);
addRoamZrListener(zr, 'mousemove', mousemoveHandler,
zInfoParsed);
addRoamZrListener(zr, 'mouseup', mouseupHandler,
zInfoParsed);
}
- if (controlType === true || (controlType === 'scale' ||
controlType === 'zoom')) {
+ if (zoomEnabled) {
addRoamZrListener(zr, 'mousewheel', mousewheelHandler,
zInfoParsed);
+ }
+ if (moveEnabled || zoomEnabled) {
addRoamZrListener(zr, 'pinch', pinchHandler, zInfoParsed);
}
}
+ this._moveEnabled = moveEnabled;
+ this._zoomEnabled = zoomEnabled;
Review Comment:
`this._controlType` is used in the idempotency check (`this._controlType !==
controlType`) but is never assigned anywhere in the class, so `enable()` will
always tear down and re-add listeners even when repeatedly called with the same
controlType. Assign `_controlType` in `enable()` so the optimization actually
works.
--
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]
