From 43b1e17eb2b1e736301b88363d3d275905e4877b Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Sun, 9 Feb 2020 10:57:45 -0700 Subject: [PATCH 1/9] split TooltipPopover from TooltipContent --- .../map/mb/tooltip_control/tooltip_control.js | 175 +--------------- .../map/mb/tooltip_control/tooltip_popover.js | 189 ++++++++++++++++++ 2 files changed, 200 insertions(+), 164 deletions(-) create mode 100644 x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js index cfb92a8677455..06ad87bf42a82 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js @@ -6,17 +6,14 @@ import _ from 'lodash'; import React from 'react'; -import { FEATURE_ID_PROPERTY_NAME, LAT_INDEX, LON_INDEX } from '../../../../../common/constants'; -import { FeaturesTooltip } from '../../features_tooltip/features_tooltip'; -import { EuiPopover, EuiText } from '@elastic/eui'; +import { FEATURE_ID_PROPERTY_NAME, LON_INDEX } from '../../../../../common/constants'; +import { TooltipPopover } from './tooltip_popover'; export const TOOLTIP_TYPE = { HOVER: 'HOVER', LOCKED: 'LOCKED', }; -const noop = () => {}; - function justifyAnchorLocation(mbLngLat, targetFeature) { let popupAnchorLocation = [mbLngLat.lng, mbLngLat.lat]; // default popup location to mouse location if (targetFeature.geometry.type === 'Point') { @@ -35,47 +32,15 @@ function justifyAnchorLocation(mbLngLat, targetFeature) { } export class TooltipControl extends React.Component { - state = { - x: undefined, - y: undefined, - }; - - constructor(props) { - super(props); - this._popoverRef = React.createRef(); - } - - static getDerivedStateFromProps(nextProps, prevState) { - if (nextProps.tooltipState) { - const nextPoint = nextProps.mbMap.project(nextProps.tooltipState.location); - if (nextPoint.x !== prevState.x || nextPoint.y !== prevState.y) { - return { - x: nextPoint.x, - y: nextPoint.y, - }; - } - } - - return null; - } - componentDidMount() { this.props.mbMap.on('mouseout', this._onMouseout); this.props.mbMap.on('mousemove', this._updateHoverTooltipState); - this.props.mbMap.on('move', this._updatePopoverPosition); this.props.mbMap.on('click', this._lockTooltip); } - componentDidUpdate() { - if (this.props.tooltipState && this._popoverRef.current) { - this._popoverRef.current.positionPopoverFluid(); - } - } - componentWillUnmount() { this.props.mbMap.off('mouseout', this._onMouseout); this.props.mbMap.off('mousemove', this._updateHoverTooltipState); - this.props.mbMap.off('move', this._updatePopoverPosition); this.props.mbMap.off('click', this._lockTooltip); } @@ -86,31 +51,6 @@ export class TooltipControl extends React.Component { } }; - _updatePopoverPosition = () => { - if (!this.props.tooltipState) { - return; - } - - const lat = this.props.tooltipState.location[LAT_INDEX]; - const lon = this.props.tooltipState.location[LON_INDEX]; - const bounds = this.props.mbMap.getBounds(); - if ( - lat > bounds.getNorth() || - lat < bounds.getSouth() || - lon < bounds.getWest() || - lon > bounds.getEast() - ) { - this.props.clearTooltipState(); - return; - } - - const nextPoint = this.props.mbMap.project(this.props.tooltipState.location); - this.setState({ - x: nextPoint.x, - y: nextPoint.y, - }); - }; - _getLayerByMbLayerId(mbLayerId) { return this.props.layerList.find(layer => { const mbLayerIds = layer.getMbLayerIds(); @@ -240,114 +180,21 @@ export class TooltipControl extends React.Component { return this.props.mbMap.queryRenderedFeatures(mbBbox, { layers: mbLayerIds }); } - // Must load original geometry instead of using geometry from mapbox feature. - // Mapbox feature geometry is from vector tile and is not the same as the original geometry. - _loadFeatureGeometry = ({ layerId, featureId }) => { - const tooltipLayer = this._findLayerById(layerId); - if (!tooltipLayer) { - return null; - } - - const targetFeature = tooltipLayer.getFeatureById(featureId); - if (!targetFeature) { - return null; - } - - return targetFeature.geometry; - }; - - _loadFeatureProperties = async ({ layerId, featureId }) => { - const tooltipLayer = this._findLayerById(layerId); - if (!tooltipLayer) { - return []; - } - - const targetFeature = tooltipLayer.getFeatureById(featureId); - if (!targetFeature) { - return []; - } - return await tooltipLayer.getPropertiesForTooltip(targetFeature.properties); - }; - - _loadPreIndexedShape = async ({ layerId, featureId }) => { - const tooltipLayer = this._findLayerById(layerId); - if (!tooltipLayer) { - return null; - } - - const targetFeature = tooltipLayer.getFeatureById(featureId); - if (!targetFeature) { - return null; - } - - return await tooltipLayer.getSource().getPreIndexedShape(targetFeature.properties); - }; - - _findLayerById = layerId => { - return this.props.layerList.find(layer => { - return layer.getId() === layerId; - }); - }; - - _getLayerName = async layerId => { - const layer = this._findLayerById(layerId); - if (!layer) { - return null; - } - - return layer.getDisplayName(); - }; - - _renderTooltipContent = () => { - const publicProps = { - addFilters: this.props.addFilters, - closeTooltip: this.props.clearTooltipState, - features: this.props.tooltipState.features, - isLocked: this.props.tooltipState.type === TOOLTIP_TYPE.LOCKED, - loadFeatureProperties: this._loadFeatureProperties, - loadFeatureGeometry: this._loadFeatureGeometry, - getLayerName: this._getLayerName, - }; - - if (this.props.renderTooltipContent) { - return this.props.renderTooltipContent(publicProps); - } - - return ( - - - - ); - }; - render() { if (!this.props.tooltipState) { return null; } - const tooltipAnchor = ( -
- ); return ( - - {this._renderTooltipContent()} - + ); } } diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js new file mode 100644 index 0000000000000..40077ea5e1723 --- /dev/null +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js @@ -0,0 +1,189 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { Component } from 'react'; +import { LAT_INDEX, LON_INDEX } from '../../../../../common/constants'; +import { FeaturesTooltip } from '../../features_tooltip/features_tooltip'; +import { EuiPopover, EuiText } from '@elastic/eui'; + +export const TOOLTIP_TYPE = { + HOVER: 'HOVER', + LOCKED: 'LOCKED', +}; + +const noop = () => {}; + +export class TooltipPopover extends Component { + state = { + x: undefined, + y: undefined, + isVisible: true, + }; + + constructor(props) { + super(props); + this._popoverRef = React.createRef(); + } + + static getDerivedStateFromProps(nextProps, prevState) { + if (nextProps.tooltipState) { + const nextPoint = nextProps.mbMap.project(nextProps.tooltipState.location); + if (nextPoint.x !== prevState.x || nextPoint.y !== prevState.y) { + return { + x: nextPoint.x, + y: nextPoint.y, + }; + } + } + + return null; + } + + componentDidMount() { + this.props.mbMap.on('move', this._updatePopoverPosition); + } + + componentDidUpdate() { + if (this.props.tooltipState && this._popoverRef.current) { + this._popoverRef.current.positionPopoverFluid(); + } + } + + componentWillUnmount() { + this.props.mbMap.off('move', this._updatePopoverPosition); + } + + _updatePopoverPosition = () => { + if (!this.props.tooltipState) { + return; + } + + const nextPoint = this.props.mbMap.project(this.props.tooltipState.location); + const lat = this.props.tooltipState.location[LAT_INDEX]; + const lon = this.props.tooltipState.location[LON_INDEX]; + const bounds = this.props.mbMap.getBounds(); + this.setState({ + x: nextPoint.x, + y: nextPoint.y, + isVisible: + lat < bounds.getNorth() && + lat > bounds.getSouth() && + lon > bounds.getWest() && + lon < bounds.getEast(), + }); + }; + + // Must load original geometry instead of using geometry from mapbox feature. + // Mapbox feature geometry is from vector tile and is not the same as the original geometry. + _loadFeatureGeometry = ({ layerId, featureId }) => { + const tooltipLayer = this._findLayerById(layerId); + if (!tooltipLayer) { + return null; + } + + const targetFeature = tooltipLayer.getFeatureById(featureId); + if (!targetFeature) { + return null; + } + + return targetFeature.geometry; + }; + + _loadFeatureProperties = async ({ layerId, featureId }) => { + const tooltipLayer = this._findLayerById(layerId); + if (!tooltipLayer) { + return []; + } + + const targetFeature = tooltipLayer.getFeatureById(featureId); + if (!targetFeature) { + return []; + } + return await tooltipLayer.getPropertiesForTooltip(targetFeature.properties); + }; + + _loadPreIndexedShape = async ({ layerId, featureId }) => { + const tooltipLayer = this._findLayerById(layerId); + if (!tooltipLayer) { + return null; + } + + const targetFeature = tooltipLayer.getFeatureById(featureId); + if (!targetFeature) { + return null; + } + + return await tooltipLayer.getSource().getPreIndexedShape(targetFeature.properties); + }; + + _findLayerById = layerId => { + return this.props.layerList.find(layer => { + return layer.getId() === layerId; + }); + }; + + _getLayerName = async layerId => { + const layer = this._findLayerById(layerId); + if (!layer) { + return null; + } + + return layer.getDisplayName(); + }; + + _renderTooltipContent = () => { + const publicProps = { + addFilters: this.props.addFilters, + closeTooltip: this.props.clearTooltipState, + features: this.props.tooltipState.features, + isLocked: this.props.tooltipState.type === TOOLTIP_TYPE.LOCKED, + loadFeatureProperties: this._loadFeatureProperties, + loadFeatureGeometry: this._loadFeatureGeometry, + getLayerName: this._getLayerName, + }; + + if (this.props.renderTooltipContent) { + return this.props.renderTooltipContent(publicProps); + } + + return ( + + + + ); + }; + + render() { + if (!this.props.tooltipState || !this.state.isVisible) { + return null; + } + + const tooltipAnchor = ( +
+ ); + return ( + + {this._renderTooltipContent()} + + ); + } +} From f6d35ac661f27c60bb6cb57a184c3b6dcee17631 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Sun, 9 Feb 2020 13:48:24 -0700 Subject: [PATCH 2/9] display multiple tooltips --- .../maps/public/actions/map_actions.js | 36 +++++++++++++++ .../map/mb/tooltip_control/index.js | 14 +++++- .../map/mb/tooltip_control/tooltip_control.js | 41 ++++++++++------- .../map/mb/tooltip_control/tooltip_popover.js | 44 +++++-------------- .../maps/public/selectors/map_selectors.js | 4 ++ 5 files changed, 87 insertions(+), 52 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/actions/map_actions.js b/x-pack/legacy/plugins/maps/public/actions/map_actions.js index 59b54c2434d17..6085100610238 100644 --- a/x-pack/legacy/plugins/maps/public/actions/map_actions.js +++ b/x-pack/legacy/plugins/maps/public/actions/map_actions.js @@ -6,6 +6,7 @@ import turf from 'turf'; import turfBooleanContains from '@turf/boolean-contains'; +import uuid from 'uuid/v4'; import { getLayerList, getLayerListRaw, @@ -15,6 +16,7 @@ import { getWaitingForMapReadyLayerListRaw, getTransientLayerId, getTooltipState, + getLockedTooltips, getQuery, } from '../selectors/map_selectors'; import { FLYOUT_STATE } from '../reducers/ui'; @@ -412,7 +414,41 @@ export function mapExtentChanged(newMapConstants) { }; } +export function openLockedTooltip({ features, location }) { + return (dispatch, getState) => { + const lockedTooltips = getLockedTooltips(getState()); + lockedTooltips.push({ + features, + location, + id: uuid(), + }); + + dispatch({ + type: SET_TOOLTIP_STATE, + tooltipState: { + ...getTooltipState(getState()), + lockedTooltips, + }, + }); + }; +} + +export function closeLockedTooltip(tooltipId) { + return (dispatch, getState) => { + dispatch({ + type: SET_TOOLTIP_STATE, + tooltipState: { + ...getTooltipState(getState()), + lockedTooltips: getLockedTooltips(getState()).filter(({ id }) => { + return tooltipId !== id; + }), + }, + }); + }; +} + export function setTooltipState(tooltipState) { + console.log(tooltipState); return { type: 'SET_TOOLTIP_STATE', tooltipState: tooltipState, diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/index.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/index.js index 6bc9511c6c580..7610c4af8e64c 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/index.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/index.js @@ -6,9 +6,14 @@ import { connect } from 'react-redux'; import { TooltipControl } from './tooltip_control'; -import { setTooltipState } from '../../../../actions/map_actions'; +import { + closeLockedTooltip, + openLockedTooltip, + setTooltipState, +} from '../../../../actions/map_actions'; import { getLayerList, + getLockedTooltips, getTooltipState, isDrawingFilter, } from '../../../../selectors/map_selectors'; @@ -18,11 +23,18 @@ function mapStateToProps(state = {}) { layerList: getLayerList(state), tooltipState: getTooltipState(state), isDrawingFilter: isDrawingFilter(state), + lockedTooltips: getLockedTooltips(state), }; } function mapDispatchToProps(dispatch) { return { + closeLockedTooltip(tooltipId) { + dispatch(closeLockedTooltip(tooltipId)); + }, + openLockedTooltip({ features, location }) { + dispatch(openLockedTooltip({ features, location })); + }, setTooltipState(tooltipState) { dispatch(setTooltipState(tooltipState)); }, diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js index 06ad87bf42a82..a2390c745426b 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js @@ -46,7 +46,7 @@ export class TooltipControl extends React.Component { _onMouseout = () => { this._updateHoverTooltipState.cancel(); - if (this.props.tooltipState && this.props.tooltipState.type !== TOOLTIP_TYPE.LOCKED) { + if (this.props.lockedTooltips.length === 0) { this.props.clearTooltipState(); } }; @@ -104,8 +104,7 @@ export class TooltipControl extends React.Component { const popupAnchorLocation = justifyAnchorLocation(e.lngLat, targetMbFeataure); const features = this._getIdsForFeatures(mbFeatures); - this.props.setTooltipState({ - type: TOOLTIP_TYPE.LOCKED, + this.props.openLockedTooltip({ features: features, location: popupAnchorLocation, }); @@ -117,7 +116,7 @@ export class TooltipControl extends React.Component { return; } - if (this.props.tooltipState && this.props.tooltipState.type === TOOLTIP_TYPE.LOCKED) { + if (this.props.lockedTooltips.length) { //ignore hover events when tooltip is locked return; } @@ -181,20 +180,28 @@ export class TooltipControl extends React.Component { } render() { - if (!this.props.tooltipState) { - return null; + if (this.props.lockedTooltips.length) { + return this.props.lockedTooltips.map(({ features, location, id }) => { + const closeTooltip = () => { + this.props.closeLockedTooltip(id); + }; + return ( + + ); + }); } - return ( - - ); + return null; } } diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js index 40077ea5e1723..c80051567190e 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js @@ -9,11 +9,6 @@ import { LAT_INDEX, LON_INDEX } from '../../../../../common/constants'; import { FeaturesTooltip } from '../../features_tooltip/features_tooltip'; import { EuiPopover, EuiText } from '@elastic/eui'; -export const TOOLTIP_TYPE = { - HOVER: 'HOVER', - LOCKED: 'LOCKED', -}; - const noop = () => {}; export class TooltipPopover extends Component { @@ -28,26 +23,13 @@ export class TooltipPopover extends Component { this._popoverRef = React.createRef(); } - static getDerivedStateFromProps(nextProps, prevState) { - if (nextProps.tooltipState) { - const nextPoint = nextProps.mbMap.project(nextProps.tooltipState.location); - if (nextPoint.x !== prevState.x || nextPoint.y !== prevState.y) { - return { - x: nextPoint.x, - y: nextPoint.y, - }; - } - } - - return null; - } - componentDidMount() { + this._updatePopoverPosition(); this.props.mbMap.on('move', this._updatePopoverPosition); } componentDidUpdate() { - if (this.props.tooltipState && this._popoverRef.current) { + if (this._popoverRef.current) { this._popoverRef.current.positionPopoverFluid(); } } @@ -57,13 +39,9 @@ export class TooltipPopover extends Component { } _updatePopoverPosition = () => { - if (!this.props.tooltipState) { - return; - } - - const nextPoint = this.props.mbMap.project(this.props.tooltipState.location); - const lat = this.props.tooltipState.location[LAT_INDEX]; - const lon = this.props.tooltipState.location[LON_INDEX]; + const nextPoint = this.props.mbMap.project(this.props.location); + const lat = this.props.location[LAT_INDEX]; + const lon = this.props.location[LON_INDEX]; const bounds = this.props.mbMap.getBounds(); this.setState({ x: nextPoint.x, @@ -137,9 +115,9 @@ export class TooltipPopover extends Component { _renderTooltipContent = () => { const publicProps = { addFilters: this.props.addFilters, - closeTooltip: this.props.clearTooltipState, - features: this.props.tooltipState.features, - isLocked: this.props.tooltipState.type === TOOLTIP_TYPE.LOCKED, + closeTooltip: this.props.closeTooltip, + features: this.props.features, + isLocked: this.props.isLocked, loadFeatureProperties: this._loadFeatureProperties, loadFeatureGeometry: this._loadFeatureGeometry, getLayerName: this._getLayerName, @@ -162,13 +140,11 @@ export class TooltipPopover extends Component { }; render() { - if (!this.props.tooltipState || !this.state.isVisible) { + if (!this.state.isVisible) { return null; } - const tooltipAnchor = ( -
- ); + const tooltipAnchor =
; return ( { return map.tooltipState; }; +export const getLockedTooltips = ({ map }) => { + return map.tooltipState && map.tooltipState.lockedTooltips ? map.tooltipState.lockedTooltips : []; +}; + export const getMapReady = ({ map }) => map && map.ready; export const getMapInitError = ({ map }) => map.mapInitError; From cae6a41a0ea871b8c49b404b24dfbd216bd70466 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Sun, 9 Feb 2020 14:01:56 -0700 Subject: [PATCH 3/9] hack to fix spacing --- .../map/mb/tooltip_control/tooltip_control.js | 3 ++- .../map/mb/tooltip_control/tooltip_popover.js | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js index a2390c745426b..12f5b51fccf1c 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js @@ -181,7 +181,7 @@ export class TooltipControl extends React.Component { render() { if (this.props.lockedTooltips.length) { - return this.props.lockedTooltips.map(({ features, location, id }) => { + return this.props.lockedTooltips.map(({ features, location, id }, index) => { const closeTooltip = () => { this.props.closeLockedTooltip(id); }; @@ -197,6 +197,7 @@ export class TooltipControl extends React.Component { location={location} closeTooltip={closeTooltip} isLocked={true} + index={index} /> ); }); diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js index c80051567190e..867c779bc4dba 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.js @@ -145,6 +145,10 @@ export class TooltipPopover extends Component { } const tooltipAnchor =
; + // Although tooltip anchors are not visible, they take up horizontal space. + // This horizontal spacing needs to be accounted for in the translate function, + // otherwise the anchors get increasingly pushed to the right away from the actual location. + const offset = this.props.index * 26; return ( {this._renderTooltipContent()} From efc87bafa09a7f2b104a959802fec7c0ae74960c Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 10 Feb 2020 09:49:28 -0700 Subject: [PATCH 4/9] hover tooltip --- .../maps/public/actions/map_actions.js | 124 ++++++++++-------- .../connected_components/map/mb/index.js | 2 - .../map/mb/tooltip_control/index.js | 31 ++--- .../map/mb/tooltip_control/tooltip_control.js | 60 +++++---- .../plugins/maps/public/reducers/map.js | 8 +- .../maps/public/selectors/map_selectors.js | 10 +- 6 files changed, 129 insertions(+), 106 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/actions/map_actions.js b/x-pack/legacy/plugins/maps/public/actions/map_actions.js index 6085100610238..773dd6e1e1341 100644 --- a/x-pack/legacy/plugins/maps/public/actions/map_actions.js +++ b/x-pack/legacy/plugins/maps/public/actions/map_actions.js @@ -15,8 +15,7 @@ import { getMapReady, getWaitingForMapReadyLayerListRaw, getTransientLayerId, - getTooltipState, - getLockedTooltips, + getOpenTooltips, getQuery, } from '../selectors/map_selectors'; import { FLYOUT_STATE } from '../reducers/ui'; @@ -65,7 +64,7 @@ export const CLEAR_GOTO = 'CLEAR_GOTO'; export const TRACK_CURRENT_LAYER_STATE = 'TRACK_CURRENT_LAYER_STATE'; export const ROLLBACK_TO_TRACKED_LAYER_STATE = 'ROLLBACK_TO_TRACKED_LAYER_STATE'; export const REMOVE_TRACKED_LAYER_STATE = 'REMOVE_TRACKED_LAYER_STATE'; -export const SET_TOOLTIP_STATE = 'SET_TOOLTIP_STATE'; +export const SET_OPEN_TOOLTIPS = 'SET_OPEN_TOOLTIPS'; export const UPDATE_DRAW_STATE = 'UPDATE_DRAW_STATE'; export const SET_SCROLL_ZOOM = 'SET_SCROLL_ZOOM'; export const SET_MAP_INIT_ERROR = 'SET_MAP_INIT_ERROR'; @@ -223,34 +222,36 @@ function setLayerDataLoadErrorStatus(layerId, errorMessage) { export function cleanTooltipStateForLayer(layerId, layerFeatures = []) { return (dispatch, getState) => { - const tooltipState = getTooltipState(getState()); - - if (!tooltipState) { - return; - } - - const nextTooltipFeatures = tooltipState.features.filter(tooltipFeature => { - if (tooltipFeature.layerId !== layerId) { - // feature from another layer, keep it - return true; - } - - // Keep feature if it is still in layer - return layerFeatures.some(layerFeature => { - return layerFeature.properties[FEATURE_ID_PROPERTY_NAME] === tooltipFeature.id; + let featuresRemoved = false; + const openTooltips = getOpenTooltips(getState()) + .map(tooltipState => { + const nextFeatures = tooltipState.features.filter(tooltipFeature => { + if (tooltipFeature.layerId !== layerId) { + // feature from another layer, keep it + return true; + } + + // Keep feature if it is still in layer + return layerFeatures.some(layerFeature => { + return layerFeature.properties[FEATURE_ID_PROPERTY_NAME] === tooltipFeature.id; + }); + }); + + if (tooltipState.features.length !== nextFeatures.length) { + featuresRemoved = true; + } + + return { ...tooltipState, features: nextFeatures }; + }) + .filter(tooltipState => { + return tooltipState.features.length > 0; }); - }); - - if (tooltipState.features.length === nextTooltipFeatures.length) { - // no features got removed, nothing to update - return; - } - if (nextTooltipFeatures.length === 0) { - // all features removed from tooltip, close tooltip - dispatch(setTooltipState(null)); - } else { - dispatch(setTooltipState({ ...tooltipState, features: nextTooltipFeatures })); + if (featuresRemoved) { + dispatch({ + type: SET_OPEN_TOOLTIPS, + openTooltips, + }); } }; } @@ -414,44 +415,57 @@ export function mapExtentChanged(newMapConstants) { }; } -export function openLockedTooltip({ features, location }) { +export function closeOnClickTooltip(tooltipId) { return (dispatch, getState) => { - const lockedTooltips = getLockedTooltips(getState()); - lockedTooltips.push({ - features, - location, + dispatch({ + type: SET_OPEN_TOOLTIPS, + openTooltips: getOpenTooltips(getState()).filter(({ id }) => { + return tooltipId !== id; + }), + }); + }; +} + +export function openOnClickTooltip(tooltipState) { + return (dispatch, getState) => { + const openTooltips = getOpenTooltips(getState()).filter(({ isLocked }) => { + return isLocked; + }); + + openTooltips.push({ + ...tooltipState, + isLocked: true, id: uuid(), }); dispatch({ - type: SET_TOOLTIP_STATE, - tooltipState: { - ...getTooltipState(getState()), - lockedTooltips, - }, + type: SET_OPEN_TOOLTIPS, + openTooltips, }); }; } -export function closeLockedTooltip(tooltipId) { +export function closeOnHoverTooltip() { return (dispatch, getState) => { - dispatch({ - type: SET_TOOLTIP_STATE, - tooltipState: { - ...getTooltipState(getState()), - lockedTooltips: getLockedTooltips(getState()).filter(({ id }) => { - return tooltipId !== id; - }), - }, - }); + if (getOpenTooltips(getState()).length) { + dispatch({ + type: SET_OPEN_TOOLTIPS, + openTooltips: [], + }); + } }; } -export function setTooltipState(tooltipState) { - console.log(tooltipState); +export function openOnHoverTooltip(tooltipState) { return { - type: 'SET_TOOLTIP_STATE', - tooltipState: tooltipState, + type: SET_OPEN_TOOLTIPS, + openTooltips: [ + { + ...tooltipState, + isLocked: false, + id: uuid(), + }, + ], }; } @@ -862,9 +876,9 @@ export function setJoinsForLayer(layer, joins) { } export function updateDrawState(drawState) { - return async dispatch => { + return dispatch => { if (drawState !== null) { - await dispatch(setTooltipState(null)); //tooltips just get in the way + dispatch({ type: SET_OPEN_TOOLTIPS, openTooltips: [] }); // tooltips just get in the way } dispatch({ type: UPDATE_DRAW_STATE, diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/index.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/index.js index 0274f849daf3d..9148fbdfd2d1e 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/index.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/index.js @@ -16,7 +16,6 @@ import { setMapInitError, } from '../../../actions/map_actions'; import { - getTooltipState, getLayerList, getMapReady, getGoto, @@ -33,7 +32,6 @@ function mapStateToProps(state = {}) { layerList: getLayerList(state), goto: getGoto(state), inspectorAdapters: getInspectorAdapters(state), - tooltipState: getTooltipState(state), scrollZoom: getScrollZoom(state), disableInteractive: isInteractiveDisabled(state), disableTooltipControl: isTooltipControlDisabled(state), diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/index.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/index.js index 7610c4af8e64c..d3cdbfeca3e57 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/index.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/index.js @@ -7,39 +7,40 @@ import { connect } from 'react-redux'; import { TooltipControl } from './tooltip_control'; import { - closeLockedTooltip, - openLockedTooltip, - setTooltipState, + closeOnClickTooltip, + openOnClickTooltip, + closeOnHoverTooltip, + openOnHoverTooltip, } from '../../../../actions/map_actions'; import { getLayerList, - getLockedTooltips, - getTooltipState, + getOpenTooltips, + getHasLockedTooltips, isDrawingFilter, } from '../../../../selectors/map_selectors'; function mapStateToProps(state = {}) { return { layerList: getLayerList(state), - tooltipState: getTooltipState(state), + hasLockedTooltips: getHasLockedTooltips(state), isDrawingFilter: isDrawingFilter(state), - lockedTooltips: getLockedTooltips(state), + openTooltips: getOpenTooltips(state), }; } function mapDispatchToProps(dispatch) { return { - closeLockedTooltip(tooltipId) { - dispatch(closeLockedTooltip(tooltipId)); + closeOnClickTooltip(tooltipId) { + dispatch(closeOnClickTooltip(tooltipId)); }, - openLockedTooltip({ features, location }) { - dispatch(openLockedTooltip({ features, location })); + openOnClickTooltip(tooltipState) { + dispatch(openOnClickTooltip(tooltipState)); }, - setTooltipState(tooltipState) { - dispatch(setTooltipState(tooltipState)); + closeOnHoverTooltip() { + dispatch(closeOnHoverTooltip()); }, - clearTooltipState() { - dispatch(setTooltipState(null)); + openOnHoverTooltip(tooltipState) { + dispatch(openOnHoverTooltip(tooltipState)); }, }; } diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js index 12f5b51fccf1c..b8f6710faa16b 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js @@ -9,11 +9,6 @@ import React from 'react'; import { FEATURE_ID_PROPERTY_NAME, LON_INDEX } from '../../../../../common/constants'; import { TooltipPopover } from './tooltip_popover'; -export const TOOLTIP_TYPE = { - HOVER: 'HOVER', - LOCKED: 'LOCKED', -}; - function justifyAnchorLocation(mbLngLat, targetFeature) { let popupAnchorLocation = [mbLngLat.lng, mbLngLat.lat]; // default popup location to mouse location if (targetFeature.geometry.type === 'Point') { @@ -46,8 +41,8 @@ export class TooltipControl extends React.Component { _onMouseout = () => { this._updateHoverTooltipState.cancel(); - if (this.props.lockedTooltips.length === 0) { - this.props.clearTooltipState(); + if (!this.props.hasLockedTooltips) { + this.props.closeOnHoverTooltip(); } }; @@ -88,7 +83,7 @@ export class TooltipControl extends React.Component { _lockTooltip = e => { if (this.props.isDrawingFilter) { - //ignore click events when in draw mode + // ignore click events when in draw mode return; } @@ -96,7 +91,7 @@ export class TooltipControl extends React.Component { const mbFeatures = this._getFeaturesUnderPointer(e.point); if (!mbFeatures.length) { - this.props.clearTooltipState(); + // No features at click location so there is no tooltip to open return; } @@ -104,41 +99,36 @@ export class TooltipControl extends React.Component { const popupAnchorLocation = justifyAnchorLocation(e.lngLat, targetMbFeataure); const features = this._getIdsForFeatures(mbFeatures); - this.props.openLockedTooltip({ + this.props.openOnClickTooltip({ features: features, location: popupAnchorLocation, }); }; _updateHoverTooltipState = _.debounce(e => { - if (this.props.isDrawingFilter) { - //ignore hover events when in draw mode - return; - } - - if (this.props.lockedTooltips.length) { - //ignore hover events when tooltip is locked + if (this.props.isDrawingFilter || this.props.hasLockedTooltips) { + // ignore hover events when in draw mode or when there are locked tooltips return; } const mbFeatures = this._getFeaturesUnderPointer(e.point); if (!mbFeatures.length) { - this.props.clearTooltipState(); + this.props.closeOnHoverTooltip(); return; } const targetMbFeature = mbFeatures[0]; - if (this.props.tooltipState) { - const firstFeature = this.props.tooltipState.features[0]; + if (this.props.openTooltips[0]) { + const firstFeature = this.props.openTooltips[0].features[0]; if (targetMbFeature.properties[FEATURE_ID_PROPERTY_NAME] === firstFeature.id) { + // ignore hover events when hover tooltip is all ready opened for feature return; } } const popupAnchorLocation = justifyAnchorLocation(e.lngLat, targetMbFeature); const features = this._getIdsForFeatures(mbFeatures); - this.props.setTooltipState({ - type: TOOLTIP_TYPE.HOVER, + this.props.openOnHoverTooltip({ features: features, location: popupAnchorLocation, }); @@ -180,10 +170,14 @@ export class TooltipControl extends React.Component { } render() { - if (this.props.lockedTooltips.length) { - return this.props.lockedTooltips.map(({ features, location, id }, index) => { + if (this.props.openTooltips.length === 0) { + return null; + } + + if (this.props.hasLockedTooltips) { + return this.props.openTooltips.map(({ features, location, id }, index) => { const closeTooltip = () => { - this.props.closeLockedTooltip(id); + this.props.closeOnClickTooltip(id); }; return ( + ); } } diff --git a/x-pack/legacy/plugins/maps/public/reducers/map.js b/x-pack/legacy/plugins/maps/public/reducers/map.js index 234584d08a311..7e81fb03dd85b 100644 --- a/x-pack/legacy/plugins/maps/public/reducers/map.js +++ b/x-pack/legacy/plugins/maps/public/reducers/map.js @@ -37,7 +37,7 @@ import { ROLLBACK_TO_TRACKED_LAYER_STATE, REMOVE_TRACKED_LAYER_STATE, UPDATE_SOURCE_DATA_REQUEST, - SET_TOOLTIP_STATE, + SET_OPEN_TOOLTIPS, SET_SCROLL_ZOOM, SET_MAP_INIT_ERROR, UPDATE_DRAW_STATE, @@ -97,7 +97,7 @@ const INITIAL_STATE = { ready: false, mapInitError: null, goto: null, - tooltipState: null, + openTooltips: [], mapState: { zoom: null, // setting this value does not adjust map zoom, read only value used to store current map zoom for persisting between sessions center: null, // setting this value does not adjust map view, read only value used to store current map center for persisting between sessions @@ -138,10 +138,10 @@ export function map(state = INITIAL_STATE, action) { return trackCurrentLayerState(state, action.layerId); case ROLLBACK_TO_TRACKED_LAYER_STATE: return rollbackTrackedLayerState(state, action.layerId); - case SET_TOOLTIP_STATE: + case SET_OPEN_TOOLTIPS: return { ...state, - tooltipState: action.tooltipState, + openTooltips: action.openTooltips, }; case SET_MOUSE_COORDINATES: return { diff --git a/x-pack/legacy/plugins/maps/public/selectors/map_selectors.js b/x-pack/legacy/plugins/maps/public/selectors/map_selectors.js index eb15c360e46da..d1048a759beca 100644 --- a/x-pack/legacy/plugins/maps/public/selectors/map_selectors.js +++ b/x-pack/legacy/plugins/maps/public/selectors/map_selectors.js @@ -42,12 +42,14 @@ function createSourceInstance(sourceDescriptor, inspectorAdapters) { return new Source(sourceDescriptor, inspectorAdapters); } -export const getTooltipState = ({ map }) => { - return map.tooltipState; +export const getOpenTooltips = ({ map }) => { + return map && map.openTooltips ? map.openTooltips : []; }; -export const getLockedTooltips = ({ map }) => { - return map.tooltipState && map.tooltipState.lockedTooltips ? map.tooltipState.lockedTooltips : []; +export const getHasLockedTooltips = state => { + return getOpenTooltips(state).some(({ isLocked }) => { + return isLocked; + }); }; export const getMapReady = ({ map }) => map && map.ready; From f15594baa27ef1b67340a3c20a1f1db3fbed1063 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 10 Feb 2020 10:34:52 -0700 Subject: [PATCH 5/9] fix jest tests for new props --- .../tooltip_control.test.js.snap | 193 ++++++++--------- .../tooltip_popover.test.js.snap | 115 +++++++++++ .../tooltip_control/tooltip_control.test.js | 195 ++++++------------ .../tooltip_control/tooltip_popover.test.js | 163 +++++++++++++++ 4 files changed, 433 insertions(+), 233 deletions(-) create mode 100644 x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_popover.test.js.snap create mode 100644 x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.test.js diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap index 7e8feeec01bbd..978931cfd4d19 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap @@ -1,117 +1,104 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`TooltipControl render tooltipState is not provided should not render tooltip popover when tooltipState is not provided 1`] = `""`; +exports[`TooltipControl render should not render tooltips when there are no open tooltips 1`] = `""`; -exports[`TooltipControl render tooltipState is provided should render tooltip popover with custom tooltip content when renderTooltipContent provided 1`] = ` - +exports[`TooltipControl render should render tooltip popover with custom tooltip content when renderTooltipContent provided 1`] = ` + -
- Custom tooltip content -
- + renderTooltipContent={[Function]} +/> `; -exports[`TooltipControl render tooltipState is provided should render tooltip popover with features tooltip content 1`] = ` - +exports[`TooltipControl render should render tooltip popover with features tooltip content 1`] = ` + - - - - + } +/> `; diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_popover.test.js.snap b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_popover.test.js.snap new file mode 100644 index 0000000000000..d95a418988ae7 --- /dev/null +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_popover.test.js.snap @@ -0,0 +1,115 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`TooltipPopover render should render tooltip popover 1`] = ` + + } + closePopover={[Function]} + display="inlineBlock" + hasArrow={true} + id="mapTooltip" + isOpen={true} + ownFocus={false} + panelPaddingSize="m" + style={ + Object { + "pointerEvents": "none", + "transform": "translate(NaNpx, 2987px)", + } + } +> + + + + +`; + +exports[`TooltipPopover render should render tooltip popover with custom tooltip content when renderTooltipContent provided 1`] = ` + + } + closePopover={[Function]} + display="inlineBlock" + hasArrow={true} + id="mapTooltip" + isOpen={true} + ownFocus={false} + panelPaddingSize="m" + style={ + Object { + "pointerEvents": "none", + "transform": "translate(NaNpx, 2987px)", + } + } +> +
+ Custom tooltip content +
+ +`; diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.test.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.test.js index b9dc668cfb016..cc2b95752236f 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.test.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.test.js @@ -13,7 +13,7 @@ jest.mock('../../features_tooltip/features_tooltip', () => ({ import sinon from 'sinon'; import React from 'react'; import { mount, shallow } from 'enzyme'; -import { TooltipControl, TOOLTIP_TYPE } from './tooltip_control'; +import { TooltipControl } from './tooltip_control'; // mutable map state let featuresAtLocation; @@ -82,16 +82,21 @@ const mockMBMap = { const defaultProps = { mbMap: mockMBMap, - clearTooltipState: () => {}, - setTooltipState: () => {}, + closeOnClickTooltip: () => {}, + openOnClickTooltip: () => {}, + closeOnHoverTooltip: () => {}, + openOnHoverTooltip: () => {}, layerList: [mockLayer], isDrawingFilter: false, addFilters: () => {}, geoFields: [{}], + openTooltips: [], + hasLockedTooltips: false, }; const hoverTooltipState = { - type: TOOLTIP_TYPE.HOVER, + id: '1', + isLocked: false, location: [-120, 30], features: [ { @@ -103,7 +108,8 @@ const hoverTooltipState = { }; const lockedTooltipState = { - type: TOOLTIP_TYPE.LOCKED, + id: '2', + isLocked: true, location: [-120, 30], features: [ { @@ -127,72 +133,76 @@ describe('TooltipControl', () => { }); describe('render', () => { - describe('tooltipState is not provided', () => { - test('should not render tooltip popover when tooltipState is not provided', () => { - const component = shallow(); + test('should not render tooltips when there are no open tooltips', () => { + const component = shallow(); - expect(component).toMatchSnapshot(); - }); + expect(component).toMatchSnapshot(); }); - describe('tooltipState is provided', () => { - test('should render tooltip popover with features tooltip content', () => { - const component = shallow( - - ); + test('should render hover tooltip', () => { + const component = shallow( + + ); - expect(component).toMatchSnapshot(); - }); + expect(component).toMatchSnapshot(); + }); - test('should render tooltip popover with custom tooltip content when renderTooltipContent provided', () => { - const component = shallow( - { - return
Custom tooltip content
; - }} - /> - ); - - expect(component).toMatchSnapshot(); - }); + test('should render locked tooltip', () => { + const component = shallow( + + ); + + expect(component).toMatchSnapshot(); + }); + + test('should un-register all map callbacks on unmount', () => { + const component = mount(); + + expect(Object.keys(mockMbMapHandlers).length).toBe(3); + + component.unmount(); + expect(Object.keys(mockMbMapHandlers).length).toBe(0); }); }); describe('on mouse out', () => { - const clearTooltipStateStub = sinon.stub(); + const closeOnHoverTooltipStub = sinon.stub(); beforeEach(() => { - clearTooltipStateStub.reset(); + closeOnHoverTooltipStub.reset(); }); test('should clear hover tooltip state', () => { mount( ); mockMbMapHandlers.mouseout(); - sinon.assert.calledOnce(clearTooltipStateStub); + sinon.assert.calledOnce(closeOnHoverTooltipStub); }); test('should not clear locked tooltip state', () => { mount( ); mockMbMapHandlers.mouseout(); - sinon.assert.notCalled(clearTooltipStateStub); + sinon.assert.notCalled(closeOnHoverTooltipStub); }); }); @@ -201,44 +211,44 @@ describe('TooltipControl', () => { point: { x: 0, y: 0 }, lngLat: { lng: 0, lat: 0 }, }; - const setTooltipStateStub = sinon.stub(); - const clearTooltipStateStub = sinon.stub(); + const openOnClickTooltipStub = sinon.stub(); + const closeOnClickTooltipStub = sinon.stub(); beforeEach(() => { - setTooltipStateStub.reset(); - clearTooltipStateStub.reset(); + openOnClickTooltipStub.reset(); + closeOnClickTooltipStub.reset(); }); test('should ignore clicks when map is in drawing mode', () => { mount( ); mockMbMapHandlers.click(mockMapMouseEvent); - sinon.assert.notCalled(clearTooltipStateStub); - sinon.assert.notCalled(setTooltipStateStub); + sinon.assert.notCalled(closeOnClickTooltipStub); + sinon.assert.notCalled(openOnClickTooltipStub); }); - test('should clear tooltip state when there are no features at clicked location', () => { + test('should not open tooltip when there are no features at clicked location', () => { featuresAtLocation = []; mount( ); mockMbMapHandlers.click(mockMapMouseEvent); - sinon.assert.calledOnce(clearTooltipStateStub); - sinon.assert.notCalled(setTooltipStateStub); + sinon.assert.notCalled(closeOnClickTooltipStub); + sinon.assert.notCalled(openOnClickTooltipStub); }); test('should set tooltip state when there are features at clicked location and remove duplicate features', () => { @@ -258,93 +268,18 @@ describe('TooltipControl', () => { mount( ); mockMbMapHandlers.click(mockMapMouseEvent); - sinon.assert.notCalled(clearTooltipStateStub); - sinon.assert.calledWith(setTooltipStateStub, { + sinon.assert.notCalled(closeOnClickTooltipStub); + sinon.assert.calledWith(openOnClickTooltipStub, { features: [{ id: 1, layerId: 'tfi3f' }], location: [100, 30], - type: 'LOCKED', }); }); }); - - describe('on map move', () => { - const clearTooltipStateStub = sinon.stub(); - - beforeEach(() => { - clearTooltipStateStub.reset(); - }); - - test('should safely handle map move when there is no tooltip location', () => { - const component = mount( - - ); - - mockMbMapHandlers.move(); - component.update(); - - sinon.assert.notCalled(clearTooltipStateStub); - }); - - test('should update popover location', () => { - const component = mount( - - ); - - // ensure x and y set from original tooltipState.location - expect(component.state('x')).toBe(12000); - expect(component.state('y')).toBe(3000); - - mapCenter = [25, -15]; - mockMbMapHandlers.move(); - component.update(); - - // ensure x and y updated from new map center with same tooltipState.location - expect(component.state('x')).toBe(14500); - expect(component.state('y')).toBe(4500); - - sinon.assert.notCalled(clearTooltipStateStub); - }); - - test('should clear tooltip state if tooltip location is outside map bounds', () => { - const component = mount( - - ); - - // move map bounds outside of hoverTooltipState.location, which is [-120, 30] - mockMbMapBounds = { - west: -180, - east: -170, - north: 90, - south: 80, - }; - mockMbMapHandlers.move(); - component.update(); - - sinon.assert.calledOnce(clearTooltipStateStub); - }); - }); - - test('should un-register all map callbacks on unmount', () => { - const component = mount(); - - expect(Object.keys(mockMbMapHandlers).length).toBe(4); - - component.unmount(); - expect(Object.keys(mockMbMapHandlers).length).toBe(0); - }); }); diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.test.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.test.js new file mode 100644 index 0000000000000..e2cb34ffdcd0e --- /dev/null +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.test.js @@ -0,0 +1,163 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +jest.mock('../../features_tooltip/features_tooltip', () => ({ + FeaturesTooltip: () => { + return
mockFeaturesTooltip
; + }, +})); + +import sinon from 'sinon'; +import React from 'react'; +import { mount, shallow } from 'enzyme'; +import { TooltipPopover } from './tooltip_popover'; + +// mutable map state +let mapCenter; +let mockMbMapBounds; + +const layerId = 'tfi3f'; +const mbLayerId = 'tfi3f_circle'; +const mockLayer = { + getMbLayerIds: () => { + return [mbLayerId]; + }, + getId: () => { + return layerId; + }, + canShowTooltip: () => { + return true; + }, + getFeatureById: () => { + return { + geometry: { + type: 'Point', + coordinates: [102.0, 0.5], + }, + }; + }, +}; + +const mockMbMapHandlers = {}; +const mockMBMap = { + project: lonLatArray => { + const lonDistanceFromCenter = Math.abs(lonLatArray[0] - mapCenter[0]); + const latDistanceFromCenter = Math.abs(lonLatArray[1] - mapCenter[1]); + return { + x: lonDistanceFromCenter * 100, + y: latDistanceFromCenter * 100, + }; + }, + on: (eventName, callback) => { + mockMbMapHandlers[eventName] = callback; + }, + off: eventName => { + delete mockMbMapHandlers[eventName]; + }, + getBounds: () => { + return { + getNorth: () => { + return mockMbMapBounds.north; + }, + getSouth: () => { + return mockMbMapBounds.south; + }, + getWest: () => { + return mockMbMapBounds.west; + }, + getEast: () => { + return mockMbMapBounds.east; + }, + }; + }, + getLayer: () => {}, +}; + +const defaultProps = { + mbMap: mockMBMap, + closeTooltip: () => {}, + layerList: [mockLayer], + isDrawingFilter: false, + addFilters: () => {}, + geoFields: [{}], + location: [-120, 30], + features: [ + { + id: 1, + layerId: layerId, + geometry: {}, + }, + ], + isLocked: false, +}; + +describe('TooltipPopover', () => { + beforeEach(() => { + mapCenter = [0, 0]; + mockMbMapBounds = { + west: -180, + east: 180, + north: 90, + south: -90, + }; + }); + + describe('render', () => { + test('should render tooltip popover', () => { + const component = shallow(); + + expect(component).toMatchSnapshot(); + }); + + test('should render tooltip popover with custom tooltip content when renderTooltipContent provided', () => { + const component = shallow( + { + return
Custom tooltip content
; + }} + /> + ); + + expect(component).toMatchSnapshot(); + }); + + test('should un-register all map callbacks on unmount', () => { + const component = mount(); + + expect(Object.keys(mockMbMapHandlers).length).toBe(1); + + component.unmount(); + expect(Object.keys(mockMbMapHandlers).length).toBe(0); + }); + }); + + describe('on map move', () => { + const closeTooltipStub = sinon.stub(); + + beforeEach(() => { + closeTooltipStub.reset(); + }); + + test('should update popover location', () => { + const component = mount(); + + // ensure x and y set from original tooltipState.location + expect(component.state('x')).toBe(12000); + expect(component.state('y')).toBe(3000); + + mapCenter = [25, -15]; + mockMbMapHandlers.move(); + component.update(); + + // ensure x and y updated from new map center with same tooltipState.location + expect(component.state('x')).toBe(14500); + expect(component.state('y')).toBe(4500); + + sinon.assert.notCalled(closeTooltipStub); + }); + }); +}); From 604626104d97cb4a0fafa6af87b40525b92550b6 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 10 Feb 2020 10:40:37 -0700 Subject: [PATCH 6/9] update snapshots --- .../__snapshots__/tooltip_control.test.js.snap | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap index 978931cfd4d19..bdcd7efa78ce7 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap @@ -2,7 +2,7 @@ exports[`TooltipControl render should not render tooltips when there are no open tooltips 1`] = `""`; -exports[`TooltipControl render should render tooltip popover with custom tooltip content when renderTooltipContent provided 1`] = ` +exports[`TooltipControl render should render hover tooltip 1`] = ` `; -exports[`TooltipControl render should render tooltip popover with features tooltip content 1`] = ` +exports[`TooltipControl render should render locked tooltip 1`] = ` Date: Mon, 10 Feb 2020 11:03:56 -0700 Subject: [PATCH 7/9] simplify jest tests --- .../tooltip_control.test.js.snap | 6 --- .../tooltip_control/tooltip_control.test.js | 47 ++----------------- .../tooltip_control/tooltip_popover.test.js | 23 +-------- 3 files changed, 4 insertions(+), 72 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap index bdcd7efa78ce7..cffa441d04ff5 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/__snapshots__/tooltip_control.test.js.snap @@ -27,7 +27,6 @@ exports[`TooltipControl render should render hover tooltip 1`] = ` Array [ Object { "canShowTooltip": [Function], - "getFeatureById": [Function], "getId": [Function], "getMbLayerIds": [Function], }, @@ -41,11 +40,9 @@ exports[`TooltipControl render should render hover tooltip 1`] = ` } mbMap={ Object { - "getBounds": [Function], "getLayer": [Function], "off": [Function], "on": [Function], - "project": [Function], "queryRenderedFeatures": [Function], } } @@ -77,7 +74,6 @@ exports[`TooltipControl render should render locked tooltip 1`] = ` Array [ Object { "canShowTooltip": [Function], - "getFeatureById": [Function], "getId": [Function], "getMbLayerIds": [Function], }, @@ -91,11 +87,9 @@ exports[`TooltipControl render should render locked tooltip 1`] = ` } mbMap={ Object { - "getBounds": [Function], "getLayer": [Function], "off": [Function], "on": [Function], - "project": [Function], "queryRenderedFeatures": [Function], } } diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.test.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.test.js index cc2b95752236f..620d7cb9ff756 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.test.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.test.js @@ -4,9 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -jest.mock('../../features_tooltip/features_tooltip', () => ({ - FeaturesTooltip: () => { - return
mockFeaturesTooltip
; +jest.mock('./tooltip_popover', () => ({ + TooltipPopover: () => { + return
mockTooltipPopover
; }, })); @@ -17,8 +17,6 @@ import { TooltipControl } from './tooltip_control'; // mutable map state let featuresAtLocation; -let mapCenter; -let mockMbMapBounds; const layerId = 'tfi3f'; const mbLayerId = 'tfi3f_circle'; @@ -32,48 +30,16 @@ const mockLayer = { canShowTooltip: () => { return true; }, - getFeatureById: () => { - return { - geometry: { - type: 'Point', - coordinates: [102.0, 0.5], - }, - }; - }, }; const mockMbMapHandlers = {}; const mockMBMap = { - project: lonLatArray => { - const lonDistanceFromCenter = Math.abs(lonLatArray[0] - mapCenter[0]); - const latDistanceFromCenter = Math.abs(lonLatArray[1] - mapCenter[1]); - return { - x: lonDistanceFromCenter * 100, - y: latDistanceFromCenter * 100, - }; - }, on: (eventName, callback) => { mockMbMapHandlers[eventName] = callback; }, off: eventName => { delete mockMbMapHandlers[eventName]; }, - getBounds: () => { - return { - getNorth: () => { - return mockMbMapBounds.north; - }, - getSouth: () => { - return mockMbMapBounds.south; - }, - getWest: () => { - return mockMbMapBounds.west; - }, - getEast: () => { - return mockMbMapBounds.east; - }, - }; - }, getLayer: () => {}, queryRenderedFeatures: () => { return featuresAtLocation; @@ -123,13 +89,6 @@ const lockedTooltipState = { describe('TooltipControl', () => { beforeEach(() => { featuresAtLocation = []; - mapCenter = [0, 0]; - mockMbMapBounds = { - west: -180, - east: 180, - north: 90, - south: -90, - }; }); describe('render', () => { diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.test.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.test.js index e2cb34ffdcd0e..bcef03c205b2b 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.test.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_popover.test.js @@ -20,26 +20,6 @@ let mapCenter; let mockMbMapBounds; const layerId = 'tfi3f'; -const mbLayerId = 'tfi3f_circle'; -const mockLayer = { - getMbLayerIds: () => { - return [mbLayerId]; - }, - getId: () => { - return layerId; - }, - canShowTooltip: () => { - return true; - }, - getFeatureById: () => { - return { - geometry: { - type: 'Point', - coordinates: [102.0, 0.5], - }, - }; - }, -}; const mockMbMapHandlers = {}; const mockMBMap = { @@ -73,13 +53,12 @@ const mockMBMap = { }, }; }, - getLayer: () => {}, }; const defaultProps = { mbMap: mockMBMap, closeTooltip: () => {}, - layerList: [mockLayer], + layerList: [], isDrawingFilter: false, addFilters: () => {}, geoFields: [{}], From a1da30bb5ab28d1f49fb55305d269cc2dcc0c708 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 10 Feb 2020 11:24:48 -0700 Subject: [PATCH 8/9] avoid opening up multiple tooltips in same location --- x-pack/legacy/plugins/maps/public/actions/map_actions.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/actions/map_actions.js b/x-pack/legacy/plugins/maps/public/actions/map_actions.js index 773dd6e1e1341..2c6c60db9a012 100644 --- a/x-pack/legacy/plugins/maps/public/actions/map_actions.js +++ b/x-pack/legacy/plugins/maps/public/actions/map_actions.js @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import _ from 'lodash'; import turf from 'turf'; import turfBooleanContains from '@turf/boolean-contains'; import uuid from 'uuid/v4'; @@ -428,8 +429,12 @@ export function closeOnClickTooltip(tooltipId) { export function openOnClickTooltip(tooltipState) { return (dispatch, getState) => { - const openTooltips = getOpenTooltips(getState()).filter(({ isLocked }) => { - return isLocked; + const openTooltips = getOpenTooltips(getState()).filter(({ features, location, isLocked }) => { + return ( + isLocked && + !_.isEqual(location, tooltipState.location) && + !_.isEqual(features, tooltipState.features) + ); }); openTooltips.push({ From 57290a36b94ea0edd11fe09866fb63e1023a0283 Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Tue, 11 Feb 2020 08:18:11 -0700 Subject: [PATCH 9/9] remove duplicated code --- .../map/mb/tooltip_control/tooltip_control.js | 60 +++++++------------ 1 file changed, 22 insertions(+), 38 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js index b8f6710faa16b..329d2b7fd2985 100644 --- a/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js +++ b/x-pack/legacy/plugins/maps/public/connected_components/map/mb/tooltip_control/tooltip_control.js @@ -174,43 +174,27 @@ export class TooltipControl extends React.Component { return null; } - if (this.props.hasLockedTooltips) { - return this.props.openTooltips.map(({ features, location, id }, index) => { - const closeTooltip = () => { - this.props.closeOnClickTooltip(id); - }; - return ( - - ); - }); - } - - return ( - - ); + return this.props.openTooltips.map(({ features, location, id, isLocked }, index) => { + const closeTooltip = isLocked + ? () => { + this.props.closeOnClickTooltip(id); + } + : this.props.closeOnHoverTooltip; + return ( + + ); + }); } }