From c3770fd70980e50bcab3fab6bbcd9cab50938653 Mon Sep 17 00:00:00 2001 From: Tyler Smalley Date: Tue, 15 Dec 2020 14:43:41 -0800 Subject: [PATCH] Revert " [Maps] unify legend for percentiles, interpolate, and custom ordinal color breaks (#85343)" This reverts commit 85dae266ebe2448e7ec2a918f0d3d783947e7fe8. --- x-pack/plugins/maps/common/i18n_getters.ts | 8 + .../components/color/color_stops_ordinal.js | 5 +- .../dynamic_color_property.test.tsx.snap | 24 +-- .../dynamic_color_property.test.tsx | 24 +-- .../properties/dynamic_color_property.tsx | 175 ++++++++---------- x-pack/test/functional/apps/maps/joins.js | 16 +- 6 files changed, 113 insertions(+), 139 deletions(-) diff --git a/x-pack/plugins/maps/common/i18n_getters.ts b/x-pack/plugins/maps/common/i18n_getters.ts index a128038e321f..02b1d1adcffc 100644 --- a/x-pack/plugins/maps/common/i18n_getters.ts +++ b/x-pack/plugins/maps/common/i18n_getters.ts @@ -9,6 +9,14 @@ import { i18n } from '@kbn/i18n'; import { $Values } from '@kbn/utility-types'; import { ES_SPATIAL_RELATIONS } from './constants'; +export const UPTO = i18n.translate('xpack.maps.upto', { + defaultMessage: 'up to', +}); + +export const GREAT_THAN = i18n.translate('xpack.maps.greatThan', { + defaultMessage: 'greater than', +}); + export function getAppTitle() { return i18n.translate('xpack.maps.appTitle', { defaultMessage: 'Maps', diff --git a/x-pack/plugins/maps/public/classes/styles/vector/components/color/color_stops_ordinal.js b/x-pack/plugins/maps/public/classes/styles/vector/components/color/color_stops_ordinal.js index 875e780d6f48..d52c769c95a7 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/components/color/color_stops_ordinal.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/components/color/color_stops_ordinal.js @@ -39,7 +39,7 @@ export const ColorStopsOrdinal = ({ return error; }; - const renderStopInput = (stop, onStopChange, index) => { + const renderStopInput = (stop, onStopChange) => { function handleOnChangeEvent(event) { const sanitizedValue = parseFloat(event.target.value); const newStopValue = isNaN(sanitizedValue) ? '' : sanitizedValue; @@ -50,10 +50,9 @@ export const ColorStopsOrdinal = ({ aria-label={i18n.translate('xpack.maps.styles.colorStops.ordinalStop.stopLabel', { defaultMessage: 'Stop', })} - value={index === 0 ? '' : stop} + value={stop} onChange={handleOnChangeEvent} compressed - disabled={index === 0} /> ); }; diff --git a/x-pack/plugins/maps/public/classes/styles/vector/properties/__snapshots__/dynamic_color_property.test.tsx.snap b/x-pack/plugins/maps/public/classes/styles/vector/properties/__snapshots__/dynamic_color_property.test.tsx.snap index a4c20ff5e0a7..7607510a10b3 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/properties/__snapshots__/dynamic_color_property.test.tsx.snap +++ b/x-pack/plugins/maps/public/classes/styles/vector/properties/__snapshots__/dynamic_color_property.test.tsx.snap @@ -166,7 +166,7 @@ exports[`renderLegendDetailRow ordinal Should render custom ordinal legend with color="#FF0000" isLinesOnly={false} isPointsOnly={true} - label="< 10_format" + label="0_format" styleName="lineColor" /> @@ -177,7 +177,7 @@ exports[`renderLegendDetailRow ordinal Should render custom ordinal legend with color="#00FF00" isLinesOnly={false} isPointsOnly={true} - label=">= 10_format" + label="10_format" styleName="lineColor" /> @@ -229,7 +229,7 @@ exports[`renderLegendDetailRow ordinal Should render interpolate bands 1`] = ` color="#ecf1f7" isLinesOnly={false} isPointsOnly={true} - label="< 13_format" + label="0_format" styleName="lineColor" /> @@ -240,7 +240,7 @@ exports[`renderLegendDetailRow ordinal Should render interpolate bands 1`] = ` color="#d9e3ef" isLinesOnly={false} isPointsOnly={true} - label="13_format up to 25_format" + label="13_format" styleName="lineColor" /> @@ -251,7 +251,7 @@ exports[`renderLegendDetailRow ordinal Should render interpolate bands 1`] = ` color="#c5d5e7" isLinesOnly={false} isPointsOnly={true} - label="25_format up to 38_format" + label="25_format" styleName="lineColor" /> @@ -262,7 +262,7 @@ exports[`renderLegendDetailRow ordinal Should render interpolate bands 1`] = ` color="#b2c7df" isLinesOnly={false} isPointsOnly={true} - label="38_format up to 50_format" + label="38_format" styleName="lineColor" /> @@ -273,7 +273,7 @@ exports[`renderLegendDetailRow ordinal Should render interpolate bands 1`] = ` color="#9eb9d8" isLinesOnly={false} isPointsOnly={true} - label="50_format up to 63_format" + label="50_format" styleName="lineColor" /> @@ -284,7 +284,7 @@ exports[`renderLegendDetailRow ordinal Should render interpolate bands 1`] = ` color="#8bacd0" isLinesOnly={false} isPointsOnly={true} - label="63_format up to 75_format" + label="63_format" styleName="lineColor" /> @@ -295,7 +295,7 @@ exports[`renderLegendDetailRow ordinal Should render interpolate bands 1`] = ` color="#769fc8" isLinesOnly={false} isPointsOnly={true} - label="75_format up to 88_format" + label="75_format" styleName="lineColor" /> @@ -306,7 +306,7 @@ exports[`renderLegendDetailRow ordinal Should render interpolate bands 1`] = ` color="#6092c0" isLinesOnly={false} isPointsOnly={true} - label=">= 88_format" + label="88_format" styleName="lineColor" /> @@ -358,7 +358,7 @@ exports[`renderLegendDetailRow ordinal Should render percentile bands 1`] = ` color="#e5ecf4" isLinesOnly={false} isPointsOnly={true} - label="< 50th: 5572_format" + label="up to 50th: 5572_format" styleName="lineColor" /> @@ -413,7 +413,7 @@ exports[`renderLegendDetailRow ordinal Should render percentile bands 1`] = ` color="#6092c0" isLinesOnly={false} isPointsOnly={true} - label=">= 99th: 16857_format" + label="greater than 99th: 16857_format" styleName="lineColor" /> diff --git a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.test.tsx b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.test.tsx index 4e6c38eaf38b..f120773086b8 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.test.tsx +++ b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.test.tsx @@ -371,7 +371,7 @@ describe('get mapbox color expression (via internal _getMbColor)', () => { expect(colorProperty._getMbColor()).toBeNull(); }); - describe('interpolate color ramp', () => { + describe('pre-defined color ramp', () => { test('should return null when color ramp is not provided', async () => { const dynamicStyleOptions = { type: COLOR_MAP_TYPE.ORDINAL, @@ -457,16 +457,7 @@ describe('get mapbox color expression (via internal _getMbColor)', () => { const colorProperty = makeProperty(dynamicStyleOptions); expect(colorProperty._getMbColor()).toEqual([ 'step', - [ - 'coalesce', - [ - 'case', - ['==', ['feature-state', 'foobar'], null], - 9, - ['max', ['min', ['to-number', ['feature-state', 'foobar']], 100], 10], - ], - 9, - ], + ['coalesce', ['feature-state', 'foobar'], 9], 'rgba(0,0,0,0)', 10, '#f7faff', @@ -492,16 +483,7 @@ describe('get mapbox color expression (via internal _getMbColor)', () => { const colorProperty = makeProperty(dynamicStyleOptions, undefined, field); expect(colorProperty._getMbColor()).toEqual([ 'step', - [ - 'coalesce', - [ - 'case', - ['==', ['get', 'foobar'], null], - 9, - ['max', ['min', ['to-number', ['get', 'foobar']], 100], 10], - ], - 9, - ], + ['coalesce', ['get', 'foobar'], 9], 'rgba(0,0,0,0)', 10, '#f7faff', diff --git a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.tsx b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.tsx index 428ccfbc5335..cc76a1d25856 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.tsx +++ b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.tsx @@ -15,6 +15,7 @@ import { getColorPalette, } from '../../color_palettes'; import { COLOR_MAP_TYPE, DATA_MAPPING_FUNCTION } from '../../../../../common/constants'; +import { GREAT_THAN, UPTO } from '../../../../../common/i18n_getters'; import { isCategoricalStopsInvalid, getOtherCategoryLabel, @@ -25,9 +26,6 @@ import { ColorDynamicOptions, OrdinalColorStop } from '../../../../../common/des import { LegendProps } from './style_property'; import { getOrdinalSuffix } from '../../../util/ordinal_suffix'; -const UP_TO = i18n.translate('xpack.maps.legend.upto', { - defaultMessage: 'up to', -}); const EMPTY_STOPS = { stops: [], defaultColor: null }; const RGBA_0000 = 'rgba(0,0,0,0)'; @@ -135,15 +133,11 @@ export class DynamicColorProperty extends DynamicStyleProperty { - return this._options.customColorRamp - ? this._options.customColorRamp.reduce( - (accumulatedStops: Array, nextStop: OrdinalColorStop) => { - return [...accumulatedStops, nextStop.stop, nextStop.color]; - }, - [] - ) - : []; - } - _getColorPaletteStops() { if (this._options.useCustomColorPalette && this._options.customColorPalette) { if (isCategoricalStopsInvalid(this._options.customColorPalette)) { @@ -298,89 +276,96 @@ export class DynamicColorProperty extends DynamicStyleProperty | null = null; - let getValuePrefix: ((i: number, isNext: boolean) => string) | null = null; - if (this._options.useCustomColorRamp) { - if (!this._options.customColorRamp) { - return []; - } - colorStops = this._getCustomRampColorStops(); - } else if (this.getDataMappingFunction() === DATA_MAPPING_FUNCTION.PERCENTILES) { + if (this._options.useCustomColorRamp && this._options.customColorRamp) { + return this._options.customColorRamp.map((ordinalColorStop) => { + return { + color: ordinalColorStop.color, + symbolId, + label: this.formatField(ordinalColorStop.stop), + }; + }); + } + + if (this.getDataMappingFunction() === DATA_MAPPING_FUNCTION.PERCENTILES) { const percentilesFieldMeta = this.getPercentilesFieldMeta(); if (!percentilesFieldMeta) { return []; } - colorStops = getPercentilesMbColorRampStops( + const colorStops = getPercentilesMbColorRampStops( this._options.color ? this._options.color : null, percentilesFieldMeta ); - getValuePrefix = function (i: number, isNext: boolean) { - const percentile = isNext - ? parseFloat(percentilesFieldMeta[i / 2 + 1].percentile) - : parseFloat(percentilesFieldMeta[i / 2].percentile); - - return `${percentile}${getOrdinalSuffix(percentile)}: `; - }; - } else { - const rangeFieldMeta = this.getRangeFieldMeta(); - if (!rangeFieldMeta || !this._options.color) { + if (!colorStops || colorStops.length <= 2) { return []; } - if (rangeFieldMeta.delta === 0) { - const colors = getColorPalette(this._options.color); - // map to last color. - return [ - { - color: colors[colors.length - 1], - label: this.formatField(dynamicRound(rangeFieldMeta.max)), - symbolId, - }, - ]; + + const breaks = []; + const lastStopIndex = colorStops.length - 2; + for (let i = 0; i < colorStops.length; i += 2) { + const hasNext = i < lastStopIndex; + const stopValue = colorStops[i]; + const formattedStopValue = this.formatField(dynamicRound(stopValue)); + const color = colorStops[i + 1] as string; + const percentile = parseFloat(percentilesFieldMeta[i / 2].percentile); + const percentileLabel = `${percentile}${getOrdinalSuffix(percentile)}`; + + let label = ''; + if (!hasNext) { + label = `${GREAT_THAN} ${percentileLabel}: ${formattedStopValue}`; + } else { + const nextStopValue = colorStops[i + 2]; + const formattedNextStopValue = this.formatField(dynamicRound(nextStopValue)); + const nextPercentile = parseFloat(percentilesFieldMeta[i / 2 + 1].percentile); + const nextPercentileLabel = `${nextPercentile}${getOrdinalSuffix(nextPercentile)}`; + + if (i === 0) { + label = `${UPTO} ${nextPercentileLabel}: ${formattedNextStopValue}`; + } else { + const begin = `${percentileLabel}: ${formattedStopValue}`; + const end = `${nextPercentileLabel}: ${formattedNextStopValue}`; + label = `${begin} ${UPTO} ${end}`; + } + } + + breaks.push({ + color, + label, + symbolId, + }); } - colorStops = getOrdinalMbColorRampStops( - this._options.color ? this._options.color : null, - rangeFieldMeta.min, - rangeFieldMeta.max - ); + return breaks; } - if (!colorStops || colorStops.length <= 2) { + if (!this._options.color) { return []; } - const breaks = []; - const lastStopIndex = colorStops.length - 2; - for (let i = 0; i < colorStops.length; i += 2) { - const hasNext = i < lastStopIndex; - const stopValue = colorStops[i]; - const formattedStopValue = this.formatField(dynamicRound(stopValue)); - const color = colorStops[i + 1] as string; - const valuePrefix = getValuePrefix ? getValuePrefix(i, false) : ''; - - let label = ''; - if (!hasNext) { - label = `>= ${valuePrefix}${formattedStopValue}`; - } else { - const nextStopValue = colorStops[i + 2]; - const formattedNextStopValue = this.formatField(dynamicRound(nextStopValue)); - const nextValuePrefix = getValuePrefix ? getValuePrefix(i, true) : ''; - - if (i === 0) { - label = `< ${nextValuePrefix}${formattedNextStopValue}`; - } else { - const begin = `${valuePrefix}${formattedStopValue}`; - const end = `${nextValuePrefix}${formattedNextStopValue}`; - label = `${begin} ${UP_TO} ${end}`; - } - } - - breaks.push({ - color, - label, - symbolId, - }); + const rangeFieldMeta = this.getRangeFieldMeta(); + if (!rangeFieldMeta) { + return []; } - return breaks; + + const colors = getColorPalette(this._options.color); + + if (rangeFieldMeta.delta === 0) { + // map to last color. + return [ + { + color: colors[colors.length - 1], + label: this.formatField(dynamicRound(rangeFieldMeta.max)), + symbolId, + }, + ]; + } + + return colors.map((color, index) => { + const rawStopValue = rangeFieldMeta.min + rangeFieldMeta.delta * (index / colors.length); + return { + color, + label: this.formatField(dynamicRound(rawStopValue)), + symbolId, + }; + }); } _getCategoricalBreaks(symbolId?: string): Break[] { diff --git a/x-pack/test/functional/apps/maps/joins.js b/x-pack/test/functional/apps/maps/joins.js index 9c769b8d9f59..0e2850dafbcc 100644 --- a/x-pack/test/functional/apps/maps/joins.js +++ b/x-pack/test/functional/apps/maps/joins.js @@ -69,14 +69,14 @@ export default function ({ getPageObjects, getService }) { expect(split[0]).to.equal('max prop1'); //bands 1-8 - expect(split[1]).to.equal('< 4.13'); - expect(split[2]).to.equal('4.13 up to 5.25'); - expect(split[3]).to.equal('5.25 up to 6.38'); - expect(split[4]).to.equal('6.38 up to 7.5'); - expect(split[5]).to.equal('7.5 up to 8.63'); - expect(split[6]).to.equal('8.63 up to 9.75'); - expect(split[7]).to.equal('9.75 up to 11'); - expect(split[8]).to.equal('>= 11'); + expect(split[1]).to.equal('3'); + expect(split[2]).to.equal('4.13'); + expect(split[3]).to.equal('5.25'); + expect(split[4]).to.equal('6.38'); + expect(split[5]).to.equal('7.5'); + expect(split[6]).to.equal('8.63'); + expect(split[7]).to.equal('9.75'); + expect(split[8]).to.equal('11'); }); it('should decorate feature properties with join property', async () => {