Skip to content

Commit

Permalink
Add a click tolerance option for HTML markers (#9640)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ChristopherChudzicki
ChristopherChudzicki authored Sep 2, 2020
1 parent eb64a62 commit 38f0072
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 17 deletions.
2 changes: 2 additions & 0 deletions src/ui/map.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);

Expand Down
19 changes: 18 additions & 1 deletion src/ui/marker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type Options = {
color?: string,
scale?: number,
draggable?: boolean,
clickTolerance?: number,
rotation?: number,
rotationAlignment?: string,
pitchAlignment?: string
Expand All @@ -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`.
Expand Down Expand Up @@ -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;
Expand All @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
164 changes: 148 additions & 16 deletions test/unit/ui/marker.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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])
Expand All @@ -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();
Expand All @@ -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);
Expand All @@ -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])
Expand All @@ -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();
Expand All @@ -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);
Expand Down

0 comments on commit 38f0072

Please sign in to comment.