From b2cca6eab86c2d2bc23a38f9f4d5f0773b47af78 Mon Sep 17 00:00:00 2001 From: Michael Barry Date: Mon, 18 Jun 2018 11:55:23 -0400 Subject: [PATCH] fix issue with fractional position in `click` event looks like Chrome 89 is sending an instance of PointerEvent as a `click` event see: https://bugs.chromium.org/p/chromium/issues/detail?id=1182909 Pointer events have fractional mouse position which means that we cannot compare directly with the position stored after `mousedown` event for now use the configurable click tolerance for click events, which was added to fix unrelated problem in the longer term we should consider switching to handling PointerEvents directly see: https://developer.mozilla.org/en-US/docs/Web/API/PointerEvent see: https://github.com/w3c/pointerevents/issues/100 --- src/ui/bind_handlers.js | 9 ++++++--- src/ui/map.js | 3 +++ test/unit/ui/map_events.test.js | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/ui/bind_handlers.js b/src/ui/bind_handlers.js index ad809fc8f9e..962ad0e858c 100644 --- a/src/ui/bind_handlers.js +++ b/src/ui/bind_handlers.js @@ -155,10 +155,13 @@ module.exports = function bindHandlers(map, options) { } function onClick(e) { - const pos = DOM.mousePos(el, e); - if (pos.equals(startPos)) { - map.fire(new MapMouseEvent('click', map, e)); + if (startPos) { + const pos = DOM.mousePos(el, e); + if (pos.dist(startPos) > options.clickTolerance) { + return; + } } + map.fire(new MapMouseEvent('click', map, e)); } function onDblClick(e) { diff --git a/src/ui/map.js b/src/ui/map.js index 1073e8ac902..ddf2c85e759 100644 --- a/src/ui/map.js +++ b/src/ui/map.js @@ -45,6 +45,8 @@ const defaultOptions = { bearingSnap: 7, + clickTolerance: 3, + attributionControl: true, failIfMajorPerformanceCaveat: false, @@ -100,6 +102,7 @@ const defaultOptions = { * bearing will snap to north. For example, with a `bearingSnap` of 7, if the user rotates * the map within 7 degrees of north, the map will automatically snap to exact north. * @param {boolean} [options.pitchWithRotate=true] If `false`, the map's pitch (tilt) control with "drag to rotate" interaction will be disabled. + * @param {number} [options.clickTolerance=3] The max number of pixels a user can shift the mouse pointer during a click for it to be considered a valid click (as opposed to a mouse drag). * @param {string} [options.logoPosition='bottom-left'] A string representing the position of the Mapbox wordmark on the map. Valid options are `top-left`,`top-right`, `bottom-left`, `bottom-right`. * @param {boolean} [options.failIfMajorPerformanceCaveat=false] If `true`, map creation will fail if the performance of Mapbox * GL JS would be dramatically worse than expected (i.e. a software renderer would be used). diff --git a/test/unit/ui/map_events.test.js b/test/unit/ui/map_events.test.js index bd5d01a5650..4f07d0acfc9 100644 --- a/test/unit/ui/map_events.test.js +++ b/test/unit/ui/map_events.test.js @@ -65,3 +65,35 @@ test(`Map#on mousedown doesn't fire subsequent click event if mousepos changes`, map.remove(); t.end(); }); + +test(`Map#on mousedown fires subsequent click event if mouse position changes less than click tolerance`, (t) => { + const map = createMap(t, { clickTolerance: 4 }); + + map.on('mousedown', e => e.preventDefault()); + + const click = t.spy(); + map.on('click', click); + const canvas = map.getCanvas(); + + simulate.drag(canvas, {clientX: 100, clientY: 100}, {clientX: 100, clientY: 103}); + t.ok(click.called); + + map.remove(); + t.end(); +}); + +test(`Map#on mousedown does not fire subsequent click event if mouse position changes more than click tolerance`, (t) => { + const map = createMap(t, { clickTolerance: 4 }); + + map.on('mousedown', e => e.preventDefault()); + + const click = t.spy(); + map.on('click', click); + const canvas = map.getCanvas(); + + simulate.drag(canvas, {clientX: 100, clientY: 100}, {clientX: 100, clientY: 104}); + t.ok(click.notCalled); + + map.remove(); + t.end(); +});