From f68975d455fa9c997bcc539d690066fde503d35d Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 19 Feb 2018 16:04:31 -0800 Subject: [PATCH 1/3] Remove unnecessary conditionals --- src/ui/bind_handlers.js | 8 ++++---- src/ui/handler/drag_pan.js | 4 ++-- src/ui/handler/drag_rotate.js | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ui/bind_handlers.js b/src/ui/bind_handlers.js index d91e941c11c..5d47a89e4b7 100644 --- a/src/ui/bind_handlers.js +++ b/src/ui/bind_handlers.js @@ -58,7 +58,7 @@ module.exports = function bindHandlers(map: Map, options: {}) { } function onMouseUp(e: MouseEvent) { - const rotating = map.dragRotate && map.dragRotate.isActive(); + const rotating = map.dragRotate.isActive(); if (contextMenuEvent && !rotating) { // This will be the case for Mac @@ -71,8 +71,8 @@ module.exports = function bindHandlers(map: Map, options: {}) { } function onMouseMove(e: MouseEvent) { - if (map.dragPan && map.dragPan.isActive()) return; - if (map.dragRotate && map.dragRotate.isActive()) return; + if (map.dragPan.isActive()) return; + if (map.dragRotate.isActive()) return; let target: any = e.toElement || e.target; while (target && target !== el) target = target.parentNode; @@ -136,7 +136,7 @@ module.exports = function bindHandlers(map: Map, options: {}) { } function onContextMenu(e: MouseEvent) { - const rotating = map.dragRotate && map.dragRotate.isActive(); + const rotating = map.dragRotate.isActive(); if (!mouseDown && !rotating) { // Windows: contextmenu fired on mouseup, so fire event now fireMouseEvent('contextmenu', e); diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index 193e3926a68..1913a9e5cf3 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -214,8 +214,8 @@ class DragPanHandler { _ignoreEvent(e: any) { const map = this._map; - if (map.boxZoom && map.boxZoom.isActive()) return true; - if (map.dragRotate && map.dragRotate.isActive()) return true; + if (map.boxZoom.isActive()) return true; + if (map.dragRotate.isActive()) return true; if (e.touches) { return (e.touches.length > 1); } else { diff --git a/src/ui/handler/drag_rotate.js b/src/ui/handler/drag_rotate.js index 61c9bf3d1af..5955d1cacde 100644 --- a/src/ui/handler/drag_rotate.js +++ b/src/ui/handler/drag_rotate.js @@ -102,8 +102,8 @@ class DragRotateHandler { } _onDown(e: MouseEvent) { - if (this._map.boxZoom && this._map.boxZoom.isActive()) return; - if (this._map.dragPan && this._map.dragPan.isActive()) return; + if (this._map.boxZoom.isActive()) return; + if (this._map.dragPan.isActive()) return; if (this.isActive()) return; if (this._button === 'right') { From fa5d2a1a646677f4fc272c272aab0a90d6918d9b Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 19 Feb 2018 16:38:11 -0800 Subject: [PATCH 2/3] Ensure drag gesture ends even if the control key is down on mouseup --- src/ui/handler/drag_pan.js | 21 +--- test/unit/ui/handler/drag_pan.test.js | 154 ++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 16 deletions(-) diff --git a/src/ui/handler/drag_pan.js b/src/ui/handler/drag_pan.js index 1913a9e5cf3..d0db7f1f47c 100644 --- a/src/ui/handler/drag_pan.js +++ b/src/ui/handler/drag_pan.js @@ -89,13 +89,16 @@ class DragPanHandler { } _onDown(e: MouseEvent | TouchEvent) { - if (this._ignoreEvent(e)) return; + if (this._map.boxZoom.isActive()) return; + if (this._map.dragRotate.isActive()) return; if (this.isActive()) return; if (e.touches) { + if ((e.touches: any).length > 1) return; window.document.addEventListener('touchmove', this._onMove); window.document.addEventListener('touchend', this._onUp); } else { + if (e.ctrlKey || e.button !== 0) return; window.document.addEventListener('mousemove', this._onMove); window.document.addEventListener('mouseup', this._onUp); } @@ -110,7 +113,6 @@ class DragPanHandler { } _onMove(e: MouseEvent | TouchEvent) { - if (this._ignoreEvent(e)) return; this._lastMoveEvent = e; e.preventDefault(); @@ -150,7 +152,7 @@ class DragPanHandler { * @private */ _onUp(e: MouseEvent | TouchEvent | FocusEvent) { - if (this._ignoreEvent(e)) return; + if (e.type === 'mouseup' && e.button !== 0) return; window.document.removeEventListener('touchmove', this._onMove); window.document.removeEventListener('touchend', this._onUp); @@ -211,19 +213,6 @@ class DragPanHandler { return this._map.fire(type, e ? { originalEvent: e } : {}); } - _ignoreEvent(e: any) { - const map = this._map; - - if (map.boxZoom.isActive()) return true; - if (map.dragRotate.isActive()) return true; - if (e.touches) { - return (e.touches.length > 1); - } else { - if (e.ctrlKey) return true; - return e.type !== 'mousemove' && e.button && e.button !== 0; // left button - } - } - _drainInertiaBuffer() { const inertia = this._inertia, now = browser.now(), diff --git a/test/unit/ui/handler/drag_pan.test.js b/test/unit/ui/handler/drag_pan.test.js index 66ce2af308e..0dd89ec389e 100644 --- a/test/unit/ui/handler/drag_pan.test.js +++ b/test/unit/ui/handler/drag_pan.test.js @@ -176,3 +176,157 @@ test('DragPanHandler can interleave with another handler', (t) => { map.remove(); t.end(); }); + +test('DragPanHandler does not begin a drag if the control key is down on mousedown', (t) => { + const map = createMap(); + map.dragRotate.disable(); + + 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.mousedown(map.getCanvas(), {ctrlKey: true}); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.mousemove(map.getCanvas(), {ctrlKey: true}); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.mouseup(map.getCanvas(), {ctrlKey: true}); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + map.remove(); + t.end(); +}); + +test('DragPanHandler still ends a drag if the control key is down on mouseup', (t) => { + const map = createMap(); + map.dragRotate.disable(); + + 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.mousedown(map.getCanvas()); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.mouseup(map.getCanvas(), {ctrlKey: true}); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.mousemove(map.getCanvas()); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + map.remove(); + t.end(); +}); + +test('DragPanHandler does not begin a drag on right button mousedown', (t) => { + const map = createMap(); + map.dragRotate.disable(); + + 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.mousedown(map.getCanvas(), {buttons: 2, button: 2}); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.mousemove(map.getCanvas(), {buttons: 2}); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.mouseup(map.getCanvas(), {buttons: 0, button: 2}); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + map.remove(); + t.end(); +}); + +test('DragPanHandler does not end a drag on right button mouseup', (t) => { + const map = createMap(); + map.dragRotate.disable(); + + 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.mousedown(map.getCanvas()); + map._updateCamera(); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + simulate.mousemove(map.getCanvas()); + map._updateCamera(); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); + t.equal(dragend.callCount, 0); + + simulate.mousedown(map.getCanvas(), {buttons: 2, button: 2}); + map._updateCamera(); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); + t.equal(dragend.callCount, 0); + + simulate.mouseup(map.getCanvas(), {buttons: 0, button: 2}); + map._updateCamera(); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1); + t.equal(dragend.callCount, 0); + + simulate.mousemove(map.getCanvas()); + map._updateCamera(); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2); + t.equal(dragend.callCount, 0); + + simulate.mouseup(map.getCanvas()); + map._updateCamera(); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2); + t.equal(dragend.callCount, 1); + + map.remove(); + t.end(); +}); From 842ae4e9923a74f728096e20a1b873d75a375d4e Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 20 Feb 2018 09:07:43 -0800 Subject: [PATCH 3/3] Left-button mouse click shouldn't end right-button drag rotate --- src/ui/handler/drag_rotate.js | 11 +- test/unit/ui/handler/drag_rotate.test.js | 138 +++++++++++++++++++++++ 2 files changed, 145 insertions(+), 4 deletions(-) diff --git a/src/ui/handler/drag_rotate.js b/src/ui/handler/drag_rotate.js index 5955d1cacde..74467c1c430 100644 --- a/src/ui/handler/drag_rotate.js +++ b/src/ui/handler/drag_rotate.js @@ -30,6 +30,7 @@ class DragRotateHandler { _enabled: boolean; _active: boolean; _button: 'right' | 'left'; + _eventButton: number; _bearingSnap: number; _pitchWithRotate: boolean; @@ -107,18 +108,18 @@ class DragRotateHandler { if (this.isActive()) return; if (this._button === 'right') { - const button = (e.ctrlKey ? 0 : 2); // ? ctrl+left button : right button - let eventButton = e.button; + this._eventButton = e.button; if (typeof window.InstallTrigger !== 'undefined' && e.button === 2 && e.ctrlKey && window.navigator.platform.toUpperCase().indexOf('MAC') >= 0) { // Fix for https://github.com/mapbox/mapbox-gl-js/issues/3131: // Firefox (detected by InstallTrigger) on Mac determines e.button = 2 when // using Control + left click - eventButton = 0; + this._eventButton = 0; } - if (eventButton !== button) return; + if (this._eventButton !== (e.ctrlKey ? 0 : 2)) return; } else { if (e.ctrlKey || e.button !== 0) return; + this._eventButton = 0; } DOM.disableDrag(); @@ -182,6 +183,8 @@ class DragRotateHandler { } _onUp(e: MouseEvent | FocusEvent) { + if (e.type === 'mouseup' && e.button !== this._eventButton) return; + window.document.removeEventListener('mousemove', this._onMove, {capture: true}); window.document.removeEventListener('mouseup', this._onUp); window.removeEventListener('blur', this._onUp); diff --git a/test/unit/ui/handler/drag_rotate.test.js b/test/unit/ui/handler/drag_rotate.test.js index c7bc1ade73a..d13baeb8b29 100644 --- a/test/unit/ui/handler/drag_rotate.test.js +++ b/test/unit/ui/handler/drag_rotate.test.js @@ -435,3 +435,141 @@ test('DragRotateHandler can interleave with another handler', (t) => { map.remove(); t.end(); }); + +test('DragRotateHandler does not begin a drag on left-button mousedown without the control key', (t) => { + const map = createMap(); + map.dragPan.disable(); + + const rotatestart = t.spy(); + const rotate = t.spy(); + const rotateend = t.spy(); + + map.on('rotatestart', rotatestart); + map.on('rotate', rotate); + map.on('rotateend', rotateend); + + simulate.mousedown(map.getCanvas()); + map._updateCamera(); + t.equal(rotatestart.callCount, 0); + t.equal(rotate.callCount, 0); + t.equal(rotateend.callCount, 0); + + simulate.mousemove(map.getCanvas()); + map._updateCamera(); + t.equal(rotatestart.callCount, 0); + t.equal(rotate.callCount, 0); + t.equal(rotateend.callCount, 0); + + simulate.mouseup(map.getCanvas()); + map._updateCamera(); + t.equal(rotatestart.callCount, 0); + t.equal(rotate.callCount, 0); + t.equal(rotateend.callCount, 0); + + map.remove(); + t.end(); +}); + +test('DragRotateHandler does not end a right-button drag on left-button mouseup', (t) => { + const map = createMap(); + map.dragPan.disable(); + + const rotatestart = t.spy(); + const rotate = t.spy(); + const rotateend = t.spy(); + + map.on('rotatestart', rotatestart); + map.on('rotate', rotate); + map.on('rotateend', rotateend); + + simulate.mousedown(map.getCanvas(), {buttons: 2, button: 2}); + map._updateCamera(); + t.equal(rotatestart.callCount, 0); + t.equal(rotate.callCount, 0); + t.equal(rotateend.callCount, 0); + + simulate.mousemove(map.getCanvas(), {buttons: 2}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 1); + t.equal(rotateend.callCount, 0); + + simulate.mousedown(map.getCanvas(), {buttons: 3, button: 0}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 1); + t.equal(rotateend.callCount, 0); + + simulate.mouseup(map.getCanvas(), {buttons: 2, button: 0}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 1); + t.equal(rotateend.callCount, 0); + + simulate.mousemove(map.getCanvas(), {buttons: 2}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 2); + t.equal(rotateend.callCount, 0); + + simulate.mouseup(map.getCanvas(), {buttons: 0, button: 2}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 2); + t.equal(rotateend.callCount, 1); + + map.remove(); + t.end(); +}); + +test('DragRotateHandler does not end a control-left-button drag on right-button mouseup', (t) => { + const map = createMap(); + map.dragPan.disable(); + + const rotatestart = t.spy(); + const rotate = t.spy(); + const rotateend = t.spy(); + + map.on('rotatestart', rotatestart); + map.on('rotate', rotate); + map.on('rotateend', rotateend); + + simulate.mousedown(map.getCanvas(), {buttons: 1, button: 0, ctrlKey: true}); + map._updateCamera(); + t.equal(rotatestart.callCount, 0); + t.equal(rotate.callCount, 0); + t.equal(rotateend.callCount, 0); + + simulate.mousemove(map.getCanvas(), {buttons: 1, ctrlKey: true}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 1); + t.equal(rotateend.callCount, 0); + + simulate.mousedown(map.getCanvas(), {buttons: 3, button: 2, ctrlKey: true}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 1); + t.equal(rotateend.callCount, 0); + + simulate.mouseup(map.getCanvas(), {buttons: 1, button: 2, ctrlKey: true}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 1); + t.equal(rotateend.callCount, 0); + + simulate.mousemove(map.getCanvas(), {buttons: 1, ctrlKey: true}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 2); + t.equal(rotateend.callCount, 0); + + simulate.mouseup(map.getCanvas(), {buttons: 0, button: 0, ctrlKey: true}); + map._updateCamera(); + t.equal(rotatestart.callCount, 1); + t.equal(rotate.callCount, 2); + t.equal(rotateend.callCount, 1); + + map.remove(); + t.end(); +});