Skip to content

Commit

Permalink
fix issue with fractional position in click event
Browse files Browse the repository at this point in the history
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: w3c/pointerevents#100
  • Loading branch information
msbarry authored and pirxpilot committed Mar 6, 2021
1 parent a1eeb44 commit b2cca6e
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/ui/bind_handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions src/ui/map.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ const defaultOptions = {

bearingSnap: 7,

clickTolerance: 3,

attributionControl: true,

failIfMajorPerformanceCaveat: false,
Expand Down Expand Up @@ -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).
Expand Down
32 changes: 32 additions & 0 deletions test/unit/ui/map_events.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});

0 comments on commit b2cca6e

Please sign in to comment.