From f42dc49467dd1e9a51e129441ec1300031badfe9 Mon Sep 17 00:00:00 2001 From: christopherchudzicki Date: Tue, 28 Apr 2020 07:52:32 -0400 Subject: [PATCH 1/9] add private _clickTolerance property to map This is so that Marker instances can access map._clickTolerance if the Markers do not have their own click tolerances. --- src/ui/map.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ui/map.js b/src/ui/map.js index 613bd549f47..126c97c2d56 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); From 7158befcb0e697da71c2ab00c30f5c882cf99ee7 Mon Sep 17 00:00:00 2001 From: christopherchudzicki Date: Tue, 28 Apr 2020 07:56:45 -0400 Subject: [PATCH 2/9] fix flow typing for Marker._positionDelta --- src/ui/marker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index 7dcdc22670c..bc5c3d1365e 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -59,7 +59,7 @@ export default class Marker extends Evented { _defaultMarker: boolean; _draggable: boolean; _state: 'inactive' | 'pending' | 'active'; // used for handling drag events - _positionDelta: ?number; + _positionDelta: ?Point; _rotation: number; _pitchAlignment: string; _rotationAlignment: string; From 21fc6f4170a8ad43d6ed81a0683b1453f24b13dc Mon Sep 17 00:00:00 2001 From: christopherchudzicki Date: Tue, 28 Apr 2020 08:53:01 -0400 Subject: [PATCH 3/9] add clickTolerance option to markers If the pointer movement ever exceeds (or equals) click tolerance, then the movement fires drag events, otherwise it fires click events. Includes getClickTolerance and setClickTolerance methods. Marker clickTolerance is inherited from the map if not provided as an option. --- src/ui/marker.js | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/ui/marker.js b/src/ui/marker.js index bc5c3d1365e..1b9838e63c6 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=null] A number specifying the click tolerance in pixels. For draggable markers, movement must exceed this tolerance to be treated as a drag. A `null` value inherits click tolerance from `Map`. * @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`. @@ -58,8 +60,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: ?Point; + _pointerdownPos: ?Point; _rotation: number; _pitchAlignment: string; _rotationAlignment: string; @@ -86,6 +91,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 || null; + this._isDragging = false; this._state = 'inactive'; this._rotation = options && options.rotation || 0; this._rotationAlignment = options && options.rotationAlignment || 'auto'; @@ -483,7 +490,49 @@ export default class Marker extends Evented { return this; } + /** + * Returns the marker's click tolerance. If the marker's `clickTolerance` + * has not been set, then returns the associated `Map`'s click tolerance. If + * there is no associated `Map`, returns 0. + * @returns {number} the 's click tolerance. + */ + getClickTolerance() { + if (this._clickTolerance !== null && this._clickTolerance !== undefined) { + return this._clickTolerance; + } + if (this._map) { + return this._map._clickTolerance; + } + return null; + } + + /** + * Set the `Marker`'s click tolerance. + * @param {number} value the click tolerance in pixels + * @returns {void} + */ + setClickTolerance(value: ?number) { + if (value === null || value === undefined) { + this._clickTolerance = null; // set to inherit from map. + } + const numeric = +value; + if (!Number.isNaN(numeric)) { + this._clickTolerance = numeric; + } + } + + _hasMovedBeyondClickTolerance(point: Point) { + const clickTolerance = this.getClickTolerance(); + if (clickTolerance === null) return false; + return point.distSqr(this._pointerdownPos) >= clickTolerance ** 2; + } + _onMove(e: MapMouseEvent | MapTouchEvent) { + if (!this._isDragging) { + this._isDragging = this._hasMovedBeyondClickTolerance(e.point); + } + if (!this._isDragging) return; + this._pos = e.point.sub(this._positionDelta); this._lngLat = this._map.unproject(this._pos); this.setLngLat(this._lngLat); @@ -524,6 +573,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); @@ -556,6 +607,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); From eebd191f33355a4c3fb71451c50f566bf1b59c31 Mon Sep 17 00:00:00 2001 From: christopherchudzicki Date: Tue, 28 Apr 2020 08:16:45 -0400 Subject: [PATCH 4/9] fix marker tests after clicktolerance update --- test/unit/ui/marker.test.js | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 20376e22dd4..9bc5e4d4850 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -385,12 +385,13 @@ 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); - simulate.mousemove(el); + t.equal(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); + simulate.mousemove(el, {clientX: 3, clientY: 1}); t.equal(dragstart.callCount, 1); t.equal(drag.callCount, 1); t.equal(dragend.callCount, 0); @@ -419,12 +420,13 @@ 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); + t.equal(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); + simulate.mousemove(el, {clientX: 3, clientY: 1}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); @@ -453,12 +455,13 @@ 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); - simulate.touchmove(el); + t.equal(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); + simulate.touchmove(el, {touches: [{clientX: 3, clientY: 1}]}); t.equal(dragstart.callCount, 1); t.equal(drag.callCount, 1); t.equal(dragend.callCount, 0); @@ -487,12 +490,13 @@ 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, {clientX: 0, clientY: 0}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); - simulate.touchmove(el); + t.equal(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); + simulate.touchmove(el, {clientX: 3, clientY: 1}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); From c0c29e3ea24e95d34269c30e6b7c6b4145c34487 Mon Sep 17 00:00:00 2001 From: christopherchudzicki Date: Tue, 28 Apr 2020 08:24:12 -0400 Subject: [PATCH 5/9] assert drag handlers do not fire below clicktol --- test/unit/ui/marker.test.js | 64 +++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 9bc5e4d4850..d5d424ed147 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -405,6 +405,38 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap t.end(); }); +test('Marker with draggable:true does NOT fire drag events if mouse movement is below clickTolerance', (t) => { + const map = createMap(t); + const marker = new Marker({draggable: true}) + .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(marker.getClickTolerance(), 3, 'subsequent movement is below clickTolerance'); + simulate.mousemove(el, {clientX: 1, clientY: 1}); + + simulate.mouseup(el); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + map.remove(); + t.end(); +}); + test('Marker with draggable:false does not fire dragstart, drag, and dragend events in response to a mouse-triggered drag', (t) => { const map = createMap(t); const marker = new Marker({}) @@ -475,6 +507,38 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap t.end(); }); +test('Marker with draggable:true does NOT fire drag events if touch movement is below clickTolerance', (t) => { + const map = createMap(t); + const marker = new Marker({draggable: true}) + .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(marker.getClickTolerance(), 3, 'subsequent movement is below clickTolerance'); + simulate.touchmove(el, {touches: [{clientX: 1, clientY: 1}]}); + + simulate.touchend(el); + t.equal(dragstart.callCount, 0); + t.equal(drag.callCount, 0); + t.equal(dragend.callCount, 0); + + map.remove(); + t.end(); +}); + test('Marker with draggable:false does not fire dragstart, drag, and dragend events in response to a touch-triggered drag', (t) => { const map = createMap(t); const marker = new Marker({}) From 1f469958ae9257ea2645d9522373b0b9da09d3a9 Mon Sep 17 00:00:00 2001 From: christopherchudzicki Date: Tue, 28 Apr 2020 09:11:52 -0400 Subject: [PATCH 6/9] add tests for getting/setting marker clicktol --- test/unit/ui/marker.test.js | 39 +++++++++++++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index d5d424ed147..27e34b02fd9 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) => { @@ -329,6 +329,41 @@ test('Popup anchors around default Marker', (t) => { t.end(); }); +test('Marker#getClickTolerance gets clickTolerance set via options', (t) => { + const marker = new Marker({clickTolerance: 5}); + t.equal(marker.getClickTolerance(), 5); + t.end(); +}); + +test('Marker#getClickTolerance gets map clickTolerance if clickTolerance has not been set', (t) => { + const map = createMap(t, {clickTolerance: 4}); + const marker = new Marker() + .setLngLat([0, 0]) + .addTo(map); + t.equal(marker.getClickTolerance(), 4); + + map.remove(); + t.end(); +}); + +test('Marker#getClickTolerance uses Marker clickTolerance in favor of Map Click Tolerance if both specified', (t) => { + const map = createMap(t, {clickTolerance: 4}); + const marker = new Marker({clickTolerance: 5}) + .setLngLat([0, 0]) + .addTo(map); + t.equal(marker.getClickTolerance(), 5); + + map.remove(); + t.end(); +}); + +test('Marker#setClickTolerance sets the click tolerance', (t) => { + const marker = new Marker({clickTolerance: 5}); + marker.setClickTolerance(10); + t.equal(marker.getClickTolerance(), 10); + t.end(); +}); + test('Marker drag functionality can be added with drag option', (t) => { const map = createMap(t); const marker = new Marker({draggable: true}) From 621bc697b9d5244f7727c9f2f1534d1c5583415f Mon Sep 17 00:00:00 2001 From: christopherchudzicki Date: Tue, 28 Apr 2020 09:31:25 -0400 Subject: [PATCH 7/9] add pointerEvents assertions to drag tests --- test/unit/ui/marker.test.js | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 27e34b02fd9..19f2061a670 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -405,7 +405,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 a mouse-triggered drag and cancels element pointer events.', (t) => { const map = createMap(t); const marker = new Marker({draggable: true}) .setLngLat([0, 0]) @@ -424,23 +424,26 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); t.equal(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); simulate.mousemove(el, {clientX: 3, clientY: 1}); t.equal(dragstart.callCount, 1); t.equal(drag.callCount, 1); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none'); simulate.mouseup(el); t.equal(dragstart.callCount, 1); t.equal(drag.callCount, 1); t.equal(dragend.callCount, 1); + t.equal(el.style.pointerEvents, 'auto'); map.remove(); t.end(); }); -test('Marker with draggable:true does NOT fire drag events if mouse movement is below clickTolerance', (t) => { +test('Marker with draggable:true does NOT fire drag events if mouse movement is below clickTolerance and allows element pointer events', (t) => { const map = createMap(t); const marker = new Marker({draggable: true}) .setLngLat([0, 0]) @@ -459,14 +462,17 @@ test('Marker with draggable:true does NOT fire drag events if mouse movement is t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); t.equal(marker.getClickTolerance(), 3, 'subsequent movement is below clickTolerance'); simulate.mousemove(el, {clientX: 1, clientY: 1}); + t.equal(el.style.pointerEvents, ''); simulate.mouseup(el); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'auto'); map.remove(); t.end(); @@ -507,7 +513,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 and cancels element pointer events.', (t) => { const map = createMap(t); const marker = new Marker({draggable: true}) .setLngLat([0, 0]) @@ -526,23 +532,26 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); t.equal(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); simulate.touchmove(el, {touches: [{clientX: 3, clientY: 1}]}); t.equal(dragstart.callCount, 1); t.equal(drag.callCount, 1); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'none'); simulate.touchend(el); t.equal(dragstart.callCount, 1); t.equal(drag.callCount, 1); t.equal(dragend.callCount, 1); + t.equal(el.style.pointerEvents, 'auto'); map.remove(); t.end(); }); -test('Marker with draggable:true does NOT fire drag events if touch movement is below clickTolerance', (t) => { +test('Marker with draggable:true does NOT fire drag events if touch movement is below clickTolerance and allows element pointer events', (t) => { const map = createMap(t); const marker = new Marker({draggable: true}) .setLngLat([0, 0]) @@ -561,14 +570,17 @@ test('Marker with draggable:true does NOT fire drag events if touch movement is t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, ''); t.equal(marker.getClickTolerance(), 3, 'subsequent movement is below clickTolerance'); simulate.touchmove(el, {touches: [{clientX: 1, clientY: 1}]}); + t.equal(el.style.pointerEvents, ''); simulate.touchend(el); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); + t.equal(el.style.pointerEvents, 'auto'); map.remove(); t.end(); From c69263ff0de3c8df32ddb52d9c1523d3d0d58f7c Mon Sep 17 00:00:00 2001 From: christopherchudzicki Date: Thu, 30 Apr 2020 17:12:34 -0400 Subject: [PATCH 8/9] reword clickTolerance jsdoc to mirror map option --- src/ui/marker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index 1b9838e63c6..41f6148d97a 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -37,7 +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=null] A number specifying the click tolerance in pixels. For draggable markers, movement must exceed this tolerance to be treated as a drag. A `null` value inherits click tolerance from `Map`. + * @param {?number} [options.clickTolerance=null] 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). * @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`. From 3688f949ae2f2ea88b712bc60b741e2ddfe2bab0 Mon Sep 17 00:00:00 2001 From: Chris Date: Sat, 25 Jul 2020 15:04:14 -0400 Subject: [PATCH 9/9] remove null clickTolerance, remove get/set methods - Make the tolerance numeric instead of ?numeric. - 0 marker clickTolerance now inherits from the map - remove get/set methods --- src/ui/marker.js | 46 ++--------- test/unit/ui/marker.test.js | 147 ++++++++++++++++++++---------------- 2 files changed, 87 insertions(+), 106 deletions(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index 41f6148d97a..77f300333b3 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -37,7 +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=null] 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). + * @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`. @@ -60,7 +60,7 @@ export default class Marker extends Evented { _scale: number; _defaultMarker: boolean; _draggable: boolean; - _clickTolerance: ?number; + _clickTolerance: number; _isDragging: boolean; _state: 'inactive' | 'pending' | 'active'; // used for handling drag events _positionDelta: ?Point; @@ -91,7 +91,7 @@ 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 || null; + this._clickTolerance = options && options.clickTolerance || 0; this._isDragging = false; this._state = 'inactive'; this._rotation = options && options.rotation || 0; @@ -490,46 +490,10 @@ export default class Marker extends Evented { return this; } - /** - * Returns the marker's click tolerance. If the marker's `clickTolerance` - * has not been set, then returns the associated `Map`'s click tolerance. If - * there is no associated `Map`, returns 0. - * @returns {number} the 's click tolerance. - */ - getClickTolerance() { - if (this._clickTolerance !== null && this._clickTolerance !== undefined) { - return this._clickTolerance; - } - if (this._map) { - return this._map._clickTolerance; - } - return null; - } - - /** - * Set the `Marker`'s click tolerance. - * @param {number} value the click tolerance in pixels - * @returns {void} - */ - setClickTolerance(value: ?number) { - if (value === null || value === undefined) { - this._clickTolerance = null; // set to inherit from map. - } - const numeric = +value; - if (!Number.isNaN(numeric)) { - this._clickTolerance = numeric; - } - } - - _hasMovedBeyondClickTolerance(point: Point) { - const clickTolerance = this.getClickTolerance(); - if (clickTolerance === null) return false; - return point.distSqr(this._pointerdownPos) >= clickTolerance ** 2; - } - _onMove(e: MapMouseEvent | MapTouchEvent) { if (!this._isDragging) { - this._isDragging = this._hasMovedBeyondClickTolerance(e.point); + const clickTolerance = this._clickTolerance || this._map._clickTolerance; + this._isDragging = e.point.dist(this._pointerdownPos) >= clickTolerance; } if (!this._isDragging) return; diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 19f2061a670..fbfbc5b33dd 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -329,41 +329,6 @@ test('Popup anchors around default Marker', (t) => { t.end(); }); -test('Marker#getClickTolerance gets clickTolerance set via options', (t) => { - const marker = new Marker({clickTolerance: 5}); - t.equal(marker.getClickTolerance(), 5); - t.end(); -}); - -test('Marker#getClickTolerance gets map clickTolerance if clickTolerance has not been set', (t) => { - const map = createMap(t, {clickTolerance: 4}); - const marker = new Marker() - .setLngLat([0, 0]) - .addTo(map); - t.equal(marker.getClickTolerance(), 4); - - map.remove(); - t.end(); -}); - -test('Marker#getClickTolerance uses Marker clickTolerance in favor of Map Click Tolerance if both specified', (t) => { - const map = createMap(t, {clickTolerance: 4}); - const marker = new Marker({clickTolerance: 5}) - .setLngLat([0, 0]) - .addTo(map); - t.equal(marker.getClickTolerance(), 5); - - map.remove(); - t.end(); -}); - -test('Marker#setClickTolerance sets the click tolerance', (t) => { - const marker = new Marker({clickTolerance: 5}); - marker.setClickTolerance(10); - t.equal(marker.getClickTolerance(), 10); - t.end(); -}); - test('Marker drag functionality can be added with drag option', (t) => { const map = createMap(t); const marker = new Marker({draggable: true}) @@ -405,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 and cancels element pointer events.', (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]) @@ -426,16 +391,28 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap t.equal(dragend.callCount, 0); t.equal(el.style.pointerEvents, ''); - t.equal(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); - simulate.mousemove(el, {clientX: 3, clientY: 1}); + 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, 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'); + 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'); @@ -443,9 +420,9 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap t.end(); }); -test('Marker with draggable:true does NOT fire drag events if mouse movement is below clickTolerance and allows element pointer events', (t) => { +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}) + const marker = new Marker({draggable: true, clickTolerance: 4}) .setLngLat([0, 0]) .addTo(map); const el = marker.getElement(); @@ -464,14 +441,29 @@ test('Marker with draggable:true does NOT fire drag events if mouse movement is t.equal(dragend.callCount, 0); t.equal(el.style.pointerEvents, ''); - t.equal(marker.getClickTolerance(), 3, 'subsequent movement is below clickTolerance'); - simulate.mousemove(el, {clientX: 1, clientY: 1}); + 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, ''); - simulate.mouseup(el); - t.equal(dragstart.callCount, 0); - t.equal(drag.callCount, 0); + // 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, 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(); @@ -498,7 +490,6 @@ test('Marker with draggable:false does not fire dragstart, drag, and dragend eve t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0); - t.equal(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); simulate.mousemove(el, {clientX: 3, clientY: 1}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); @@ -513,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 and cancels element pointer events.', (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]) @@ -534,16 +525,28 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap t.equal(dragend.callCount, 0); t.equal(el.style.pointerEvents, ''); - t.equal(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); - simulate.touchmove(el, {touches: [{clientX: 3, clientY: 1}]}); + 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, 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'); + 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'); @@ -551,9 +554,9 @@ test('Marker with draggable:true fires dragstart, drag, and dragend events at ap t.end(); }); -test('Marker with draggable:true does NOT fire drag events if touch movement is below clickTolerance and allows element pointer events', (t) => { +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}) + const marker = new Marker({draggable: true, clickTolerance: 4}) .setLngLat([0, 0]) .addTo(map); const el = marker.getElement(); @@ -572,14 +575,29 @@ test('Marker with draggable:true does NOT fire drag events if touch movement is t.equal(dragend.callCount, 0); t.equal(el.style.pointerEvents, ''); - t.equal(marker.getClickTolerance(), 3, 'subsequent movement is below clickTolerance'); - simulate.touchmove(el, {touches: [{clientX: 1, clientY: 1}]}); + 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, ''); - simulate.touchend(el); - t.equal(dragstart.callCount, 0); - t.equal(drag.callCount, 0); + // 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, {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(); @@ -601,13 +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, {clientX: 0, clientY: 0}); + 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(marker.getClickTolerance(), 3, 'subsequent movement exceeds than clickTolerance'); - simulate.touchmove(el, {clientX: 3, clientY: 1}); + simulate.touchmove(el, {touches: [{clientX: 0, clientY: 0}]}); t.equal(dragstart.callCount, 0); t.equal(drag.callCount, 0); t.equal(dragend.callCount, 0);