Skip to content

Commit

Permalink
[Enhancement] check bounds before calling fitbounds (#1348)
Browse files Browse the repository at this point in the history
Signed-off-by: Shan He <heshan0131@gmail.com>
  • Loading branch information
heshan0131 committed Dec 7, 2020
1 parent f046ac1 commit 7f3be27
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 3 deletions.
6 changes: 4 additions & 2 deletions src/layers/geojson-layer/geojson-layer.js
Expand Up @@ -264,7 +264,7 @@ export default class GeoJsonLayer extends Layer {
}

renderLayer(opts) {
const {data, gpuFilter, objectHovered, mapState} = opts;
const {data, gpuFilter, objectHovered, mapState, interactionConfig} = opts;

const {fixedRadius, featureTypes} = this.meta;
const radiusScale = this.getRadiusScaleByZoom(mapState, fixedRadius);
Expand All @@ -290,13 +290,15 @@ export default class GeoJsonLayer extends Layer {
opacity: visConfig.strokeOpacity
};

const pickable = interactionConfig.tooltip.enabled;
return [
new DeckGLGeoJsonLayer({
...defaultLayerProps,
...layerProps,
...data,
pickable,
highlightColor: HIGHLIGH_COLOR_3D,
autoHighlight: visConfig.enable3d,
autoHighlight: visConfig.enable3d && pickable,
stroked: visConfig.stroked,
filled: visConfig.filled,
extruded: visConfig.enable3d,
Expand Down
8 changes: 7 additions & 1 deletion src/reducers/map-state-updaters.js
Expand Up @@ -19,6 +19,8 @@
// THE SOFTWARE.

import geoViewport from '@mapbox/geo-viewport';
import {validateBounds} from 'utils/projection-utils';
import Console from 'global/console';

/** @typedef {import('./map-state-updaters').MapState} MapState */

Expand Down Expand Up @@ -109,7 +111,11 @@ export const updateMapUpdater = (state, action) => ({
* @public
*/
export const fitBoundsUpdater = (state, action) => {
const bounds = action.payload;
const bounds = validateBounds(action.payload);
if (!bounds) {
Console.warn('invalid map bounds provided')
return state;
}
const {zoom} = geoViewport.viewport(bounds, [state.width, state.height]);
// center being calculated by geo-vieweport.viewport has a complex logic that
// projects and then unprojects the coordinates to determine the center
Expand Down
24 changes: 24 additions & 0 deletions src/utils/projection-utils.js
@@ -0,0 +1,24 @@
function isLat(num) {
return Number.isFinite(num) && num <= 90 && num >= -90;
}
function isLng(num) {
return Number.isFinite(num) && num <= 180 && num >= -180;
}

/**
* bounds should be [minLng, minLat, maxLng, maxLat]
* @param {*} bounds
*/
export function validateBounds(bounds) {
// array: [ -180, -85.05112877980659, 180, 85.0511287798066 ]
// validate bounds
if (
Array.isArray(bounds) &&
bounds.length === 4 &&
[bounds[0], bounds[2]].every(isLng) &&
[bounds[1], bounds[3]].every(isLat)
) {
return bounds;
}
return null;
}
18 changes: 18 additions & 0 deletions test/node/reducers/map-state-test.js
Expand Up @@ -111,6 +111,24 @@ test('#mapStateReducer -> FIT_BOUNDS', t => {
t.end();
});

test('#mapStateReducer -> FIT_BOUNDS.invalid', t => {
// default input and output in @mapbox/geo-viewport
// https://github.com/mapbox/geo-viewport

const mapUpdate = {
width: 640,
height: 480
};

const stateWidthMapDimension = reducer(undefined, updateMap(mapUpdate));
const updatedState = reducer(stateWidthMapDimension, fitBounds(null));
t.equal(updatedState, stateWidthMapDimension, 'should not update state when bounds is invalid');
const updatedState2 = reducer(stateWidthMapDimension, fitBounds([500, -100, 322, 9]));
t.equal(updatedState2, stateWidthMapDimension, 'should not update state when bounds is invalid');

t.end();
});

test('#mapStateReducer -> SPLIT_MAP: toggle', t => {
let newState = reducer(INITIAL_MAP_STATE, toggleSplitMap());

Expand Down

0 comments on commit 7f3be27

Please sign in to comment.