From 6b018f797b97276644e27f58dbfac58c2858d2f3 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Fri, 8 Jul 2022 13:34:56 -0600 Subject: [PATCH] [maps] fix Tooltip loses pages on refresh (#135593) * [maps] fix Tooltip loses pages on refresh * increase debounce * fix jest test Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../public/actions/data_request_actions.ts | 21 ------- x-pack/plugins/maps/public/actions/index.ts | 1 + .../maps/public/actions/layer_actions.ts | 6 -- .../maps/public/actions/map_actions.ts | 9 --- .../maps/public/actions/tooltip_actions.ts | 55 ++----------------- .../mb_map/tooltip_control/index.ts | 4 ++ .../tooltip_control/tooltip_control.test.tsx | 3 +- .../tooltip_control/tooltip_control.tsx | 40 +++++++++++++- 8 files changed, 51 insertions(+), 88 deletions(-) diff --git a/x-pack/plugins/maps/public/actions/data_request_actions.ts b/x-pack/plugins/maps/public/actions/data_request_actions.ts index 8c521fa40b0cd..b5ce42ebefc09 100644 --- a/x-pack/plugins/maps/public/actions/data_request_actions.ts +++ b/x-pack/plugins/maps/public/actions/data_request_actions.ts @@ -35,7 +35,6 @@ import { getInspectorAdapters, ResultMeta, } from '../reducers/non_serializable_instances'; -import { updateTooltipStateForLayer } from './tooltip_actions'; import { LAYER_DATA_LOAD_ENDED, LAYER_DATA_LOAD_ERROR, @@ -307,12 +306,6 @@ function endDataLoad( }); } - if (dataId === SOURCE_DATA_REQUEST_ID) { - if (layer) { - dispatch(updateTooltipStateForLayer(layer, features)); - } - } - dispatch({ type: LAYER_DATA_LOAD_ENDED, layerId, @@ -352,13 +345,6 @@ function onDataLoadError( }); } - if (dataId === SOURCE_DATA_REQUEST_ID) { - const layer = getLayerById(layerId, getState()); - if (layer) { - dispatch(updateTooltipStateForLayer(layer)); - } - } - dispatch({ type: LAYER_DATA_LOAD_ERROR, layerId, @@ -382,13 +368,6 @@ export function updateSourceDataRequest(layerId: string, newData: object) { newData, }); - if ('features' in newData) { - const layer = getLayerById(layerId, getState()); - if (layer) { - dispatch(updateTooltipStateForLayer(layer, (newData as FeatureCollection).features)); - } - } - dispatch(updateStyleMeta(layerId)); }; } diff --git a/x-pack/plugins/maps/public/actions/index.ts b/x-pack/plugins/maps/public/actions/index.ts index 2568a787b3941..96db1cebe7d39 100644 --- a/x-pack/plugins/maps/public/actions/index.ts +++ b/x-pack/plugins/maps/public/actions/index.ts @@ -22,4 +22,5 @@ export { openOnClickTooltip, closeOnHoverTooltip, openOnHoverTooltip, + updateOpenTooltips, } from './tooltip_actions'; diff --git a/x-pack/plugins/maps/public/actions/layer_actions.ts b/x-pack/plugins/maps/public/actions/layer_actions.ts index fee8754b48d7e..4fb817c699e9e 100644 --- a/x-pack/plugins/maps/public/actions/layer_actions.ts +++ b/x-pack/plugins/maps/public/actions/layer_actions.ts @@ -50,7 +50,6 @@ import { syncDataForLayerId, updateStyleMeta, } from './data_request_actions'; -import { updateTooltipStateForLayer } from './tooltip_actions'; import { Attribution, JoinDescriptor, @@ -243,10 +242,6 @@ export function setLayerVisibility(layerId: string, makeVisible: boolean) { return; } - if (!makeVisible) { - dispatch(updateTooltipStateForLayer(layer)); - } - dispatch({ type: SET_LAYER_VISIBILITY, layerId, @@ -597,7 +592,6 @@ function removeLayerFromLayerList(layerId: string) { layerGettingRemoved.getInFlightRequestTokens().forEach((requestToken) => { dispatch(cancelRequest(requestToken)); }); - dispatch(updateTooltipStateForLayer(layerGettingRemoved)); clearInspectorAdapters(layerGettingRemoved, getInspectorAdapters(getState())); dispatch({ type: REMOVE_LAYER, diff --git a/x-pack/plugins/maps/public/actions/map_actions.ts b/x-pack/plugins/maps/public/actions/map_actions.ts index 446e0211cc295..e02ca86324083 100644 --- a/x-pack/plugins/maps/public/actions/map_actions.ts +++ b/x-pack/plugins/maps/public/actions/map_actions.ts @@ -70,7 +70,6 @@ import { Timeslice, } from '../../common/descriptor_types'; import { INITIAL_LOCATION } from '../../common/constants'; -import { updateTooltipStateForLayer } from './tooltip_actions'; import { isVectorLayer, IVectorLayer } from '../classes/layers/vector_layer'; import { SET_DRAW_MODE, pushDeletedFeatureId, clearDeletedFeatureIds } from './ui_actions'; import { expandToTileBoundaries, getTilesForExtent } from '../classes/util/geo_tile_utils'; @@ -232,14 +231,6 @@ export function mapExtentChanged(mapExtentState: MapExtentState) { } as MapViewContext, }); - if (prevZoom !== nextZoom) { - getLayerList(getState()).map((layer) => { - if (!layer.showAtZoomLevel(nextZoom)) { - dispatch(updateTooltipStateForLayer(layer)); - } - }); - } - dispatch(syncDataForAllLayers(false)); }; } diff --git a/x-pack/plugins/maps/public/actions/tooltip_actions.ts b/x-pack/plugins/maps/public/actions/tooltip_actions.ts index f1842ade4277e..22deb1fd1e930 100644 --- a/x-pack/plugins/maps/public/actions/tooltip_actions.ts +++ b/x-pack/plugins/maps/public/actions/tooltip_actions.ts @@ -7,14 +7,10 @@ import _ from 'lodash'; import { Dispatch } from 'redux'; -import { Feature } from 'geojson'; import { getOpenTooltips } from '../selectors/map_selectors'; import { SET_OPEN_TOOLTIPS } from './map_action_constants'; -import { FEATURE_VISIBLE_PROPERTY_NAME } from '../../common/constants'; -import { TooltipFeature, TooltipState } from '../../common/descriptor_types'; +import { TooltipState } from '../../common/descriptor_types'; import { MapStoreState } from '../reducers/store'; -import { ILayer } from '../classes/layers/layer'; -import { IVectorLayer, isVectorLayer } from '../classes/layers/vector_layer'; export function closeOnClickTooltip(tooltipId: string) { return (dispatch: Dispatch, getState: () => MapStoreState) => { @@ -64,50 +60,9 @@ export function openOnHoverTooltip(tooltipState: TooltipState) { }; } -export function updateTooltipStateForLayer(layer: ILayer, layerFeatures: Feature[] = []) { - return (dispatch: Dispatch, getState: () => MapStoreState) => { - if (!isVectorLayer(layer)) { - return; - } - - const openTooltips = getOpenTooltips(getState()) - .map((tooltipState) => { - const nextFeatures: TooltipFeature[] = []; - tooltipState.features.forEach((tooltipFeature) => { - if (tooltipFeature.layerId !== layer.getId()) { - // feature from another layer, keep it - nextFeatures.push(tooltipFeature); - } - - const updatedFeature = layerFeatures.find((layerFeature) => { - const isVisible = - layerFeature.properties![FEATURE_VISIBLE_PROPERTY_NAME] !== undefined - ? layerFeature.properties![FEATURE_VISIBLE_PROPERTY_NAME] - : true; - return ( - isVisible && (layer as IVectorLayer).getFeatureId(layerFeature) === tooltipFeature.id - ); - }); - - if (updatedFeature) { - nextFeatures.push({ - ...tooltipFeature, - mbProperties: { - ...updatedFeature.properties, - }, - }); - } - }); - - return { ...tooltipState, features: nextFeatures }; - }) - .filter((tooltipState) => { - return tooltipState.features.length > 0; - }); - - dispatch({ - type: SET_OPEN_TOOLTIPS, - openTooltips, - }); +export function updateOpenTooltips(openTooltips: TooltipState[]) { + return { + type: SET_OPEN_TOOLTIPS, + openTooltips, }; } diff --git a/x-pack/plugins/maps/public/connected_components/mb_map/tooltip_control/index.ts b/x-pack/plugins/maps/public/connected_components/mb_map/tooltip_control/index.ts index a9281898902e4..95c4e6bf43b83 100644 --- a/x-pack/plugins/maps/public/connected_components/mb_map/tooltip_control/index.ts +++ b/x-pack/plugins/maps/public/connected_components/mb_map/tooltip_control/index.ts @@ -15,6 +15,7 @@ import { openOnClickTooltip, closeOnHoverTooltip, openOnHoverTooltip, + updateOpenTooltips, } from '../../../actions'; import { getLayerList, @@ -52,6 +53,9 @@ function mapDispatchToProps(dispatch: ThunkDispatch {}, closeOnHoverTooltip: () => {}, openOnHoverTooltip: () => {}, + updateOpenTooltips: () => {}, layerList: [mockLayer], isDrawingFilter: false, addFilters: async () => {}, @@ -162,7 +163,7 @@ describe('TooltipControl', () => { test('should un-register all map callbacks on unmount', () => { const component = mount(); - expect(Object.keys(mockMbMapHandlers).length).toBe(4); + expect(Object.keys(mockMbMapHandlers).length).toBe(5); component.unmount(); expect(Object.keys(mockMbMapHandlers).length).toBe(0); diff --git a/x-pack/plugins/maps/public/connected_components/mb_map/tooltip_control/tooltip_control.tsx b/x-pack/plugins/maps/public/connected_components/mb_map/tooltip_control/tooltip_control.tsx index 87e39174f4922..41a69b4d3c065 100644 --- a/x-pack/plugins/maps/public/connected_components/mb_map/tooltip_control/tooltip_control.tsx +++ b/x-pack/plugins/maps/public/connected_components/mb_map/tooltip_control/tooltip_control.tsx @@ -12,6 +12,7 @@ import { Map as MbMap, MapGeoJSONFeature, MapMouseEvent, + MapSourceDataEvent, Point2D, PointLike, } from '@kbn/mapbox-gl'; @@ -19,7 +20,7 @@ import uuid from 'uuid/v4'; import { Geometry } from 'geojson'; import { Filter } from '@kbn/es-query'; import { ActionExecutionContext, Action } from '@kbn/ui-actions-plugin/public'; -import { LON_INDEX, RawValue } from '../../../../common/constants'; +import { LON_INDEX, RawValue, SPATIAL_FILTERS_LAYER_ID } from '../../../../common/constants'; import { TooltipFeature, TooltipState } from '../../../../common/descriptor_types'; import { TooltipPopover } from './tooltip_popover'; import { ILayer } from '../../../classes/layers/layer'; @@ -63,6 +64,7 @@ export interface Props { onSingleValueTrigger?: (actionId: string, key: string, value: RawValue) => void; openTooltips: TooltipState[]; renderTooltipContent?: RenderToolTipContent; + updateOpenTooltips: (openTooltips: TooltipState[]) => void; } export class TooltipControl extends Component { @@ -73,6 +75,7 @@ export class TooltipControl extends Component { this.props.mbMap.on('mousemove', this._updateHoverTooltipState); this.props.mbMap.on('click', this._lockTooltip); this.props.mbMap.on('remove', this._setIsMapRemoved); + this.props.mbMap.on('sourcedata', this._onSourceData); } componentWillUnmount() { @@ -80,12 +83,47 @@ export class TooltipControl extends Component { this.props.mbMap.off('mousemove', this._updateHoverTooltipState); this.props.mbMap.off('click', this._lockTooltip); this.props.mbMap.off('remove', this._setIsMapRemoved); + this.props.mbMap.off('sourcedata', this._onSourceData); } _setIsMapRemoved = () => { this._isMapRemoved = true; }; + _onSourceData = (e: MapSourceDataEvent) => { + if ( + e.sourceId && + e.sourceId !== SPATIAL_FILTERS_LAYER_ID && + e.dataType === 'source' && + (e.source.type === 'vector' || e.source.type === 'geojson') + ) { + // map features changed, update tooltip state to match new map state. + this._updateOpentooltips(); + } + }; + + _updateOpentooltips = _.debounce(() => { + if (this.props.openTooltips.length === 0) { + return; + } + + const openTooltips = this.props.openTooltips + .map((tooltipState) => { + const mbFeatures = this._getMbFeaturesUnderPointer( + this.props.mbMap.project(tooltipState.location) + ); + return { + ...tooltipState, + features: this._getTooltipFeatures(mbFeatures, tooltipState.isLocked, tooltipState.id), + }; + }) + .filter((tooltipState) => { + return tooltipState.features.length > 0; + }); + + this.props.updateOpenTooltips(openTooltips); + }, 300); + _onMouseout = () => { this._updateHoverTooltipState.cancel(); if (!this.props.hasLockedTooltips) {