From 68823346d7293a56bfcc6f614f03834484442539 Mon Sep 17 00:00:00 2001 From: Tom Hicks Date: Tue, 30 Jul 2024 09:02:26 +0200 Subject: [PATCH 1/4] Allow pinch trackpad gesture while cooperative gestures is enabled --- src/ui/handler/cooperative_gestures.test.ts | 39 +++++++++++++++++++-- src/ui/handler/cooperative_gestures.ts | 21 ++++++++--- src/ui/handler/scroll_zoom.ts | 12 +++---- 3 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/ui/handler/cooperative_gestures.test.ts b/src/ui/handler/cooperative_gestures.test.ts index 4320de2f11..c376803af4 100644 --- a/src/ui/handler/cooperative_gestures.test.ts +++ b/src/ui/handler/cooperative_gestures.test.ts @@ -1,8 +1,8 @@ -import {browser} from '../../util/browser'; +import { browser } from '../../util/browser'; import {Map} from '../map'; -import {DOM} from '../../util/dom'; +import { DOM } from '../../util/dom'; import simulate from '../../../test/unit/lib/simulate_interaction'; -import {beforeMapTest, sleep} from '../../util/test/util'; +import { beforeMapTest, sleep } from '../../util/test/util'; function createMap(cooperativeGestures) { return new Map({ @@ -97,6 +97,39 @@ describe('CoopGesturesHandler', () => { map.remove(); }); + test('Zooms on trackpad pinch when metaKey is the bypass key', () => { + // NOTE: This should pass regardless of whether cooperativeGestures is enabled or not + const browserNow = jest.spyOn(browser, 'now'); + let now = 1555555555555; + browserNow.mockReturnValue(now); + + const map = createMap(true); + + // pretend we're on a Mac, where the ctrlKey isn't the bypassKey + map.cooperativeGestures._bypassKey = 'metaKey'; + map._renderTaskQueue.run(); + + const startZoom = map.getZoom(); + // simulate a single 'wheel' trackpad pinch event + simulate.wheel(map.getCanvas(), { + type: 'wheel', + deltaY: -simulate.magicWheelZoomDelta, + + // this is how a browser identifies a trackpad pinch + ctrlKey: true + }); + map._renderTaskQueue.run(); + + now += 400; + browserNow.mockReturnValue(now); + map._renderTaskQueue.run(); + + const endZoom = map.getZoom(); + expect(endZoom - startZoom).toBeCloseTo(0.0285, 3); + + 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'); diff --git a/src/ui/handler/cooperative_gestures.ts b/src/ui/handler/cooperative_gestures.ts index 2af9786101..61b84246e6 100644 --- a/src/ui/handler/cooperative_gestures.ts +++ b/src/ui/handler/cooperative_gestures.ts @@ -1,7 +1,7 @@ -import {DOM} from '../../util/dom'; -import {Handler} from '../handler_manager'; +import { DOM } from '../../util/dom'; +import { Handler } from '../handler_manager'; -import type {Map} from '../map'; +import type { Map } from '../map'; /** * The {@link CooperativeGesturesHandler} options object for the gesture settings @@ -97,7 +97,20 @@ export class CooperativeGesturesHandler implements Handler { if (!this._map.scrollZoom.isEnabled()) { return; } - this._onCooperativeGesture(!e[this._bypassKey]); + + const isPrevented = this.shouldPreventWheelEvent(e); + this._onCooperativeGesture(isPrevented); + } + + shouldPreventWheelEvent(e: WheelEvent) { + if (!this.isEnabled()) { + return false; + } + + const isTrackpadPinch = e.ctrlKey; + const isBypassed = e[this._bypassKey] || isTrackpadPinch; + + return !isBypassed; } _onCooperativeGesture(showNotification: boolean) { diff --git a/src/ui/handler/scroll_zoom.ts b/src/ui/handler/scroll_zoom.ts index 56ecfefefd..21d31db9b0 100644 --- a/src/ui/handler/scroll_zoom.ts +++ b/src/ui/handler/scroll_zoom.ts @@ -1,15 +1,15 @@ -import {DOM} from '../../util/dom'; +import { DOM } from '../../util/dom'; import {defaultEasing, bezier} from '../../util/util'; import {browser} from '../../util/browser'; -import {interpolates} from '@maplibre/maplibre-gl-style-spec'; -import {LngLat} from '../../geo/lng_lat'; -import {TransformProvider} from './transform-provider'; +import { interpolates } from '@maplibre/maplibre-gl-style-spec'; +import { LngLat } from '../../geo/lng_lat'; +import { TransformProvider } from './transform-provider'; import type {Map} from '../map'; import type Point from '@mapbox/point-geometry'; import type {AroundCenterOptions} from './two_fingers_touch'; -import {Handler} from '../handler_manager'; +import { Handler } from '../handler_manager'; // deltaY value for mouse scroll wheel identification const wheelZoomDelta = 4.000244140625; @@ -151,7 +151,7 @@ export class ScrollZoomHandler implements Handler { wheel(e: WheelEvent) { if (!this.isEnabled()) return; - if (this._map.cooperativeGestures.isEnabled() && !e[this._map.cooperativeGestures._bypassKey]) { + if (this._map.cooperativeGestures.shouldPreventWheelEvent(e)) { return; } let value = e.deltaMode === WheelEvent.DOM_DELTA_LINE ? e.deltaY * 40 : e.deltaY; From e54bf8c08ff57d7763ba340bfb30da7f0f262406 Mon Sep 17 00:00:00 2001 From: Tom Hicks Date: Tue, 30 Jul 2024 09:12:20 +0200 Subject: [PATCH 2/4] Add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a676a9980e..0ec51b6a9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## main +- Allow trackpad pinch gestures to break through the `cooperativeGestures` setting, bringing it in line with other embedded map behaviours, such as Google Maps and Mapbox. ([#4465](https://github.com/maplibre/maplibre-gl-js/pull/4465)) + ### ✨ Features and improvements - Expose projection matrix parameters ([#3136](https://github.com/maplibre/maplibre-gl-js/pull/3136)) From 896a699f3fa6559ee8c85795feec887aecaa00d7 Mon Sep 17 00:00:00 2001 From: Tom Hicks Date: Tue, 30 Jul 2024 13:00:19 +0200 Subject: [PATCH 3/4] Fix linting --- src/ui/handler/cooperative_gestures.test.ts | 6 +++--- src/ui/handler/cooperative_gestures.ts | 8 ++++---- src/ui/handler/scroll_zoom.ts | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/ui/handler/cooperative_gestures.test.ts b/src/ui/handler/cooperative_gestures.test.ts index c376803af4..b6f5861c9a 100644 --- a/src/ui/handler/cooperative_gestures.test.ts +++ b/src/ui/handler/cooperative_gestures.test.ts @@ -1,8 +1,8 @@ -import { browser } from '../../util/browser'; +import {browser} from '../../util/browser'; import {Map} from '../map'; -import { DOM } from '../../util/dom'; +import {DOM} from '../../util/dom'; import simulate from '../../../test/unit/lib/simulate_interaction'; -import { beforeMapTest, sleep } from '../../util/test/util'; +import {beforeMapTest, sleep} from '../../util/test/util'; function createMap(cooperativeGestures) { return new Map({ diff --git a/src/ui/handler/cooperative_gestures.ts b/src/ui/handler/cooperative_gestures.ts index 61b84246e6..9fe8eee3fb 100644 --- a/src/ui/handler/cooperative_gestures.ts +++ b/src/ui/handler/cooperative_gestures.ts @@ -1,7 +1,7 @@ -import { DOM } from '../../util/dom'; -import { Handler } from '../handler_manager'; +import {DOM} from '../../util/dom'; +import {Handler} from '../handler_manager'; -import type { Map } from '../map'; +import type {Map} from '../map'; /** * The {@link CooperativeGesturesHandler} options object for the gesture settings @@ -97,7 +97,7 @@ export class CooperativeGesturesHandler implements Handler { if (!this._map.scrollZoom.isEnabled()) { return; } - + const isPrevented = this.shouldPreventWheelEvent(e); this._onCooperativeGesture(isPrevented); } diff --git a/src/ui/handler/scroll_zoom.ts b/src/ui/handler/scroll_zoom.ts index 21d31db9b0..095f337175 100644 --- a/src/ui/handler/scroll_zoom.ts +++ b/src/ui/handler/scroll_zoom.ts @@ -1,15 +1,15 @@ -import { DOM } from '../../util/dom'; +import {DOM} from '../../util/dom'; import {defaultEasing, bezier} from '../../util/util'; import {browser} from '../../util/browser'; -import { interpolates } from '@maplibre/maplibre-gl-style-spec'; -import { LngLat } from '../../geo/lng_lat'; -import { TransformProvider } from './transform-provider'; +import {interpolates} from '@maplibre/maplibre-gl-style-spec'; +import {LngLat} from '../../geo/lng_lat'; +import {TransformProvider} from './transform-provider'; import type {Map} from '../map'; import type Point from '@mapbox/point-geometry'; import type {AroundCenterOptions} from './two_fingers_touch'; -import { Handler } from '../handler_manager'; +import {Handler} from '../handler_manager'; // deltaY value for mouse scroll wheel identification const wheelZoomDelta = 4.000244140625; From 00971b13c40c30f6357abb37e80bb4282089d580 Mon Sep 17 00:00:00 2001 From: Tom Hicks Date: Tue, 30 Jul 2024 14:00:01 +0200 Subject: [PATCH 4/4] Update test size expectation --- test/build/min.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/build/min.test.ts b/test/build/min.test.ts index 8cc9002488..b0d0fd23f0 100644 --- a/test/build/min.test.ts +++ b/test/build/min.test.ts @@ -1,5 +1,5 @@ import fs from 'fs'; -import packageJson from '../../package.json' with {type: 'json'}; +import packageJson from '../../package.json' with { type: 'json' }; const minBundle = fs.readFileSync('dist/maplibre-gl.js', 'utf8'); @@ -36,7 +36,7 @@ describe('test min build', () => { const decreaseQuota = 4096; // feel free to update this value after you've checked that it has changed on purpose :-) - const expectedBytes = 799999; + const expectedBytes = 800300; expect(actualBytes - expectedBytes).toBeLessThan(increaseQuota); expect(expectedBytes - actualBytes).toBeLessThan(decreaseQuota);