From 3f8d2c73e20ab0f03efa10736e5c9fc5ce25654d Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 30 Aug 2021 20:30:08 -0400 Subject: [PATCH] [Maps] cleanup mapExtentChanged thunk (#110098) (#110540) * [Maps] cleanup mapExtentChanged thunk * rename for clarity * cleanup * review feedback Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Nathan Reese --- ...ap_actions.test.js => map_actions.test.ts} | 211 ++++++++++-------- .../maps/public/actions/map_actions.ts | 77 +++---- .../connected_components/mb_map/index.ts | 2 +- .../connected_components/mb_map/mb_map.tsx | 10 +- .../plugins/maps/public/reducers/map/map.ts | 8 +- .../plugins/maps/public/reducers/map/types.ts | 16 +- 6 files changed, 168 insertions(+), 156 deletions(-) rename x-pack/plugins/maps/public/actions/{map_actions.test.js => map_actions.test.ts} (68%) diff --git a/x-pack/plugins/maps/public/actions/map_actions.test.js b/x-pack/plugins/maps/public/actions/map_actions.test.ts similarity index 68% rename from x-pack/plugins/maps/public/actions/map_actions.test.js rename to x-pack/plugins/maps/public/actions/map_actions.test.ts index 763ce459dcc25..d222d8e5b0466 100644 --- a/x-pack/plugins/maps/public/actions/map_actions.test.js +++ b/x-pack/plugins/maps/public/actions/map_actions.test.ts @@ -5,6 +5,8 @@ * 2.0. */ +/* eslint @typescript-eslint/no-var-requires: 0 */ + jest.mock('../selectors/map_selectors', () => ({})); jest.mock('./data_request_actions', () => { return { @@ -30,7 +32,7 @@ describe('map_actions', () => { }); describe('mapExtentChanged', () => { - describe('store mapState is empty', () => { + describe('mapState.buffer is undefined', () => { beforeEach(() => { require('../selectors/map_selectors').getDataFilters = () => { return { @@ -43,20 +45,12 @@ describe('map_actions', () => { }; }); - it('should add newMapConstants to dispatch action mapState', async () => { - const action = mapExtentChanged({ zoom: 5 }); - await action(dispatchMock, getStoreMock); - - expect(dispatchMock).toHaveBeenCalledWith({ - mapState: { - zoom: 5, - }, - type: 'MAP_EXTENT_CHANGED', - }); - }); - - it('should add buffer to dispatch action mapState', async () => { + it('should set buffer', () => { const action = mapExtentChanged({ + center: { + lat: 7.5, + lon: 97.5, + }, extent: { maxLat: 10, maxLon: 100, @@ -65,30 +59,36 @@ describe('map_actions', () => { }, zoom: 5, }); - await action(dispatchMock, getStoreMock); + action(dispatchMock, getStoreMock); - expect(dispatchMock).toHaveBeenCalledWith({ - mapState: { - zoom: 5, - extent: { - maxLat: 10, - maxLon: 100, - minLat: 5, - minLon: 95, - }, - buffer: { - maxLat: 11.1784, - maxLon: 101.25, - minLat: 0, - minLon: 90, + expect(dispatchMock.mock.calls[0]).toEqual([ + { + mapViewContext: { + center: { + lat: 7.5, + lon: 97.5, + }, + zoom: 5, + extent: { + maxLat: 10, + maxLon: 100, + minLat: 5, + minLon: 95, + }, + buffer: { + maxLat: 11.1784, + maxLon: 101.25, + minLat: 0, + minLon: 90, + }, }, + type: 'MAP_EXTENT_CHANGED', }, - type: 'MAP_EXTENT_CHANGED', - }); + ]); }); }); - describe('store mapState is populated', () => { + describe('mapState.buffer is defined', () => { const initialZoom = 10; beforeEach(() => { require('../selectors/map_selectors').getDataFilters = () => { @@ -104,8 +104,12 @@ describe('map_actions', () => { }; }); - it('should not update buffer if extent is contained in existing buffer', async () => { + it('should not update buffer if extent is contained in existing buffer', () => { const action = mapExtentChanged({ + center: { + lat: 8.5, + lon: 98.5, + }, zoom: initialZoom, extent: { maxLat: 11, @@ -114,30 +118,40 @@ describe('map_actions', () => { minLon: 96, }, }); - await action(dispatchMock, getStoreMock); - - expect(dispatchMock).toHaveBeenCalledWith({ - mapState: { - zoom: 10, - extent: { - maxLat: 11, - maxLon: 101, - minLat: 6, - minLon: 96, - }, - buffer: { - maxLat: 12.5, - maxLon: 102.5, - minLat: 2.5, - minLon: 92.5, + action(dispatchMock, getStoreMock); + + expect(dispatchMock.mock.calls[0]).toEqual([ + { + mapViewContext: { + center: { + lat: 8.5, + lon: 98.5, + }, + zoom: 10, + extent: { + maxLat: 11, + maxLon: 101, + minLat: 6, + minLon: 96, + }, + buffer: { + maxLat: 12.5, + maxLon: 102.5, + minLat: 2.5, + minLon: 92.5, + }, }, + type: 'MAP_EXTENT_CHANGED', }, - type: 'MAP_EXTENT_CHANGED', - }); + ]); }); - it('should update buffer if extent is outside of existing buffer', async () => { + it('should update buffer if extent is outside of existing buffer', () => { const action = mapExtentChanged({ + center: { + lat: 2.5, + lon: 87.5, + }, zoom: initialZoom, extent: { maxLat: 5, @@ -146,30 +160,40 @@ describe('map_actions', () => { minLon: 85, }, }); - await action(dispatchMock, getStoreMock); - - expect(dispatchMock).toHaveBeenCalledWith({ - mapState: { - zoom: 10, - extent: { - maxLat: 5, - maxLon: 90, - minLat: 0, - minLon: 85, - }, - buffer: { - maxLat: 5.26601, - maxLon: 90.35156, - minLat: -0.35156, - minLon: 84.72656, + action(dispatchMock, getStoreMock); + + expect(dispatchMock.mock.calls[0]).toEqual([ + { + mapViewContext: { + center: { + lat: 2.5, + lon: 87.5, + }, + zoom: 10, + extent: { + maxLat: 5, + maxLon: 90, + minLat: 0, + minLon: 85, + }, + buffer: { + maxLat: 5.26601, + maxLon: 90.35156, + minLat: -0.35156, + minLon: 84.72656, + }, }, + type: 'MAP_EXTENT_CHANGED', }, - type: 'MAP_EXTENT_CHANGED', - }); + ]); }); - it('should update buffer when zoom changes', async () => { + it('should update buffer when zoom changes', () => { const action = mapExtentChanged({ + center: { + lat: 8.5, + lon: 98.5, + }, zoom: initialZoom + 1, extent: { maxLat: 11, @@ -178,26 +202,32 @@ describe('map_actions', () => { minLon: 96, }, }); - await action(dispatchMock, getStoreMock); - - expect(dispatchMock).toHaveBeenCalledWith({ - mapState: { - zoom: 11, - extent: { - maxLat: 11, - maxLon: 101, - minLat: 6, - minLon: 96, - }, - buffer: { - maxLat: 11.0059, - maxLon: 101.07422, - minLat: 5.96575, - minLon: 95.97656, + action(dispatchMock, getStoreMock); + + expect(dispatchMock.mock.calls[0]).toEqual([ + { + mapViewContext: { + center: { + lat: 8.5, + lon: 98.5, + }, + zoom: 11, + extent: { + maxLat: 11, + maxLon: 101, + minLat: 6, + minLon: 96, + }, + buffer: { + maxLat: 11.0059, + maxLon: 101.07422, + minLat: 5.96575, + minLon: 95.97656, + }, }, + type: 'MAP_EXTENT_CHANGED', }, - type: 'MAP_EXTENT_CHANGED', - }); + ]); }); }); }); @@ -262,13 +292,12 @@ describe('map_actions', () => { params: { query: 'png' }, }, query: { match_phrase: { extension: 'png' } }, - $state: { store: 'appState' }, }, ]; const searchSessionId = '1234'; beforeEach(() => { - //Mocks the "previous" state + // Mocks the "previous" state require('../selectors/map_selectors').getQuery = () => { return query; }; diff --git a/x-pack/plugins/maps/public/actions/map_actions.ts b/x-pack/plugins/maps/public/actions/map_actions.ts index af8d6cc6bedc3..45f3299db9041 100644 --- a/x-pack/plugins/maps/public/actions/map_actions.ts +++ b/x-pack/plugins/maps/public/actions/map_actions.ts @@ -13,6 +13,7 @@ import turfBooleanContains from '@turf/boolean-contains'; import { Filter, Query, TimeRange } from 'src/plugins/data/public'; import { Geometry, Position } from 'geojson'; import { DRAW_MODE, DRAW_SHAPE } from '../../common/constants'; +import type { MapExtentState, MapViewContext } from '../reducers/map/types'; import { MapStoreState } from '../reducers/store'; import { getDataFilters, @@ -52,25 +53,13 @@ import { import { autoFitToBounds, syncDataForAllLayers, syncDataForLayer } from './data_request_actions'; import { addLayer, addLayerWithoutDataSync } from './layer_actions'; import { MapSettings } from '../reducers/map'; -import { - DrawState, - MapCenter, - MapCenterAndZoom, - MapExtent, - Timeslice, -} from '../../common/descriptor_types'; +import { DrawState, MapCenterAndZoom, MapExtent, Timeslice } from '../../common/descriptor_types'; import { INITIAL_LOCATION } from '../../common/constants'; import { cleanTooltipStateForLayer } from './tooltip_actions'; import { VectorLayer } from '../classes/layers/vector_layer'; import { SET_DRAW_MODE } from './ui_actions'; import { expandToTileBoundaries } from '../../common/geo_tile_utils'; -export interface MapExtentState { - zoom: number; - extent: MapExtent; - center: MapCenter; -} - export function setMapInitError(errorMessage: string) { return { type: SET_MAP_INIT_ERROR, @@ -138,56 +127,50 @@ export function mapDestroyed() { } export function mapExtentChanged(mapExtentState: MapExtentState) { - return async ( + return ( dispatch: ThunkDispatch, getState: () => MapStoreState ) => { - const dataFilters = getDataFilters(getState()); - const { extent, zoom: newZoom } = mapExtentState; - const { buffer, zoom: currentZoom } = dataFilters; - - if (extent) { - let doesBufferContainExtent = false; - if (buffer) { - const bufferGeometry = turfBboxPolygon([ - buffer.minLon, - buffer.minLat, - buffer.maxLon, - buffer.maxLat, - ]); - const extentGeometry = turfBboxPolygon([ - extent.minLon, - extent.minLat, - extent.maxLon, - extent.maxLat, - ]); - - doesBufferContainExtent = turfBooleanContains(bufferGeometry, extentGeometry); - } - - if (!doesBufferContainExtent || currentZoom !== newZoom) { - // snap to the smallest tile-bounds, to avoid jitter in the bounds - dataFilters.buffer = expandToTileBoundaries(extent, Math.ceil(newZoom)); - } + const { extent, zoom: nextZoom } = mapExtentState; + const { buffer: prevBuffer, zoom: prevZoom } = getDataFilters(getState()); + + let doesPrevBufferContainNextExtent = true; + if (prevBuffer) { + const bufferGeometry = turfBboxPolygon([ + prevBuffer.minLon, + prevBuffer.minLat, + prevBuffer.maxLon, + prevBuffer.maxLat, + ]); + const extentGeometry = turfBboxPolygon([ + extent.minLon, + extent.minLat, + extent.maxLon, + extent.maxLat, + ]); + doesPrevBufferContainNextExtent = turfBooleanContains(bufferGeometry, extentGeometry); } dispatch({ type: MAP_EXTENT_CHANGED, - mapState: { - ...dataFilters, + mapViewContext: { ...mapExtentState, - }, + buffer: + !prevBuffer || !doesPrevBufferContainNextExtent || prevZoom !== nextZoom + ? expandToTileBoundaries(extent, Math.ceil(nextZoom)) + : prevBuffer, + } as MapViewContext, }); - if (currentZoom !== newZoom) { + if (prevZoom !== nextZoom) { getLayerList(getState()).map((layer) => { - if (!layer.showAtZoomLevel(newZoom)) { + if (!layer.showAtZoomLevel(nextZoom)) { dispatch(cleanTooltipStateForLayer(layer.getId())); } }); } - await dispatch(syncDataForAllLayers()); + dispatch(syncDataForAllLayers()); }; } diff --git a/x-pack/plugins/maps/public/connected_components/mb_map/index.ts b/x-pack/plugins/maps/public/connected_components/mb_map/index.ts index 3084d3b9c9f3f..9936d412de9e6 100644 --- a/x-pack/plugins/maps/public/connected_components/mb_map/index.ts +++ b/x-pack/plugins/maps/public/connected_components/mb_map/index.ts @@ -14,7 +14,6 @@ import { clearMouseCoordinates, mapDestroyed, mapExtentChanged, - MapExtentState, mapReady, setAreTilesLoaded, setMapInitError, @@ -35,6 +34,7 @@ import { getInspectorAdapters } from '../../reducers/non_serializable_instances' import { MapStoreState } from '../../reducers/store'; import { DRAW_MODE } from '../../../common'; import { TileMetaFeature } from '../../../common/descriptor_types'; +import type { MapExtentState } from '../../reducers/map/types'; function mapStateToProps(state: MapStoreState) { return { diff --git a/x-pack/plugins/maps/public/connected_components/mb_map/mb_map.tsx b/x-pack/plugins/maps/public/connected_components/mb_map/mb_map.tsx index a60aef95318f0..053e410b8c712 100644 --- a/x-pack/plugins/maps/public/connected_components/mb_map/mb_map.tsx +++ b/x-pack/plugins/maps/public/connected_components/mb_map/mb_map.tsx @@ -49,10 +49,10 @@ import { } from './utils'; import { ResizeChecker } from '../../../../../../src/plugins/kibana_utils/public'; import { RenderToolTipContent } from '../../classes/tooltips/tooltip_property'; -import { MapExtentState } from '../../actions'; import { TileStatusTracker } from './tile_status_tracker'; import { DrawFeatureControl } from './draw_control/draw_feature_control'; import { TiledVectorLayer } from '../../classes/layers/tiled_vector_layer/tiled_vector_layer'; +import type { MapExtentState } from '../../reducers/map/types'; export interface Props { isMapReady: boolean; @@ -150,7 +150,7 @@ export class MbMap extends Component { } }, 256); - _getMapState() { + _getMapExtentState(): MapExtentState { const zoom = this.state.mbMap!.getZoom(); const mbCenter = this.state.mbMap!.getCenter(); const mbBounds = this.state.mbMap!.getBounds(); @@ -257,7 +257,7 @@ export class MbMap extends Component { this._loadMakiSprites(mbMap); this._initResizerChecker(); this._registerMapEventListeners(mbMap); - this.props.onMapReady(this._getMapState()); + this.props.onMapReady(this._getMapExtentState()); }); } @@ -269,7 +269,7 @@ export class MbMap extends Component { mbMap.on( 'moveend', _.debounce(() => { - this.props.extentChanged(this._getMapState()); + this.props.extentChanged(this._getMapExtentState()); }, 100) ); @@ -413,7 +413,7 @@ export class MbMap extends Component { // hack to update extent after zoom update finishes moving map. if (zoomRangeChanged) { setTimeout(() => { - this.props.extentChanged(this._getMapState()); + this.props.extentChanged(this._getMapExtentState()); }, 300); } } diff --git a/x-pack/plugins/maps/public/reducers/map/map.ts b/x-pack/plugins/maps/public/reducers/map/map.ts index de74adf55ba9a..30db4ef8120ad 100644 --- a/x-pack/plugins/maps/public/reducers/map/map.ts +++ b/x-pack/plugins/maps/public/reducers/map/map.ts @@ -218,13 +218,7 @@ export function map(state: MapState = DEFAULT_MAP_STATE, action: Record & { scrollZoom: boolean; - buffer?: MapExtent; - extent?: MapExtent; mouseCoordinates?: { lat: number; lon: number;