Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Enhancement] check bounds before calling fitbounds #1348

Merged
merged 1 commit into from
Dec 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/layers/geojson-layer/geojson-layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,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 Down Expand Up @@ -368,13 +368,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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