From c434f924f0a2f3b1ffe56b420e169c6a41600e62 Mon Sep 17 00:00:00 2001 From: Harel M Date: Sat, 18 May 2024 19:37:10 +0300 Subject: [PATCH] Move the recalculateZoom to the end of the movement. (#4132) * Move the recalculateZoom to the end of the movement. * Add changelog --- CHANGELOG.md | 2 ++ src/ui/camera.ts | 5 +++-- src/ui/handler_manager.ts | 15 ++++++++------- src/ui/hash.test.ts | 5 +++-- src/ui/map.ts | 1 - src/ui/map_tests/map_events.test.ts | 18 ++++++++++++++++++ test/unit/lib/simulate_interaction.ts | 7 +++++++ 7 files changed, 41 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc14ef42b3..f8970e42a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ - _...Add new stuff here..._ ### 🐞 Bug fixes + +- Fix an issue with `moveend` zoom being different than the actual current zoom ([#4132](https://github.com/maplibre/maplibre-gl-js/pull/4132)) - _...Add new stuff here..._ ## 4.3.1 diff --git a/src/ui/camera.ts b/src/ui/camera.ts index 8640950b36..5a3ae31603 100644 --- a/src/ui/camera.ts +++ b/src/ui/camera.ts @@ -13,6 +13,7 @@ import type {LngLatLike} from '../geo/lng_lat'; import type {LngLatBoundsLike} from '../geo/lng_lat_bounds'; import type {TaskID} from '../util/task_queue'; import type {PaddingOptions} from '../geo/edge_insets'; +import type {HandlerManager} from './handler_manager'; /** * A [Point](https://github.com/mapbox/point-geometry) or an array of two numbers representing `x` and `y` screen coordinates in pixels. * @@ -242,6 +243,7 @@ export type CameraUpdateTransformFunction = (next: { export abstract class Camera extends Evented { transform: Transform; terrain: Terrain; + handlers: HandlerManager; _moving: boolean; _zooming: boolean; @@ -1400,8 +1402,7 @@ export abstract class Camera extends Evented { onEaseEnd.call(this, easeId); } if (!allowGestures) { - const handlers = (this as any).handlers; - if (handlers) handlers.stop(false); + this.handlers?.stop(false); } return this; } diff --git a/src/ui/handler_manager.ts b/src/ui/handler_manager.ts index c9ca0ff898..04a3a631b0 100644 --- a/src/ui/handler_manager.ts +++ b/src/ui/handler_manager.ts @@ -21,7 +21,7 @@ import {extend} from '../util/util'; import {browser} from '../util/browser'; import Point from '@mapbox/point-geometry'; -const isMoving = p => p.zoom || p.drag || p.pitch || p.rotate; +const isMoving = (p: EventsInProgress) => p.zoom || p.drag || p.pitch || p.rotate; class RenderFrameEvent extends Event { type: 'renderFrame'; @@ -519,11 +519,6 @@ export class HandlerManager { this._terrainMovement = true; this._map._elevationFreeze = true; tr.setLocationAtPoint(loc, around); - this._map.once('moveend', () => { - this._map._elevationFreeze = false; - this._terrainMovement = false; - tr.recalculateZoom(map.terrain); - }); } else if (combinedEventsInProgress.drag && this._terrainMovement) { // drag map tr.center = tr.pointLocation(tr.centerPoint.sub(panDelta)); @@ -590,7 +585,13 @@ export class HandlerManager { } const stillMoving = isMoving(this._eventsInProgress); - if (allowEndAnimation && (wasMoving || nowMoving) && !stillMoving) { + const finishedMoving = (wasMoving || nowMoving) && !stillMoving; + if (finishedMoving && this._terrainMovement) { + this._map._elevationFreeze = false; + this._terrainMovement = false; + this._map.transform.recalculateZoom(this._map.terrain); + } + if (allowEndAnimation && finishedMoving) { this._updatingCamera = true; const inertialEase = this._inertia._onMoveEnd(this._map.dragPan._inertiaOptions); diff --git a/src/ui/hash.test.ts b/src/ui/hash.test.ts index d0fd3cb12d..6ed1a2cb4a 100644 --- a/src/ui/hash.test.ts +++ b/src/ui/hash.test.ts @@ -1,5 +1,6 @@ import {Hash} from './hash'; import {createMap as globalCreateMap, beforeMapTest} from '../util/test/util'; +import type {Map} from './map'; describe('hash', () => { function createHash(name: string = undefined) { @@ -15,7 +16,7 @@ describe('hash', () => { return globalCreateMap({container}, undefined); } - let map; + let map: Map; beforeEach(() => { beforeMapTest(); @@ -23,7 +24,7 @@ describe('hash', () => { }); afterEach(() => { - if (map.removed === false) { + if (map._removed === false) { map.remove(); } window.location.hash = ''; diff --git a/src/ui/map.ts b/src/ui/map.ts index e1af475e57..cce68da252 100644 --- a/src/ui/map.ts +++ b/src/ui/map.ts @@ -432,7 +432,6 @@ const defaultOptions = { export class Map extends Camera { style: Style; painter: Painter; - handlers: HandlerManager; _container: HTMLElement; _canvasContainer: HTMLElement; diff --git a/src/ui/map_tests/map_events.test.ts b/src/ui/map_tests/map_events.test.ts index b18aed8a39..6fac66939c 100644 --- a/src/ui/map_tests/map_events.test.ts +++ b/src/ui/map_tests/map_events.test.ts @@ -738,6 +738,24 @@ describe('map events', () => { await sourcePromise; }); + test('getZoom on moveend is the same as after the map end moving, with terrain on', () => { + const map = createMap({interactive: true, clickTolerance: 4}); + map.terrain = { + pointCoordinate: () => null, + getElevationForLngLatZoom: () => 1000, + } as any; + let actualZoom: number; + map.on('moveend', () => { + // this can't use a promise due to race condition + actualZoom = map.getZoom(); + }); + const canvas = map.getCanvas(); + simulate.dragWithMove(canvas, {x: 100, y: 100}, {x: 100, y: 150}); + map._renderTaskQueue.run(); + + expect(actualZoom).toBe(map.getZoom()); + }); + describe('error event', () => { test('logs errors to console when it has NO listeners', () => { // to avoid seeing error in the console in Jest diff --git a/test/unit/lib/simulate_interaction.ts b/test/unit/lib/simulate_interaction.ts index 9c92d07b2e..6abe84cf7a 100644 --- a/test/unit/lib/simulate_interaction.ts +++ b/test/unit/lib/simulate_interaction.ts @@ -14,6 +14,12 @@ function drag(target: HTMLElement | Window, mousedownOptions, mouseUpOptions) { target.dispatchEvent(new MouseEvent('click', mouseUpOptions)); } +function dragWithMove(target: HTMLElement | Window, start: {x: number; y: number}, end: {x: number; y: number}) { + target.dispatchEvent(new MouseEvent('mousedown', {bubbles: true, clientX: start.x, clientY: start.y})); + document.dispatchEvent(new MouseEvent('mousemove', {bubbles: true, buttons: 1, clientX: end.x, clientY: end.y})); + target.dispatchEvent(new MouseEvent('mouseup', {bubbles: true, clientX: end.x, clientY: end.y})); +} + function dblclick(target: HTMLElement | Window) { const options = {bubbles: true}; target.dispatchEvent(new MouseEvent('mousedown', options)); @@ -64,6 +70,7 @@ function focusBlueFunctionFactory(event: string) { const events = { click, drag, + dragWithMove, dblclick, keydown: keyFunctionFactory('keydown'), keyup: keyFunctionFactory('keyup'),