From 63f937ee509313b0752ec14d2e20bb254ead2311 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 25 Jun 2019 18:14:07 -0400 Subject: [PATCH 01/28] add pagination --- .../public/components/map/feature_tooltip.js | 93 +++++++++++++------ .../maps/public/components/map/mb/view.js | 13 +++ 2 files changed, 76 insertions(+), 30 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index b4749ab4e56a4..5c0a37c30dbc0 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -10,6 +10,7 @@ import { EuiCallOut, EuiLoadingSpinner, EuiTextAlign, + EuiPagination } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; @@ -19,6 +20,8 @@ export class FeatureTooltip extends React.Component { state = { properties: undefined, loadPropertiesErrorMsg: undefined, + pageNumber: 0, + selectedLayer: null }; componentDidMount() { @@ -37,11 +40,13 @@ export class FeatureTooltip extends React.Component { } _loadProperties = () => { + + const feature = this.props.tooltipState.features[this.state.pageNumber]; this._fetchProperties({ - nextFeatureId: this.props.tooltipState.featureId, - nextLayerId: this.props.tooltipState.layerId, + nextFeatureId: feature.id, + nextLayerId: feature.layerId }); - } + }; _fetchProperties = async ({ nextLayerId, nextFeatureId }) => { if (this.prevLayerId === nextLayerId && this.prevFeatureId === nextFeatureId) { @@ -109,6 +114,36 @@ export class FeatureTooltip extends React.Component { } _renderProperties() { + + if (this.state.loadPropertiesErrorMsg) { + return ( + +

+ {this.state.loadPropertiesErrorMsg} +

+ + ); + } + + if (!this.state.properties) { + const loadingMsg = i18n.translate('xpack.maps.tooltip.loadingMsg', { + defaultMessage: 'Loading' + }); + return ( + + + {loadingMsg} + + ); + } + const rows = this.state.properties.map(tooltipProperty => { const label = tooltipProperty.getPropertyName(); return ( @@ -158,40 +193,38 @@ export class FeatureTooltip extends React.Component { ); } - render() { - if (!this.state.properties) { - const loadingMsg = i18n.translate('xpack.maps.tooltip.loadingMsg', { - defaultMessage: 'Loading' - }); - return ( - - - {loadingMsg} - - ); - } + _onPageChange = (pageNumber) => { + console.log('must change page', pageNumber); + this.setState({ + pageNumber: pageNumber, + properties: null + }); + this._loadProperties(); + }; - if (this.state.loadPropertiesErrorMsg) { - return ( - -

- {this.state.loadPropertiesErrorMsg} -

- - ); + _renderPagination() { + + console.log('render feature list ... ', this.props); + + if (!this.props.showFeatureList) { + return null; } + return ( + + ); + } + + render() { return ( {this._renderCloseButton()} {this._renderProperties()} + {this._renderPagination()} ); } diff --git a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js index b1191e59c1107..ba5178dce45d1 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js +++ b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js @@ -126,6 +126,15 @@ export class MBMapContainer extends React.Component { } }, 256); + _getIdsForFeatures(features) { + return features.map((feature) => { + const layer = this._getLayerByMbLayerId(feature.layer.id); + return { + id: feature.properties[FEATURE_ID_PROPERTY_NAME], + layerId: layer.getId() + }; + }); + } _lockTooltip = (e) => { @@ -145,10 +154,12 @@ export class MBMapContainer extends React.Component { const targetFeature = features[0]; const layer = this._getLayerByMbLayerId(targetFeature.layer.id); const popupAnchorLocation = this._justifyAnchorLocation(e.lngLat, targetFeature); + this.props.setTooltipState({ type: TOOLTIP_TYPE.LOCKED, layerId: layer.getId(), featureId: targetFeature.properties[FEATURE_ID_PROPERTY_NAME], + features: this._getIdsForFeatures(features), location: popupAnchorLocation }); }; @@ -186,6 +197,7 @@ export class MBMapContainer extends React.Component { type: TOOLTIP_TYPE.HOVER, featureId: targetFeature.properties[FEATURE_ID_PROPERTY_NAME], layerId: layer.getId(), + features: this._getIdsForFeatures(features), location: popupAnchorLocation }); @@ -391,6 +403,7 @@ export class MBMapContainer extends React.Component { closeTooltip={this._onTooltipClose} showFilterButtons={this.props.isFilterable && isLocked} showCloseButton={isLocked} + showFeatureList={isLocked} /> ), this._tooltipContainer); From d1f67f7518d6e7fbe7d323a6a11744eec0c3ad59 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 26 Jun 2019 16:12:15 -0400 Subject: [PATCH 02/28] add layer filtering --- .../public/components/map/feature_tooltip.js | 153 ++++++++++++++++-- .../maps/public/components/map/mb/view.js | 61 +++---- 2 files changed, 172 insertions(+), 42 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 5c0a37c30dbc0..97e1997887af4 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -9,19 +9,26 @@ import { EuiButtonIcon, EuiCallOut, EuiLoadingSpinner, + EuiText, EuiTextAlign, - EuiPagination + EuiPagination, + EuiSuperSelect } from '@elastic/eui'; +import _ from 'lodash'; import { i18n } from '@kbn/i18n'; +const ALL_LAYERS = '_ALL_LAYERS_'; +const DEFAULT_PAGE_NUMBER = 0; + export class FeatureTooltip extends React.Component { state = { - properties: undefined, - loadPropertiesErrorMsg: undefined, - pageNumber: 0, - selectedLayer: null + uniqueLayers: [], + properties: null, + loadPropertiesErrorMsg: null, + pageNumber: DEFAULT_PAGE_NUMBER, + layerIdFilter: ALL_LAYERS }; componentDidMount() { @@ -33,15 +40,74 @@ export class FeatureTooltip extends React.Component { componentDidUpdate() { this._loadProperties(); + this._loadUniqueLayers(); } componentWillUnmount() { this._isMounted = false; } + _onLayerChange = (layerId) => { + + if (this.state.layerIdFilter === layerId) { + return; + } + + this.setState({ + properties: null, + pageNumber: DEFAULT_PAGE_NUMBER, + layerIdFilter: layerId + }, () => { + this.prevLayerId = null; + this.prevFeatureId = null; + this._loadProperties(); + }); + }; + + _loadUniqueLayers = async () => { + + const uniqueLayerIds = []; + for (let i = 0; i < this.props.tooltipState.features.length; i++) { + if (uniqueLayerIds.indexOf(this.props.tooltipState.features[i].layerId) < 0) { + uniqueLayerIds.push(this.props.tooltipState.features[i].layerId); + } + } + + const layers = uniqueLayerIds.map(layerId => { + return this.props.findLayerById(layerId); + }); + + const layerNamePromises = layers.map(layer => { + return layer.getDisplayName(); + }); + + const layerNames = await Promise.all(layerNamePromises); + + + const options = layers.map((layer, index) => { + return { + displayName: layerNames[index], + id: layer.getId() + }; + }); + + if (this._isMounted) { + if (!_.isEqual(this.state.uniqueLayers, options)) { + this.setState({ + uniqueLayers: options + }); + } + } + }; + _loadProperties = () => { + const features = this._filterFeatures(); + const feature = features[this.state.pageNumber]; + + if (!feature) { + return; + } - const feature = this.props.tooltipState.features[this.state.pageNumber]; this._fetchProperties({ nextFeatureId: feature.id, nextLayerId: feature.layerId @@ -84,7 +150,7 @@ export class FeatureTooltip extends React.Component { properties }); } - } + }; _renderFilterCell(tooltipProperty) { if (!this.props.showFilterButtons || !tooltipProperty.isFilterable()) { @@ -175,6 +241,41 @@ export class FeatureTooltip extends React.Component { ); } + _renderLayerFilterBox() { + + if (!this.props.showFeatureList) { + return null; + } + + if (!this.state.uniqueLayers || this.state.uniqueLayers.length < 2) { + return null; + } + + const layerOptions = this.state.uniqueLayers.map(({ id, displayName })=> { + return { + value: id, + inputDisplay: ({displayName}) + }; + }); + + const options = [ + { + value: ALL_LAYERS, + inputDisplay: (All layers) + }, + ...layerOptions + ]; + + return ( + + ); + + } + _renderCloseButton() { if (!this.props.showCloseButton) { return null; @@ -194,25 +295,43 @@ export class FeatureTooltip extends React.Component { } _onPageChange = (pageNumber) => { - console.log('must change page', pageNumber); + this.prevLayerId = null; + this.prevFeatureId = null; this.setState({ pageNumber: pageNumber, properties: null + }, () => { + this._loadProperties(); }); - this._loadProperties(); }; - _renderPagination() { + _filterFeatures() { + if (this.state.layerIdFilter === ALL_LAYERS) { + return this.props.tooltipState.features; + } + + return this.props.tooltipState.features.filter((feature) => { + return feature.layerId === this.state.layerIdFilter; + }); + } - console.log('render feature list ... ', this.props); + _renderPagination(filteredFeatures) { - if (!this.props.showFeatureList) { + if (filteredFeatures.length === 1) { return null; } + if (!this.props.showFeatureList) { + return ( + + 1 of {filteredFeatures.length} + + ); + } + return ( @@ -220,11 +339,15 @@ export class FeatureTooltip extends React.Component { } render() { + + const filteredFeatures = this._filterFeatures(); + return ( {this._renderCloseButton()} - {this._renderProperties()} - {this._renderPagination()} + {this._renderLayerFilterBox()} + {this._renderProperties(filteredFeatures)} + {this._renderPagination(filteredFeatures)} ); } diff --git a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js index ba5178dce45d1..64d799c30c3ab 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js +++ b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js @@ -126,11 +126,11 @@ export class MBMapContainer extends React.Component { } }, 256); - _getIdsForFeatures(features) { - return features.map((feature) => { - const layer = this._getLayerByMbLayerId(feature.layer.id); + _getIdsForFeatures(mbFeatures) { + return mbFeatures.map((mbFeature) => { + const layer = this._getLayerByMbLayerId(mbFeature.layer.id); return { - id: feature.properties[FEATURE_ID_PROPERTY_NAME], + id: mbFeature.properties[FEATURE_ID_PROPERTY_NAME], layerId: layer.getId() }; }); @@ -145,21 +145,23 @@ export class MBMapContainer extends React.Component { this._updateHoverTooltipState.cancel();//ignore any possible moves - const features = this._getFeaturesUnderPointer(e.point); - if (!features.length) { + const mbFeatures = this._getFeaturesUnderPointer(e.point); + if (!mbFeatures.length) { this.props.setTooltipState(null); return; } - const targetFeature = features[0]; - const layer = this._getLayerByMbLayerId(targetFeature.layer.id); - const popupAnchorLocation = this._justifyAnchorLocation(e.lngLat, targetFeature); + const targetMbFeataure = mbFeatures[0]; + // const layer = this._getLayerByMbLayerId(targetMbFeataure.layer.id); + const popupAnchorLocation = this._justifyAnchorLocation(e.lngLat, targetMbFeataure); + const features = this._getIdsForFeatures(mbFeatures); + console.log('f', features); this.props.setTooltipState({ type: TOOLTIP_TYPE.LOCKED, - layerId: layer.getId(), - featureId: targetFeature.properties[FEATURE_ID_PROPERTY_NAME], - features: this._getIdsForFeatures(features), + // layerId: layer.getId(), + // featureId: targetMbFeataure.properties[FEATURE_ID_PROPERTY_NAME], + features: features, location: popupAnchorLocation }); }; @@ -176,28 +178,28 @@ export class MBMapContainer extends React.Component { return; } - const features = this._getFeaturesUnderPointer(e.point); - if (!features.length) { + const mbFeatures = this._getFeaturesUnderPointer(e.point); + if (!mbFeatures.length) { this.props.setTooltipState(null); return; } - const targetFeature = features[0]; - + const targetMbFeature = mbFeatures[0]; if (this.props.tooltipState) { - if (targetFeature.properties[FEATURE_ID_PROPERTY_NAME] === this.props.tooltipState.featureId) { + const firstFeature = this.props.tooltipState.features[0]; + if (targetMbFeature.properties[FEATURE_ID_PROPERTY_NAME] === firstFeature.id) { return; } } - const layer = this._getLayerByMbLayerId(targetFeature.layer.id); - const popupAnchorLocation = this._justifyAnchorLocation(e.lngLat, targetFeature); - + // const layer = this._getLayerByMbLayerId(targetMbFeature.layer.id); + const popupAnchorLocation = this._justifyAnchorLocation(e.lngLat, targetMbFeature); + const features = this._getIdsForFeatures(mbFeatures); this.props.setTooltipState({ type: TOOLTIP_TYPE.HOVER, - featureId: targetFeature.properties[FEATURE_ID_PROPERTY_NAME], - layerId: layer.getId(), - features: this._getIdsForFeatures(features), + // featureId: targetMbFeature.properties[FEATURE_ID_PROPERTY_NAME], + // layerId: layer.getId(), + features: features, location: popupAnchorLocation }); @@ -400,6 +402,7 @@ export class MBMapContainer extends React.Component { { - const tooltipLayer = this.props.layerList.find(layer => { - return layer.getId() === layerId; - }); + const tooltipLayer = this._findLayerById(layerId); if (!tooltipLayer) { return []; } @@ -424,7 +425,13 @@ export class MBMapContainer extends React.Component { return []; } return await tooltipLayer.getPropertiesForTooltip(targetFeature.properties); - } + }; + + _findLayerById = (layerId) => { + return this.props.layerList.find(layer => { + return layer.getId() === layerId; + }); + }; _syncTooltipState() { if (this.props.tooltipState) { From c7d0900de7c86a74d83752649a671f2eac60cdba Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 26 Jun 2019 16:38:32 -0400 Subject: [PATCH 03/28] change ordering --- .../plugins/maps/public/components/map/feature_tooltip.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 97e1997887af4..05195f9d104e1 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -317,10 +317,6 @@ export class FeatureTooltip extends React.Component { _renderPagination(filteredFeatures) { - if (filteredFeatures.length === 1) { - return null; - } - if (!this.props.showFeatureList) { return ( @@ -345,8 +341,8 @@ export class FeatureTooltip extends React.Component { return ( {this._renderCloseButton()} - {this._renderLayerFilterBox()} {this._renderProperties(filteredFeatures)} + {this._renderLayerFilterBox()} {this._renderPagination(filteredFeatures)} ); From 72f0aacd0e5f6383aebe6f243eb269d9cab4cd24 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 26 Jun 2019 16:51:53 -0400 Subject: [PATCH 04/28] reset layer selection on change --- .../public/components/map/feature_tooltip.js | 23 +++++++++++++++---- .../maps/public/components/map/mb/view.js | 3 --- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 05195f9d104e1..f0b49be731e40 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -64,6 +64,15 @@ export class FeatureTooltip extends React.Component { }); }; + _onCloseTooltip = () => { + this.setState({ + layerIdFilter: ALL_LAYERS, + pageNumber: DEFAULT_PAGE_NUMBER + }, () => { + this.props.closeTooltip(); + }); + }; + _loadUniqueLayers = async () => { const uniqueLayerIds = []; @@ -94,7 +103,9 @@ export class FeatureTooltip extends React.Component { if (this._isMounted) { if (!_.isEqual(this.state.uniqueLayers, options)) { this.setState({ - uniqueLayers: options + uniqueLayers: options, + layerIdFilter: ALL_LAYERS, + pageNumber: DEFAULT_PAGE_NUMBER }); } } @@ -166,7 +177,7 @@ export class FeatureTooltip extends React.Component { defaultMessage: 'Filter on property' })} onClick={() => { - this.props.closeTooltip(); + this._onCloseTooltip(); const filterAction = tooltipProperty.getFilterAction(); filterAction(); }} @@ -283,7 +294,7 @@ export class FeatureTooltip extends React.Component { return ( @@ -342,8 +357,8 @@ export class FeatureTooltip extends React.Component { {this._renderCloseButton()} {this._renderProperties(filteredFeatures)} - {this._renderLayerFilterBox()} {this._renderPagination(filteredFeatures)} + {this._renderLayerFilterBox()} ); } diff --git a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js index 64d799c30c3ab..60f7e95cb01be 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js +++ b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js @@ -156,11 +156,8 @@ export class MBMapContainer extends React.Component { const popupAnchorLocation = this._justifyAnchorLocation(e.lngLat, targetMbFeataure); const features = this._getIdsForFeatures(mbFeatures); - console.log('f', features); this.props.setTooltipState({ type: TOOLTIP_TYPE.LOCKED, - // layerId: layer.getId(), - // featureId: targetMbFeataure.properties[FEATURE_ID_PROPERTY_NAME], features: features, location: popupAnchorLocation }); From ea4bcc739ea34f6af743bc611dae7999c3b17b23 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 26 Jun 2019 17:02:46 -0400 Subject: [PATCH 05/28] show count --- .../public/components/map/feature_tooltip.js | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index f0b49be731e40..18a89e5a65dc8 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -77,12 +77,20 @@ export class FeatureTooltip extends React.Component { const uniqueLayerIds = []; for (let i = 0; i < this.props.tooltipState.features.length; i++) { - if (uniqueLayerIds.indexOf(this.props.tooltipState.features[i].layerId) < 0) { - uniqueLayerIds.push(this.props.tooltipState.features[i].layerId); + let index = uniqueLayerIds.findIndex(({ layerId }) => { + return layerId === this.props.tooltipState.features[i].layerId; + }); + if (index < 0) { + uniqueLayerIds.push({ + layerId: this.props.tooltipState.features[i].layerId, + count: 0 + }); + index = uniqueLayerIds.length - 1; } + uniqueLayerIds[index].count++; } - const layers = uniqueLayerIds.map(layerId => { + const layers = uniqueLayerIds.map(({ layerId }) => { return this.props.findLayerById(layerId); }); @@ -91,12 +99,11 @@ export class FeatureTooltip extends React.Component { }); const layerNames = await Promise.all(layerNamePromises); - - const options = layers.map((layer, index) => { return { displayName: layerNames[index], - id: layer.getId() + id: layer.getId(), + count: uniqueLayerIds[index].count }; }); @@ -141,7 +148,7 @@ export class FeatureTooltip extends React.Component { let properties; try { properties = await this.props.loadFeatureProperties({ layerId: nextLayerId, featureId: nextFeatureId }); - } catch(error) { + } catch (error) { if (this._isMounted) { this.setState({ properties: [], @@ -164,7 +171,7 @@ export class FeatureTooltip extends React.Component { }; _renderFilterCell(tooltipProperty) { - if (!this.props.showFilterButtons || !tooltipProperty.isFilterable()) { + if (!this.props.showFilterButtons || !tooltipProperty.isFilterable()) { return null; } @@ -215,7 +222,7 @@ export class FeatureTooltip extends React.Component { }); return ( - + {loadingMsg} ); @@ -262,10 +269,10 @@ export class FeatureTooltip extends React.Component { return null; } - const layerOptions = this.state.uniqueLayers.map(({ id, displayName })=> { + const layerOptions = this.state.uniqueLayers.map(({ id, displayName, count }) => { return { value: id, - inputDisplay: ({displayName}) + inputDisplay: (({count}) {displayName}) }; }); From 94c0e588ce2fc3b8454483f9475395c41cf2dfcd Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 11:20:51 -0400 Subject: [PATCH 06/28] factor out feature properties --- .../components/map/feature_properties.js | 175 ++++++++++++++++++ .../public/components/map/feature_tooltip.js | 160 +--------------- 2 files changed, 184 insertions(+), 151 deletions(-) create mode 100644 x-pack/legacy/plugins/maps/public/components/map/feature_properties.js diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_properties.js b/x-pack/legacy/plugins/maps/public/components/map/feature_properties.js new file mode 100644 index 0000000000000..938a9edd5df09 --- /dev/null +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_properties.js @@ -0,0 +1,175 @@ +/* + * 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 from 'react'; +import { + EuiCallOut, + EuiLoadingSpinner, + EuiTextAlign, + EuiButtonIcon +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; + + +export class FeatureProperties extends React.Component { + + state = { + properties: null, + loadPropertiesErrorMsg: null, + }; + + componentDidMount() { + this._isMounted = true; + this.prevLayerId = undefined; + this.prevFeatureId = undefined; + this._loadProperties(); + } + + componentDidUpdate() { + this._loadProperties(); + } + + componentWillUnmount() { + this._isMounted = false; + } + + _loadProperties = () => { + this._fetchProperties({ + nextFeatureId: this.props.featureId, + nextLayerId: this.props.layerId + }); + }; + + _fetchProperties = async ({ nextLayerId, nextFeatureId }) => { + if (this.prevLayerId === nextLayerId && this.prevFeatureId === nextFeatureId) { + // do not reload same feature properties + return; + } + + this.prevLayerId = nextLayerId; + this.prevFeatureId = nextFeatureId; + this.setState({ + properties: undefined, + loadPropertiesErrorMsg: undefined, + }); + + let properties; + try { + properties = await this.props.loadFeatureProperties({ layerId: nextLayerId, featureId: nextFeatureId }); + } catch (error) { + if (this._isMounted) { + this.setState({ + properties: [], + loadPropertiesErrorMsg: error.message + }); + } + return; + } + + if (this.prevLayerId !== nextLayerId && this.prevFeatureId !== nextFeatureId) { + // ignore results for old request + return; + } + + if (this._isMounted) { + this.setState({ + properties + }); + } + }; + + + _renderFilterCell(tooltipProperty) { + if (!this.props.showFilterButtons || !tooltipProperty.isFilterable()) { + return null; + } + + return ( + + { + this._onCloseTooltip(); + const filterAction = tooltipProperty.getFilterAction(); + filterAction(); + }} + aria-label={i18n.translate('xpack.maps.tooltip.filterOnPropertyAriaLabel', { + defaultMessage: 'Filter on property' + })} + data-test-subj="mapTooltipCreateFilterButton" + /> + + ); + } + + render() { + + if (this.state.loadPropertiesErrorMsg) { + return ( + +

+ {this.state.loadPropertiesErrorMsg} +

+ + ); + } + + if (!this.state.properties) { + const loadingMsg = i18n.translate('xpack.maps.tooltip.loadingMsg', { + defaultMessage: 'Loading' + }); + return ( + + + {loadingMsg} + + ); + } + + const rows = this.state.properties.map(tooltipProperty => { + const label = tooltipProperty.getPropertyName(); + return ( + + + {label} + + + {this._renderFilterCell(tooltipProperty)} + + ); + }); + + return ( + + + {rows} + +
+ ); + } + +} + diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 18a89e5a65dc8..d41730828a386 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -7,8 +7,6 @@ import React, { Fragment } from 'react'; import { EuiButtonIcon, - EuiCallOut, - EuiLoadingSpinner, EuiText, EuiTextAlign, EuiPagination, @@ -16,6 +14,7 @@ import { } from '@elastic/eui'; import _ from 'lodash'; import { i18n } from '@kbn/i18n'; +import { FeatureProperties } from './feature_properties'; const ALL_LAYERS = '_ALL_LAYERS_'; @@ -25,21 +24,15 @@ export class FeatureTooltip extends React.Component { state = { uniqueLayers: [], - properties: null, - loadPropertiesErrorMsg: null, pageNumber: DEFAULT_PAGE_NUMBER, layerIdFilter: ALL_LAYERS }; componentDidMount() { this._isMounted = true; - this.prevLayerId = undefined; - this.prevFeatureId = undefined; - this._loadProperties(); } componentDidUpdate() { - this._loadProperties(); this._loadUniqueLayers(); } @@ -54,13 +47,8 @@ export class FeatureTooltip extends React.Component { } this.setState({ - properties: null, pageNumber: DEFAULT_PAGE_NUMBER, layerIdFilter: layerId - }, () => { - this.prevLayerId = null; - this.prevFeatureId = null; - this._loadProperties(); }); }; @@ -118,144 +106,19 @@ export class FeatureTooltip extends React.Component { } }; - _loadProperties = () => { - const features = this._filterFeatures(); - const feature = features[this.state.pageNumber]; + _renderProperties(features) { + const feature = features[this.state.pageNumber]; if (!feature) { - return; - } - - this._fetchProperties({ - nextFeatureId: feature.id, - nextLayerId: feature.layerId - }); - }; - - _fetchProperties = async ({ nextLayerId, nextFeatureId }) => { - if (this.prevLayerId === nextLayerId && this.prevFeatureId === nextFeatureId) { - // do not reload same feature properties - return; - } - - this.prevLayerId = nextLayerId; - this.prevFeatureId = nextFeatureId; - this.setState({ - properties: undefined, - loadPropertiesErrorMsg: undefined, - }); - - let properties; - try { - properties = await this.props.loadFeatureProperties({ layerId: nextLayerId, featureId: nextFeatureId }); - } catch (error) { - if (this._isMounted) { - this.setState({ - properties: [], - loadPropertiesErrorMsg: error.message - }); - } - return; - } - - if (this.prevLayerId !== nextLayerId && this.prevFeatureId !== nextFeatureId) { - // ignore results for old request - return; - } - - if (this._isMounted) { - this.setState({ - properties - }); - } - }; - - _renderFilterCell(tooltipProperty) { - if (!this.props.showFilterButtons || !tooltipProperty.isFilterable()) { return null; } - return ( - - { - this._onCloseTooltip(); - const filterAction = tooltipProperty.getFilterAction(); - filterAction(); - }} - aria-label={i18n.translate('xpack.maps.tooltip.filterOnPropertyAriaLabel', { - defaultMessage: 'Filter on property' - })} - data-test-subj="mapTooltipCreateFilterButton" - /> - - ); - } - - _renderProperties() { - - if (this.state.loadPropertiesErrorMsg) { - return ( - -

- {this.state.loadPropertiesErrorMsg} -

- - ); - } - - if (!this.state.properties) { - const loadingMsg = i18n.translate('xpack.maps.tooltip.loadingMsg', { - defaultMessage: 'Loading' - }); - return ( - - - {loadingMsg} - - ); - } - - const rows = this.state.properties.map(tooltipProperty => { - const label = tooltipProperty.getPropertyName(); - return ( - - - {label} - - - {this._renderFilterCell(tooltipProperty)} - - ); - }); - - return ( - - - {rows} - -
+ ); } @@ -317,9 +180,6 @@ export class FeatureTooltip extends React.Component { this.prevFeatureId = null; this.setState({ pageNumber: pageNumber, - properties: null - }, () => { - this._loadProperties(); }); }; @@ -357,9 +217,7 @@ export class FeatureTooltip extends React.Component { } render() { - const filteredFeatures = this._filterFeatures(); - return ( {this._renderCloseButton()} From c9c8d5fd84f694720db62263b717c53aecf85888 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 11:49:51 -0400 Subject: [PATCH 07/28] rearrange inputs --- .../public/components/map/feature_tooltip.js | 87 +++++++++++++++---- 1 file changed, 69 insertions(+), 18 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index d41730828a386..2e293535452d3 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -10,7 +10,10 @@ import { EuiText, EuiTextAlign, EuiPagination, - EuiSuperSelect + EuiSuperSelect, + EuiHorizontalRule, + EuiFlexGroup, + EuiFlexItem } from '@elastic/eui'; import _ from 'lodash'; import { i18n } from '@kbn/i18n'; @@ -157,6 +160,42 @@ export class FeatureTooltip extends React.Component { } + + _renderHeader() { + + if (!this.props.showCloseButton && !this.props.showFeatureList) { + return null; + } + + return ( + + + + {this._renderLayerFilterBox()} + + + {this._renderCloseButton()} + + + + + ); + } + + _renderFooter(filteredFeatures) { + + if (filteredFeatures.length === 1) { + return null; + } + + return ( + + + {this._renderPagination(filteredFeatures)} + + ); + } + _renderCloseButton() { if (!this.props.showCloseButton) { return null; @@ -175,6 +214,7 @@ export class FeatureTooltip extends React.Component { ); } + _onPageChange = (pageNumber) => { this.prevLayerId = null; this.prevFeatureId = null; @@ -195,35 +235,46 @@ export class FeatureTooltip extends React.Component { _renderPagination(filteredFeatures) { - if (filteredFeatures.length === 1) { - return null; - } - - if (!this.props.showFeatureList) { - return ( - - 1 of {filteredFeatures.length} - - ); - } + const pageNumberReadout = ( + + {(this.state.pageNumber + 1)} of {filteredFeatures.length} + + ); - return ( - + compressed + />); + } else { + cycleArrows = null; + } + + return ( + + + + {pageNumberReadout} + + + {cycleArrows} + + + ); + } render() { const filteredFeatures = this._filterFeatures(); return ( - {this._renderCloseButton()} + {this._renderHeader()} {this._renderProperties(filteredFeatures)} - {this._renderPagination(filteredFeatures)} - {this._renderLayerFilterBox()} + {this._renderFooter(filteredFeatures)} ); } From d4b935b23060987aa4ba84b40c838bcef6f415bb Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 12:36:50 -0400 Subject: [PATCH 08/28] small changes --- .../public/components/map/feature_tooltip.js | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 2e293535452d3..dfa2426f14359 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -10,7 +10,8 @@ import { EuiText, EuiTextAlign, EuiPagination, - EuiSuperSelect, + EuiSelect, + EuiIcon, EuiHorizontalRule, EuiFlexGroup, EuiFlexItem @@ -43,8 +44,9 @@ export class FeatureTooltip extends React.Component { this._isMounted = false; } - _onLayerChange = (layerId) => { + _onLayerChange = (e) => { + const layerId = e.target.value; if (this.state.layerIdFilter === layerId) { return; } @@ -138,29 +140,32 @@ export class FeatureTooltip extends React.Component { const layerOptions = this.state.uniqueLayers.map(({ id, displayName, count }) => { return { value: id, - inputDisplay: (({count}) {displayName}) + text: `(${count}) ${displayName}` }; }); const options = [ { value: ALL_LAYERS, - inputDisplay: (All layers) + text: i18n.translate('xpack.maps.tooltip.allLayersLabel', { + defaultMessage: 'All layers' + }) }, ...layerOptions ]; return ( - ); - } - _renderHeader() { if (!this.props.showCloseButton && !this.props.showFeatureList) { @@ -259,6 +264,9 @@ export class FeatureTooltip extends React.Component { {pageNumberReadout} + + + {cycleArrows} From 2e5eb1cfcbb6524c9332427598bcf39ac3622984 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 13:48:58 -0400 Subject: [PATCH 09/28] change condition --- .../maps/public/components/map/feature_tooltip.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index dfa2426f14359..54bc4b1f5a1ac 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -11,7 +11,7 @@ import { EuiTextAlign, EuiPagination, EuiSelect, - EuiIcon, + EuiIconTip, EuiHorizontalRule, EuiFlexGroup, EuiFlexItem @@ -172,6 +172,7 @@ export class FeatureTooltip extends React.Component { return null; } + const divider = (this.state.uniqueLayers && this.state.uniqueLayers.length > 1) ? : null; return ( @@ -182,7 +183,7 @@ export class FeatureTooltip extends React.Component { {this._renderCloseButton()} - + {divider} ); } @@ -265,7 +266,12 @@ export class FeatureTooltip extends React.Component { {pageNumberReadout} - + {cycleArrows} From 7ae18c03c353b4135ba2ea6e77b242e3d965efba Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 14:03:29 -0400 Subject: [PATCH 10/28] toggle hint --- .../maps/public/components/map/feature_tooltip.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 54bc4b1f5a1ac..7cd6c2f3771ac 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -259,6 +259,13 @@ export class FeatureTooltip extends React.Component { cycleArrows = null; } + const hint = this.props.showFeatureList ? () : null; + return ( @@ -266,12 +273,7 @@ export class FeatureTooltip extends React.Component { {pageNumberReadout} - + {hint} {cycleArrows} From 73add06503e5ceddb6524c36bcc298fbbbc1f6cf Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 14:08:03 -0400 Subject: [PATCH 11/28] use html the right way --- .../plugins/maps/public/components/map/feature_tooltip.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 7cd6c2f3771ac..542886b661765 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -243,7 +243,7 @@ export class FeatureTooltip extends React.Component { const pageNumberReadout = ( - {(this.state.pageNumber + 1)} of {filteredFeatures.length} + {(this.state.pageNumber + 1)} of {filteredFeatures.length} ); From 54c9f2deb78af2539b4b6338e4ce2a48574b46a5 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 14:14:44 -0400 Subject: [PATCH 12/28] fix on close tooltip --- .../plugins/maps/public/components/map/feature_properties.js | 2 +- .../plugins/maps/public/components/map/feature_tooltip.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_properties.js b/x-pack/legacy/plugins/maps/public/components/map/feature_properties.js index 938a9edd5df09..079684692e23f 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_properties.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_properties.js @@ -96,7 +96,7 @@ export class FeatureProperties extends React.Component { defaultMessage: 'Filter on property' })} onClick={() => { - this._onCloseTooltip(); + this.props.onCloseTooltip(); const filterAction = tooltipProperty.getFilterAction(); filterAction(); }} diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 542886b661765..ea49f2f82c9a9 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -123,6 +123,7 @@ export class FeatureTooltip extends React.Component { layerId={feature.layerId} loadFeatureProperties={this.props.loadFeatureProperties} showFilterButtons={this.props.showFilterButtons} + onCloseTooltip={this.props._onCloseTooltip} /> ); } From b5d37467306c6bba066409f25641e20feee481d7 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 14:18:36 -0400 Subject: [PATCH 13/28] typo --- .../plugins/maps/public/components/map/feature_tooltip.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index ea49f2f82c9a9..8db8eebf0f83e 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -123,7 +123,7 @@ export class FeatureTooltip extends React.Component { layerId={feature.layerId} loadFeatureProperties={this.props.loadFeatureProperties} showFilterButtons={this.props.showFilterButtons} - onCloseTooltip={this.props._onCloseTooltip} + onCloseTooltip={this._onCloseTooltip} /> ); } From e28e95d6573a2505a400adef598246d920bd032d Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 14:52:46 -0400 Subject: [PATCH 14/28] remove cruft --- .../legacy/plugins/maps/public/components/map/mb/view.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js index 60f7e95cb01be..f63671477d8dc 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js +++ b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js @@ -152,7 +152,6 @@ export class MBMapContainer extends React.Component { } const targetMbFeataure = mbFeatures[0]; - // const layer = this._getLayerByMbLayerId(targetMbFeataure.layer.id); const popupAnchorLocation = this._justifyAnchorLocation(e.lngLat, targetMbFeataure); const features = this._getIdsForFeatures(mbFeatures); @@ -189,13 +188,10 @@ export class MBMapContainer extends React.Component { } } - // const layer = this._getLayerByMbLayerId(targetMbFeature.layer.id); const popupAnchorLocation = this._justifyAnchorLocation(e.lngLat, targetMbFeature); const features = this._getIdsForFeatures(mbFeatures); this.props.setTooltipState({ type: TOOLTIP_TYPE.HOVER, - // featureId: targetMbFeature.properties[FEATURE_ID_PROPERTY_NAME], - // layerId: layer.getId(), features: features, location: popupAnchorLocation }); @@ -226,8 +222,8 @@ export class MBMapContainer extends React.Component { }, []); - //ensure all layers that are actually on the map - //the raw list may contain layer-ids that have not been added to the map yet. + //Ensure that all layers are actually on the map. + //The raw list may contain layer-ids that have not been added to the map yet. //For example: //a vector or heatmap layer will not add a source and layer to the mapbox-map, until that data is available. //during that data-fetch window, the app should not query for layers that do not exist. From 1d58ec483726476818ab41437559396850b6cee1 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 15:06:21 -0400 Subject: [PATCH 15/28] add feature_properties test --- .../feature_properties.test.js.snap | 113 ++++++++ .../feature_tooltip.test.js.snap | 252 +++++++----------- .../components/map/feature_properties.test.js | 107 ++++++++ .../components/map/feature_tooltip.test.js | 21 +- 4 files changed, 338 insertions(+), 155 deletions(-) create mode 100644 x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_properties.test.js.snap create mode 100644 x-pack/legacy/plugins/maps/public/components/map/feature_properties.test.js diff --git a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_properties.test.js.snap b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_properties.test.js.snap new file mode 100644 index 0000000000000..57375b4578ec7 --- /dev/null +++ b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_properties.test.js.snap @@ -0,0 +1,113 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`FeatureTooltip (single) should not show filter button 1`] = ` + + + + + + + + + +
+ foo + +
+ foo + +
+`; + +exports[`FeatureTooltip (single) should show error message if unable to load tooltip content 1`] = ` + +

+ Simulated load properties error +

+ +`; + +exports[`FeatureTooltip (single) should show only filter button for filterable properties 1`] = ` + + + + + + + + + + +
+ foo + + + +
+ foo + +
+`; diff --git a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap index 36f2ef7465fad..a7900c39e0fff 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap @@ -1,176 +1,128 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`FeatureTooltip should not show close button and not show filter button 1`] = ` +exports[`FeatureTooltip (single) should not show close button and not show filter button 1`] = ` - - -
+ `; -exports[`FeatureTooltip should show both filter buttons and close button 1`] = ` +exports[`FeatureTooltip (single) should show both filter buttons and close button 1`] = ` - - - - - - - - - - + - - - -
- foo - - - -
- foo - -
+ + + + `; -exports[`FeatureTooltip should show close button, but not filter button 1`] = ` +exports[`FeatureTooltip (single) should show close button, but not filter button 1`] = ` - - - - - -
+ + + + + + + `; -exports[`FeatureTooltip should show error message if unable to load tooltip content 1`] = ` - -

- Simulated load properties error -

- -`; - -exports[`FeatureTooltip should show only filter button for filterable properties 1`] = ` +exports[`FeatureTooltip (single) should show error message if unable to load tooltip content 1`] = ` - - - - - - - + + - - - -
- foo - - - -
- foo - -
+ + + + + +`; + +exports[`FeatureTooltip (single) should show only filter button for filterable properties 1`] = ` + + `; diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_properties.test.js b/x-pack/legacy/plugins/maps/public/components/map/feature_properties.test.js new file mode 100644 index 0000000000000..d0ad5e4941cbc --- /dev/null +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_properties.test.js @@ -0,0 +1,107 @@ +/* + * 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 from 'react'; +import { shallow } from 'enzyme'; +import { FeatureProperties } from './feature_properties'; + +class MockTooltipProperty { + constructor(key, value, isFilterable) { + this._key = key; + this._value = value; + this._isFilterable = isFilterable; + } + + isFilterable() { + return this._isFilterable; + } + + getFilterAction() { + return () => {}; + } + + getHtmlDisplayValue() { + return this._value; + } + + getPropertyName() { + return this._key; + } +} + +const defaultProps = { + loadFeatureProperties: () => { return []; }, + featureId: `feature`, + layerId: `layer`, + onCloseTooltip: () => {}, + showFilterButtons: false +}; + +const mockTooltipProperties = [ + new MockTooltipProperty('foo', 'bar', true), + new MockTooltipProperty('foo', 'bar', false) +]; + +describe('FeatureTooltip (single)', async () => { + + test('should not show filter button', async () => { + const component = shallow( + { return mockTooltipProperties; }} + /> + ); + + // Ensure all promises resolve + await new Promise(resolve => process.nextTick(resolve)); + // Ensure the state changes are reflected + component.update(); + + expect(component) + .toMatchSnapshot(); + }); + + + test('should show only filter button for filterable properties', async () => { + const component = shallow( + { return mockTooltipProperties; }} + /> + ); + + // Ensure all promises resolve + await new Promise(resolve => process.nextTick(resolve)); + // Ensure the state changes are reflected + component.update(); + + expect(component) + .toMatchSnapshot(); + }); + + + test('should show error message if unable to load tooltip content', async () => { + const component = shallow( + { throw new Error('Simulated load properties error'); }} + /> + ); + + // Ensure all promises resolve + await new Promise(resolve => process.nextTick(resolve)); + // Ensure the state changes are reflected + component.update(); + + expect(component) + .toMatchSnapshot(); + }); + + +}); diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js index 1abb22fe93521..054c5198d084a 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js @@ -32,15 +32,26 @@ class MockTooltipProperty { } } +class MockLayer { + + constructor(id) { + this._id = id; + } + async getDisplayName() { + return `display + ${this._id}`; + } + +} + const defaultProps = { loadFeatureProperties: () => { return []; }, - tooltipState: { - layerId: 'layer1', - featureId: 'feature1', + findLayerById: (id) => { + return new MockLayer(id); }, closeTooltip: () => {}, showFilterButtons: false, - showCloseButton: false + showCloseButton: false, + showFeatureList: false }; @@ -49,7 +60,7 @@ const mockTooltipProperties = [ new MockTooltipProperty('foo', 'bar', false) ]; -describe('FeatureTooltip', async () => { +describe('FeatureTooltip (single)', async () => { test('should not show close button and not show filter button', async () => { const component = shallow( From bd4cd5217d31b365f572ca26c7a949291de6a183 Mon Sep 17 00:00:00 2001 From: Ryan Keairns Date: Thu, 27 Jun 2019 14:30:45 -0500 Subject: [PATCH 16/28] design adjustments --- .../components/map/_feature_tooltip.scss | 1 + .../public/components/map/feature_tooltip.js | 36 +++++++++---------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/_feature_tooltip.scss b/x-pack/legacy/plugins/maps/public/components/map/_feature_tooltip.scss index 416ba8c6c01f0..1ce7738f37fda 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/_feature_tooltip.scss +++ b/x-pack/legacy/plugins/maps/public/components/map/_feature_tooltip.scss @@ -1,4 +1,5 @@ .mapFeatureTooltip_table { + width: 100%; td { padding: $euiSizeXS; } diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 542886b661765..dd128f551a98e 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -8,7 +8,6 @@ import React, { Fragment } from 'react'; import { EuiButtonIcon, EuiText, - EuiTextAlign, EuiPagination, EuiSelect, EuiIconTip, @@ -159,6 +158,7 @@ export class FeatureTooltip extends React.Component { options={options} onChange={this._onLayerChange} valueOfSelected={this.state.layerIdFilter} + compressed aria-label={i18n.translate('xpack.maps.tooltip.layerFilterLabel', { defaultMessage: 'Filter results by layer' })} @@ -172,7 +172,7 @@ export class FeatureTooltip extends React.Component { return null; } - const divider = (this.state.uniqueLayers && this.state.uniqueLayers.length > 1) ? : null; + const divider = (this.state.uniqueLayers && this.state.uniqueLayers.length > 1) ? : null; return ( @@ -196,7 +196,7 @@ export class FeatureTooltip extends React.Component { return ( - + {this._renderPagination(filteredFeatures)} ); @@ -207,16 +207,14 @@ export class FeatureTooltip extends React.Component { return null; } return ( - - - + ); } @@ -242,9 +240,7 @@ export class FeatureTooltip extends React.Component { _renderPagination(filteredFeatures) { const pageNumberReadout = ( - - {(this.state.pageNumber + 1)} of {filteredFeatures.length} - + {(this.state.pageNumber + 1)} of {filteredFeatures.length} ); let cycleArrows; @@ -262,18 +258,18 @@ export class FeatureTooltip extends React.Component { const hint = this.props.showFeatureList ? () : null; return ( - + - {pageNumberReadout} + {hint} - {hint} + {pageNumberReadout} {cycleArrows} From 4fe564d4ac257aaa990600243469e0030681a41f Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 15:32:02 -0400 Subject: [PATCH 17/28] modify tooltip test --- .../feature_tooltip.test.js.snap | 138 +++++++++++++++--- .../public/components/map/feature_tooltip.js | 13 +- .../components/map/feature_tooltip.test.js | 81 +++++----- .../maps/public/components/map/mb/view.js | 2 +- 4 files changed, 166 insertions(+), 68 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap index a7900c39e0fff..3240cab183347 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap @@ -1,17 +1,44 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`FeatureTooltip (single) should not show close button and not show filter button 1`] = ` +exports[`FeatureTooltip (multi) should not show close button / should show count 1`] = ` + + + + + + + 1 + + of + + 3 + + + + + + + `; -exports[`FeatureTooltip (single) should show both filter buttons and close button 1`] = ` +exports[`FeatureTooltip (multi) should show close button / should show count / should show arrows / should show layer filter 1`] = ` + + + + + + + 1 + + of + + 3 + + + + + + + + + + + `; -exports[`FeatureTooltip (single) should show close button, but not filter button 1`] = ` +exports[`FeatureTooltip (multi) should show close button / should show count 1`] = ` + + + + + + + 1 + + of + + 3 + + + + + + + `; -exports[`FeatureTooltip (single) should show error message if unable to load tooltip content 1`] = ` +exports[`FeatureTooltip (single) should not show close button 1`] = ` + + + +`; + +exports[`FeatureTooltip (single) should show close button 1`] = ` - -`; - -exports[`FeatureTooltip (single) should show only filter button for filterable properties 1`] = ` - - `; diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 8db8eebf0f83e..f2fd896d1fdc0 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -69,13 +69,13 @@ export class FeatureTooltip extends React.Component { _loadUniqueLayers = async () => { const uniqueLayerIds = []; - for (let i = 0; i < this.props.tooltipState.features.length; i++) { + for (let i = 0; i < this.props.features.length; i++) { let index = uniqueLayerIds.findIndex(({ layerId }) => { - return layerId === this.props.tooltipState.features[i].layerId; + return layerId === this.props.features[i].layerId; }); if (index < 0) { uniqueLayerIds.push({ - layerId: this.props.tooltipState.features[i].layerId, + layerId: this.props.features[i].layerId, count: 0 }); index = uniqueLayerIds.length - 1; @@ -130,14 +130,17 @@ export class FeatureTooltip extends React.Component { _renderLayerFilterBox() { + console.log('SHOW FILTER BOX'); if (!this.props.showFeatureList) { return null; } if (!this.state.uniqueLayers || this.state.uniqueLayers.length < 2) { + console.log('SKIP'); return null; } + console.log('SHOW FILTER LYOPTIONS'); const layerOptions = this.state.uniqueLayers.map(({ id, displayName, count }) => { return { value: id, @@ -232,10 +235,10 @@ export class FeatureTooltip extends React.Component { _filterFeatures() { if (this.state.layerIdFilter === ALL_LAYERS) { - return this.props.tooltipState.features; + return this.props.features; } - return this.props.tooltipState.features.filter((feature) => { + return this.props.features.filter((feature) => { return feature.layerId === this.state.layerIdFilter; }); } diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js index 054c5198d084a..c04121d1b0b8d 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js @@ -8,30 +8,6 @@ import React from 'react'; import { shallow } from 'enzyme'; import { FeatureTooltip } from './feature_tooltip'; -class MockTooltipProperty { - constructor(key, value, isFilterable) { - this._key = key; - this._value = value; - this._isFilterable = isFilterable; - } - - isFilterable() { - return this._isFilterable; - } - - getFilterAction() { - return () => {}; - } - - getHtmlDisplayValue() { - return this._value; - } - - getPropertyName() { - return this._key; - } -} - class MockLayer { constructor(id) { @@ -43,9 +19,33 @@ class MockLayer { } + +const MULTI_FEATURE_MULTI_LAYER = [ + { + 'id': 'feature1', + 'layerId': 'layer1' + }, + { + 'id': 'feature2', + 'layerId': 'layer1' + }, + { + 'id': 'feature1', + 'layerId': 'layer2' + } +]; + +const SINGLE_FEATURE = [ + { + 'id': 'feature1', + 'layerId': 'layer1' + } +]; + const defaultProps = { loadFeatureProperties: () => { return []; }, findLayerById: (id) => { + console.log('fin by id', id); return new MockLayer(id); }, closeTooltip: () => {}, @@ -54,18 +54,13 @@ const defaultProps = { showFeatureList: false }; - -const mockTooltipProperties = [ - new MockTooltipProperty('foo', 'bar', true), - new MockTooltipProperty('foo', 'bar', false) -]; - describe('FeatureTooltip (single)', async () => { - test('should not show close button and not show filter button', async () => { + test('should not show close button', async () => { const component = shallow( ); @@ -78,11 +73,12 @@ describe('FeatureTooltip (single)', async () => { .toMatchSnapshot(); }); - test('should show close button, but not filter button', async () => { + test('should show close button', async () => { const component = shallow( ); @@ -95,12 +91,15 @@ describe('FeatureTooltip (single)', async () => { .toMatchSnapshot(); }); - test('should show only filter button for filterable properties', async () => { +}); + +describe('FeatureTooltip (multi)', async () => { + + test('should not show close button / should show count', async () => { const component = shallow( { return mockTooltipProperties; }} + features={MULTI_FEATURE_MULTI_LAYER} /> ); @@ -113,13 +112,12 @@ describe('FeatureTooltip (single)', async () => { .toMatchSnapshot(); }); - test('should show both filter buttons and close button', async () => { + test('should show close button / should show count', async () => { const component = shallow( { return mockTooltipProperties; }} + features={MULTI_FEATURE_MULTI_LAYER} /> ); @@ -132,13 +130,13 @@ describe('FeatureTooltip (single)', async () => { .toMatchSnapshot(); }); - test('should show error message if unable to load tooltip content', async () => { + test('should show close button / should show count / should show arrows / should show layer filter', async () => { const component = shallow( { throw new Error('Simulated load properties error'); }} + features={MULTI_FEATURE_MULTI_LAYER} + showFeatureList={true} /> ); @@ -152,4 +150,5 @@ describe('FeatureTooltip (single)', async () => { }); + }); diff --git a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js index f63671477d8dc..c501220f7ea19 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js +++ b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js @@ -393,7 +393,7 @@ export class MBMapContainer extends React.Component { const isLocked = this.props.tooltipState.type === TOOLTIP_TYPE.LOCKED; ReactDOM.render(( Date: Thu, 27 Jun 2019 15:54:18 -0400 Subject: [PATCH 18/28] dont always show hint --- .../public/components/map/feature_tooltip.js | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 3ec85d491c286..518aa68f0ebbf 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -259,19 +259,21 @@ export class FeatureTooltip extends React.Component { cycleArrows = null; } - const hint = this.props.showFeatureList ? () : null; + const hint = (this.props.showFeatureList && filteredFeatures.length > 20) ? ( + + + + ) : null; return ( - - {hint} - + {hint} {pageNumberReadout} From 5f0a086732a405c1bcd4da3387124b83d8ee2901 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 27 Jun 2019 16:00:39 -0400 Subject: [PATCH 19/28] update test --- .../feature_tooltip.test.js.snap | 171 ++++++++---------- 1 file changed, 75 insertions(+), 96 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap index 3240cab183347..6adb0f972ae75 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap @@ -9,28 +9,27 @@ exports[`FeatureTooltip (multi) should not show close button / should show count onCloseTooltip={[Function]} showFilterButtons={false} /> - + - - + - - - 1 - - of - - 3 - - - + + 1 + + of + + 3 + + - @@ -49,19 +48,15 @@ exports[`FeatureTooltip (multi) should show close button / should show count / s - - - + - + - - - - - 1 - - of - - 3 - - - - - + + + 1 + + of + + 3 + + - - - + - + - - + - - - 1 - - of - - 3 - - - + + 1 + + of + + 3 + + - @@ -198,19 +181,15 @@ exports[`FeatureTooltip (single) should show close button 1`] = ` - - - + Date: Fri, 28 Jun 2019 10:11:45 -0400 Subject: [PATCH 20/28] remove cruft --- .../plugins/maps/public/components/map/feature_tooltip.js | 4 ---- .../maps/public/components/map/feature_tooltip.test.js | 1 - 2 files changed, 5 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 518aa68f0ebbf..fa3f15f86d860 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -129,17 +129,13 @@ export class FeatureTooltip extends React.Component { _renderLayerFilterBox() { - console.log('SHOW FILTER BOX'); if (!this.props.showFeatureList) { return null; } if (!this.state.uniqueLayers || this.state.uniqueLayers.length < 2) { - console.log('SKIP'); return null; } - - console.log('SHOW FILTER LYOPTIONS'); const layerOptions = this.state.uniqueLayers.map(({ id, displayName, count }) => { return { value: id, diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js index c04121d1b0b8d..94bf95fa22bc0 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js @@ -45,7 +45,6 @@ const SINGLE_FEATURE = [ const defaultProps = { loadFeatureProperties: () => { return []; }, findLayerById: (id) => { - console.log('fin by id', id); return new MockLayer(id); }, closeTooltip: () => {}, From 2835360204babf6558634a9e474587ee2fad7e89 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Fri, 28 Jun 2019 10:18:35 -0400 Subject: [PATCH 21/28] improve test --- .../feature_properties.test.js.snap | 30 +++++++++---------- .../components/map/feature_properties.test.js | 7 ++--- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_properties.test.js.snap b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_properties.test.js.snap index 57375b4578ec7..bdcc13a872b5e 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_properties.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_properties.test.js.snap @@ -1,40 +1,40 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`FeatureTooltip (single) should not show filter button 1`] = ` +exports[`FeatureProperties should not show filter button 1`] = `
- foo + prop1
- foo + prop2 @@ -43,7 +43,7 @@ exports[`FeatureTooltip (single) should not show filter button 1`] = `
`; -exports[`FeatureTooltip (single) should show error message if unable to load tooltip content 1`] = ` +exports[`FeatureProperties should show error message if unable to load tooltip content 1`] = ` `; -exports[`FeatureTooltip (single) should show only filter button for filterable properties 1`] = ` +exports[`FeatureProperties should show only filter button for filterable properties 1`] = `
- foo + prop1 @@ -92,18 +92,18 @@ exports[`FeatureTooltip (single) should show only filter button for filterable p
- foo + prop2 diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_properties.test.js b/x-pack/legacy/plugins/maps/public/components/map/feature_properties.test.js index d0ad5e4941cbc..aa39ad85bd820 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_properties.test.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_properties.test.js @@ -41,11 +41,11 @@ const defaultProps = { }; const mockTooltipProperties = [ - new MockTooltipProperty('foo', 'bar', true), - new MockTooltipProperty('foo', 'bar', false) + new MockTooltipProperty('prop1', 'foobar1', true), + new MockTooltipProperty('prop2', 'foobar2', false) ]; -describe('FeatureTooltip (single)', async () => { +describe('FeatureProperties', async () => { test('should not show filter button', async () => { const component = shallow( @@ -89,7 +89,6 @@ describe('FeatureTooltip (single)', async () => { { throw new Error('Simulated load properties error'); }} /> ); From 5eefcd199d522af5b660f43af06dffbbec05f666 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Fri, 28 Jun 2019 10:39:40 -0400 Subject: [PATCH 22/28] dont recalculate layer-list --- .../maps/public/components/map/feature_tooltip.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index fa3f15f86d860..890641092f63d 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -31,6 +31,11 @@ export class FeatureTooltip extends React.Component { layerIdFilter: ALL_LAYERS }; + constructor() { + super(); + this._prevFeatures = null; + } + componentDidMount() { this._isMounted = true; } @@ -67,6 +72,11 @@ export class FeatureTooltip extends React.Component { _loadUniqueLayers = async () => { + + if (this._prevFeatures === this.props.features) { + return; + } + const uniqueLayerIds = []; for (let i = 0; i < this.props.features.length; i++) { let index = uniqueLayerIds.findIndex(({ layerId }) => { @@ -100,6 +110,7 @@ export class FeatureTooltip extends React.Component { }); if (this._isMounted) { + this._prevFeatures = this.props.features; if (!_.isEqual(this.state.uniqueLayers, options)) { this.setState({ uniqueLayers: options, From 19c8eabc507ccdd17bc62ab7c2ae3ba2bfff1076 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Fri, 28 Jun 2019 11:08:08 -0400 Subject: [PATCH 23/28] cut off earlier, so doesnt update on pagenumber or layer-filter changes --- .../public/components/map/feature_tooltip.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 890641092f63d..279eae4978e1e 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -15,7 +15,6 @@ import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; -import _ from 'lodash'; import { i18n } from '@kbn/i18n'; import { FeatureProperties } from './feature_properties'; @@ -77,6 +76,8 @@ export class FeatureTooltip extends React.Component { return; } + this._prevFeatures = this.props.features; + const uniqueLayerIds = []; for (let i = 0; i < this.props.features.length; i++) { let index = uniqueLayerIds.findIndex(({ layerId }) => { @@ -110,14 +111,11 @@ export class FeatureTooltip extends React.Component { }); if (this._isMounted) { - this._prevFeatures = this.props.features; - if (!_.isEqual(this.state.uniqueLayers, options)) { - this.setState({ - uniqueLayers: options, - layerIdFilter: ALL_LAYERS, - pageNumber: DEFAULT_PAGE_NUMBER - }); - } + this.setState({ + uniqueLayers: options, + layerIdFilter: ALL_LAYERS, + pageNumber: DEFAULT_PAGE_NUMBER + }); } }; From fd5748470b0803ff6ca00618cd996501df6139e3 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Fri, 28 Jun 2019 16:55:27 -0400 Subject: [PATCH 24/28] lint --- .../maps/public/components/map/feature_tooltip.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 279eae4978e1e..b65671ce2a3bf 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -177,11 +177,12 @@ export class FeatureTooltip extends React.Component { _renderHeader() { - if (!this.props.showCloseButton && !this.props.showFeatureList) { + if (!this.props.showCloseButton && !this.props.showFeatureList) { return null; } - const divider = (this.state.uniqueLayers && this.state.uniqueLayers.length > 1) ? : null; + const divider = (this.state.uniqueLayers && this.state.uniqueLayers.length > 1) ? + : null; return ( @@ -205,7 +206,7 @@ export class FeatureTooltip extends React.Component { return ( - + {this._renderPagination(filteredFeatures)} ); @@ -248,7 +249,7 @@ export class FeatureTooltip extends React.Component { _renderPagination(filteredFeatures) { - const pageNumberReadout = ( + const pageNumberReadout = ( {(this.state.pageNumber + 1)} of {filteredFeatures.length} ); From cdb7e50ae422d7630fbe82d55e147c7fcef6a961 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Mon, 1 Jul 2019 09:39:55 -0400 Subject: [PATCH 25/28] feedback --- .../public/components/map/feature_tooltip.js | 32 ++++++------------- .../components/map/feature_tooltip.test.js | 10 +++--- .../maps/public/components/map/mb/view.js | 5 +-- 3 files changed, 16 insertions(+), 31 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index b65671ce2a3bf..067522246e414 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -71,7 +71,6 @@ export class FeatureTooltip extends React.Component { _loadUniqueLayers = async () => { - if (this._prevFeatures === this.props.features) { return; } @@ -137,11 +136,6 @@ export class FeatureTooltip extends React.Component { } _renderLayerFilterBox() { - - if (!this.props.showFeatureList) { - return null; - } - if (!this.state.uniqueLayers || this.state.uniqueLayers.length < 2) { return null; } @@ -177,7 +171,7 @@ export class FeatureTooltip extends React.Component { _renderHeader() { - if (!this.props.showCloseButton && !this.props.showFeatureList) { + if (!this.props.isLocked) { return null; } @@ -213,9 +207,6 @@ export class FeatureTooltip extends React.Component { } _renderCloseButton() { - if (!this.props.showCloseButton) { - return null; - } return ( {(this.state.pageNumber + 1)} of {filteredFeatures.length} ); - let cycleArrows; - if (this.props.showFeatureList) { - cycleArrows = (); - } else { - cycleArrows = null; - } + const cycleArrows = (this.props.isLocked) ? () : null; - const hint = (this.props.showFeatureList && filteredFeatures.length > 20) ? ( + const hint = (this.props.isLocked && filteredFeatures.length > 20) ? ( diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js index 94bf95fa22bc0..328402957bd42 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.test.js @@ -49,8 +49,7 @@ const defaultProps = { }, closeTooltip: () => {}, showFilterButtons: false, - showCloseButton: false, - showFeatureList: false + isLocked: false }; describe('FeatureTooltip (single)', async () => { @@ -76,7 +75,7 @@ describe('FeatureTooltip (single)', async () => { const component = shallow( ); @@ -115,7 +114,7 @@ describe('FeatureTooltip (multi)', async () => { const component = shallow( ); @@ -133,9 +132,8 @@ describe('FeatureTooltip (multi)', async () => { const component = shallow( ); diff --git a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js index c501220f7ea19..d86f11c06f0ea 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js +++ b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js @@ -398,8 +398,9 @@ export class MBMapContainer extends React.Component { findLayerById={this._findLayerById} closeTooltip={this._onTooltipClose} showFilterButtons={this.props.isFilterable && isLocked} - showCloseButton={isLocked} - showFeatureList={isLocked} + // showCloseButton={isLocked} + // showFeatureList={isLocked} + isLocked={isLocked} /> ), this._tooltipContainer); From 89af06d090ce7743ddfa644db419e2a94c014dd6 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Mon, 1 Jul 2019 09:44:27 -0400 Subject: [PATCH 26/28] update snapshot --- .../map/__snapshots__/feature_tooltip.test.js.snap | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap index 6adb0f972ae75..fa9511840f253 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/components/map/__snapshots__/feature_tooltip.test.js.snap @@ -153,7 +153,14 @@ exports[`FeatureTooltip (multi) should show close button / should show count 1`] + > + + `; From 42e61d3154c95e141eeb90ee0bf41040366a37d2 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Mon, 1 Jul 2019 10:08:05 -0400 Subject: [PATCH 27/28] use map --- .../public/components/map/feature_tooltip.js | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 067522246e414..8a6289819bc01 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -77,23 +77,20 @@ export class FeatureTooltip extends React.Component { this._prevFeatures = this.props.features; - const uniqueLayerIds = []; + + const countByLayerId = new Map(); for (let i = 0; i < this.props.features.length; i++) { - let index = uniqueLayerIds.findIndex(({ layerId }) => { - return layerId === this.props.features[i].layerId; - }); - if (index < 0) { - uniqueLayerIds.push({ - layerId: this.props.features[i].layerId, - count: 0 - }); - index = uniqueLayerIds.length - 1; + let count = countByLayerId.get(this.props.features[i].layerId); + if (!count) { + count = 0; } - uniqueLayerIds[index].count++; + count++; + countByLayerId.set(this.props.features[i].layerId, count); } - const layers = uniqueLayerIds.map(({ layerId }) => { - return this.props.findLayerById(layerId); + const layers = []; + countByLayerId.forEach((count, layerId) => { + layers.push(this.props.findLayerById(layerId)); }); const layerNamePromises = layers.map(layer => { @@ -105,7 +102,7 @@ export class FeatureTooltip extends React.Component { return { displayName: layerNames[index], id: layer.getId(), - count: uniqueLayerIds[index].count + count: countByLayerId.get(layer.getId()) }; }); From 17697d2c98ec8af1ef079a65d150d0c7cc6d518f Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Mon, 1 Jul 2019 11:20:58 -0400 Subject: [PATCH 28/28] sloppy sloppy tsk tsk --- .../plugins/maps/public/components/map/feature_tooltip.js | 2 -- x-pack/legacy/plugins/maps/public/components/map/mb/view.js | 2 -- 2 files changed, 4 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js index 8a6289819bc01..6c9710b661840 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js +++ b/x-pack/legacy/plugins/maps/public/components/map/feature_tooltip.js @@ -218,8 +218,6 @@ export class FeatureTooltip extends React.Component { _onPageChange = (pageNumber) => { - this.prevLayerId = null; - this.prevFeatureId = null; this.setState({ pageNumber: pageNumber, }); diff --git a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js index d86f11c06f0ea..8267c18e244b7 100644 --- a/x-pack/legacy/plugins/maps/public/components/map/mb/view.js +++ b/x-pack/legacy/plugins/maps/public/components/map/mb/view.js @@ -398,8 +398,6 @@ export class MBMapContainer extends React.Component { findLayerById={this._findLayerById} closeTooltip={this._onTooltipClose} showFilterButtons={this.props.isFilterable && isLocked} - // showCloseButton={isLocked} - // showFeatureList={isLocked} isLocked={isLocked} /> ), this._tooltipContainer);