Skip to content

Commit 8a2bf7c

Browse files
Call setMinZoom, setMaxZoom, setMinPitch and setMaxPitch in the right order to avoid error in Maplibre (#2573)
* Add updateZoomConstraint helper * Fix types, prevent duplicate call to setMinZoom or setMaxZoom * Add the same logic for pitch, format code * Return whether or not changes were applied from the updatePitchConstraint and updateZoomConstraint func * Add back propPresent * yarn lint fix * refactor: cleanup tests, prop names, code duplication, comments --------- Co-authored-by: Xiaoji Chen <Pessimistress@users.noreply.github.com>
1 parent a4050bc commit 8a2bf7c

3 files changed

Lines changed: 243 additions & 15 deletions

File tree

modules/react-maplibre/src/maplibre/maplibre.ts

Lines changed: 68 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import {transformToViewState, applyViewStateToTransform} from '../utils/transform';
1+
import {
2+
transformToViewState,
3+
applyViewStateToTransform,
4+
updateZoomConstraint,
5+
updatePitchConstraint
6+
} from '../utils/transform';
27
import {normalizeStyle} from '../utils/style-utils';
38
import {deepEqual} from '../utils/deep-equal';
49

@@ -76,6 +81,31 @@ export type MaplibreProps = Partial<ViewState> &
7681
interactiveLayerIds?: string[];
7782
/** CSS cursor */
7883
cursor?: string;
84+
85+
/** Minimum zoom available to the map.
86+
* @default 0
87+
*/
88+
minZoom?: number;
89+
/** Maximum zoom available to the map.
90+
* @default 22
91+
*/
92+
maxZoom?: number;
93+
/** Minimum pitch available to the map.
94+
* @default 0
95+
*/
96+
minPitch?: number;
97+
/** Maximum pitch available to the map.
98+
* @default 85
99+
*/
100+
maxPitch?: number;
101+
/** Bounds of the map.
102+
* @default [-180, -85.051129, 180, 85.051129]
103+
*/
104+
maxBounds?: [number, number, number, number];
105+
/** Whether to render copies of the world or not.
106+
* @default true
107+
*/
108+
renderWorldCopies?: boolean;
79109
};
80110

