Skip to content

Commit

Permalink
fix pinching while touching markers (#9683)
Browse files Browse the repository at this point in the history
* fix multi-finger gestures touching markers

Previously we were using e.targetTouches as the list of touches relevant
to the map. It contains only the touches that first touched the same
target. Touches that hit a marker have a different target.

This fixes uses e.touches and filters out touches who's target is not a
child of the map.

fix #9675

* update tests and add regression test
  • Loading branch information
ansis authored and karimnaaji committed May 14, 2020
1 parent f3aa871 commit 704e677
Show file tree
Hide file tree
Showing 12 changed files with 227 additions and 95 deletions.
2 changes: 1 addition & 1 deletion src/ui/handler/handler_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import assert from 'assert';

export function indexTouches(touches: TouchList, points: Array<Point>) {
export function indexTouches(touches: Array<Touch>, points: Array<Point>) {
assert(touches.length === points.length);
const obj = {};
for (let i = 0; i < touches.length; i++) {
Expand Down
20 changes: 10 additions & 10 deletions src/ui/handler/tap_drag_zoom.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,27 +30,27 @@ export default class TapDragZoomHandler {
this._tap.reset();
}

touchstart(e: TouchEvent, points: Array<Point>) {
touchstart(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (this._swipePoint) return;

if (this._tapTime && e.timeStamp - this._tapTime > MAX_TAP_INTERVAL) {
this.reset();
}

if (!this._tapTime) {
this._tap.touchstart(e, points);
} else if (e.targetTouches.length > 0) {
this._tap.touchstart(e, points, mapTouches);
} else if (mapTouches.length > 0) {
this._swipePoint = points[0];
this._swipeTouch = e.targetTouches[0].identifier;
this._swipeTouch = mapTouches[0].identifier;
}

}

touchmove(e: TouchEvent, points: Array<Point>) {
touchmove(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (!this._tapTime) {
this._tap.touchmove(e, points);
this._tap.touchmove(e, points, mapTouches);
} else if (this._swipePoint) {
if (e.targetTouches[0].identifier !== this._swipeTouch) {
if (mapTouches[0].identifier !== this._swipeTouch) {
return;
}

Expand All @@ -67,14 +67,14 @@ export default class TapDragZoomHandler {
}
}

touchend(e: TouchEvent) {
touchend(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (!this._tapTime) {
const point = this._tap.touchend(e);
const point = this._tap.touchend(e, points, mapTouches);
if (point) {
this._tapTime = e.timeStamp;
}
} else if (this._swipePoint) {
if (e.targetTouches.length === 0) {
if (mapTouches.length === 0) {
this.reset();
}
}
Expand Down
28 changes: 14 additions & 14 deletions src/ui/handler/tap_recognizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ export class SingleTapRecognizer {
this.aborted = false;
}

touchstart(e: TouchEvent, points: Array<Point>) {
touchstart(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {

if (this.centroid || e.targetTouches.length > this.numTouches) {
if (this.centroid || mapTouches.length > this.numTouches) {
this.aborted = true;
}
if (this.aborted) {
Expand All @@ -48,16 +48,16 @@ export class SingleTapRecognizer {
this.startTime = e.timeStamp;
}

if (e.targetTouches.length === this.numTouches) {
if (mapTouches.length === this.numTouches) {
this.centroid = getCentroid(points);
this.touches = indexTouches(e.targetTouches, points);
this.touches = indexTouches(mapTouches, points);
}
}

touchmove(e: TouchEvent, points: Array<Point>) {
touchmove(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (this.aborted || !this.centroid) return;

const newTouches = indexTouches(e.targetTouches, points);
const newTouches = indexTouches(mapTouches, points);
for (const id in this.touches) {
const prevPos = this.touches[id];
const pos = newTouches[id];
Expand All @@ -67,12 +67,12 @@ export class SingleTapRecognizer {
}
}

touchend(e: TouchEvent) {
touchend(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (!this.centroid || e.timeStamp - this.startTime > MAX_TOUCH_TIME) {
this.aborted = true;
}

if (e.targetTouches.length === 0) {
if (mapTouches.length === 0) {
const centroid = !this.aborted && this.centroid;
this.reset();
if (centroid) return centroid;
Expand Down Expand Up @@ -102,16 +102,16 @@ export class TapRecognizer {
this.singleTap.reset();
}

touchstart(e: TouchEvent, points: Array<Point>) {
this.singleTap.touchstart(e, points);
touchstart(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
this.singleTap.touchstart(e, points, mapTouches);
}

touchmove(e: TouchEvent, points: Array<Point>) {
this.singleTap.touchmove(e, points);
touchmove(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
this.singleTap.touchmove(e, points, mapTouches);
}

touchend(e: TouchEvent) {
const tap = this.singleTap.touchend(e);
touchend(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
const tap = this.singleTap.touchend(e, points, mapTouches);
if (tap) {
const soonEnough = e.timeStamp - this.lastTime < MAX_TAP_INTERVAL;
const closeEnough = !this.lastTap || this.lastTap.dist(tap) < MAX_DIST;
Expand Down
18 changes: 9 additions & 9 deletions src/ui/handler/tap_zoom.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,19 @@ export default class TapZoomHandler {
this._zoomOut.reset();
}

touchstart(e: TouchEvent, points: Array<Point>) {
this._zoomIn.touchstart(e, points);
this._zoomOut.touchstart(e, points);
touchstart(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
this._zoomIn.touchstart(e, points, mapTouches);
this._zoomOut.touchstart(e, points, mapTouches);
}

touchmove(e: TouchEvent, points: Array<Point>) {
this._zoomIn.touchmove(e, points);
this._zoomOut.touchmove(e, points);
touchmove(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
this._zoomIn.touchmove(e, points, mapTouches);
this._zoomOut.touchmove(e, points, mapTouches);
}

touchend(e: TouchEvent) {
const zoomInPoint = this._zoomIn.touchend(e);
const zoomOutPoint = this._zoomOut.touchend(e);
touchend(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
const zoomInPoint = this._zoomIn.touchend(e, points, mapTouches);
const zoomOutPoint = this._zoomOut.touchend(e, points, mapTouches);

if (zoomInPoint) {
this._active = true;
Expand Down
20 changes: 10 additions & 10 deletions src/ui/handler/touch_pan.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,20 @@ export default class TouchPanHandler {
this._sum = new Point(0, 0);
}

touchstart(e: TouchEvent, points: Array<Point>) {
return this._calculateTransform(e, points);
touchstart(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
return this._calculateTransform(e, points, mapTouches);
}

touchmove(e: TouchEvent, points: Array<Point>) {
touchmove(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (!this._active) return;
e.preventDefault();
return this._calculateTransform(e, points);
return this._calculateTransform(e, points, mapTouches);
}

touchend(e: TouchEvent, points: Array<Point>) {
this._calculateTransform(e, points);
touchend(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
this._calculateTransform(e, points, mapTouches);

if (this._active && e.targetTouches.length < this._minTouches) {
if (this._active && mapTouches.length < this._minTouches) {
this.reset();
}
}
Expand All @@ -46,10 +46,10 @@ export default class TouchPanHandler {
this.reset();
}

_calculateTransform(e: TouchEvent, points: Array<Point>) {
if (e.targetTouches.length > 0) this._active = true;
_calculateTransform(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (mapTouches.length > 0) this._active = true;

const touches = indexTouches(e.targetTouches, points);
const touches = indexTouches(mapTouches, points);

const touchPointSum = new Point(0, 0);
const touchDeltaSum = new Point(0, 0);
Expand Down
28 changes: 15 additions & 13 deletions src/ui/handler/touch_zoom_rotate.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,28 @@ class TwoTouchHandler {
_start(points: [Point, Point]) {} //eslint-disable-line
_move(points: [Point, Point], pinchAround: Point, e: TouchEvent) { return {}; } //eslint-disable-line

touchstart(e: TouchEvent, points: Array<Point>) {
if (this._firstTwoTouches || e.targetTouches.length < 2) return;
touchstart(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
//console.log(e.target, e.targetTouches.length ? e.targetTouches[0].target : null);
//log('touchstart', points, e.target.innerHTML, e.targetTouches.length ? e.targetTouches[0].target.innerHTML: undefined);
if (this._firstTwoTouches || mapTouches.length < 2) return;

this._firstTwoTouches = [
e.targetTouches[0].identifier,
e.targetTouches[1].identifier
mapTouches[0].identifier,
mapTouches[1].identifier
];

// implemented by child classes
this._start([points[0], points[1]]);
}

touchmove(e: TouchEvent, points: Array<Point>) {
touchmove(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (!this._firstTwoTouches) return;

e.preventDefault();

const [idA, idB] = this._firstTwoTouches;
const a = getTouchById(e, points, idA);
const b = getTouchById(e, points, idB);
const a = getTouchById(mapTouches, points, idA);
const b = getTouchById(mapTouches, points, idB);
if (!a || !b) return;
const pinchAround = this._aroundCenter ? null : a.add(b).div(2);

Expand All @@ -52,12 +54,12 @@ class TwoTouchHandler {

}

touchend(e: TouchEvent, points: Array<Point>) {
touchend(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (!this._firstTwoTouches) return;

const [idA, idB] = this._firstTwoTouches;
const a = getTouchById(e, points, idA);
const b = getTouchById(e, points, idB);
const a = getTouchById(mapTouches, points, idA);
const b = getTouchById(mapTouches, points, idB);
if (a && b) return;

if (this._active) DOM.suppressClick();
Expand Down Expand Up @@ -88,9 +90,9 @@ class TwoTouchHandler {
}
}

function getTouchById(e: TouchEvent, points: Array<Point>, identifier: number) {
for (let i = 0; i < e.targetTouches.length; i++) {
if (e.targetTouches[i].identifier === identifier) return points[i];
function getTouchById(mapTouches: Array<Touch>, points: Array<Point>, identifier: number) {
for (let i = 0; i < mapTouches.length; i++) {
if (mapTouches[i].identifier === identifier) return points[i];
}
}

Expand Down
26 changes: 18 additions & 8 deletions src/ui/handler_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ export interface Handler {

// Handlers can optionally implement these methods.
// They are called with dom events whenever those dom evens are received.
+touchstart?: (e: TouchEvent, points: Array<Point>) => HandlerResult | void;
+touchmove?: (e: TouchEvent, points: Array<Point>) => HandlerResult | void;
+touchend?: (e: TouchEvent, points: Array<Point>) => HandlerResult | void;
+touchcancel?: (e: TouchEvent, points: Array<Point>) => HandlerResult | void;
+touchstart?: (e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) => HandlerResult | void;
+touchmove?: (e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) => HandlerResult | void;
+touchend?: (e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) => HandlerResult | void;
+touchcancel?: (e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) => HandlerResult | void;
+mousedown?: (e: MouseEvent, point: Point) => HandlerResult | void;
+mousemove?: (e: MouseEvent, point: Point) => HandlerResult | void;
+mouseup?: (e: MouseEvent, point: Point) => HandlerResult | void;
Expand Down Expand Up @@ -277,6 +277,17 @@ class HandlerManager {
this.handleEvent(e, `${e.type}Window`);
}

_getMapTouches(touches: TouchList) {
const mapTouches = [];
for (const t of touches) {
const target = ((t.target: any): Node);
if (this._el.contains(target)) {
mapTouches.push(t);
}
}
return ((mapTouches: any): TouchList);
}

handleEvent(e: InputEvent | RenderFrameEvent, eventName?: string) {

if (e.type === 'blur') {
Expand All @@ -298,9 +309,8 @@ class HandlerManager {
const eventsInProgress = {};
const activeHandlers = {};

const points = e ? (e.targetTouches ?
DOM.touchPos(this._el, ((e: any): TouchEvent).targetTouches) :
DOM.mousePos(this._el, ((e: any): MouseEvent))) : null;
const mapTouches = e.touches ? this._getMapTouches(((e: any): TouchEvent).touches) : undefined;
const points = mapTouches ? DOM.touchPos(this._el, mapTouches) : DOM.mousePos(this._el, ((e: any): MouseEvent));

for (const {handlerName, handler, allowed} of this._handlers) {
if (!handler.isEnabled()) continue;
Expand All @@ -311,7 +321,7 @@ class HandlerManager {

} else {
if ((handler: any)[eventName || e.type]) {
data = (handler: any)[eventName || e.type](e, points);
data = (handler: any)[eventName || e.type](e, points, mapTouches);
this.mergeHandlerResult(mergedHandlerResult, eventsInProgress, data, handlerName, inputEvent);
if (data && data.needsRenderFrame) {
this._triggerRenderFrame();
Expand Down
6 changes: 3 additions & 3 deletions test/unit/ui/handler/dblclick_zoom.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ function createMap(t) {
function simulateDoubleTap(map, delay = 100) {
const canvas = map.getCanvas();
return new Promise(resolve => {
simulate.touchstart(canvas, {targetTouches: [{clientX: 0, clientY: 0}]});
simulate.touchstart(canvas, {touches: [{target: canvas, clientX: 0, clientY: 0}]});
simulate.touchend(canvas);
setTimeout(() => {
simulate.touchstart(canvas, {targetTouches: [{clientX: 0, clientY: 0}]});
simulate.touchstart(canvas, {touches: [{target: canvas, clientX: 0, clientY: 0}]});
simulate.touchend(canvas);
map._renderTaskQueue.run();
resolve();
Expand Down Expand Up @@ -122,7 +122,7 @@ test('DoubleClickZoomHandler zooms on the second touchend event of a double tap'
map.on('zoomstart', zoom);

const canvas = map.getCanvas();
const touchOptions = {targetTouches: [{clientX: 0.5, clientY: 0.5}]};
const touchOptions = {touches: [{target: canvas, clientX: 0.5, clientY: 0.5}]};

simulate.touchstart(canvas, touchOptions);
simulate.touchend(canvas);
Expand Down
Loading

0 comments on commit 704e677

Please sign in to comment.