Touch navigation control fixes (#1852)

* Fix compass button behavior on touch devices * Rename buttonStillPressed in the mouse handling module to buttonNoLongerPressed as this is what it is checking * Restore an accidentally deleted event listener * Added a test for resetting pitch / bearing when clicking the compass button. This also required catching an issue with prefersReducedMotion. * Add tests to change pitch / bearing by dragging the compass button * #1869 require valid node version (#1870) Resolves #1870 * #1869 require specific node version * #1869 downgrade to oldest version possible Co-authored-by: Vsevolod (Seva) Dolgopolov <zavalit@gmail.com> * Restored original prefersReducedMotion getter and replace it with a call to setMatchMedia in the navigationControl tests * Add tests to change pitch / bearing by dragging the compass button using a touch device * Refactored the touch interactions for the navigation control by creating a touch_button_pitch_rotate handler and using it's classes to handle map interactions * Rename classes for handlers when rotating / pitching using a button * Refactored compass button touch implementation with improved naming, typing, and a more compositional style * Fix type name in two_fingers_touch.ts after previous refactor * Move rotation / pitch change one touch handlers to static factory methods Co-authored-by: Moshe Jonathan Gordon Radian <moshe.gr@albosys.com> Co-authored-by: Vsevolod Dolgopolov (aka Seva) <zavalit@googlemail.com> Co-authored-by: Vsevolod (Seva) Dolgopolov <zavalit@gmail.com>
maplibre · Dec 2, 2022 · 8449998 · 8449998
1 parent 9756943
commit 8449998
Show file tree

Hide file tree

Showing 11 changed files with 310 additions and 59 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@
 - fix issue [#860](https://github.com/maplibre/maplibre-gl-js/issues/860) fill-pattern with pixelRatio > 1 is now switched correctly at runtime. ([#1765](https://github.com/maplibre/maplibre-gl-js/pull/1765))
 - Fix the exception that would be thrown on `map.setStyle` when it is passed with transformStyle option and map is initialized without an initial style. ([#1824](https://github.com/maplibre/maplibre-gl-js/pull/1824))
 - fix issue [#1582](https://github.com/maplibre/maplibre-gl-js/issues/1582) source maps are now properly generated
+- Fix the behavior of the compass button on touch devices.
 
 
 ## 3.0.0-pre.1

diff --git a/src/css/maplibre-gl.css b/src/css/maplibre-gl.css
@@ -16,6 +16,10 @@
     height: 100%;
 }
 
+.maplibregl-ctrl-group button.maplibregl-ctrl-compass {
+    touch-action: none;
+}
+
 .maplibregl-canvas-container.maplibregl-interactive,
 .maplibregl-ctrl-group button.maplibregl-ctrl-compass {
     cursor: grab;

diff --git a/src/ui/control/navigation_control.test.ts b/src/ui/control/navigation_control.test.ts
@@ -153,4 +153,88 @@ describe('NavigationControl', () => {
         expect(spySetPitch).toHaveBeenCalled();
         expect(spySetBearing).toHaveBeenCalled();
     });
+
+    test('compass button touch drag horizontal', () => {
+        const navControl = new NavigationControl({
+            visualizePitch: true,
+            showZoom: true,
+            showCompass: true
+        });
+        map.addControl(navControl);
+
+        const spySetPitch = jest.spyOn(map, 'setPitch');
+        const spySetBearing = jest.spyOn(map, 'setBearing');
+
+        const navButton = map.getContainer().querySelector('.maplibregl-ctrl-compass');
+        const navRect = navButton.getClientRects();
+
+        const buttonX = (navRect.x ?? 0) + (navRect.width ?? 0) / 2;
+        const buttonY = (navRect.y ?? 0) + (navRect.height ?? 0) / 2;
+
+        simulate.touchstart(navButton, {touches: [{clientX: buttonX, clientY: buttonY}], targetTouches: [{clientX: buttonX, clientY: buttonY}]});
+        simulate.touchmove(window, {touches: [{clientX: buttonX - 50, clientY: buttonY}], targetTouches: [{clientX: buttonX - 50, clientY: buttonY}]});
+        simulate.touchmove(window, {touches: [{clientX: buttonX - 100, clientY: buttonY}], targetTouches: [{clientX: buttonX - 100, clientY: buttonY}]});
+        simulate.touchend(window);
+
+        map._renderTaskQueue.run();
+
+        expect(spySetPitch).not.toHaveBeenCalled();
+        expect(spySetBearing).toHaveBeenCalled();
+    });
+
+    test('compass button touch drag vertical', () => {
+        const navControl = new NavigationControl({
+            visualizePitch: true,
+            showZoom: true,
+            showCompass: true
+        });
+        map.addControl(navControl);
+
+        const spySetPitch = jest.spyOn(map, 'setPitch');
+        const spySetBearing = jest.spyOn(map, 'setBearing');
+
+        const navButton = map.getContainer().querySelector('.maplibregl-ctrl-compass');
+        const navRect = navButton.getClientRects();
+
+        const buttonX = (navRect.x ?? 0) + (navRect.width ?? 0) / 2;
+        const buttonY = (navRect.y ?? 0) + (navRect.height ?? 0) / 2;
+
+        simulate.touchstart(navButton, {touches: [{clientX: buttonX, clientY: buttonY}], targetTouches: [{clientX: buttonX, clientY: buttonY}]});
+        simulate.touchmove(window, {touches: [{clientX: buttonX, clientY: buttonY - 50}], targetTouches: [{clientX: buttonX, clientY: buttonY - 50}]});
+        simulate.touchmove(window, {touches: [{clientX: buttonX, clientY: buttonY - 100}], targetTouches: [{clientX: buttonX, clientY: buttonY - 100}]});
+        simulate.touchend(window);
+
+        map._renderTaskQueue.run();
+
+        expect(spySetPitch).toHaveBeenCalled();
+        expect(spySetBearing).not.toHaveBeenCalled();
+    });
+
+    test('compass button touch drag diagonal', () => {
+        const navControl = new NavigationControl({
+            visualizePitch: true,
+            showZoom: true,
+            showCompass: true
+        });
+        map.addControl(navControl);
+
+        const spySetPitch = jest.spyOn(map, 'setPitch');
+        const spySetBearing = jest.spyOn(map, 'setBearing');
+
+        const navButton = map.getContainer().querySelector('.maplibregl-ctrl-compass');
+        const navRect = navButton.getClientRects();
+
+        const buttonX = (navRect.x ?? 0) + (navRect.width ?? 0) / 2;
+        const buttonY = (navRect.y ?? 0) + (navRect.height ?? 0) / 2;
+
+        simulate.touchstart(navButton, {touches: [{clientX: buttonX, clientY: buttonY}], targetTouches: [{clientX: buttonX, clientY: buttonY}]});
+        simulate.touchmove(window, {touches: [{clientX: buttonX - 50, clientY: buttonY - 50}], targetTouches: [{clientX: buttonX - 50, clientY: buttonY - 50}]});
+        simulate.touchmove(window, {touches: [{clientX: buttonX - 100, clientY: buttonY - 100}], targetTouches: [{clientX: buttonX - 100, clientY: buttonY - 100}]});
+        simulate.touchend(window);
+
+        map._renderTaskQueue.run();
+
+        expect(spySetPitch).toHaveBeenCalled();
+        expect(spySetBearing).toHaveBeenCalled();
+    });
 });
diff --git a/src/ui/control/navigation_control.ts b/src/ui/control/navigation_control.ts
@@ -3,6 +3,7 @@ import Point from '@mapbox/point-geometry';
 import DOM from '../../util/dom';
 import {extend, bindAll} from '../../util/util';
 import {MouseRotateHandler, MousePitchHandler} from '../handler/mouse';
+import {OneFingerTouchHandler} from '../handler/one_finger_touch_drag';
 
 import type Map from '../map';
 import type {IControl} from './control';
@@ -149,48 +150,71 @@ class MouseRotateWrapper {
     map: Map;
     _clickTolerance: number;
     element: HTMLElement;
+    // Rotation and pitch handlers are separated due to different _clickTolerance values
     mouseRotate: MouseRotateHandler;
+    touchRotate: OneFingerTouchHandler;
     mousePitch: MousePitchHandler;
+    touchPitch: OneFingerTouchHandler;
     _startPos: Point;
     _lastPos: Point;
 
     constructor(map: Map, element: HTMLElement, pitch: boolean = false) {
         this._clickTolerance = 10;
+        const mapRotateTolerance = map.dragRotate._mouseRotate._clickTolerance;
+        const mapPitchTolerance = map.dragRotate._mousePitch._clickTolerance;
         this.element = element;
-        this.mouseRotate = new MouseRotateHandler({clickTolerance: map.dragRotate._mouseRotate._clickTolerance});
+        this.mouseRotate = new MouseRotateHandler({clickTolerance: mapRotateTolerance});
+        this.touchRotate = OneFingerTouchHandler.generateRotationHandler({clickTolerance: mapRotateTolerance});
         this.map = map;
-        if (pitch) this.mousePitch = new MousePitchHandler({clickTolerance: map.dragRotate._mousePitch._clickTolerance});
+        if (pitch) {
+            this.mousePitch = new MousePitchHandler({clickTolerance: mapPitchTolerance});
+            this.touchPitch = OneFingerTouchHandler.generatePitchHandler({clickTolerance: mapPitchTolerance});
+        }
 
         bindAll(['mousedown', 'mousemove', 'mouseup', 'touchstart', 'touchmove', 'touchend', 'reset'], this);
         DOM.addEventListener(element, 'mousedown', this.mousedown);
         DOM.addEventListener(element, 'touchstart', this.touchstart, {passive: false});
-        DOM.addEventListener(element, 'touchmove', this.touchmove);
-        DOM.addEventListener(element, 'touchend', this.touchend);
         DOM.addEventListener(element, 'touchcancel', this.reset);
     }
 
-    down(e: MouseEvent, point: Point) {
+    startMouse(e: MouseEvent, point: Point) {
         this.mouseRotate.mousedown(e, point);
         if (this.mousePitch) this.mousePitch.mousedown(e, point);
         DOM.disableDrag();
     }
 
-    move(e: MouseEvent, point: Point) {
+    startTouch(e: TouchEvent, point: Point) {
+        this.touchRotate.touchstart(e, point);
+        if (this.touchPitch) this.touchPitch.touchstart(e, point);
+        DOM.disableDrag();
+    }
+
+    moveMouse(e: MouseEvent, point: Point) {
         const map = this.map;
-        const r = this.mouseRotate.mousemoveWindow(e, point) as any;
-        if (r && r.bearingDelta) map.setBearing(map.getBearing() + r.bearingDelta);
+        const {bearingDelta} = this.mouseRotate.mousemoveWindow(e, point) || {};
+        if (bearingDelta) map.setBearing(map.getBearing() + bearingDelta);
         if (this.mousePitch) {
-            const p = this.mousePitch.mousemoveWindow(e, point) as any;
-            if (p && p.pitchDelta) map.setPitch(map.getPitch() + p.pitchDelta);
+            const {pitchDelta} = this.mousePitch.mousemoveWindow(e, point) || {};
+            if (pitchDelta) map.setPitch(map.getPitch() + pitchDelta);
+        }
+    }
+
+    moveTouch(e: TouchEvent, point: Point) {
+        const map = this.map;
+        const {bearingDelta} = this.touchRotate.touchmoveWindow(e, point) || {};
+        if (bearingDelta) map.setBearing(map.getBearing() + bearingDelta);
+        if (this.touchPitch) {
+            const {pitchDelta} = this.touchPitch.touchmoveWindow(e, point) || {};
+            if (pitchDelta) map.setPitch(map.getPitch() + pitchDelta);
         }
     }
 
     off() {
         const element = this.element;
         DOM.removeEventListener(element, 'mousedown', this.mousedown);
         DOM.removeEventListener(element, 'touchstart', this.touchstart, {passive: false});
-        DOM.removeEventListener(element, 'touchmove', this.touchmove);
-        DOM.removeEventListener(element, 'touchend', this.touchend);
+        DOM.removeEventListener(window, 'touchmove', this.touchmove, {passive: false});
+        DOM.removeEventListener(window, 'touchend', this.touchend);
         DOM.removeEventListener(element, 'touchcancel', this.reset);
         this.offTemp();
     }
@@ -199,16 +223,18 @@ class MouseRotateWrapper {
         DOM.enableDrag();
         DOM.removeEventListener(window, 'mousemove', this.mousemove);
         DOM.removeEventListener(window, 'mouseup', this.mouseup);
+        DOM.removeEventListener(window, 'touchmove', this.touchmove, {passive: false});
+        DOM.removeEventListener(window, 'touchend', this.touchend);
     }
 
     mousedown(e: MouseEvent) {
-        this.down(extend({}, e, {ctrlKey: true, preventDefault: () => e.preventDefault()}), DOM.mousePos(this.element, e));
+        this.startMouse(extend({}, e, {ctrlKey: true, preventDefault: () => e.preventDefault()}), DOM.mousePos(this.element, e));
         DOM.addEventListener(window, 'mousemove', this.mousemove);
         DOM.addEventListener(window, 'mouseup', this.mouseup);
     }
 
     mousemove(e: MouseEvent) {
-        this.move(e, DOM.mousePos(this.element, e));
+        this.moveMouse(e, DOM.mousePos(this.element, e));
     }
 
     mouseup(e: MouseEvent) {
@@ -222,7 +248,9 @@ class MouseRotateWrapper {
             this.reset();
         } else {
             this._startPos = this._lastPos = DOM.touchPos(this.element, e.targetTouches)[0];
-            this.down((({type: 'mousedown', button: 0, ctrlKey: true, preventDefault: () => e.preventDefault()} as any as MouseEvent)), this._startPos);
+            this.startTouch(e, this._startPos);
+            DOM.addEventListener(window, 'touchmove', this.touchmove, {passive: false});
+            DOM.addEventListener(window, 'touchend', this.touchend);
         }
     }
 
@@ -231,7 +259,7 @@ class MouseRotateWrapper {
             this.reset();
         } else {
             this._lastPos = DOM.touchPos(this.element, e.targetTouches)[0];
-            this.move((({preventDefault: () => e.preventDefault()} as any as MouseEvent)), this._lastPos);
+            this.moveTouch(e, this._lastPos);
         }
     }
 
@@ -242,12 +270,16 @@ class MouseRotateWrapper {
             this._startPos.dist(this._lastPos) < this._clickTolerance) {
             this.element.click();
         }
-        this.reset();
+        delete this._startPos;
+        delete this._lastPos;
+        this.offTemp();
     }
 
     reset() {
         this.mouseRotate.reset();
         if (this.mousePitch) this.mousePitch.reset();
+        this.touchRotate.reset();
+        if (this.touchPitch) this.touchPitch.reset();
         delete this._startPos;
         delete this._lastPos;
         this.offTemp();

diff --git a/src/ui/handler/mouse.ts b/src/ui/handler/mouse.ts
@@ -1,6 +1,13 @@
 import DOM from '../../util/dom';
 import type Point from '@mapbox/point-geometry';
 
+interface MouseMovementResult {
+    bearingDelta?: number;
+    pitchDelta?: number;
+    around?: Point;
+    panDelta?: Point;
+}
+
 const LEFT_BUTTON = 0;
 const RIGHT_BUTTON = 2;
 
@@ -42,7 +49,7 @@ class MouseHandler {
         return false; // implemented by child
     }
 
-    _move(lastPoint: Point, point: Point) {  //eslint-disable-line
+    _move(lastPoint: Point, point: Point): MouseMovementResult {  //eslint-disable-line
         return {}; // implemented by child
     }
 
@@ -116,7 +123,7 @@ export class MousePanHandler extends MouseHandler {
         return button === LEFT_BUTTON && !e.ctrlKey;
     }
 
-    _move(lastPoint: Point, point: Point) {
+    _move(lastPoint: Point, point: Point): MouseMovementResult {
         return {
             around: point,
             panDelta: point.sub(lastPoint)
@@ -129,7 +136,7 @@ export class MouseRotateHandler extends MouseHandler {
         return (button === LEFT_BUTTON && e.ctrlKey) || (button === RIGHT_BUTTON);
     }
 
-    _move(lastPoint: Point, point: Point) {
+    _move(lastPoint: Point, point: Point): MouseMovementResult {
         const degreesPerPixelMoved = 0.8;
         const bearingDelta = (point.x - lastPoint.x) * degreesPerPixelMoved;
         if (bearingDelta) {
@@ -150,7 +157,7 @@ export class MousePitchHandler extends MouseHandler {
         return (button === LEFT_BUTTON && e.ctrlKey) || (button === RIGHT_BUTTON);
     }
 
-    _move(lastPoint: Point, point: Point) {
+    _move(lastPoint: Point, point: Point): MouseMovementResult {
         const degreesPerPixelMoved = -0.5;
         const pitchDelta = (point.y - lastPoint.y) * degreesPerPixelMoved;
         if (pitchDelta) {