Fix double-tap zoom (#6217) (#8086)

* Fix/add tests for existing dblclick-zoom functionality

* Add tests for existing dbltap-zoom functionality

* Prevent dbltap-zoom if touch points differ, add test

* Trigger dbltap-zoom on second touchend, add tests

* Increase double-tap maxDelta to 30

* Prevent duplicate zoom on dblclick if double-tap zoom is triggered

* Address review comments from @mourner & @asheemmamoowala

- Use proper Point obj w/ .dist() method
- Remove touchcancel handler on touchend
- Attach touchend/cancel handlers with .once()
- Move distance threshold to top-level constant, add doc comment
- Refactor tests to avoid repetition

* Clarify comments re passing touchstart vs. -end event

* Fix type-only import

src/ui/handler/dblclick_zoom.js

@@ -4,6 +4,10 @@ import { bindAll } from '../../util/util';
 
 import type Map from '../map';
 import type {MapMouseEvent, MapTouchEvent} from '../events';
+import type Point from '@mapbox/point-geometry';
+
+// maximum distance between two tap Points for them to qualify as a double-tap
+const maxDist = 30;
 
 /**
  * The `DoubleClickZoomHandler` allows the user to zoom the map at a point by
@@ -14,6 +18,7 @@ class DoubleClickZoomHandler {
     _enabled: boolean;
     _active: boolean;
     _tapped: ?TimeoutID;
+    _tappedPoint: ?Point;
 
     /**
      * @private
@@ -72,14 +77,43 @@ class DoubleClickZoomHandler {
         if (e.points.length > 1) return;
 
         if (!this._tapped) {
-            this._tapped = setTimeout(() => { this._tapped = null; }, 300);
+            this._tappedPoint = e.points[0];
+            this._tapped = setTimeout(() => { this._tapped = null; this._tappedPoint = null; }, 300);
         } else {
-            clearTimeout(this._tapped);
-            this._tapped = null;
-            this._zoom(e);
+            const newTap = e.points[0];
+            const firstTap = this._tappedPoint;
+
+            if (firstTap && firstTap.dist(newTap) <= maxDist) {
+                e.originalEvent.preventDefault(); // prevent duplicate zoom on dblclick
+
+                const onTouchEnd = () => { // ignore the touchend event, as it has no point we can zoom to
+                    if (this._tapped) { // make sure we are still within the timeout window
+                        this._zoom(e); // pass the original touchstart event, with the tapped point
+                    }
+                    this._map.off('touchcancel', onTouchCancel);
+                    this._resetTapped();
+                };
+
+                const onTouchCancel = () => {
+                    this._map.off('touchend', onTouchEnd);
+                    this._resetTapped();
+                };
+
+                this._map.once('touchend', onTouchEnd);
+                this._map.once('touchcancel', onTouchCancel);
+
+            } else { // touches are too far apart, don't zoom
+                this._resetTapped();
+            }
         }
     }
 
+    _resetTapped() {
+        clearTimeout(this._tapped);
+        this._tapped = null;
+        this._tappedPoint = null;
+    }
+
     onDblClick(e: MapMouseEvent) {
         if (!this.isEnabled()) return;
         e.originalEvent.preventDefault();

test/unit/ui/handler/dblclick_zoom.test.js

@@ -9,6 +9,35 @@ function createMap(t) {
     return new Map({ container: DOM.create('div', '', window.document.body) });
 }
 
+function simulateDoubleTap(map, delay = 100) {
+    const canvas = map.getCanvas();
+    return new Promise(resolve => {
+        simulate.touchstart(canvas);
+        simulate.touchend(canvas);
+        setTimeout(() => {
+            simulate.touchstart(canvas);
+            simulate.touchend(canvas);
+            map._renderTaskQueue.run();
+            resolve();
+        }, delay);
+    });
+}
+
+test('DoubleClickZoomHandler zooms on dblclick event', (t) => {
+    const map = createMap(t);
+
+    const zoom = t.spy();
+    map.on('zoom', zoom);
+
+    simulate.dblclick(map.getCanvas());
+    map._renderTaskQueue.run();
+
+    t.ok(zoom.called);
+
+    map.remove();
+    t.end();
+});
+
 test('DoubleClickZoomHandler does not zoom if preventDefault is called on the dblclick event', (t) => {
     const map = createMap(t);
 
@@ -18,9 +47,133 @@ test('DoubleClickZoomHandler does not zoom if preventDefault is called on the db
     map.on('zoom', zoom);
 
     simulate.dblclick(map.getCanvas());
+    map._renderTaskQueue.run();
 
     t.equal(zoom.callCount, 0);
 
     map.remove();
     t.end();
 });
+
+test('DoubleClickZoomHandler zooms on double tap if touchstart events are < 300ms apart', (t) => {
+    const map = createMap(t);
+
+    const zoom = t.spy();
+    map.on('zoom', zoom);
+
+    simulateDoubleTap(map, 100).then(() => {
+        t.ok(zoom.called);
+
+        map.remove();
+        t.end();
+    });
+
+});
+
+test('DoubleClickZoomHandler does not zoom on double tap if touchstart events are > 300ms apart', (t) => {
+    const map = createMap(t);
+
+    const zoom = t.spy();
+    map.on('zoom', zoom);
+
+    simulateDoubleTap(map, 300).then(() => {
+        t.equal(zoom.callCount, 0);
+
+        map.remove();
+        t.end();
+    });
+
+});
+
+test('DoubleClickZoomHandler does not zoom on double tap if touchstart events are in different locations', (t) => {
+    const map = createMap(t);
+
+    const zoom = t.spy();
+    map.on('zoom', zoom);
+
+    const canvas = map.getCanvas();
+
+    const simulateTwoDifferentTaps = () => {
+        return new Promise(resolve => {
+            simulate.touchstart(canvas, {touches: [{clientX: 0, clientY: 0}]});
+            simulate.touchend(canvas);
+            setTimeout(() => {
+                simulate.touchstart(canvas, {touches: [{clientX: 30.5, clientY: 30.5}]});
+                simulate.touchend(canvas);
+                map._renderTaskQueue.run();
+                resolve();
+            }, 100);
+        });
+    };
+
+    simulateTwoDifferentTaps().then(() => {
+        t.equal(zoom.callCount, 0);
+
+        map.remove();
+        t.end();
+    });
+
+});
+
+test('DoubleClickZoomHandler zooms on the second touchend event of a double tap', (t) => {
+    const map = createMap(t);
+
+    const zoom = t.spy();
+    map.on('zoom', zoom);
+
+    const canvas = map.getCanvas();
+    const touchOptions = {touches: [{clientX: 0.5, clientY: 0.5}]};
+
+    simulate.touchstart(canvas, touchOptions);
+    simulate.touchend(canvas);
+    simulate.touchstart(canvas, touchOptions);
+    map._renderTaskQueue.run();
+    t.notOk(zoom.called, 'should not trigger zoom before second touchend');
+
+    simulate.touchcancel(canvas);
+    simulate.touchend(canvas);
+    map._renderTaskQueue.run();
+    t.notOk(zoom.called, 'should not trigger zoom if second touch is canceled');
+
+    simulate.touchstart(canvas, touchOptions);
+    simulate.touchend(canvas);
+    simulate.touchstart(canvas, touchOptions);
+    map._renderTaskQueue.run();
+    t.notOk(zoom.called);
+
+    simulate.touchend(canvas);
+    map._renderTaskQueue.run();
+
+    t.ok(zoom.called, 'should trigger zoom after second touchend');
+    t.deepEquals(zoom.getCall(0).args[0].point, { x: 0.5, y: 0.5 }, 'should zoom to correct point');
+
+    t.end();
+});
+
+test('DoubleClickZoomHandler does not zoom on double tap if second touchend is >300ms after first touchstart', (t) => {
+    const map = createMap(t);
+
+    const zoom = t.spy();
+    map.on('zoom', zoom);
+
+    const canvas = map.getCanvas();
+
+    const simulateSlowSecondTap = () => {
+        return new Promise(resolve => {
+            simulate.touchstart(canvas);
+            simulate.touchend(canvas);
+            simulate.touchstart(canvas);
+            setTimeout(() => {
+                simulate.touchend(canvas);
+                map._renderTaskQueue.run();
+                resolve();
+            }, 300);
+        });
+    };
+
+    simulateSlowSecondTap().then(() => {
+        t.notOk(zoom.called);
+
+        t.end();
+    });
+});