From 38f0072d4dd8122b1f12337468c592cc60938e2c Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 2 Sep 2020 17:03:14 -0400 Subject: [PATCH] Add a click tolerance option for HTML markers (#9640) --- src/ui/map.js | 2 + src/ui/marker.js | 19 ++++- test/unit/ui/marker.test.js | 164 ++++++++++++++++++++++++++++++++---- 3 files changed, 168 insertions(+), 17 deletions(-) diff --git a/src/ui/map.js b/src/ui/map.js index 7549e59b704..aa5341a17e1 100755 --- a/src/ui/map.js +++ b/src/ui/map.js @@ -308,6 +308,7 @@ class Map extends Camera { _requestManager: RequestManager; _locale: Object; _removed: boolean; + _clickTolerance: number; /** * The map's {@link ScrollZoomHandler}, which implements zooming in and out with a scroll wheel or trackpad. @@ -398,6 +399,7 @@ class Map extends Camera { this._controls = []; this._mapId = uniqueId(); this._locale = extend({}, defaultLocale, options.locale); + this._clickTolerance = options.clickTolerance; this._requestManager = new RequestManager(options.transformRequest, options.accessToken); diff --git a/src/ui/marker.js b/src/ui/marker.js index 76a0deadf16..29d844d0edf 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -21,6 +21,7 @@ type Options = { color?: string, scale?: number, draggable?: boolean, + clickTolerance?: number, rotation?: number, rotationAlignment?: string, pitchAlignment?: string @@ -36,6 +37,7 @@ type Options = { * @param {string} [options.color='#3FB1CE'] The color to use for the default marker if options.element is not provided. The default is light blue. * @param {number} [options.scale=1] The scale to use for the default marker if options.element is not provided. The default scale corresponds to a height of `41px` and a width of `27px`. * @param {boolean} [options.draggable=false] A boolean indicating whether or not a marker is able to be dragged to a new position on the map. + * @param {number} [options.clickTolerance=0] The max number of pixels a user can shift the mouse pointer during a click on the marker for it to be considered a valid click (as opposed to a marker drag). The default is to inherit map's clickTolerance. * @param {number} [options.rotation=0] The rotation angle of the marker in degrees, relative to its respective `rotationAlignment` setting. A positive value will rotate the marker clockwise. * @param {string} [options.pitchAlignment='auto'] `map` aligns the `Marker` to the plane of the map. `viewport` aligns the `Marker` to the plane of the viewport. `auto` automatically matches the value of `rotationAlignment`. * @param {string} [options.rotationAlignment='auto'] `map` aligns the `Marker`'s rotation relative to the map, maintaining a bearing as the map rotates. `viewport` aligns the `Marker`'s rotation relative to the viewport, agnostic to map rotations. `auto` is equivalent to `viewport`. @@ -65,8 +67,11 @@ export default class Marker extends Evented { _scale: number; _defaultMarker: boolean; _draggable: boolean; + _clickTolerance: number; + _isDragging: boolean; _state: 'inactive' | 'pending' | 'active'; // used for handling drag events - _positionDelta: ?number; + _positionDelta: ?Point; + _pointerdownPos: ?Point; _rotation: number; _pitchAlignment: string; _rotationAlignment: string; @@ -93,6 +98,8 @@ export default class Marker extends Evented { this._color = options && options.color || '#3FB1CE'; this._scale = options && options.scale || 1; this._draggable = options && options.draggable || false; + this._clickTolerance = options && options.clickTolerance || 0; + this._isDragging = false; this._state = 'inactive'; this._rotation = options && options.rotation || 0; this._rotationAlignment = options && options.rotationAlignment || 'auto'; @@ -491,6 +498,12 @@ export default class Marker extends Evented { } _onMove(e: MapMouseEvent | MapTouchEvent) { + if (!this._isDragging) { + const clickTolerance = this._clickTolerance || this._map._clickTolerance; + this._isDragging = e.point.dist(this._pointerdownPos) >= clickTolerance; + } + if (!this._isDragging) return; + this._pos = e.point.sub(this._positionDelta); this._lngLat = this._map.unproject(this._pos); this.setLngLat(this._lngLat); @@ -531,6 +544,8 @@ export default class Marker extends Evented { // revert to normal pointer event handling this._element.style.pointerEvents = 'auto'; this._positionDelta = null; + this._pointerdownPos = null; + this._isDragging = false; this._map.off('mousemove', this._onMove); this._map.off('touchmove', this._onMove); @@ -563,6 +578,8 @@ export default class Marker extends Evented { // creating a jarring UX effect. this._positionDelta = e.point.sub(this._pos).add(this._offset); + this._pointerdownPos = e.point; + this._state = 'pending'; this._map.on('mousemove', this._onMove); this._map.on('touchmove', this._onMove); diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 20376e22dd4..fbfbc5b33dd 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -7,11 +7,11 @@ import LngLat from '../../../src/geo/lng_lat'; import Point from '@mapbox/point-geometry'; import simulate from '../../util/simulate_interaction'; -function createMap(t) { +function createMap(t, options = {}) { const container = window.document.createElement('div'); Object.defineProperty(container, 'clientWidth', {value: 512}); Object.defineProperty(container, 'clientHeight', {value: 512}); - return globalCreateMap(t, {container}); + return globalCreateMap(t, {container, ...options}); } test('Marker uses a default marker element with an appropriate offset', (t) => { @@ -370,7 +370,7 @@ test('Marker#setDraggable turns off drag functionality', (t) => { t.end(); }); -test('Marker with draggable:true fires dragstart, drag, and dragend events at appropriate times in response to a mouse-triggered drag', (t) => { +test('Marker with draggable:true fires dragstart, drag, and dragend events at appropriate times in response to mouse-triggered drag with map-inherited clickTolerance', (t) => { const map = createMap(t); const marker = new Marker({draggable: true}) .setLngLat([0, 0]) @@ -385,20 +385,86 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap marker.on('drag', drag); marker.on('dragend', dragend); - simulate.mousedown(el); + simulate.mousedown(el, {clientX: 0, clientY: 0}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); - simulate.mousemove(el); + simulate.mousemove(el, {clientX: 2.9, clientY: 0}); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0, "drag not called yet, movement below marker's map-inherited click tolerance"); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); + + // above map's click tolerance + simulate.mousemove(el, {clientX: 3.1, clientY: 0}); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1, 'drag fired once click tolerance exceeded'); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none', 'cancels pointer events while dragging'); + + simulate.mousemove(el, {clientX: 0, clientY: 0}); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2, 'drag fired when moving back within clickTolerance of mousedown'); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none', 'cancels pointer events while dragging'); + + simulate.mouseup(el); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2); + t.equal(dragend.callCount, 1); + t.equal(el.style.pointerEvents, 'auto'); + + map.remove(); + t.end(); +}); + +test('Marker with draggable:true fires dragstart, drag, and dragend events at appropriate times in response to mouse-triggered drag with marker-specific clickTolerance', (t) => { + const map = createMap(t); + const marker = new Marker({draggable: true, clickTolerance: 4}) + .setLngLat([0, 0]) + .addTo(map); + const el = marker.getElement(); + + const dragstart = t.spy(); + const drag = t.spy(); + const dragend = t.spy(); + + marker.on('dragstart', dragstart); + marker.on('drag', drag); + marker.on('dragend', dragend); + + simulate.mousedown(el, {clientX: 0, clientY: 0}); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); + + simulate.mousemove(el, {clientX: 3.9, clientY: 0}); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0, "drag not called yet, movement below marker's map-inherited click tolerance"); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); + + // above map's click tolerance + simulate.mousemove(el, {clientX: 4.1, clientY: 0}); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1, 'drag fired once click tolerance exceeded'); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none', 'cancels pointer events while dragging'); + + simulate.mousemove(el, {clientX: 0, clientY: 0}); t.equal(dragstart.callCount, 1); - t.equal(drag.callCount, 1); + t.equal(drag.callCount, 2, 'drag fired when moving back within clickTolerance of mousedown'); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none', 'cancels pointer events while dragging'); simulate.mouseup(el); t.equal(dragstart.callCount, 1); - t.equal(drag.callCount, 1); + t.equal(drag.callCount, 2); t.equal(dragend.callCount, 1); + t.equal(el.style.pointerEvents, 'auto'); map.remove(); t.end(); @@ -419,12 +485,12 @@ test('Marker with draggable:false does not fire dragstart, drag, and dragend eve marker.on('drag', drag); marker.on('dragend', dragend); - simulate.mousedown(el); + simulate.mousedown(el, {clientX: 0, clientY: 0}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); - simulate.mousemove(el); + simulate.mousemove(el, {clientX: 3, clientY: 1}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); @@ -438,7 +504,7 @@ test('Marker with draggable:false does not fire dragstart, drag, and dragend eve t.end(); }); -test('Marker with draggable:true fires dragstart, drag, and dragend events at appropriate times in response to a touch-triggered drag', (t) => { +test('Marker with draggable:true fires dragstart, drag, and dragend events at appropriate times in response to a touch-triggered drag with map-inherited clickTolerance', (t) => { const map = createMap(t); const marker = new Marker({draggable: true}) .setLngLat([0, 0]) @@ -453,20 +519,86 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap marker.on('drag', drag); marker.on('dragend', dragend); - simulate.touchstart(el); + simulate.touchstart(el, {touches: [{clientX: 0, clientY: 0}]}); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); + + simulate.touchmove(el, {touches: [{clientX: 2.9, clientY: 0}]}); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0, "drag not called yet, movement below marker's map-inherited click tolerance"); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); + + // above map's click tolerance + simulate.touchmove(el, {touches: [{clientX: 3.1, clientY: 0}]}); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1, 'drag fired once click tolerance exceeded'); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none', 'cancels pointer events while dragging'); + + simulate.touchmove(el, {touches: [{clientX: 0, clientY: 0}]}); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2, 'drag fired when moving back within clickTolerance of touchstart'); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none', 'cancels pointer events while dragging'); + + simulate.touchend(el); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 2); + t.equal(dragend.callCount, 1); + t.equal(el.style.pointerEvents, 'auto'); + + map.remove(); + t.end(); +}); + +test('Marker with draggable:true fires dragstart, drag, and dragend events at appropriate times in response to a touch-triggered drag with marker-specific clickTolerance', (t) => { + const map = createMap(t); + const marker = new Marker({draggable: true, clickTolerance: 4}) + .setLngLat([0, 0]) + .addTo(map); + const el = marker.getElement(); + + const dragstart = t.spy(); + const drag = t.spy(); + const dragend = t.spy(); + + marker.on('dragstart', dragstart); + marker.on('drag', drag); + marker.on('dragend', dragend); + + simulate.touchstart(el, {touches: [{clientX: 0, clientY: 0}]}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); + + simulate.touchmove(el, {touches: [{clientX: 3.9, clientY: 0}]}); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0, "drag not called yet, movement below marker's map-inherited click tolerance"); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); + + // above map's click tolerance + simulate.touchmove(el, {touches: [{clientX: 4.1, clientY: 0}]}); + t.equal(dragstart.callCount, 1); + t.equal(drag.callCount, 1, 'drag fired once click tolerance exceeded'); + t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none', 'cancels pointer events while dragging'); - simulate.touchmove(el); + simulate.touchmove(el, {touches: [{clientX: 0, clientY: 0}]}); t.equal(dragstart.callCount, 1); - t.equal(drag.callCount, 1); + t.equal(drag.callCount, 2, 'drag fired when moving back within clickTolerance of touchstart'); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none', 'cancels pointer events while dragging'); simulate.touchend(el); t.equal(dragstart.callCount, 1); - t.equal(drag.callCount, 1); + t.equal(drag.callCount, 2); t.equal(dragend.callCount, 1); + t.equal(el.style.pointerEvents, 'auto'); map.remove(); t.end(); @@ -487,12 +619,12 @@ test('Marker with draggable:false does not fire dragstart, drag, and dragend eve marker.on('drag', drag); marker.on('dragend', dragend); - simulate.touchstart(el); + simulate.touchstart(el, {touches: [{clientX: 0, clientY: 0}]}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); - simulate.touchmove(el); + simulate.touchmove(el, {touches: [{clientX: 0, clientY: 0}]}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0);