Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix drag handler corner cases #6193

Merged
merged 3 commits into from
Feb 20, 2018
Merged

Fix drag handler corner cases #6193

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f68975d
Remove unnecessary conditionals
jfirebaugh Feb 20, 2018
fa5d2a1
Ensure drag gesture ends even if the control key is down on mouseup
jfirebaugh Feb 20, 2018
842ae4e
Left-button mouse click shouldn't end right-button drag rotate
jfirebaugh Feb 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/ui/bind_handlers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
21 changes: 5 additions & 16 deletions src/ui/handler/drag_pan.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is e.button guaranteed to be defined in this case? (Asking because in _ignoreEvent, we checked e.button && e.button !== 0.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mousedown will always have e.button defined. I think it was guarded before because _ignoreEvent was also used for mousemove.

window.document.addEventListener('mousemove', this._onMove);
window.document.addEventListener('mouseup', this._onUp);
}
Expand All @@ -110,7 +113,6 @@ class DragPanHandler {
}

_onMove(e: MouseEvent | TouchEvent) {
if (this._ignoreEvent(e)) return;
this._lastMoveEvent = e;
e.preventDefault();

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -211,19 +213,6 @@ class DragPanHandler {
return this._map.fire(type, e ? { originalEvent: e } : {});
}

_ignoreEvent(e: any) {
const map = this._map;

if (map.boxZoom && map.boxZoom.isActive()) return true;
if (map.dragRotate && 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(),
Expand Down
15 changes: 9 additions & 6 deletions src/ui/handler/drag_rotate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class DragRotateHandler {
_enabled: boolean;
_active: boolean;
_button: 'right' | 'left';
_eventButton: number;
_bearingSnap: number;
_pitchWithRotate: boolean;

Expand Down Expand Up @@ -102,23 +103,23 @@ 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') {
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();
Expand Down Expand Up @@ -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);
Expand Down
154 changes: 154 additions & 0 deletions test/unit/ui/handler/drag_pan.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Loading