Skip to content

Commit

Permalink
[maps] fix Tooltip loses pages on refresh (#135593)
Browse files Browse the repository at this point in the history
* [maps] fix Tooltip loses pages on refresh

* increase debounce

* fix jest test

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
nreese and kibanamachine authored Jul 8, 2022
1 parent 3c13cfe commit 6b018f7
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 88 deletions.
21 changes: 0 additions & 21 deletions x-pack/plugins/maps/public/actions/data_request_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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));
};
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/maps/public/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ export {
openOnClickTooltip,
closeOnHoverTooltip,
openOnHoverTooltip,
updateOpenTooltips,
} from './tooltip_actions';
6 changes: 0 additions & 6 deletions x-pack/plugins/maps/public/actions/layer_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ import {
syncDataForLayerId,
updateStyleMeta,
} from './data_request_actions';
import { updateTooltipStateForLayer } from './tooltip_actions';
import {
Attribution,
JoinDescriptor,
Expand Down Expand Up @@ -243,10 +242,6 @@ export function setLayerVisibility(layerId: string, makeVisible: boolean) {
return;
}

if (!makeVisible) {
dispatch(updateTooltipStateForLayer(layer));
}

dispatch({
type: SET_LAYER_VISIBILITY,
layerId,
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 0 additions & 9 deletions x-pack/plugins/maps/public/actions/map_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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));
};
}
Expand Down
55 changes: 5 additions & 50 deletions x-pack/plugins/maps/public/actions/tooltip_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
openOnClickTooltip,
closeOnHoverTooltip,
openOnHoverTooltip,
updateOpenTooltips,
} from '../../../actions';
import {
getLayerList,
Expand Down Expand Up @@ -52,6 +53,9 @@ function mapDispatchToProps(dispatch: ThunkDispatch<MapStoreState, void, AnyActi
openOnHoverTooltip(tooltipState: TooltipState) {
dispatch(openOnHoverTooltip(tooltipState));
},
updateOpenTooltips(openTooltips: TooltipState[]) {
dispatch(updateOpenTooltips(openTooltips));
},
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ const defaultProps = {
openOnClickTooltip: () => {},
closeOnHoverTooltip: () => {},
openOnHoverTooltip: () => {},
updateOpenTooltips: () => {},
layerList: [mockLayer],
isDrawingFilter: false,
addFilters: async () => {},
Expand Down Expand Up @@ -162,7 +163,7 @@ describe('TooltipControl', () => {
test('should un-register all map callbacks on unmount', () => {
const component = mount(<TooltipControl {...defaultProps} />);

expect(Object.keys(mockMbMapHandlers).length).toBe(4);
expect(Object.keys(mockMbMapHandlers).length).toBe(5);

component.unmount();
expect(Object.keys(mockMbMapHandlers).length).toBe(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ import {
Map as MbMap,
MapGeoJSONFeature,
MapMouseEvent,
MapSourceDataEvent,
Point2D,
PointLike,
} from '@kbn/mapbox-gl';
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';
Expand Down Expand Up @@ -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<Props, {}> {
Expand All @@ -73,19 +75,55 @@ export class TooltipControl extends Component<Props, {}> {
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() {
this.props.mbMap.off('mouseout', this._onMouseout);
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) {
Expand Down

0 comments on commit 6b018f7

Please sign in to comment.