From 3a8bde26ac78bbbde3dfe13ffe44858bd09a7f9d Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Thu, 28 Mar 2019 12:43:59 -0700 Subject: [PATCH 01/15] Pass empty touches list in simulated touchend/touchcancel to reflect how we usually expect these events to look --- test/node_modules/mapbox-gl-js-test/simulate_interaction.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/node_modules/mapbox-gl-js-test/simulate_interaction.js b/test/node_modules/mapbox-gl-js-test/simulate_interaction.js index feb462b4c3f..7d309a8b1a6 100644 --- a/test/node_modules/mapbox-gl-js-test/simulate_interaction.js +++ b/test/node_modules/mapbox-gl-js-test/simulate_interaction.js @@ -62,7 +62,8 @@ exports.magicWheelZoomDelta = 4.000244140625; [ 'touchstart', 'touchend', 'touchmove', 'touchcancel' ].forEach((event) => { exports[event] = function (target, options) { // Should be using Touch constructor here, but https://github.com/jsdom/jsdom/issues/2152. - options = Object.assign({bubbles: true, touches: [{clientX: 0, clientY: 0}]}, options); + const defaultTouches = event.endsWith('end') || event.endsWith('cancel') ? [] : [{clientX: 0, clientY: 0}]; + options = Object.assign({bubbles: true, touches: defaultTouches}, options); const TouchEvent = window(target).TouchEvent; target.dispatchEvent(new TouchEvent(event, options)); }; From f01f27d8e15e7f4f4d43eddccfcd8c8d09823e8d Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Wed, 27 Mar 2019 18:15:07 -0700 Subject: [PATCH 02/15] Improve single vs. multi-touch interaction in dragPan handler, add tests - Stop an active single-touch drag if the user adds additional fingers and touchZoomRotate is enabled, so the latter can take over interaction - Start a single-touch drag if the user removes all but one finger - Add tests for expected single vs. multi-touch interactions --- src/ui/handler/drag_pan.js | 48 ++++++- test/unit/ui/handler/drag_pan.test.js | 185 ++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 3 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index 9d78e877d71..2aa50b09c9f 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -47,6 +47,7 @@ class DragPanHandler { '_onMove', '_onMouseUp', '_onTouchEnd', + '_onMultiTouchEnd', '_onBlur', '_onDragFrame' ], this); @@ -125,8 +126,18 @@ class DragPanHandler { } onTouchStart(e: TouchEvent) { - if (this._state !== 'enabled') return; - if (e.touches.length > 1) return; + if (!this.isEnabled()) return; + if (e.touches && e.touches.length > 1) { // multi-finger touch + // Listen for the multi-touch end in case it's reduced to a single touch + DOM.addEventListener(window.document, 'touchend', this._onMultiTouchEnd); + + // If we're already dragging with one finger and the user adds another finger(s), + // let TouchZoomRotateHandler handler take over if it's enabled by stopping the drag + if ((this._state === 'pending' || this._state === 'active') && this._map.touchZoomRotate.isEnabled()) { + this._onMultiTouchStart(e); + } + return; + } // Bind window-level event listeners for touchmove/end events. In the absence of // the pointer capture API, which is not supported by all necessary platforms, @@ -231,7 +242,29 @@ class DragPanHandler { } } - _onBlur(e: FocusEvent) { + _onMultiTouchEnd(e: TouchEvent) { + // A multi-finger touch is ending; decide whether to activate dragging + + if (e.touches && e.touches.length > 1) return; // there are still multiple fingers touching; wait + + DOM.removeEventListener(window.document, 'touchend', this._onMultiTouchEnd); + + if (!e.touches || e.touches.length === 0) return; // all fingers were removed at once; nothing left to do + + // Back to single-touch (e.touches.length === 1); treat this like a touchstart & activate drag + + // Bind window-level event listeners for touchmove/end events. In the absence of + // the pointer capture API, which is not supported by all necessary platforms, + // window-level event listeners give us the best shot at capturing events that + // fall outside the map canvas element. Use `{capture: true}` for the move event + // to prevent map move events from being fired during a drag. + DOM.addEventListener(window.document, 'touchmove', this._onMove, {capture: true, passive: false}); + DOM.addEventListener(window.document, 'touchend', this._onTouchEnd); + + this._start(e); + } + + _abort(e: FocusEvent | TouchEvent) { switch (this._state) { case 'active': this._state = 'enabled'; @@ -250,6 +283,15 @@ class DragPanHandler { } } + _onMultiTouchStart(e: TouchEvent) { + // Additional fingers are now touching; treat this almost like a touchend & stop dragging, + this._abort(e); + } + + _onBlur(e: FocusEvent) { + this._abort(e); + } + _unbind() { DOM.removeEventListener(window.document, 'touchmove', this._onMove, {capture: true, passive: false}); DOM.removeEventListener(window.document, 'touchend', this._onTouchEnd); diff --git a/test/unit/ui/handler/drag_pan.test.js b/test/unit/ui/handler/drag_pan.test.js index 97f18ef7870..c41f3ae2c21 100644 --- a/test/unit/ui/handler/drag_pan.test.js +++ b/test/unit/ui/handler/drag_pan.test.js @@ -4,6 +4,7 @@ import Map from '../../../../src/ui/map'; import DOM from '../../../../src/util/dom'; import simulate from 'mapbox-gl-js-test/simulate_interaction'; + function createMap(t, clickTolerance) { t.stub(Map.prototype, '_detectMissingCSS'); return new Map({ container: DOM.create('div', '', window.document.body), clickTolerance: clickTolerance || 0 }); @@ -741,3 +742,187 @@ test('DragPanHandler does not begin a touch drag if moved less than click tolera map.remove(); t.end(); }); + + +test('DragPanHandler does not begin a touch drag on multi-finger touchstart event', (t) => { + const map = createMap(t); + + const dragstart = t.spy(); + const drag = t.spy(); + const dragend = t.spy(); + + map.on('dragstart', dragstart); + map.on('drag', drag); + map.on('dragend', dragend); + + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}, {clientX: 20, clientY: 20}]}); + map._renderTaskQueue.run(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); + map._renderTaskQueue.run(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.touchend(map.getCanvas()); + map._renderTaskQueue.run(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + map.remove(); + t.end(); +}); + + +test('DragPanHandler ends a touch-triggered drag if a second touch is started & touchZoomRotate is enabled', (t) => { + const map = createMap(t); + map.touchZoomRotate.enable(); + + const dragend = t.spy(); + map.on('dragend', dragend); + + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}]}); + map._renderTaskQueue.run(); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); + map._renderTaskQueue.run(); + t.ok(map.dragPan.isActive()); + + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.equal(dragend.callCount, 1); + + map.remove(); + t.end(); +}); + +test('DragPanHandler does not end a touch-triggered drag if a second touch is started but touchZoomRotate is disabled', (t) => { + const map = createMap(t); + map.touchZoomRotate.disable(); + + const dragend = t.spy(); + map.on('dragend', dragend); + + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}]}); + map._renderTaskQueue.run(); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); + map._renderTaskQueue.run(); + t.ok(map.dragPan.isActive()); + + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); + map._renderTaskQueue.run(); + t.ok(map.dragPan.isActive()); + t.equal(dragend.callCount, 0); + + map.remove(); + t.end(); +}); + +test('DragPanHandler starts a touch-triggered drag if a multi-finger touch becomes a single-finger touch', (t) => { + const map = createMap(t); + + const dragstart = t.spy(); + map.on('dragstart', dragstart); + + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.notOk(dragstart.called); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}, {clientX: 40, clientY: 40}]}); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.notOk(dragstart.called); + + simulate.touchend(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}]}); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.equals(map.dragPan._state, 'pending'); + t.notOk(dragstart.called); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); + map._renderTaskQueue.run(); + t.ok(map.dragPan.isActive()); + t.equal(dragstart.callCount, 1); + + map.remove(); + t.end(); +}); + + + + + +test('DragPanHandler stops/starts touch-triggered drag appropriately when transitioning between single- and multi-finger touch', (t) => { + const map = createMap(t); + + const dragstart = t.spy(); + const drag = t.spy(); + const dragend = t.spy(); + + map.on('dragstart', dragstart); + map.on('drag', drag); + map.on('dragend', dragend); + + // Single-finger touch starts drag + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}]}); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); + map._renderTaskQueue.run(); + t.ok(map.dragPan.isActive()); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); + t.equal(dragend.callCount, 0); + + // Adding a second finger stops drag (will trigger touchZoomRotate instead) + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}, {clientX: 20, clientY: 20}]}); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); + t.equal(dragend.callCount, 1); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); + t.equal(dragend.callCount, 1); + + // Removing all but one finger starts another drag + simulate.touchend(map.getCanvas(), {touches: [{clientX: 30, clientY: 30}]}); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); + t.equal(dragend.callCount, 1); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); + map._renderTaskQueue.run(); + t.ok(map.dragPan.isActive()); + t.equal(dragstart.callCount, 2); + t.equal(drag.callCount, 2); + t.equal(dragend.callCount, 1); + + // Removing last finger stops drag + simulate.touchend(map.getCanvas()); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.equal(dragstart.callCount, 2); + t.equal(drag.callCount, 2); + t.equal(dragend.callCount, 2); + + map.remove(); + t.end(); +}); From 7237386e914832c6ff15ca47cfff68026b3bbdc2 Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Mon, 26 Aug 2019 17:33:32 -0700 Subject: [PATCH 03/15] Update/add drag pan tests (failing) to catch two-finger-pan bug Additional tests/cases to capture the fact that we only expect touchZoomRotateHandler to take over once we've actually begun zoom/rotate gesture; two-finger straight pan should still be handled by dragPanHandler. See comment on #8100 for more details. Tests currently failing. --- test/unit/ui/handler/drag_pan.test.js | 73 +++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 9 deletions(-) diff --git a/test/unit/ui/handler/drag_pan.test.js b/test/unit/ui/handler/drag_pan.test.js index c41f3ae2c21..613075d08c5 100644 --- a/test/unit/ui/handler/drag_pan.test.js +++ b/test/unit/ui/handler/drag_pan.test.js @@ -743,8 +743,7 @@ test('DragPanHandler does not begin a touch drag if moved less than click tolera t.end(); }); - -test('DragPanHandler does not begin a touch drag on multi-finger touchstart event', (t) => { +test('DragPanHandler does not begin a touch drag on multi-finger touch event if zooming', (t) => { const map = createMap(t); const dragstart = t.spy(); @@ -755,13 +754,13 @@ test('DragPanHandler does not begin a touch drag on multi-finger touchstart even map.on('drag', drag); map.on('dragend', dragend); - simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}, {clientX: 20, clientY: 20}]}); + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 0}, {clientX: 20, clientY: 0}]}); map._renderTaskQueue.run(); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); - simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 5, clientY: 10}, {clientX: 25, clientY: 10}]}); map._renderTaskQueue.run(); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); @@ -885,23 +884,39 @@ test('DragPanHandler stops/starts touch-triggered drag appropriately when transi t.equal(drag.callCount, 1); t.equal(dragend.callCount, 0); - // Adding a second finger stops drag (will trigger touchZoomRotate instead) + // Adding a second finger and panning (without zoom/rotate) continues the drag simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}, {clientX: 20, clientY: 20}]}); map._renderTaskQueue.run(); - t.notOk(map.dragPan.isActive()); + t.ok(map.dragPan.isActive()); t.equal(dragstart.callCount, 1); t.equal(drag.callCount, 1); + t.equal(dragend.callCount, 0); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 10, clientY: 20}, {clientX: 20, clientY: 30}]}); + map._renderTaskQueue.run(); + t.ok(map.dragPan.isActive()); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2); + t.equal(dragend.callCount, 0); + + // Starting a two-finger zoom/rotate stops drag (will trigger touchZoomRotate instead) + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 10, clientY: 20}, {clientX: 30, clientY: 30}]}); + map._renderTaskQueue.run(); + t.notOk(map.dragPan.isActive()); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2); t.equal(dragend.callCount, 1); - simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); + // Continuing to pan with two fingers does not start a drag (handled by touchZoomRotate instead) + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}, {clientX: 30, clientY: 20}]}); map._renderTaskQueue.run(); t.notOk(map.dragPan.isActive()); t.equal(dragstart.callCount, 1); - t.equal(drag.callCount, 1); + t.equal(drag.callCount, 2); t.equal(dragend.callCount, 1); // Removing all but one finger starts another drag - simulate.touchend(map.getCanvas(), {touches: [{clientX: 30, clientY: 30}]}); + simulate.touchend(map.getCanvas(), {touches: [{clientX: 30, clientY: 20}]}); map._renderTaskQueue.run(); t.notOk(map.dragPan.isActive()); t.equal(dragstart.callCount, 1); @@ -926,3 +941,43 @@ test('DragPanHandler stops/starts touch-triggered drag appropriately when transi map.remove(); t.end(); }); + + +test('DragPanHandler fires dragstart, drag, dragend events in response to multi-touch pan', (t) => { + const map = createMap(t); + + const dragstart = t.spy(); + const drag = t.spy(); + const dragend = t.spy(); + + map.on('dragstart', dragstart); + map.on('drag', drag); + map.on('dragend', dragend); + + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 0, clientY: 0}, {clientX: 5, clientY: 0}]}); + map._renderTaskQueue.run(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 0, clientY: 10}, {clientX: 5, clientY: 10}]}); + map._renderTaskQueue.run(); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); + t.equal(dragend.callCount, 0); + + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 0, clientY: 5}, {clientX: 5, clientY: 5}]}); + map._renderTaskQueue.run(); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2); + t.equal(dragend.callCount, 0); + + simulate.touchend(map.getCanvas(), {touches: []}); + map._renderTaskQueue.run(); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2); + t.equal(dragend.callCount, 1); + + map.remove(); + t.end(); +}); From ea0e416530590251a4555072b6c5dedd96aae295 Mon Sep 17 00:00:00 2001 From: Arindam Bose Date: Tue, 27 Aug 2019 11:57:13 -0700 Subject: [PATCH 04/15] Defer drag_pan disabling to requestAnimationFrame Co-authored-by: Anjana Vakil --- src/ui/handler/drag_pan.js | 34 ++++++++++++++++------------- src/ui/handler/touch_zoom_rotate.js | 11 +++++++++- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index 2aa50b09c9f..08eab50b399 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -129,13 +129,8 @@ class DragPanHandler { if (!this.isEnabled()) return; if (e.touches && e.touches.length > 1) { // multi-finger touch // Listen for the multi-touch end in case it's reduced to a single touch + DOM.removeEventListener(window.document, 'touchend', this._onTouchEnd); DOM.addEventListener(window.document, 'touchend', this._onMultiTouchEnd); - - // If we're already dragging with one finger and the user adds another finger(s), - // let TouchZoomRotateHandler handler take over if it's enabled by stopping the drag - if ((this._state === 'pending' || this._state === 'active') && this._map.touchZoomRotate.isEnabled()) { - this._onMultiTouchStart(e); - } return; } @@ -173,14 +168,6 @@ class DragPanHandler { this._drainInertiaBuffer(); this._inertia.push([browser.now(), this._lastPos]); - if (this._state === 'pending') { - // we treat the first move event (rather than the mousedown event) - // as the start of the drag - this._state = 'active'; - this._fireEvent('dragstart', e); - this._fireEvent('movestart', e); - } - if (!this._frameId) { this._frameId = this._map._requestRenderFrame(this._onDragFrame); } @@ -193,8 +180,21 @@ class DragPanHandler { _onDragFrame() { this._frameId = null; + if (this._map.touchZoomRotate.isActive()) { + this._abort(); + return; + } + const e = this._lastMoveEvent; if (!e) return; + + if (this._state === 'pending') { + // we treat the first move event (rather than the mousedown event) + // as the start of the drag + this._state = 'active'; + this._fireEvent('dragstart', e); + this._fireEvent('movestart', e); + } const tr = this._map.transform; tr.setLocationAtPoint(tr.pointLocation(this._startPos), this._lastPos); this._fireEvent('drag', e); @@ -249,7 +249,11 @@ class DragPanHandler { DOM.removeEventListener(window.document, 'touchend', this._onMultiTouchEnd); - if (!e.touches || e.touches.length === 0) return; // all fingers were removed at once; nothing left to do + // all fingers were removed at once; nothing left to do + if (!e.touches || e.touches.length === 0) { + this._onTouchEnd(e); + return; + } // Back to single-touch (e.touches.length === 1); treat this like a touchstart & activate drag diff --git a/src/ui/handler/touch_zoom_rotate.js b/src/ui/handler/touch_zoom_rotate.js index 0f2ab959d81..404721c30d1 100644 --- a/src/ui/handler/touch_zoom_rotate.js +++ b/src/ui/handler/touch_zoom_rotate.js @@ -112,6 +112,16 @@ class TouchZoomRotateHandler { this._rotationDisabled = false; } + /** + * Returns true if the handler is enabled and has detected the start of a zoom/rotate gesture. + * + * @returns {boolean} + * @memberof TouchZoomRotateHandler + */ + isActive(): boolean { + return this.isEnabled() && !!this._gestureIntent; + } + onStart(e: TouchEvent) { if (!this.isEnabled()) return; if (e.touches.length !== 2) return; @@ -197,7 +207,6 @@ class TouchZoomRotateHandler { } tr.zoom = tr.scaleZoom(this._startScale * scale); - tr.setLocationAtPoint(this._startAround, aroundPoint); this._map.fire(new Event(gestureIntent, {originalEvent: this._lastTouchEvent})); From 2f2967bcc36ab17070b484b64a7cd4c2a1edaf8e Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Wed, 28 Aug 2019 15:53:01 -0700 Subject: [PATCH 05/15] Consolidate touchend handlers Co-authored-by: Arindam Bose --- src/ui/handler/drag_pan.js | 67 ++++++++++---------------------------- 1 file changed, 18 insertions(+), 49 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index 08eab50b399..2a7d8db20e8 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -47,7 +47,6 @@ class DragPanHandler { '_onMove', '_onMouseUp', '_onTouchEnd', - '_onMultiTouchEnd', '_onBlur', '_onDragFrame' ], this); @@ -128,10 +127,9 @@ class DragPanHandler { onTouchStart(e: TouchEvent) { if (!this.isEnabled()) return; if (e.touches && e.touches.length > 1) { // multi-finger touch - // Listen for the multi-touch end in case it's reduced to a single touch - DOM.removeEventListener(window.document, 'touchend', this._onTouchEnd); - DOM.addEventListener(window.document, 'touchend', this._onMultiTouchEnd); - return; + // If we are already dragging (e.g. with one finger) and add another finger, + // keep the handler active but don't attempt to ._start() again + if (this._state === 'pending' || this._state === 'active') return; } // Bind window-level event listeners for touchmove/end events. In the absence of @@ -225,47 +223,23 @@ class DragPanHandler { } _onTouchEnd(e: TouchEvent) { + if (!e.touches || e.touches.length === 0) { // only stop drag if all fingers have been removed switch (this._state) { - case 'active': - this._state = 'enabled'; - this._unbind(); - this._deactivate(); - this._inertialPan(e); - break; - case 'pending': - this._state = 'enabled'; - this._unbind(); - break; - default: - assert(false); - break; + case 'active': + this._state = 'enabled'; + this._unbind(); + this._deactivate(); + this._inertialPan(e); + break; + case 'pending': + this._state = 'enabled'; + this._unbind(); + break; + default: + assert(false); + break; } - } - - _onMultiTouchEnd(e: TouchEvent) { - // A multi-finger touch is ending; decide whether to activate dragging - - if (e.touches && e.touches.length > 1) return; // there are still multiple fingers touching; wait - - DOM.removeEventListener(window.document, 'touchend', this._onMultiTouchEnd); - - // all fingers were removed at once; nothing left to do - if (!e.touches || e.touches.length === 0) { - this._onTouchEnd(e); - return; - } - - // Back to single-touch (e.touches.length === 1); treat this like a touchstart & activate drag - - // Bind window-level event listeners for touchmove/end events. In the absence of - // the pointer capture API, which is not supported by all necessary platforms, - // window-level event listeners give us the best shot at capturing events that - // fall outside the map canvas element. Use `{capture: true}` for the move event - // to prevent map move events from being fired during a drag. - DOM.addEventListener(window.document, 'touchmove', this._onMove, {capture: true, passive: false}); - DOM.addEventListener(window.document, 'touchend', this._onTouchEnd); - - this._start(e); + } } _abort(e: FocusEvent | TouchEvent) { @@ -287,11 +261,6 @@ class DragPanHandler { } } - _onMultiTouchStart(e: TouchEvent) { - // Additional fingers are now touching; treat this almost like a touchend & stop dragging, - this._abort(e); - } - _onBlur(e: FocusEvent) { this._abort(e); } From 4fb4c6ccc9b25877891d708c2366e8a66b23ecaf Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Wed, 28 Aug 2019 15:53:51 -0700 Subject: [PATCH 06/15] Update tests for new drag interaction behavior Co-authored-by: Arindam Bose --- test/unit/ui/handler/drag_pan.test.js | 75 ++++++--------------------- 1 file changed, 16 insertions(+), 59 deletions(-) diff --git a/test/unit/ui/handler/drag_pan.test.js b/test/unit/ui/handler/drag_pan.test.js index 613075d08c5..5b3a6515a20 100644 --- a/test/unit/ui/handler/drag_pan.test.js +++ b/test/unit/ui/handler/drag_pan.test.js @@ -777,87 +777,44 @@ test('DragPanHandler does not begin a touch drag on multi-finger touch event if }); -test('DragPanHandler ends a touch-triggered drag if a second touch is started & touchZoomRotate is enabled', (t) => { - const map = createMap(t); - map.touchZoomRotate.enable(); - - const dragend = t.spy(); - map.on('dragend', dragend); - - simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}]}); - map._renderTaskQueue.run(); - - simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); - map._renderTaskQueue.run(); - t.ok(map.dragPan.isActive()); - - simulate.touchstart(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); - map._renderTaskQueue.run(); - t.notOk(map.dragPan.isActive()); - t.equal(dragend.callCount, 1); - - map.remove(); - t.end(); -}); - -test('DragPanHandler does not end a touch-triggered drag if a second touch is started but touchZoomRotate is disabled', (t) => { - const map = createMap(t); - map.touchZoomRotate.disable(); - - const dragend = t.spy(); - map.on('dragend', dragend); - - simulate.touchstart(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}]}); - map._renderTaskQueue.run(); - - simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); - map._renderTaskQueue.run(); - t.ok(map.dragPan.isActive()); - - simulate.touchstart(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); - map._renderTaskQueue.run(); - t.ok(map.dragPan.isActive()); - t.equal(dragend.callCount, 0); - - map.remove(); - t.end(); -}); - -test('DragPanHandler starts a touch-triggered drag if a multi-finger touch becomes a single-finger touch', (t) => { +test('DragPanHandler starts a drag on a multi-finger no-zoom touch, and continues if it becomes a single-finger touch', (t) => { const map = createMap(t); const dragstart = t.spy(); map.on('dragstart', dragstart); + const drag = t.spy(); + map.on('drag', drag); + simulate.touchstart(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}, {clientX: 30, clientY: 30}]}); map._renderTaskQueue.run(); t.notOk(map.dragPan.isActive()); + t.equals(map.dragPan._state, 'pending'); t.notOk(dragstart.called); + t.notOk(drag.called); simulate.touchmove(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}, {clientX: 40, clientY: 40}]}); map._renderTaskQueue.run(); - t.notOk(map.dragPan.isActive()); - t.notOk(dragstart.called); + t.ok(map.dragPan.isActive()); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); simulate.touchend(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}]}); map._renderTaskQueue.run(); - t.notOk(map.dragPan.isActive()); - t.equals(map.dragPan._state, 'pending'); - t.notOk(dragstart.called); + t.ok(map.dragPan.isActive()); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); map._renderTaskQueue.run(); t.ok(map.dragPan.isActive()); t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2); map.remove(); t.end(); }); - - - - test('DragPanHandler stops/starts touch-triggered drag appropriately when transitioning between single- and multi-finger touch', (t) => { const map = createMap(t); @@ -920,14 +877,14 @@ test('DragPanHandler stops/starts touch-triggered drag appropriately when transi map._renderTaskQueue.run(); t.notOk(map.dragPan.isActive()); t.equal(dragstart.callCount, 1); - t.equal(drag.callCount, 1); + t.equal(drag.callCount, 2); t.equal(dragend.callCount, 1); simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); map._renderTaskQueue.run(); t.ok(map.dragPan.isActive()); t.equal(dragstart.callCount, 2); - t.equal(drag.callCount, 2); + t.equal(drag.callCount, 3); t.equal(dragend.callCount, 1); // Removing last finger stops drag @@ -935,7 +892,7 @@ test('DragPanHandler stops/starts touch-triggered drag appropriately when transi map._renderTaskQueue.run(); t.notOk(map.dragPan.isActive()); t.equal(dragstart.callCount, 2); - t.equal(drag.callCount, 2); + t.equal(drag.callCount, 3); t.equal(dragend.callCount, 2); map.remove(); From 2d59350a897d0f4ec4a6993786539229d5748d1f Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Wed, 28 Aug 2019 16:54:24 -0700 Subject: [PATCH 07/15] Update handler state on move event, but defer dragstart To account for the fact that move event handlers might be inspecting the DragPanHandler's (in)active state, update the state in the onMove handler rather than in onDragFrame. However, don't fire the dragstart/movestart events immediately on the move event as we had been doing before; schedule these to be fired on the first drag frame. Co-authored-by: Arindam Bose --- src/ui/handler/drag_pan.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index 2a7d8db20e8..f332727ce5d 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -31,6 +31,7 @@ class DragPanHandler { _inertia: Array<[number, Point]>; _frameId: ?TaskID; _clickTolerance: number; + _shouldStart: ?boolean; /** * @private @@ -166,6 +167,11 @@ class DragPanHandler { this._drainInertiaBuffer(); this._inertia.push([browser.now(), this._lastPos]); + if (this._state === 'pending') { + this._state = 'active'; + this._shouldStart = true; + } + if (!this._frameId) { this._frameId = this._map._requestRenderFrame(this._onDragFrame); } @@ -186,12 +192,12 @@ class DragPanHandler { const e = this._lastMoveEvent; if (!e) return; - if (this._state === 'pending') { - // we treat the first move event (rather than the mousedown event) - // as the start of the drag - this._state = 'active'; - this._fireEvent('dragstart', e); - this._fireEvent('movestart', e); + if (this._shouldStart) { + // we treat the first drag frame (rather than the mousedown event) + // as the start of the drag + this._fireEvent('dragstart', e); + this._fireEvent('movestart', e); + this._shouldStart = false; } const tr = this._map.transform; tr.setLocationAtPoint(tr.pointLocation(this._startPos), this._lastPos); From 709d4a3ae52b96ac5ef097a14fe63b6b3fc3cb2e Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Wed, 28 Aug 2019 16:58:06 -0700 Subject: [PATCH 08/15] Account for disabling handler on dragstart Co-authored-by: Arindam Bose --- src/ui/handler/drag_pan.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index f332727ce5d..2a79b93ef30 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -199,6 +199,9 @@ class DragPanHandler { this._fireEvent('movestart', e); this._shouldStart = false; } + + if (!this.isActive()) return; // It's possible for the dragstart event to trigger a disable() call (#2419) so we must account for that + const tr = this._map.transform; tr.setLocationAtPoint(tr.pointLocation(this._startPos), this._lastPos); this._fireEvent('drag', e); From 9643f6675cd8c228471f9801b5db9d9efdd4aecc Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Fri, 30 Aug 2019 10:22:19 -0700 Subject: [PATCH 09/15] Don't fire dragend if we never fired dragstart, reset ._shouldStart on .deactivate() --- src/ui/handler/drag_pan.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index 2a79b93ef30..76e01a37a4e 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -257,8 +257,11 @@ class DragPanHandler { this._state = 'enabled'; this._unbind(); this._deactivate(); - this._fireEvent('dragend', e); - this._fireEvent('moveend', e); + if (!this._shouldStart) { // If we scheduled the dragstart but never fired, nothing to end + // We already started the drag, end it + this._fireEvent('dragend', e); + this._fireEvent('moveend', e); + } break; case 'pending': this._state = 'enabled'; @@ -291,6 +294,7 @@ class DragPanHandler { delete this._startPos; delete this._mouseDownPos; delete this._lastPos; + // delete this._shouldStart; } _inertialPan(e: MouseEvent | TouchEvent) { From 3fc41d3fcd3e167748bd076e13c28090239c65a3 Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Fri, 30 Aug 2019 10:27:07 -0700 Subject: [PATCH 10/15] Fix multi-finger no-zoom test --- test/unit/ui/handler/drag_pan.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/ui/handler/drag_pan.test.js b/test/unit/ui/handler/drag_pan.test.js index 5b3a6515a20..bc3ddba47d9 100644 --- a/test/unit/ui/handler/drag_pan.test.js +++ b/test/unit/ui/handler/drag_pan.test.js @@ -793,7 +793,7 @@ test('DragPanHandler starts a drag on a multi-finger no-zoom touch, and continue t.notOk(dragstart.called); t.notOk(drag.called); - simulate.touchmove(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}, {clientX: 40, clientY: 40}]}); + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 10, clientY: 10}, {clientX: 20, clientY: 20}]}); map._renderTaskQueue.run(); t.ok(map.dragPan.isActive()); t.equal(dragstart.callCount, 1); @@ -805,7 +805,7 @@ test('DragPanHandler starts a drag on a multi-finger no-zoom touch, and continue t.equal(dragstart.callCount, 1); t.equal(drag.callCount, 1); - simulate.touchmove(map.getCanvas(), {touches: [{clientX: 20, clientY: 20}]}); + simulate.touchmove(map.getCanvas(), {touches: [{clientX: 0, clientY: 0}]}); map._renderTaskQueue.run(); t.ok(map.dragPan.isActive()); t.equal(dragstart.callCount, 1); From 7dc031d7b022f9ff5d2726c2b0e2a2b0bf65dd8e Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Mon, 9 Sep 2019 16:49:34 -0700 Subject: [PATCH 11/15] Fix DragPanHandler behavior on touch end/start --- src/ui/handler/drag_pan.js | 61 ++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index 76e01a37a4e..d5aa5aa5dfb 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -27,6 +27,8 @@ class DragPanHandler { _startPos: Point; _mouseDownPos: Point; _lastPos: Point; + _startTouch: Array | void; + _lastTouch: Array | void; _lastMoveEvent: MouseEvent | TouchEvent | void; _inertia: Array<[number, Point]>; _frameId: ?TaskID; @@ -151,19 +153,33 @@ class DragPanHandler { this._state = 'pending'; this._startPos = this._mouseDownPos = this._lastPos = DOM.mousePos(this._el, e); + this._startTouch = this._lastTouch = e.touches ? DOM.touchPos(this._el, e) : null ; this._inertia = [[browser.now(), this._startPos]]; } + _touchesMatch(lastTouch: Array, thisTouch: Array) { + if (lastTouch.length !== thisTouch.length) return false; + for (let i = 0; i < lastTouch.length; i++) { + if (!lastTouch[i].equals(thisTouch[i])) return false; + } + return true; + } + _onMove(e: MouseEvent | TouchEvent) { e.preventDefault(); + const touchPos = e.touches ? DOM.touchPos(this._el, e) : null; const pos = DOM.mousePos(this._el, e); - if (this._lastPos.equals(pos) || (this._state === 'pending' && pos.dist(this._mouseDownPos) < this._clickTolerance)) { + + const matchesLastPos = touchPos ? this._touchesMatch(this._lastTouch, touchPos) : this._lastPos.equals(pos); + + if (matchesLastPos || (this._state === 'pending' && pos.dist(this._mouseDownPos) < this._clickTolerance)) { return; } this._lastMoveEvent = e; this._lastPos = pos; + this._lastTouch = touchPos; this._drainInertiaBuffer(); this._inertia.push([browser.now(), this._lastPos]); @@ -184,14 +200,14 @@ class DragPanHandler { _onDragFrame() { this._frameId = null; - if (this._map.touchZoomRotate.isActive()) { - this._abort(); - return; - } - const e = this._lastMoveEvent; if (!e) return; + if (this._map.touchZoomRotate.isActive()) { + this._abort(e); + return; + } + if (this._shouldStart) { // we treat the first drag frame (rather than the mousedown event) // as the start of the drag @@ -244,10 +260,27 @@ class DragPanHandler { this._state = 'enabled'; this._unbind(); break; + case 'enabled': + this._unbind(); + break; default: assert(false); break; } + } else { // some finger(s) still touching the screen + switch (this._state) { + case 'pending': + case 'active': + // we are already dragging; continue + break; + case 'enabled': + // not currently dragging; get ready to start a new drag + this.onTouchStart(e); + break; + default: + assert(false); + break; + } } } @@ -255,18 +288,26 @@ class DragPanHandler { switch (this._state) { case 'active': this._state = 'enabled'; - this._unbind(); - this._deactivate(); if (!this._shouldStart) { // If we scheduled the dragstart but never fired, nothing to end // We already started the drag, end it this._fireEvent('dragend', e); this._fireEvent('moveend', e); } + this._unbind(); + this._deactivate(); + if (e.touches && e.touches.length > 1) { + // If there are multiple fingers touching, reattach touchend listner in case + // all but one finger is removed and we need to restart a drag on touchend + DOM.addEventListener(window.document, 'touchend', this._onTouchEnd); + } break; case 'pending': this._state = 'enabled'; this._unbind(); break; + case 'enabled': + this._unbind(); + break; default: assert(false); break; @@ -294,7 +335,9 @@ class DragPanHandler { delete this._startPos; delete this._mouseDownPos; delete this._lastPos; - // delete this._shouldStart; + delete this._startTouch; + delete this._lastTouch; + delete this._shouldStart; } _inertialPan(e: MouseEvent | TouchEvent) { From 620fe88535247f720d35ba4a55779a5d1f0e4bf9 Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Tue, 10 Sep 2019 18:15:16 -0700 Subject: [PATCH 12/15] Fix lint & flow errors --- src/ui/handler/drag_pan.js | 94 +++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index d5aa5aa5dfb..e1e74eb5041 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -27,8 +27,8 @@ class DragPanHandler { _startPos: Point; _mouseDownPos: Point; _lastPos: Point; - _startTouch: Array | void; - _lastTouch: Array | void; + _startTouch: ?Array; + _lastTouch: ?Array; _lastMoveEvent: MouseEvent | TouchEvent | void; _inertia: Array<[number, Point]>; _frameId: ?TaskID; @@ -153,16 +153,16 @@ class DragPanHandler { this._state = 'pending'; this._startPos = this._mouseDownPos = this._lastPos = DOM.mousePos(this._el, e); - this._startTouch = this._lastTouch = e.touches ? DOM.touchPos(this._el, e) : null ; + this._startTouch = this._lastTouch = e.touches ? DOM.touchPos(this._el, e) : null; this._inertia = [[browser.now(), this._startPos]]; } - _touchesMatch(lastTouch: Array, thisTouch: Array) { - if (lastTouch.length !== thisTouch.length) return false; - for (let i = 0; i < lastTouch.length; i++) { - if (!lastTouch[i].equals(thisTouch[i])) return false; - } - return true; + _touchesMatch(lastTouch: ?Array, thisTouch: ?Array) { + if (!lastTouch || !thisTouch || lastTouch.length !== thisTouch.length) return false; + for (let i = 0; i < lastTouch.length; i++) { + if (!lastTouch[i].equals(thisTouch[i])) return false; + } + return true; } _onMove(e: MouseEvent | TouchEvent) { @@ -184,8 +184,8 @@ class DragPanHandler { this._inertia.push([browser.now(), this._lastPos]); if (this._state === 'pending') { - this._state = 'active'; - this._shouldStart = true; + this._state = 'active'; + this._shouldStart = true; } if (!this._frameId) { @@ -204,16 +204,16 @@ class DragPanHandler { if (!e) return; if (this._map.touchZoomRotate.isActive()) { - this._abort(e); - return; + this._abort(e); + return; } if (this._shouldStart) { - // we treat the first drag frame (rather than the mousedown event) - // as the start of the drag - this._fireEvent('dragstart', e); - this._fireEvent('movestart', e); - this._shouldStart = false; + // we treat the first drag frame (rather than the mousedown event) + // as the start of the drag + this._fireEvent('dragstart', e); + this._fireEvent('movestart', e); + this._shouldStart = false; } if (!this.isActive()) return; // It's possible for the dragstart event to trigger a disable() call (#2419) so we must account for that @@ -248,27 +248,27 @@ class DragPanHandler { } _onTouchEnd(e: TouchEvent) { - if (!e.touches || e.touches.length === 0) { // only stop drag if all fingers have been removed - switch (this._state) { - case 'active': - this._state = 'enabled'; - this._unbind(); - this._deactivate(); - this._inertialPan(e); - break; - case 'pending': - this._state = 'enabled'; - this._unbind(); - break; - case 'enabled': - this._unbind(); - break; - default: - assert(false); - break; - } - } else { // some finger(s) still touching the screen - switch (this._state) { + if (!e.touches || e.touches.length === 0) { // only stop drag if all fingers have been removed + switch (this._state) { + case 'active': + this._state = 'enabled'; + this._unbind(); + this._deactivate(); + this._inertialPan(e); + break; + case 'pending': + this._state = 'enabled'; + this._unbind(); + break; + case 'enabled': + this._unbind(); + break; + default: + assert(false); + break; + } + } else { // some finger(s) still touching the screen + switch (this._state) { case 'pending': case 'active': // we are already dragging; continue @@ -280,23 +280,23 @@ class DragPanHandler { default: assert(false); break; - } - } + } + } } - _abort(e: FocusEvent | TouchEvent) { + _abort(e: FocusEvent | MouseEvent | TouchEvent) { switch (this._state) { case 'active': this._state = 'enabled'; if (!this._shouldStart) { // If we scheduled the dragstart but never fired, nothing to end - // We already started the drag, end it - this._fireEvent('dragend', e); - this._fireEvent('moveend', e); + // We already started the drag, end it + this._fireEvent('dragend', e); + this._fireEvent('moveend', e); } this._unbind(); this._deactivate(); - if (e.touches && e.touches.length > 1) { - // If there are multiple fingers touching, reattach touchend listner in case + if (e instanceof TouchEvent && e.touches.length > 1) { + // If there are multiple fingers touching, reattach touchend listener in case // all but one finger is removed and we need to restart a drag on touchend DOM.addEventListener(window.document, 'touchend', this._onTouchEnd); } From 226e17c00fc88a2664b2a5686f0fcd952661dd7e Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Fri, 13 Sep 2019 13:54:22 -0700 Subject: [PATCH 13/15] Fix another flow error --- src/ui/handler/drag_pan.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index e1e74eb5041..44ef91ddb0c 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -295,7 +295,7 @@ class DragPanHandler { } this._unbind(); this._deactivate(); - if (e instanceof TouchEvent && e.touches.length > 1) { + if (e instanceof window.TouchEvent && e.touches.length > 1) { // If there are multiple fingers touching, reattach touchend listener in case // all but one finger is removed and we need to restart a drag on touchend DOM.addEventListener(window.document, 'touchend', this._onTouchEnd); From 03cb5dba3b7e9143b02c88501c5ada7aff9d0d91 Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Fri, 13 Sep 2019 15:31:33 -0700 Subject: [PATCH 14/15] Lint & Flow are my BFFs --- src/ui/handler/drag_pan.js | 4 ++-- test/unit/ui/handler/drag_pan.test.js | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index d87596ade32..d5b5b1fe6b1 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -154,7 +154,7 @@ class DragPanHandler { this._state = 'pending'; this._startPos = this._mouseDownPos = this._prevPos = this._lastPos = DOM.mousePos(this._el, e); - this._startTouch = this._lastTouch = e.touches ? DOM.touchPos(this._el, e) : null; + this._startTouch = this._lastTouch = e instanceof window.TouchEvent ? DOM.touchPos(this._el, e) : null; this._inertia = [[browser.now(), this._startPos]]; } @@ -169,7 +169,7 @@ class DragPanHandler { _onMove(e: MouseEvent | TouchEvent) { e.preventDefault(); - const touchPos = e.touches ? DOM.touchPos(this._el, e) : null; + const touchPos = e instanceof window.TouchEvent ? DOM.touchPos(this._el, e) : null; const pos = DOM.mousePos(this._el, e); const matchesLastPos = touchPos ? this._touchesMatch(this._lastTouch, touchPos) : this._lastPos.equals(pos); diff --git a/test/unit/ui/handler/drag_pan.test.js b/test/unit/ui/handler/drag_pan.test.js index f786061ace4..62aeba5bf2b 100644 --- a/test/unit/ui/handler/drag_pan.test.js +++ b/test/unit/ui/handler/drag_pan.test.js @@ -4,7 +4,6 @@ import Map from '../../../../src/ui/map'; import DOM from '../../../../src/util/dom'; import simulate from '../../../util/simulate_interaction'; - function createMap(t, clickTolerance) { t.stub(Map.prototype, '_detectMissingCSS'); return new Map({ container: DOM.create('div', '', window.document.body), clickTolerance: clickTolerance || 0 }); @@ -776,7 +775,6 @@ test('DragPanHandler does not begin a touch drag on multi-finger touch event if t.end(); }); - test('DragPanHandler starts a drag on a multi-finger no-zoom touch, and continues if it becomes a single-finger touch', (t) => { const map = createMap(t); @@ -899,7 +897,6 @@ test('DragPanHandler stops/starts touch-triggered drag appropriately when transi t.end(); }); - test('DragPanHandler fires dragstart, drag, dragend events in response to multi-touch pan', (t) => { const map = createMap(t); From b50f108fa85748c6c139a3e2fb4f237305b2a9fe Mon Sep 17 00:00:00 2001 From: Anjana Vakil Date: Tue, 17 Sep 2019 18:03:02 -0700 Subject: [PATCH 15/15] Replace array loop with .every per reviewer comment --- src/ui/handler/drag_pan.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index d5b5b1fe6b1..8cc618e5b95 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -160,10 +160,7 @@ class DragPanHandler { _touchesMatch(lastTouch: ?Array, thisTouch: ?Array) { if (!lastTouch || !thisTouch || lastTouch.length !== thisTouch.length) return false; - for (let i = 0; i < lastTouch.length; i++) { - if (!lastTouch[i].equals(thisTouch[i])) return false; - } - return true; + return lastTouch.every((pos, i) => thisTouch[i] === pos); } _onMove(e: MouseEvent | TouchEvent) {