Skip to content

Commit

Permalink
Move the recalculateZoom to the end of the movement. (#4132)
Browse files Browse the repository at this point in the history
* Move the recalculateZoom to the end of the movement.

* Add changelog
  • Loading branch information
HarelM committed May 18, 2024
1 parent f7d5e62 commit c434f92
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/ui/camera.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -242,6 +243,7 @@ export type CameraUpdateTransformFunction = (next: {
export abstract class Camera extends Evented {
transform: Transform;
terrain: Terrain;
handlers: HandlerManager;

_moving: boolean;
_zooming: boolean;
Expand Down Expand Up @@ -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;
}
Expand Down
15 changes: 8 additions & 7 deletions src/ui/handler_manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);

Expand Down
5 changes: 3 additions & 2 deletions src/ui/hash.test.ts
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -15,15 +16,15 @@ describe('hash', () => {
return globalCreateMap({container}, undefined);
}

let map;
let map: Map;

beforeEach(() => {
beforeMapTest();
map = createMap();
});

afterEach(() => {
if (map.removed === false) {
if (map._removed === false) {
map.remove();
}
window.location.hash = '';
Expand Down
1 change: 0 additions & 1 deletion src/ui/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,6 @@ const defaultOptions = {
export class Map extends Camera {
style: Style;
painter: Painter;
handlers: HandlerManager;

_container: HTMLElement;
_canvasContainer: HTMLElement;
Expand Down
18 changes: 18 additions & 0 deletions src/ui/map_tests/map_events.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions test/unit/lib/simulate_interaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -64,6 +70,7 @@ function focusBlueFunctionFactory(event: string) {
const events = {
click,
drag,
dragWithMove,
dblclick,
keydown: keyFunctionFactory('keydown'),
keyup: keyFunctionFactory('keyup'),
Expand Down

0 comments on commit c434f92

Please sign in to comment.