Cooperative gestures redesign (#3430)

* Initial commit for adding a cooperative gesture control instead of the current solution in the map object. * Fix all the relevant issues, still needs to be tested on a touch device... * Fix relevant code, fix lint * Add changelog * Fix some documentations and default values * More fixes after testing on a touch device * Fix lint * Improve robustness of render test
maplibre · Dec 3, 2023 · 3c78f81 · 3c78f81
1 parent 4ad19a6
commit 3c78f81
Show file tree

Hide file tree

Showing 15 changed files with 282 additions and 343 deletions.
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
@@ -1,4 +1,4 @@
 {
     "name": "MapLibre container using node.js",
-    "postCreateCommand": "/bin/bash -l -c \"sudo apt-get update && sudo apt-get install -y build-essential git libglew-dev libxi-dev default-jre default-jdk xvfb && source $NVM_DIR/nvm.sh && nvm install && npm install\""
+    "postCreateCommand": "/bin/bash -l -c \"source $NVM_DIR/nvm.sh && nvm install && npm install && echo 'done npm install' && sudo apt-get update && sudo apt-get install -y build-essential git libglew-dev libxi-dev default-jre default-jdk xvfb\""
 }
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,11 +6,13 @@
 - ⚠️ Removed callback usage from `map.loadImage` in continue to below change ([#3422](https://github.com/maplibre/maplibre-gl-js/pull/3422))
 - ⚠️ Changed the `GeoJSONSource`'s `getClusterExpansionZoom`, `getClusterChildren`, `getClusterLeaves` methods to return a `Promise` instead of a callback usage ([#3421](https://github.com/maplibre/maplibre-gl-js/pull/3421))
 - ⚠️ Changed the `setRTLTextPlugin` function to return a promise instead of using callback ([#3418](https://github.com/maplibre/maplibre-gl-js/pull/3418)) this also changed how the RTL pluing code is handled internally by splitting the main thread and worker thread code.
+- ⚠️ Remove `setCooperativeGestures` and `getCooperativeGestures` functions in favor of `cooperativeGestures` handler which now has an `enabled()` or `disabled()` methods ([#3430](https://github.com/maplibre/maplibre-gl-js/pull/3430))
 - Created a new example showing how to place a threejs scene as a `CustomLayer` over maplibre 3d-terrain ([#3429](https://github.com/maplibre/maplibre-gl-js/pull/3429))
 - _...Add new stuff here..._
 
 ### 🐞 Bug fixes
 - Fix zooming outside the central globe when terrain 3D is enabled ([#3425](https://github.com/maplibre/maplibre-gl-js/pull/3425))
+- Fix a bug in showing cooperative gestures when scroll zoom is disabled ([#2498](https://github.com/maplibre/maplibre-gl-js/pull/2498))
 - Handle loading of empty raster tiles (204 No Content) ([#3428](https://github.com/maplibre/maplibre-gl-js/pull/3428))
 - _...Add new stuff here..._
 

diff --git a/src/ui/control/fullscreen_control.test.ts b/src/ui/control/fullscreen_control.test.ts
@@ -88,33 +88,11 @@ describe('FullscreenControl', () => {
 
         // Simulate a click to the fullscreen button
         fullscreen._fullscreenButton.dispatchEvent(click);
-        expect(map.getCooperativeGestures()).toBeFalsy();
+        expect(map.cooperativeGestures.isEnabled()).toBeFalsy();
 
         // Second simulated click would exit fullscreen mode
         fullscreen._fullscreenButton.dispatchEvent(click);
-        expect(map.getCooperativeGestures()).toBe(cooperativeGestures);
-    });
-
-    test('reenables cooperative gestures custom options when fullscreen exits', () => {
-        const cooperativeGestures = {
-            'windowsHelpText': 'Custom message',
-            'macHelpText': 'Custom message',
-            'mobileHelpText': 'Custom message',
-        };
-        const map = createMap({cooperativeGestures});
-        const fullscreen = new FullscreenControl({});
-
-        map.addControl(fullscreen);
-
-        const click = new window.Event('click');
-
-        // Simulate a click to the fullscreen button
-        fullscreen._fullscreenButton.dispatchEvent(click);
-        expect(map.getCooperativeGestures()).toBeFalsy();
-
-        // Second simulated click would exit fullscreen mode
-        fullscreen._fullscreenButton.dispatchEvent(click);
-        expect(map.getCooperativeGestures()).toEqual(cooperativeGestures);
+        expect(map.cooperativeGestures.isEnabled()).toBeTruthy();
     });
 
     test('if never set, cooperative gestures remain disabled when fullscreen exits', () => {
@@ -127,10 +105,10 @@ describe('FullscreenControl', () => {
 
         // Simulate a click to the fullscreen button
         fullscreen._fullscreenButton.dispatchEvent(click);
-        expect(map.getCooperativeGestures()).toBeFalsy();
+        expect(map.cooperativeGestures.isEnabled()).toBeFalsy();
 
         // Second simulated click would exit fullscreen mode
         fullscreen._fullscreenButton.dispatchEvent(click);
-        expect(map.getCooperativeGestures()).toBeFalsy();
+        expect(map.cooperativeGestures.isEnabled()).toBeFalsy();
     });
 });
diff --git a/src/ui/control/fullscreen_control.ts b/src/ui/control/fullscreen_control.ts
@@ -3,7 +3,7 @@ import {DOM} from '../../util/dom';
 import {warnOnce} from '../../util/util';
 
 import {Event, Evented} from '../../util/evented';
-import type {Map, GestureOptions} from '../map';
+import type {Map} from '../map';
 import type {IControl} from './control';
 
 /**
@@ -44,7 +44,7 @@ export class FullscreenControl extends Evented implements IControl {
     _fullscreenchange: string;
     _fullscreenButton: HTMLButtonElement;
     _container: HTMLElement;
-    _prevCooperativeGestures: boolean | GestureOptions;
+    _prevCooperativeGesturesEnabled: boolean;
 
     constructor(options: FullscreenOptions = {}) {
         super();
@@ -128,15 +128,12 @@ export class FullscreenControl extends Evented implements IControl {
 
         if (this._fullscreen) {
             this.fire(new Event('fullscreenstart'));
-            if (this._map._cooperativeGestures) {
-                this._prevCooperativeGestures = this._map._cooperativeGestures;
-                this._map.setCooperativeGestures();
-            }
+            this._prevCooperativeGesturesEnabled = this._map.cooperativeGestures.isEnabled();
+            this._map.cooperativeGestures.disable();
         } else {
             this.fire(new Event('fullscreenend'));
-            if (this._prevCooperativeGestures) {
-                this._map.setCooperativeGestures(this._prevCooperativeGestures);
-                delete this._prevCooperativeGestures;
+            if (this._prevCooperativeGesturesEnabled) {
+                this._map.cooperativeGestures.enable();
             }
         }
     }

diff --git a/src/ui/handler/cooperative_gestures.test.ts b/src/ui/handler/cooperative_gestures.test.ts
@@ -2,7 +2,7 @@ import {browser} from '../../util/browser';
 import {Map} from '../map';
 import {DOM} from '../../util/dom';
 import simulate from '../../../test/unit/lib/simulate_interaction';
-import {beforeMapTest} from '../../util/test/util';
+import {beforeMapTest, sleep} from '../../util/test/util';
 
 function createMap(cooperativeGestures) {
     return new Map({
@@ -22,7 +22,7 @@ beforeEach(() => {
 
 describe('CoopGesturesHandler', () => {
 
-    test('Does not zoom on wheel if no key is down', () => {
+    test('Does not zoom on wheel if no key is down', async () => {
         const browserNow = jest.spyOn(browser, 'now');
         let now = 1555555555555;
         browserNow.mockReturnValue(now);
@@ -34,6 +34,7 @@ describe('CoopGesturesHandler', () => {
         // simulate a single 'wheel' event
         simulate.wheel(map.getCanvas(), {type: 'wheel', deltaY: -simulate.magicWheelZoomDelta});
         map._renderTaskQueue.run();
+        expect(map.getContainer().querySelector('.maplibregl-cooperative-gesture-screen.maplibregl-show')).toBeInstanceOf(HTMLDivElement);
 
         now += 400;
         browserNow.mockReturnValue(now);
@@ -42,6 +43,9 @@ describe('CoopGesturesHandler', () => {
         const endZoom = map.getZoom();
         expect(endZoom).toBeCloseTo(startZoom);
 
+        await sleep(200);
+
+        expect(map.getContainer().querySelector('.maplibregl-cooperative-gesture-screen.maplibregl-show')).toBeNull();
         map.remove();
     });
 
@@ -51,7 +55,7 @@ describe('CoopGesturesHandler', () => {
         browserNow.mockReturnValue(now);
 
         const map = createMap(true);
-        map.setCooperativeGestures(false);
+        map.cooperativeGestures.disable();
         map._renderTaskQueue.run();
 
         const startZoom = map.getZoom();
@@ -93,6 +97,24 @@ describe('CoopGesturesHandler', () => {
         map.remove();
     });
 
+    test('Does not show message if scrollZoom is disabled', () => {
+        // NOTE: This should pass regardless of whether cooperativeGestures is enabled or not
+        const browserNow = jest.spyOn(browser, 'now');
+        const now = 1555555555555;
+        browserNow.mockReturnValue(now);
+
+        const map = createMap(true);
+        map.scrollZoom.disable();
+        map._renderTaskQueue.run();
+
+        // simulate a single 'wheel' event
+        simulate.wheel(map.getCanvas(), {type: 'wheel', deltaY: -simulate.magicWheelZoomDelta});
+        map._renderTaskQueue.run();
+        expect(map.getContainer().querySelector('.maplibregl-cooperative-gesture-screen.maplibregl-show')).toBeNull();
+
+        map.remove();
+    });
+
     test('Does not pan on touchmove with a single touch', () => {
         const map = createMap(true);
         const target = map.getCanvas();
@@ -113,6 +135,8 @@ describe('CoopGesturesHandler', () => {
         simulate.touchmove(target, {touches: [{target, clientX: 10, clientY: 10}]});
         map._renderTaskQueue.run();
 
+        expect(map.getContainer().querySelector('.maplibregl-cooperative-gesture-screen.maplibregl-show')).toBeInstanceOf(HTMLDivElement);
+
         simulate.touchend(target);
         map._renderTaskQueue.run();
 
@@ -125,25 +149,19 @@ describe('CoopGesturesHandler', () => {
 
     test('Pans on touchmove with a single touch after disabling cooperative gestures', () => {
         const map = createMap(true);
-        map.setCooperativeGestures(false);
-        const target = map.getCanvas();
+        map.cooperativeGestures.disable();
+        const target = map.getCanvasContainer();
         const startCenter = map.getCenter();
         map._renderTaskQueue.run();
 
-        const dragstart = jest.fn();
-        const drag      = jest.fn();
-        const dragend   = jest.fn();
-
-        map.on('dragstart', dragstart);
-        map.on('drag',      drag);
-        map.on('dragend',   dragend);
-
         simulate.touchstart(target, {touches: [{target, clientX: 0, clientY: 0}, {target, clientX: 1, clientY: 1}]});
         map._renderTaskQueue.run();
 
         simulate.touchmove(target, {touches: [{target, clientX: 10, clientY: 10}, {target, clientX: 11, clientY: 11}]});
         map._renderTaskQueue.run();
 
+        expect(map.getContainer().querySelector('.maplibregl-cooperative-gesture-screen.maplibregl-show')).toBeNull();
+
         simulate.touchend(target);
         map._renderTaskQueue.run();
 
@@ -154,33 +172,28 @@ describe('CoopGesturesHandler', () => {
         map.remove();
     });
 
-    test('Does pan on touchmove with a double touch', () => {
-        // NOTE: This should pass regardless of whether cooperativeGestures is enabled or not
+    test('Does pan on touchmove with a double touch but does not change pitch', () => {
         const map = createMap(true);
         const target = map.getCanvas();
         const startCenter = map.getCenter();
+        const startPitch = map.getPitch();
         map._renderTaskQueue.run();
 
-        const dragstart = jest.fn();
-        const drag      = jest.fn();
-        const dragend   = jest.fn();
-
-        map.on('dragstart', dragstart);
-        map.on('drag',      drag);
-        map.on('dragend',   dragend);
-
         simulate.touchstart(target, {touches: [{target, clientX: 0, clientY: 0}, {target, clientX: 1, clientY: 1}]});
         map._renderTaskQueue.run();
 
         simulate.touchmove(target, {touches: [{target, clientX: 10, clientY: 10}, {target, clientX: 11, clientY: 11}]});
         map._renderTaskQueue.run();
 
+        expect(map.getContainer().querySelector('.maplibregl-cooperative-gesture-screen.maplibregl-show')).toBeNull();
+
         simulate.touchend(target);
         map._renderTaskQueue.run();
 
         const endCenter = map.getCenter();
         expect(endCenter.lng).toBeGreaterThan(startCenter.lng);
         expect(endCenter.lat).toBeGreaterThan(startCenter.lat);
+        expect(startPitch).toBe(map.getPitch());
 
         map.remove();
     });
@@ -192,20 +205,14 @@ describe('CoopGesturesHandler', () => {
         const startPitch = map.getPitch();
         map._renderTaskQueue.run();
 
-        const dragstart = jest.fn();
-        const drag      = jest.fn();
-        const dragend   = jest.fn();
-
-        map.on('dragstart', dragstart);
-        map.on('drag',      drag);
-        map.on('dragend',   dragend);
-
         simulate.touchstart(target, {touches: [{target, clientX: 0, clientY: 0}, {target, clientX: 1, clientY: 1}, {target, clientX: 2, clientY: 2}]});
         map._renderTaskQueue.run();
 
         simulate.touchmove(target, {touches: [{target, clientX: 0, clientY: -10}, {target, clientX: 1, clientY: -11}, {target, clientX: 2, clientY: -12}]});
         map._renderTaskQueue.run();
 
+        expect(map.getContainer().querySelector('.maplibregl-cooperative-gesture-screen.maplibregl-show')).toBeNull();
+
         simulate.touchend(target);
         map._renderTaskQueue.run();
 
@@ -236,7 +243,7 @@ describe('CoopGesturesHandler', () => {
         expect(midZoom - startZoom).toBeCloseTo(0.0285, 3);
 
         // Enable cooperative gestures
-        map.setCooperativeGestures(true);
+        map.cooperativeGestures.enable();
 
         // This 'wheel' event should not zoom
         simulate.wheel(map.getCanvas(), {type: 'wheel', deltaY: -simulate.magicWheelZoomDelta});