81111
const DEFAULT_STYLE = {version: 8, sources: {}, layers: []} as StyleSpecification;
@@ -138,15 +168,7 @@ const otherEvents = {
138168
sourcedata: 'onSourceData',
139169
error: 'onError'
140170
};
141-
const settingNames = [
142-
'minZoom',
143-
'maxZoom',
144-
'minPitch',
145-
'maxPitch',
146-
'maxBounds',
147-
'projection',
148-
'renderWorldCopies'
149-
];
171+
const settingNames = ['maxBounds', 'projection', 'renderWorldCopies'] as const;
150172
const handlerNames = [
151173
'scrollZoom',
152174
'boxZoom',
@@ -414,25 +436,57 @@ export default class Maplibre {
414436
return false;
415437
}
416438

439+
/* Update camera constraints to match props
440+
@param {object} nextProps
441+
@param {object} currProps
442+
@returns {bool} true if anything is changed
443+
*/
444+
private _updateConstraints(nextProps: MaplibreProps, currProps: MaplibreProps): boolean {
445+
const didUpdateZoom = updateZoomConstraint(
446+
this._map,
447+
{
448+
min: nextProps.minZoom ?? DEFAULT_SETTINGS.minZoom,
449+
max: nextProps.maxZoom ?? DEFAULT_SETTINGS.maxZoom
450+
},
451+
{
452+
min: currProps.minZoom ?? DEFAULT_SETTINGS.minZoom,
453+
max: currProps.maxZoom ?? DEFAULT_SETTINGS.maxZoom
454+
}
455+
);
456+
const didUpdatePitch = updatePitchConstraint(
457+
this._map,
458+
{
459+
min: nextProps.minPitch ?? DEFAULT_SETTINGS.minPitch,
460+
max: nextProps.maxPitch ?? DEFAULT_SETTINGS.maxPitch
461+
},
462+
{
463+
min: currProps.minPitch ?? DEFAULT_SETTINGS.minPitch,
464+
max: currProps.maxPitch ?? DEFAULT_SETTINGS.maxPitch
465+
}
466+
);
467+
468+
return didUpdateZoom || didUpdatePitch;
469+
}
470+
417471
/* Update camera constraints and projection settings to match props
418472
@param {object} nextProps
419473
@param {object} currProps
420474
@returns {bool} true if anything is changed
421475
*/
422476
private _updateSettings(nextProps: MaplibreProps, currProps: MaplibreProps): boolean {
423477
const map = this._map;
424-
let changed = false;
478+
let settingsChanged = false;
425479
for (const propName of settingNames) {
426480
const propPresent = propName in nextProps || propName in currProps;
427-
428481
if (propPresent && !deepEqual(nextProps[propName], currProps[propName])) {
429-
changed = true;
482+
settingsChanged = true;
430483
const nextValue = propName in nextProps ? nextProps[propName] : DEFAULT_SETTINGS[propName];
431484
const setter = map[`set${propName[0].toUpperCase()}${propName.slice(1)}`];
432485
setter?.call(map, nextValue);
433486
}
434487
}
435-
return changed;
488+
const constraintsChanged = this._updateConstraints(nextProps, currProps);
489+
return settingsChanged || constraintsChanged;
436490
}
437491

438492
/* Update map style to match props */

modules/react-maplibre/src/utils/transform.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type {MaplibreProps} from '../maplibre/maplibre';
22
import type {ViewState} from '../types/common';
33
import type {TransformLike} from '../types/internal';
4+
import type {MapInstance} from '../types/lib';
45
import {deepEqual} from './deep-equal';
56

67
/**
@@ -56,3 +57,68 @@ export function applyViewStateToTransform(
5657
}
5758
return changes;
5859
}
60+
61+
/**
62+
* Update a min/max constraint pair in the right order to avoid
63+
* temporarily setting min > max (which maplibre rejects).
64+
* @param nextRange - the desired constraint range
65+
* @param currentRange - the current constraint range
66+
* @param setMin - setter for the minimum value
67+
* @param setMax - setter for the maximum value
68+
*/
69+
function updateConstraint(
70+
nextRange: {min: number; max: number},
71+
currentRange: {min: number; max: number},
72+
setMin: (v: number) => void,
73+
setMax: (v: number) => void
74+
): boolean {
75+
if (nextRange.min === currentRange.min && nextRange.max === currentRange.max) {
76+
return false;
77+
}
78+
79+
// When moving up (min increasing), update max first to make room
80+
if (nextRange.min >= currentRange.min) {
81+
if (nextRange.max !== currentRange.max) {
82+
setMax(nextRange.max);
83+
}
84+
if (nextRange.min !== currentRange.min) {
85+
setMin(nextRange.min);
86+
}
87+
} else {
88+
// When moving down (min decreasing), update min first to make room
89+
if (nextRange.min !== currentRange.min) {
90+
setMin(nextRange.min);
91+
}
92+
if (nextRange.max !== currentRange.max) {
93+
setMax(nextRange.max);
94+
}
95+
}
96+
97+
return true;
98+
}
99+
100+
export function updateZoomConstraint(
101+
map: MapInstance,
102+
nextRange: {min: number; max: number},
103+
currentRange: {min: number; max: number}
104+
): boolean {
105+
return updateConstraint(
106+
nextRange,
107+
currentRange,
108+
v => map.setMinZoom(v),
109+
v => map.setMaxZoom(v)
110+
);
111+
}
112+
113+
export function updatePitchConstraint(
114+
map: MapInstance,
115+
nextRange: {min: number; max: number},
116+
currentRange: {min: number; max: number}
117+
): boolean {
118+
return updateConstraint(
119+
nextRange,
120+
currentRange,
121+
v => map.setMinPitch(v),
122+
v => map.setMaxPitch(v)
123+
);
124+
}

modules/react-maplibre/test/utils/transform.spec.js

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import test from 'tape-promise/tape';
22
import {
33
transformToViewState,
4-
applyViewStateToTransform
4+
applyViewStateToTransform,
5+
updateZoomConstraint,
6+
updatePitchConstraint
57
} from '@vis.gl/react-maplibre/utils/transform';
68
import maplibregl from 'maplibre-gl';
79

@@ -64,3 +66,109 @@ test('applyViewStateToTransform', t => {
6466

6567
t.end();
6668
});
69+
70+
function createConstraintMap(setMinName, setMaxName) {
71+
let first = null;
72+
let currentMin = 0;
73+
let currentMax = 0;
74+
const map = {
75+
[setMinName]: nextMin => {
76+
if (nextMin > currentMax) {
77+
throw new Error(`Setting ${setMinName} (${nextMin}) > current max (${currentMax})`);
78+
}
79+
currentMin = nextMin;
80+
if (!first) {
81+
first = 'min';
82+
}
83+
},
84+
[setMaxName]: nextMax => {
85+
if (nextMax < currentMin) {
86+
throw new Error(`Setting ${setMaxName} (${nextMax}) < current min (${currentMin})`);
87+
}
88+
currentMax = nextMax;
89+
if (!first) {
90+
first = 'max';
91+
}
92+
}
93+
};
94+
return {
95+
map,
96+
reset(min, max) {
97+
currentMin = min;
98+
currentMax = max;
99+
first = null;
100+
},
101+
getFirst() {
102+
return first;
103+
}
104+
};
105+
}
106+
107+
function testConstraintUpdate(t, updateFn, setMinName, setMaxName, label) {
108+
const helper = createConstraintMap(setMinName, setMaxName);
109+
110+
// Range shifting down
111+
helper.reset(5, 10);
112+
updateFn(helper.map, {min: 1, max: 3}, {min: 5, max: 10});
113+
t.equal(helper.getFirst(), 'min', `${label}: 5 - 10 -> 1 - 3, update min first`);
114+
115+
// Range shifting up
116+
helper.reset(1, 3);
117+
updateFn(helper.map, {min: 5, max: 10}, {min: 1, max: 3});
118+
t.equal(helper.getFirst(), 'max', `${label}: 1 - 3 -> 5 - 10, update max first`);
119+
120+
// Range expanding
121+
helper.reset(5, 18);
122+
updateFn(helper.map, {min: 3, max: 22}, {min: 5, max: 18});
123+
t.equal(helper.getFirst(), 'min', `${label}: 5 - 18 -> 3 - 22, update min first`);
124+
125+
// Only min changing (decreasing)
126+
helper.reset(5, 18);
127+
updateFn(helper.map, {min: 3, max: 18}, {min: 5, max: 18});
128+
t.equal(helper.getFirst(), 'min', `${label}: 5 - 18 -> 3 - 18, update min first`);
129+
130+
// Range contracting
131+
helper.reset(3, 22);
132+
updateFn(helper.map, {min: 5, max: 18}, {min: 3, max: 22});
133+
t.equal(helper.getFirst(), 'max', `${label}: 3 - 22 -> 5 - 18, update max first`);
134+
135+
// Range shifting down with high start
136+
helper.reset(12, 22);
137+
updateFn(helper.map, {min: 5, max: 10}, {min: 12, max: 22});
138+
t.equal(helper.getFirst(), 'min', `${label}: 12 - 22 -> 5 - 10, update min first`);
139+
140+
// Locked to single value (min === max)
141+
helper.reset(3, 10);
142+
updateFn(helper.map, {min: 5, max: 5}, {min: 3, max: 10});
143+
t.equal(helper.getFirst(), 'max', `${label}: 3 - 10 -> 5 - 5, lock to single value`);
144+
145+
// Unlock from single value
146+
helper.reset(5, 5);
147+
updateFn(helper.map, {min: 3, max: 10}, {min: 5, max: 5});
148+
t.equal(helper.getFirst(), 'min', `${label}: 5 - 5 -> 3 - 10, unlock from single value`);
149+
150+
// Partial overlap (shifting up)
151+
helper.reset(3, 8);
152+
updateFn(helper.map, {min: 6, max: 10}, {min: 3, max: 8});
153+
t.equal(helper.getFirst(), 'max', `${label}: 3 - 8 -> 6 - 10, partial overlap shifting up`);
154+
155+
// Partial overlap (shifting down)
156+
helper.reset(6, 10);
157+
updateFn(helper.map, {min: 3, max: 8}, {min: 6, max: 10});
158+
t.equal(helper.getFirst(), 'min', `${label}: 6 - 10 -> 3 - 8, partial overlap shifting down`);
159+
160+
// No change returns false
161+
helper.reset(3, 10);
162+
const changed = updateFn(helper.map, {min: 3, max: 10}, {min: 3, max: 10});
163+
t.equal(changed, false, `${label}: no change returns false`);
164+
}
165+
166+
test('updateZoomConstraint', t => {
167+
testConstraintUpdate(t, updateZoomConstraint, 'setMinZoom', 'setMaxZoom', 'zoom');
168+
t.end();
169+
});
170+
171+
test('updatePitchConstraint', t => {
172+
testConstraintUpdate(t, updatePitchConstraint, 'setMinPitch', 'setMaxPitch', 'pitch');
173+
t.end();
174+
});

0 commit comments

Comments
 (0)