From 27f8825e06d3de75a81cc3bc98befce1d56da667 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 25 Feb 2020 15:31:39 -0500 Subject: [PATCH 01/25] remove ordinal scaling --- .../maps/public/layers/styles/color_utils.js | 8 ++++-- .../layers/styles/heatmap/heatmap_style.js | 2 +- .../properties/dynamic_color_property.js | 10 +++---- .../dynamic_orientation_property.js | 4 --- .../properties/dynamic_size_property.js | 24 +++++++++------- .../properties/dynamic_style_property.js | 11 ++------ .../properties/dynamic_text_property.js | 4 --- .../public/layers/styles/vector/style_util.js | 16 ----------- .../layers/styles/vector/style_util.test.js | 28 +------------------ 9 files changed, 29 insertions(+), 78 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js index fc305f8daed59..7f33481140ba8 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js @@ -64,13 +64,17 @@ export function getColorRampCenterColor(colorRampName) { // Returns an array of color stops // [ stop_input_1: number, stop_output_1: color, stop_input_n: number, stop_output_n: color ] -export function getOrdinalColorRampStops(colorRampName, numberColors = GRADIENT_INTERVALS) { +export function getOrdinalColorRampStops(colorRampName, min, max) { if (!colorRampName) { return null; } + + const numberColors = GRADIENT_INTERVALS; + const delta = max - min; return getHexColorRangeStrings(colorRampName, numberColors).reduce( (accu, stopColor, idx, srcArr) => { - const stopNumber = idx / srcArr.length; // number between 0 and 1, increasing as index increases + // const stopNumber = idx / srcArr.length; // number between 0 and 1, increasing as index increases + const stopNumber = delta > 0 ? min + (delta * idx) / srcArr.length : 1; return [...accu, stopNumber, stopColor]; }, [] diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js b/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js index 1dd219d4c4cad..4e7575f00cb91 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js @@ -81,7 +81,7 @@ export class HeatmapStyle extends AbstractStyle { const { colorRampName } = this._descriptor; if (colorRampName && colorRampName !== DEFAULT_HEATMAP_COLOR_RAMP_NAME) { - const colorStops = getOrdinalColorRampStops(colorRampName); + const colorStops = getOrdinalColorRampStops(colorRampName, 0, 1); mbMap.setPaintProperty(layerId, 'heatmap-color', [ 'interpolate', ['linear'], diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index 70e905907bc79..209452d240b78 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -88,10 +88,6 @@ export class DynamicColorProperty extends DynamicStyleProperty { return true; } - isOrdinalScaled() { - return this.isOrdinal() && !this.isCustomOrdinalColorRamp(); - } - isOrdinalRanged() { return this.isOrdinal() && !this.isCustomOrdinalColorRamp(); } @@ -232,7 +228,11 @@ export class DynamicColorProperty extends DynamicStyleProperty { return [...accumulatedStops, nextStop.stop, nextStop.color]; }, []); } else { - return getOrdinalColorRampStops(this._options.color); + const rangeFieldMeta = this.getFieldMeta(); + if (!rangeFieldMeta) { + return null; + } + return getOrdinalColorRampStops(this._options.color, rangeFieldMeta.min, rangeFieldMeta.max); } } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_orientation_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_orientation_property.js index 1d2457142c082..5b00709d51a7b 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_orientation_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_orientation_property.js @@ -25,8 +25,4 @@ export class DynamicOrientationProperty extends DynamicStyleProperty { supportsFeatureState() { return false; } - - isOrdinalScaled() { - return false; - } } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js index e137e15730827..45e2a0b534e99 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js @@ -121,25 +121,28 @@ export class DynamicSizeProperty extends DynamicStyleProperty { } getMbSizeExpression() { - if (this._isSizeDynamicConfigComplete(this._options)) { - return this._getMbDataDrivenSize({ - targetName: this.getComputedFieldName(), - minSize: this._options.minSize, - maxSize: this._options.maxSize, - }); + if (!this._isSizeDynamicConfigComplete(this._options)) { + return null; } - return null; + + return this._getMbDataDrivenSize({ + targetName: this.getComputedFieldName(), + minSize: this._options.minSize, + maxSize: this._options.maxSize, + }); } _getMbDataDrivenSize({ targetName, minSize, maxSize }) { + const rangeFieldMeta = this.getFieldMeta(); + const lookup = this.supportsFeatureState() ? 'feature-state' : 'get'; return [ 'interpolate', ['linear'], ['coalesce', [lookup, targetName], 0], - 0, + rangeFieldMeta.min, minSize, - 1, + rangeFieldMeta.max, maxSize, ]; } @@ -149,7 +152,8 @@ export class DynamicSizeProperty extends DynamicStyleProperty { this._field && this._field.isValid() && _.has(this._options, 'minSize') && - _.has(this._options, 'maxSize') + _.has(this._options, 'maxSize') && + this.getFieldMeta() ); } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js index ef19e9b23b10d..893a55a40fa5c 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js @@ -8,7 +8,7 @@ import _ from 'lodash'; import { AbstractStyleProperty } from './style_property'; import { DEFAULT_SIGMA } from '../vector_style_defaults'; import { COLOR_PALETTE_MAX_SIZE, STYLE_TYPE } from '../../../../../common/constants'; -import { scaleValue, getComputedFieldName } from '../style_util'; +import { getComputedFieldName } from '../style_util'; import React from 'react'; import { OrdinalLegend } from './components/ordinal_legend'; import { CategoricalLegend } from './components/categorical_legend'; @@ -84,7 +84,7 @@ export class DynamicStyleProperty extends AbstractStyleProperty { supportsFieldMeta() { if (this.isOrdinal()) { - return this.isComplete() && this.isOrdinalScaled() && this._field.supportsFieldMeta(); + return this.isComplete() && this._field.supportsFieldMeta(); } else if (this.isCategorical()) { return this.isComplete() && this._field.supportsFieldMeta(); } else { @@ -109,10 +109,6 @@ export class DynamicStyleProperty extends AbstractStyleProperty { return true; } - isOrdinalScaled() { - return true; - } - getFieldMetaOptions() { return _.get(this.getOptions(), 'fieldMetaOptions', {}); } @@ -245,9 +241,6 @@ export class DynamicStyleProperty extends AbstractStyleProperty { } const valueAsFloat = parseFloat(value); - if (this.isOrdinalScaled()) { - return scaleValue(valueAsFloat, this.getFieldMeta()); - } if (isNaN(valueAsFloat)) { return 0; } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_text_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_text_property.js index 6a40a80a1a7a6..152b0d196dd52 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_text_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_text_property.js @@ -28,8 +28,4 @@ export class DynamicTextProperty extends DynamicStyleProperty { supportsFeatureState() { return false; } - - isOrdinalScaled() { - return false; - } } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js index 2859b8c0e5a56..1c5573e9d5c67 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js @@ -34,22 +34,6 @@ export function isOnlySingleFeatureType(featureType, supportedFeatures, hasFeatu }, true); } -export function scaleValue(value, range) { - if (isNaN(value) || !range) { - return -1; //Nothing to scale, put outside scaled range - } - - if (range.delta === 0 || value >= range.max) { - return 1; //snap to end of scaled range - } - - if (value <= range.min) { - return 0; //snap to beginning of scaled range - } - - return (value - range.min) / range.delta; -} - export function assignCategoriesToPalette({ categories, paletteValues }) { const stops = []; let fallback = null; diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.test.js index 2be31c0107193..76bbfc84e3892 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.test.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { isOnlySingleFeatureType, scaleValue, assignCategoriesToPalette } from './style_util'; +import { isOnlySingleFeatureType, assignCategoriesToPalette } from './style_util'; import { VECTOR_SHAPE_TYPES } from '../../sources/vector_feature_types'; describe('isOnlySingleFeatureType', () => { @@ -62,32 +62,6 @@ describe('isOnlySingleFeatureType', () => { }); }); -describe('scaleValue', () => { - test('Should scale value between 0 and 1', () => { - expect(scaleValue(5, { min: 0, max: 10, delta: 10 })).toBe(0.5); - }); - - test('Should snap value less then range min to 0', () => { - expect(scaleValue(-1, { min: 0, max: 10, delta: 10 })).toBe(0); - }); - - test('Should snap value greater then range max to 1', () => { - expect(scaleValue(11, { min: 0, max: 10, delta: 10 })).toBe(1); - }); - - test('Should snap value to 1 when tere is not range delta', () => { - expect(scaleValue(10, { min: 10, max: 10, delta: 0 })).toBe(1); - }); - - test('Should put value as -1 when value is not provided', () => { - expect(scaleValue(undefined, { min: 0, max: 10, delta: 10 })).toBe(-1); - }); - - test('Should put value as -1 when range is not provided', () => { - expect(scaleValue(5, undefined)).toBe(-1); - }); -}); - describe('assignCategoriesToPalette', () => { test('Categories and palette values have same length', () => { const categories = [{ key: 'alpah' }, { key: 'bravo' }, { key: 'charlie' }, { key: 'delta' }]; From 55785712a3813359c4618b1463ecac296d0203a1 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 25 Feb 2020 16:49:07 -0500 Subject: [PATCH 02/25] fix unit test --- .../plugins/maps/public/layers/styles/color_utils.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js index 5a8289ba903f3..1a56eb9428a4e 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js @@ -61,7 +61,7 @@ describe('getColorRampCenterColor', () => { describe('getColorRampStops', () => { it('Should create color stops for color ramp', () => { - expect(getOrdinalColorRampStops('Blues')).toEqual([ + expect(getOrdinalColorRampStops('Blues'), 0, 1).toEqual([ 0, '#f7faff', 0.125, From acbe878e9911289c7863fe04177bb50fa18f7548 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 25 Feb 2020 16:50:55 -0500 Subject: [PATCH 03/25] fix --- .../plugins/maps/public/layers/styles/color_utils.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js index 1a56eb9428a4e..d59d36973f446 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js @@ -61,7 +61,7 @@ describe('getColorRampCenterColor', () => { describe('getColorRampStops', () => { it('Should create color stops for color ramp', () => { - expect(getOrdinalColorRampStops('Blues'), 0, 1).toEqual([ + expect(getOrdinalColorRampStops('Blues', 0, 1)).toEqual([ 0, '#f7faff', 0.125, From df2f28f64d2b486b591fe581888bc9c64a71fd4d Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 25 Feb 2020 23:55:42 -0500 Subject: [PATCH 04/25] snap to end of range --- .../maps/public/layers/styles/color_utils.js | 24 +++++++++++------- .../public/layers/styles/color_utils.test.js | 25 +++++++++++++++++++ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js index 7f33481140ba8..4eb2bb767ff19 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js @@ -65,20 +65,26 @@ export function getColorRampCenterColor(colorRampName) { // Returns an array of color stops // [ stop_input_1: number, stop_output_1: color, stop_input_n: number, stop_output_n: color ] export function getOrdinalColorRampStops(colorRampName, min, max) { + console.log('mi', min, max); if (!colorRampName) { return null; } - const numberColors = GRADIENT_INTERVALS; + if (min > max) { + return null; + } + + const hexColors = getHexColorRangeStrings(colorRampName, GRADIENT_INTERVALS); + if (max === min) { + //just return single stop value + return [max, hexColors[hexColors.length - 1]]; + } + const delta = max - min; - return getHexColorRangeStrings(colorRampName, numberColors).reduce( - (accu, stopColor, idx, srcArr) => { - // const stopNumber = idx / srcArr.length; // number between 0 and 1, increasing as index increases - const stopNumber = delta > 0 ? min + (delta * idx) / srcArr.length : 1; - return [...accu, stopNumber, stopColor]; - }, - [] - ); + return hexColors.reduce((accu, stopColor, idx, srcArr) => { + const stopNumber = min + (delta * idx) / srcArr.length; + return [...accu, stopNumber, stopColor]; + }, []); } export const COLOR_GRADIENTS = Object.keys(vislibColorMaps).map(colorRampName => ({ diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js index d59d36973f446..26b12bc0bcc6d 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js @@ -80,6 +80,31 @@ describe('getColorRampStops', () => { '#072f6b', ]); }); + + it('Should create color stops for custom range', () => { + expect(getOrdinalColorRampStops('Blues', 0, 1000)).toEqual([ + 0, + '#f7faff', + 125, + '#ddeaf7', + 250, + '#c5daee', + 375, + '#9dc9e0', + 500, + '#6aadd5', + 625, + '#4191c5', + 750, + '#2070b4', + 875, + '#072f6b', + ]); + }); + + it('Should snap to end of color stops for identical range', () => { + expect(getOrdinalColorRampStops('Blues', 23, 23)).toEqual([23, '#072f6b']); + }); }); describe('getLinearGradient', () => { From 8b193db8d6cbfbddafd19ecc9667dc7f6a899e20 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 25 Feb 2020 23:56:47 -0500 Subject: [PATCH 05/25] remove cruft --- x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js index 4eb2bb767ff19..29bec20bcaa1e 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.js @@ -65,7 +65,6 @@ export function getColorRampCenterColor(colorRampName) { // Returns an array of color stops // [ stop_input_1: number, stop_output_1: color, stop_input_n: number, stop_output_n: color ] export function getOrdinalColorRampStops(colorRampName, min, max) { - console.log('mi', min, max); if (!colorRampName) { return null; } From 86de74e089fbcf54e818440b67402ad4d94c03f6 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 26 Feb 2020 13:38:21 -0500 Subject: [PATCH 06/25] tmp commit --- .../properties/dynamic_color_property.js | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index 209452d240b78..577f6aa9e5f4c 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -133,15 +133,29 @@ export class DynamicColorProperty extends DynamicStyleProperty { 'rgba(0,0,0,0)', // MB will assign the base value to any features that is below the first stop value ...colorStops, ]; + } else { + const fieldMeta = this.getFieldMeta(); + if (!fieldMeta) { + return null; + } + + return [ + 'interpolate', + ['linear'], + [ + 'coalesce', + [ + 'max', + ['min', ['to-number', ['feature-state', targetName]], fieldMeta.max], + fieldMeta.min, + ], + -1, + ], + -1, + 'rgba(0,0,0,0)', + ...colorStops, + ]; } - return [ - 'interpolate', - ['linear'], - ['coalesce', ['feature-state', targetName], -1], - -1, - 'rgba(0,0,0,0)', - ...colorStops, - ]; } _getColorPaletteStops() { From b80e3c3aff445d9309716bb21dfd307fa7d4422a Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 26 Feb 2020 14:03:28 -0500 Subject: [PATCH 07/25] coerce to number and snap value --- .../properties/dynamic_size_property.js | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js index a9215eda910f0..4778c39cd19ee 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js @@ -75,16 +75,26 @@ export class DynamicSizeProperty extends DynamicStyleProperty { syncIconSizeWithMb(symbolLayerId, mbMap) { if (this._isSizeDynamicConfigComplete(this._options)) { + const rangeFieldMeta = this.getFieldMeta(); + const halfIconPixels = this.getIconPixelSize() / 2; const targetName = this.getComputedFieldName(); // Using property state instead of feature-state because layout properties do not support feature-state mbMap.setLayoutProperty(symbolLayerId, 'icon-size', [ 'interpolate', ['linear'], - ['coalesce', ['get', targetName], 0], - 0, + [ + 'coalesce', + [ + 'max', + ['min', ['to-number', ['get', targetName]], rangeFieldMeta.max], + rangeFieldMeta.min, + ], + 0, + ], + rangeFieldMeta.min, this._options.minSize / halfIconPixels, - 1, + rangeFieldMeta.max, this._options.maxSize / halfIconPixels, ]); } else { @@ -131,7 +141,15 @@ export class DynamicSizeProperty extends DynamicStyleProperty { return [ 'interpolate', ['linear'], - ['coalesce', [lookup, targetName], 0], + [ + 'coalesce', + [ + 'max', + ['min', ['to-number', [lookup, targetName]], rangeFieldMeta.max], + rangeFieldMeta.min, + ], + 0, + ], rangeFieldMeta.min, minSize, rangeFieldMeta.max, From 78c92a693e1979a337fda2939454967b89faf08f Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 26 Feb 2020 14:53:20 -0500 Subject: [PATCH 08/25] reduce dupe --- .../public/layers/styles/color_utils.test.js | 21 -------- .../properties/dynamic_color_property.js | 34 ++++++------ .../properties/dynamic_size_property.js | 54 +++++++++---------- .../public/layers/styles/vector/style_util.js | 14 +++++ 4 files changed, 58 insertions(+), 65 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js index 26b12bc0bcc6d..9a5ece01d5206 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/color_utils.test.js @@ -60,27 +60,6 @@ describe('getColorRampCenterColor', () => { }); describe('getColorRampStops', () => { - it('Should create color stops for color ramp', () => { - expect(getOrdinalColorRampStops('Blues', 0, 1)).toEqual([ - 0, - '#f7faff', - 0.125, - '#ddeaf7', - 0.25, - '#c5daee', - 0.375, - '#9dc9e0', - 0.5, - '#6aadd5', - 0.625, - '#4191c5', - 0.75, - '#2070b4', - 0.875, - '#072f6b', - ]); - }); - it('Should create color stops for custom range', () => { expect(getOrdinalColorRampStops('Blues', 0, 1000)).toEqual([ 0, diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index 577f6aa9e5f4c..493d8ccf3206e 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -6,7 +6,11 @@ import { DynamicStyleProperty } from './dynamic_style_property'; import _ from 'lodash'; -import { getComputedFieldName, getOtherCategoryLabel } from '../style_util'; +import { + getComputedFieldName, + getOtherCategoryLabel, + makeMbClampedNumberExpression, +} from '../style_util'; import { getOrdinalColorRampStops, getColorPalette } from '../../color_utils'; import { ColorGradient } from '../../components/color_gradient'; import React from 'react'; @@ -23,6 +27,7 @@ import { COLOR_MAP_TYPE } from '../../../../../common/constants'; import { isCategoricalStopsInvalid } from '../components/color/color_stops_utils'; const EMPTY_STOPS = { stops: [], defaultColor: null }; +const RGBA_0000 = 'rgba(0,0,0,0)'; export class DynamicColorProperty extends DynamicStyleProperty { syncCircleColorWithMb(mbLayerId, mbMap, alpha) { @@ -130,29 +135,28 @@ export class DynamicColorProperty extends DynamicStyleProperty { return [ 'step', ['coalesce', ['feature-state', targetName], lessThenFirstStopValue], - 'rgba(0,0,0,0)', // MB will assign the base value to any features that is below the first stop value + RGBA_0000, // MB will assign the base value to any features that is below the first stop value ...colorStops, ]; } else { - const fieldMeta = this.getFieldMeta(); - if (!fieldMeta) { + const rangeFieldMeta = this.getFieldMeta(); + if (!rangeFieldMeta) { return null; } + const lessThenFirstStopValue = rangeFieldMeta.min - 1; return [ 'interpolate', ['linear'], - [ - 'coalesce', - [ - 'max', - ['min', ['to-number', ['feature-state', targetName]], fieldMeta.max], - fieldMeta.min, - ], - -1, - ], - -1, - 'rgba(0,0,0,0)', + makeMbClampedNumberExpression({ + minValue: rangeFieldMeta.min, + maxValue: rangeFieldMeta.max, + lookupFunction: 'feature-state', + fallback: lessThenFirstStopValue, + fieldName: targetName, + }), + lessThenFirstStopValue, + RGBA_0000, ...colorStops, ]; } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js index 4778c39cd19ee..e0cfd648aec43 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js @@ -5,6 +5,7 @@ */ import { DynamicStyleProperty } from './dynamic_style_property'; +import { makeMbClampedNumberExpression } from '../style_util'; import { HALF_LARGE_MAKI_ICON_SIZE, @@ -74,24 +75,21 @@ export class DynamicSizeProperty extends DynamicStyleProperty { } syncIconSizeWithMb(symbolLayerId, mbMap) { - if (this._isSizeDynamicConfigComplete(this._options)) { - const rangeFieldMeta = this.getFieldMeta(); - + const rangeFieldMeta = this.getFieldMeta(); + if (this._isSizeDynamicConfigComplete(this._options) && rangeFieldMeta) { const halfIconPixels = this.getIconPixelSize() / 2; const targetName = this.getComputedFieldName(); // Using property state instead of feature-state because layout properties do not support feature-state mbMap.setLayoutProperty(symbolLayerId, 'icon-size', [ 'interpolate', ['linear'], - [ - 'coalesce', - [ - 'max', - ['min', ['to-number', ['get', targetName]], rangeFieldMeta.max], - rangeFieldMeta.min, - ], - 0, - ], + makeMbClampedNumberExpression({ + minValue: rangeFieldMeta.min, + maxValue: rangeFieldMeta.max, + fallback: 0, + lookupFunction: 'get', + fieldName: targetName, + }), rangeFieldMeta.min, this._options.minSize / halfIconPixels, rangeFieldMeta.max, @@ -123,7 +121,8 @@ export class DynamicSizeProperty extends DynamicStyleProperty { } getMbSizeExpression() { - if (!this._isSizeDynamicConfigComplete(this._options)) { + const rangeFieldMeta = this.getFieldMeta(); + if (!this._isSizeDynamicConfigComplete(this._options) || !rangeFieldMeta) { return null; } @@ -131,28 +130,26 @@ export class DynamicSizeProperty extends DynamicStyleProperty { targetName: this.getComputedFieldName(), minSize: this._options.minSize, maxSize: this._options.maxSize, + minValue: rangeFieldMeta.min, + maxValue: rangeFieldMeta.max, }); } - _getMbDataDrivenSize({ targetName, minSize, maxSize }) { - const rangeFieldMeta = this.getFieldMeta(); - + _getMbDataDrivenSize({ targetName, minSize, maxSize, minValue, maxValue }) { const lookup = this.supportsFeatureState() ? 'feature-state' : 'get'; return [ 'interpolate', ['linear'], - [ - 'coalesce', - [ - 'max', - ['min', ['to-number', [lookup, targetName]], rangeFieldMeta.max], - rangeFieldMeta.min, - ], - 0, - ], - rangeFieldMeta.min, + makeMbClampedNumberExpression({ + lookupFunction: lookup, + maxValue, + minValue, + fieldName: targetName, + fallback: 0, + }), + minValue, minSize, - rangeFieldMeta.max, + maxValue, maxSize, ]; } @@ -162,8 +159,7 @@ export class DynamicSizeProperty extends DynamicStyleProperty { this._field && this._field.isValid() && _.has(this._options, 'minSize') && - _.has(this._options, 'maxSize') && - this.getFieldMeta() + _.has(this._options, 'maxSize') ); } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js index 1c5573e9d5c67..684cea37638c1 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js @@ -54,3 +54,17 @@ export function assignCategoriesToPalette({ categories, paletteValues }) { fallback, }; } + +export function makeMbClampedNumberExpression({ + lookupFunction, + fieldName, + minValue, + maxValue, + fallback, +}) { + return [ + 'coalesce', + ['max', ['min', ['to-number', [lookupFunction, fieldName]], maxValue], minValue], + fallback, + ]; +} From bceda65dcd11c4914e1820d73c4b0056db0796a3 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 5 Mar 2020 17:16:14 -0500 Subject: [PATCH 09/25] fixx merge --- .../layers/styles/vector/properties/dynamic_color_property.js | 4 ++-- .../layers/styles/vector/properties/dynamic_size_property.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index 249dbec9d1784..b781f4e067b62 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -139,7 +139,7 @@ export class DynamicColorProperty extends DynamicStyleProperty { ...colorStops, ]; } else { - const rangeFieldMeta = this.getFieldMeta(); + const rangeFieldMeta = this.getRangeFieldMeta(); if (!rangeFieldMeta) { return null; } @@ -246,7 +246,7 @@ export class DynamicColorProperty extends DynamicStyleProperty { return [...accumulatedStops, nextStop.stop, nextStop.color]; }, []); } else { - const rangeFieldMeta = this.getFieldMeta(); + const rangeFieldMeta = this.getRangeFieldMeta(); if (!rangeFieldMeta) { return null; } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js index cc0cf2175d3ae..533e8766e6613 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js @@ -74,7 +74,7 @@ export class DynamicSizeProperty extends DynamicStyleProperty { } syncIconSizeWithMb(symbolLayerId, mbMap) { - const rangeFieldMeta = this.getFieldMeta(); + const rangeFieldMeta = this.getRangeFieldMeta(); if (this._isSizeDynamicConfigComplete(this._options) && rangeFieldMeta) { const halfIconPixels = this.getIconPixelSize() / 2; const targetName = this.getComputedFieldName(); @@ -120,7 +120,7 @@ export class DynamicSizeProperty extends DynamicStyleProperty { } getMbSizeExpression() { - const rangeFieldMeta = this.getFieldMeta(); + const rangeFieldMeta = this.getRangeFieldMeta(); if (!this._isSizeDynamicConfigComplete(this._options) || !rangeFieldMeta) { return null; } From 14e5fd4e573ec69cbdad211cc4da40b5cdbab63a Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 5 Mar 2020 17:45:30 -0500 Subject: [PATCH 10/25] fix --- .../styles/vector/properties/dynamic_style_property.js | 7 ++----- .../maps/public/layers/styles/vector/style_util.js | 8 +++++++- .../maps/public/layers/styles/vector/vector_style.js | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js index 0069a59699c0b..bf3a85887ef18 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js @@ -286,16 +286,13 @@ export class DynamicStyleProperty extends AbstractStyleProperty { } } - getMbValue(value) { + getMbFeatureStateValue(value) { if (!this.isOrdinal()) { return this.formatField(value); } const valueAsFloat = parseFloat(value); - if (isNaN(valueAsFloat)) { - return 0; - } - return valueAsFloat; + return isNaN(valueAsFloat) ? null : valueAsFloat; } renderBreakedLegend() { diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js index 684cea37638c1..0820568468439 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/style_util.js @@ -62,9 +62,15 @@ export function makeMbClampedNumberExpression({ maxValue, fallback, }) { + const clamp = ['max', ['min', ['to-number', [lookupFunction, fieldName]], maxValue], minValue]; return [ 'coalesce', - ['max', ['min', ['to-number', [lookupFunction, fieldName]], maxValue], minValue], + [ + 'case', + ['==', [lookupFunction, fieldName], null], + minValue - 1, //== does a JS-y like check where returns true for null and undefined + clamp, + ], fallback, ]; } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js index 528c5a9bfdc85..ddb2803d94e5f 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js @@ -507,7 +507,7 @@ export class VectorStyle extends AbstractStyle { const dynamicStyleProp = dynamicStyleProps[j]; const name = dynamicStyleProp.getField().getName(); const computedName = getComputedFieldName(dynamicStyleProp.getStyleName(), name); - const styleValue = dynamicStyleProp.getMbValue(feature.properties[name]); + const styleValue = dynamicStyleProp.getMbFeatureStateValue(feature.properties[name]); if (dynamicStyleProp.supportsFeatureState()) { tmpFeatureState[computedName] = styleValue; } else { From b08e9b0b60393c3a3a74f272764a9f94dbfe7e65 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 5 Mar 2020 18:13:06 -0500 Subject: [PATCH 11/25] add unit test --- .../properties/dynamic_color_property.js | 5 +- .../properties/dynamic_color_property.test.js | 58 +++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index b781f4e067b62..573158eb15086 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -102,13 +102,12 @@ export class DynamicColorProperty extends DynamicStyleProperty { } _getMbColor() { - const isDynamicConfigComplete = - _.has(this._options, 'field.name') && _.has(this._options, 'color'); + const isDynamicConfigComplete = this.getFieldName() && _.has(this._options, 'color'); if (!isDynamicConfigComplete) { return null; } - const targetName = getComputedFieldName(this._styleName, this._options.field.name); + const targetName = getComputedFieldName(this._styleName, this.getFieldName()); if (this.isCategorical()) { return this._getMbDataDrivenCategoricalColor({ targetName }); } else { diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js index c2f7a1313d02a..937a14e3a16fe 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js @@ -236,3 +236,61 @@ test('Should pluck the categorical style-meta from fieldmeta', async () => { ], }); }); + +test('Should set the correct mapbox style', async () => { + const colorStyle = makeProperty({ + color: 'Blues', + type: undefined, + }); + + const calls = []; + + const mockMbMap = { + setPaintProperty(...args) { + calls.push(args); + }, + }; + + colorStyle.syncCircleColorWithMb('foobarid', mockMbMap, 1); + expect(calls[0]).toEqual([ + 'foobarid', + 'circle-color', + [ + 'interpolate', + ['linear'], + [ + 'coalesce', + [ + 'case', + ['==', ['feature-state', '__kbn__dynamic__foobar__lineColor'], null], + -1, + [ + 'max', + ['min', ['to-number', ['feature-state', '__kbn__dynamic__foobar__lineColor']], 100], + 0, + ], + ], + -1, + ], + -1, + 'rgba(0,0,0,0)', + 0, + '#f7faff', + 12.5, + '#ddeaf7', + 25, + '#c5daee', + 37.5, + '#9dc9e0', + 50, + '#6aadd5', + 62.5, + '#4191c5', + 75, + '#2070b4', + 87.5, + '#072f6b', + ], + ]); + expect(calls[1]).toEqual(['foobarid', 'circle-opacity', 1]); +}); From 52d3b8e25ac2dfac908fd62bac1eca012492f0e8 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 12 Mar 2020 11:06:13 -0400 Subject: [PATCH 12/25] fix functional test --- .../functional/apps/maps/mapbox_styles.js | 107 ++++++++++++------ 1 file changed, 75 insertions(+), 32 deletions(-) diff --git a/x-pack/test/functional/apps/maps/mapbox_styles.js b/x-pack/test/functional/apps/maps/mapbox_styles.js index d3280bae8582e..6378c9d4e5d60 100644 --- a/x-pack/test/functional/apps/maps/mapbox_styles.js +++ b/x-pack/test/functional/apps/maps/mapbox_styles.js @@ -16,9 +16,7 @@ export const MAPBOX_STYLES = { ['==', ['get', '__kbn_isvisibleduetojoin__'], true], ['any', ['==', ['geometry-type'], 'Point'], ['==', ['geometry-type'], 'MultiPoint']], ], - layout: { - visibility: 'visible', - }, + layout: { visibility: 'visible' }, paint: { 'circle-color': [ 'interpolate', @@ -26,32 +24,56 @@ export const MAPBOX_STYLES = { [ 'coalesce', [ - 'feature-state', - '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', + 'case', + [ + '==', + [ + 'feature-state', + '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', + ], + null, + ], + 2, + [ + 'max', + [ + 'min', + [ + 'to-number', + [ + 'feature-state', + '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', + ], + ], + 12, + ], + 3, + ], ], - -1, + 2, ], - -1, + 2, 'rgba(0,0,0,0)', - 0, + 3, '#f7faff', - 0.125, + 4.125, '#ddeaf7', - 0.25, + 5.25, '#c5daee', - 0.375, + 6.375, '#9dc9e0', - 0.5, + 7.5, '#6aadd5', - 0.625, + 8.625, '#4191c5', - 0.75, + 9.75, '#2070b4', - 0.875, + 10.875, '#072f6b', ], - 'circle-opacity': 0.75, // Obtained dynamically - /* 'circle-stroke-color': '' */ 'circle-stroke-opacity': 0.75, + 'circle-opacity': 0.75, + 'circle-stroke-color': '#41937c', + 'circle-stroke-opacity': 0.75, 'circle-stroke-width': 1, 'circle-radius': 10, }, @@ -67,9 +89,7 @@ export const MAPBOX_STYLES = { ['==', ['get', '__kbn_isvisibleduetojoin__'], true], ['any', ['==', ['geometry-type'], 'Polygon'], ['==', ['geometry-type'], 'MultiPolygon']], ], - layout: { - visibility: 'visible', - }, + layout: { visibility: 'visible' }, paint: { 'fill-color': [ 'interpolate', @@ -77,28 +97,51 @@ export const MAPBOX_STYLES = { [ 'coalesce', [ - 'feature-state', - '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', + 'case', + [ + '==', + [ + 'feature-state', + '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', + ], + null, + ], + 2, + [ + 'max', + [ + 'min', + [ + 'to-number', + [ + 'feature-state', + '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', + ], + ], + 12, + ], + 3, + ], ], - -1, + 2, ], - -1, + 2, 'rgba(0,0,0,0)', - 0, + 3, '#f7faff', - 0.125, + 4.125, '#ddeaf7', - 0.25, + 5.25, '#c5daee', - 0.375, + 6.375, '#9dc9e0', - 0.5, + 7.5, '#6aadd5', - 0.625, + 8.625, '#4191c5', - 0.75, + 9.75, '#2070b4', - 0.875, + 10.875, '#072f6b', ], 'fill-opacity': 0.75, From 2479cbe2df062610a8031f580bd9f583a29e57d7 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Mon, 16 Mar 2020 22:12:23 -0400 Subject: [PATCH 13/25] feedback --- .../maps/public/layers/styles/heatmap/heatmap_style.js | 5 ++++- .../styles/vector/properties/dynamic_color_property.js | 10 +++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js b/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js index 933d5f3964a87..fde6e1dbfbbc4 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js @@ -14,6 +14,9 @@ import { getOrdinalColorRampStops } from '../color_utils'; import { i18n } from '@kbn/i18n'; import { EuiIcon } from '@elastic/eui'; +const MIN_RANGE = 0; +const MAX_RANGE = 1; + export class HeatmapStyle extends AbstractStyle { static type = LAYER_STYLE_TYPE.HEATMAP; @@ -80,7 +83,7 @@ export class HeatmapStyle extends AbstractStyle { const { colorRampName } = this._descriptor; if (colorRampName && colorRampName !== DEFAULT_HEATMAP_COLOR_RAMP_NAME) { - const colorStops = getOrdinalColorRampStops(colorRampName, 0, 1); + const colorStops = getOrdinalColorRampStops(colorRampName, MIN_RANGE, MAX_RANGE); mbMap.setPaintProperty(layerId, 'heatmap-color', [ 'interpolate', ['linear'], diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index 0e7c99ecb932b..1e23da201167e 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -122,10 +122,10 @@ export class DynamicColorProperty extends DynamicStyleProperty { return [...accumulatedStops, nextStop.stop, nextStop.color]; }, []); const firstStopValue = colorStops[0]; - const lessThenFirstStopValue = firstStopValue - 1; + const lessThanFirstStopValue = firstStopValue - 1; return [ 'step', - ['coalesce', ['feature-state', targetName], lessThenFirstStopValue], + ['coalesce', ['feature-state', targetName], lessThanFirstStopValue], RGBA_0000, // MB will assign the base value to any features that is below the first stop value ...colorStops, ]; @@ -144,7 +144,7 @@ export class DynamicColorProperty extends DynamicStyleProperty { return null; } - const lessThenFirstStopValue = rangeFieldMeta.min - 1; + const lessThanFirstStopValue = rangeFieldMeta.min - 1; return [ 'interpolate', ['linear'], @@ -152,10 +152,10 @@ export class DynamicColorProperty extends DynamicStyleProperty { minValue: rangeFieldMeta.min, maxValue: rangeFieldMeta.max, lookupFunction: 'feature-state', - fallback: lessThenFirstStopValue, + fallback: lessThanFirstStopValue, fieldName: targetName, }), - lessThenFirstStopValue, + lessThanFirstStopValue, RGBA_0000, ...colorStops, ]; From d824d116465b5cddd187302eee3d934a863bcf6c Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Mon, 16 Mar 2020 22:29:50 -0400 Subject: [PATCH 14/25] improve docs --- x-pack/legacy/plugins/maps/public/layers/heatmap_layer.js | 2 +- .../plugins/maps/public/layers/styles/heatmap/heatmap_style.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/heatmap_layer.js b/x-pack/legacy/plugins/maps/public/layers/heatmap_layer.js index 29223d6a67c6b..541481bc6b0ec 100644 --- a/x-pack/legacy/plugins/maps/public/layers/heatmap_layer.js +++ b/x-pack/legacy/plugins/maps/public/layers/heatmap_layer.js @@ -72,7 +72,7 @@ export class HeatmapLayer extends VectorLayer { const propertyKey = this._getPropKeyOfSelectedMetric(); const dataBoundToMap = AbstractLayer.getBoundDataForSource(mbMap, this.getId()); if (featureCollection !== dataBoundToMap) { - let max = 0; + let max = 1; //max will be at least one, since counts or sums will be at least one. for (let i = 0; i < featureCollection.features.length; i++) { max = Math.max(featureCollection.features[i].properties[propertyKey], max); } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js b/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js index fde6e1dbfbbc4..d769fe0da9ec2 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/heatmap/heatmap_style.js @@ -14,6 +14,8 @@ import { getOrdinalColorRampStops } from '../color_utils'; import { i18n } from '@kbn/i18n'; import { EuiIcon } from '@elastic/eui'; +//The heatmap range chosen hear runs from 0 to 1. It is arbitrary. +//Weighting is on the raw count/sum values. const MIN_RANGE = 0; const MAX_RANGE = 1; From 8c3ea164db8288109fbe848e545edac077a0defd Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 17 Mar 2020 17:01:44 -0400 Subject: [PATCH 15/25] remove duplicated feature-state system properties --- .../properties/dynamic_color_property.js | 8 ++--- .../properties/dynamic_icon_property.js | 4 +-- .../properties/dynamic_size_property.js | 4 +-- .../properties/dynamic_style_property.d.ts | 2 -- .../properties/dynamic_style_property.js | 13 ++++---- .../properties/label_border_size_property.js | 30 +++++++++++-------- .../layers/styles/vector/vector_style.js | 9 ++++-- 7 files changed, 36 insertions(+), 34 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index 1e23da201167e..5498a799e3832 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -5,11 +5,7 @@ */ import { DynamicStyleProperty } from './dynamic_style_property'; -import { - getComputedFieldName, - getOtherCategoryLabel, - makeMbClampedNumberExpression, -} from '../style_util'; +import { getOtherCategoryLabel, makeMbClampedNumberExpression } from '../style_util'; import { getOrdinalColorRampStops, getColorPalette } from '../../color_utils'; import { ColorGradient } from '../../components/color_gradient'; import React from 'react'; @@ -111,7 +107,7 @@ export class DynamicColorProperty extends DynamicStyleProperty { } _getOrdinalColorMbExpression() { - const targetName = getComputedFieldName(this._styleName, this._field.getName()); + const targetName = this._field.getName(); if (this._options.useCustomColorRamp) { if (!this._options.customColorRamp || !this._options.customColorRamp.length) { // custom color ramp config is not complete diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_icon_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_icon_property.js index c492efbdf4ba3..05e2ad06842ce 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_icon_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_icon_property.js @@ -81,7 +81,7 @@ export class DynamicIconProperty extends DynamicStyleProperty { mbStops.push(getMakiIconId(style, iconPixelSize)); }); mbStops.push(getMakiIconId(fallback, iconPixelSize)); //last item is fallback style for anything that does not match provided stops - return ['match', ['to-string', ['get', this._options.field.name]], ...mbStops]; + return ['match', ['to-string', ['get', this._field.getName()]], ...mbStops]; } _getMbIconAnchorExpression() { @@ -98,7 +98,7 @@ export class DynamicIconProperty extends DynamicStyleProperty { mbStops.push(getMakiSymbolAnchor(style)); }); mbStops.push(getMakiSymbolAnchor(fallback)); //last item is fallback style for anything that does not match provided stops - return ['match', ['to-string', ['get', this._options.field.name]], ...mbStops]; + return ['match', ['to-string', ['get', this._field.getName()]], ...mbStops]; } _isIconDynamicConfigComplete() { diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js index d651549a84a4a..d3a87587c8d77 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js @@ -77,7 +77,7 @@ export class DynamicSizeProperty extends DynamicStyleProperty { const rangeFieldMeta = this.getRangeFieldMeta(); if (this._isSizeDynamicConfigComplete(this._options) && rangeFieldMeta) { const halfIconPixels = this.getIconPixelSize() / 2; - const targetName = this.getComputedFieldName(); + const targetName = this.getCompletedFieldName(); // Using property state instead of feature-state because layout properties do not support feature-state mbMap.setLayoutProperty(symbolLayerId, 'icon-size', [ 'interpolate', @@ -126,7 +126,7 @@ export class DynamicSizeProperty extends DynamicStyleProperty { } return this._getMbDataDrivenSize({ - targetName: this.getComputedFieldName(), + targetName: this.getCompletedFieldName(), minSize: this._options.minSize, maxSize: this._options.maxSize, minValue: rangeFieldMeta.min, diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.d.ts b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.d.ts index f4c487b28757e..d551c60e67861 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.d.ts +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.d.ts @@ -9,8 +9,6 @@ import { IStyleProperty } from './style_property'; import { FIELD_ORIGIN } from '../../../../../common/constants'; import { FieldMetaOptions } from '../../../../../common/style_property_descriptor_types'; import { IField } from '../../../fields/field'; -import { IVectorLayer } from '../../../vector_layer'; -import { IVectorSource } from '../../../sources/vector_source'; import { CategoryFieldMeta, RangeFieldMeta } from '../../../../../common/descriptor_types'; export interface IDynamicStyleProperty extends IStyleProperty { diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js index 420de30a54071..62be0836ada60 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js @@ -13,7 +13,6 @@ import { SOURCE_META_ID_ORIGIN, FIELD_ORIGIN, } from '../../../../../common/constants'; -import { getComputedFieldName } from '../style_util'; import React from 'react'; import { OrdinalLegend } from './components/ordinal_legend'; import { CategoricalLegend } from './components/categorical_legend'; @@ -109,11 +108,11 @@ export class DynamicStyleProperty extends AbstractStyleProperty { return this._field ? this._field.getName() : ''; } - getComputedFieldName() { + getCompletedFieldName() { if (!this.isComplete()) { return null; } - return getComputedFieldName(this._styleName, this.getField().getName()); + return this.getField().getName(); } isDynamic() { @@ -292,11 +291,11 @@ export class DynamicStyleProperty extends AbstractStyleProperty { } } - getMbFeatureStateValue(value) { - if (!this.isOrdinal()) { - return this.formatField(value); - } + getFormattedValue(value) { + return this.isOrdinal() ? this.getNumericalMbFeatureStateValue(value) : this.formatField(value); + } + getNumericalMbFeatureStateValue(value) { const valueAsFloat = parseFloat(value); return isNaN(valueAsFloat) ? null : valueAsFloat; } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/label_border_size_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/label_border_size_property.js index 7119b659c1232..3016b15d0a05c 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/label_border_size_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/label_border_size_property.js @@ -31,21 +31,27 @@ export class LabelBorderSizeProperty extends AbstractStyleProperty { } syncLabelBorderSizeWithMb(mbLayerId, mbMap) { - const widthRatio = getWidthRatio(this.getOptions().size); - if (this.getOptions().size === LABEL_BORDER_SIZES.NONE) { mbMap.setPaintProperty(mbLayerId, 'text-halo-width', 0); - } else if (this._labelSizeProperty.isDynamic() && this._labelSizeProperty.isComplete()) { + return; + } + + const widthRatio = getWidthRatio(this.getOptions().size); + + if (this._labelSizeProperty.isDynamic() && this._labelSizeProperty.isComplete()) { const labelSizeExpression = this._labelSizeProperty.getMbSizeExpression(); - mbMap.setPaintProperty(mbLayerId, 'text-halo-width', [ - 'max', - ['*', labelSizeExpression, widthRatio], - 1, - ]); - } else { - const labelSize = _.get(this._labelSizeProperty.getOptions(), 'size', DEFAULT_LABEL_SIZE); - const labelBorderSize = Math.max(labelSize * widthRatio, 1); - mbMap.setPaintProperty(mbLayerId, 'text-halo-width', labelBorderSize); + if (labelSizeExpression) { + mbMap.setPaintProperty(mbLayerId, 'text-halo-width', [ + 'max', + ['*', labelSizeExpression, widthRatio], + 1, + ]); + return; + } } + + const labelSize = _.get(this._labelSizeProperty.getOptions(), 'size', DEFAULT_LABEL_SIZE); + const labelBorderSize = Math.max(labelSize * widthRatio, 1); + mbMap.setPaintProperty(mbLayerId, 'text-halo-width', labelBorderSize); } } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js index eecbb8c829448..3839a28f098b9 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js @@ -507,11 +507,14 @@ export class VectorStyle extends AbstractStyle { const dynamicStyleProp = dynamicStyleProps[j]; const name = dynamicStyleProp.getField().getName(); const computedName = getComputedFieldName(dynamicStyleProp.getStyleName(), name); - const styleValue = dynamicStyleProp.getMbFeatureStateValue(feature.properties[name]); + const rawValue = feature.properties[name]; if (dynamicStyleProp.supportsMbFeatureState()) { - tmpFeatureState[computedName] = styleValue; + tmpFeatureState[name] = dynamicStyleProp.getNumericalMbFeatureStateValue(rawValue); //the same value will be potentially overridden multiple times, if the name remains identical } else { - feature.properties[computedName] = styleValue; + //in practice, a new system property will only be created for: + // - label text: this requires the value to be formatted first. + // - icon orientation: this is a lay-out property which do not support feature-state (but we're still coercing to a number) + feature.properties[computedName] = dynamicStyleProp.getFormattedValue(rawValue); } } tmpFeatureIdentifier.source = mbSourceId; From 974c2214635e447bcfa4d675490875ac3045cb15 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 18 Mar 2020 14:57:06 -0400 Subject: [PATCH 16/25] fix tests --- .../properties/dynamic_color_property.test.js | 14 +++----------- x-pack/test/functional/apps/maps/mapbox_styles.js | 7 ++----- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js index 3fca2f57cf34a..81cd490ff4b45 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js @@ -289,17 +289,9 @@ describe('get mapbox color expression', () => { 'coalesce', [ 'case', - ['==', ['feature-state', '__kbn__dynamic__foobar__lineColor'], null], + ['==', ['feature-state', 'foobar'], null], -1, - [ - 'max', - [ - 'min', - ['to-number', ['feature-state', '__kbn__dynamic__foobar__lineColor']], - 100, - ], - 0, - ], + ['max', ['min', ['to-number', ['feature-state', 'foobar']], 100], 0], ], -1, ], @@ -359,7 +351,7 @@ describe('get mapbox color expression', () => { const colorProperty = makeProperty(dynamicStyleOptions); expect(colorProperty._getMbColor()).toEqual([ 'step', - ['coalesce', ['feature-state', '__kbn__dynamic__foobar__lineColor'], 9], + ['coalesce', ['feature-state', 'foobar'], 9], 'rgba(0,0,0,0)', 10, '#f7faff', diff --git a/x-pack/test/functional/apps/maps/mapbox_styles.js b/x-pack/test/functional/apps/maps/mapbox_styles.js index 6378c9d4e5d60..f48733dcfe706 100644 --- a/x-pack/test/functional/apps/maps/mapbox_styles.js +++ b/x-pack/test/functional/apps/maps/mapbox_styles.js @@ -27,10 +27,7 @@ export const MAPBOX_STYLES = { 'case', [ '==', - [ - 'feature-state', - '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', - ], + ['feature-state', 'kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name'], null, ], 2, @@ -42,7 +39,7 @@ export const MAPBOX_STYLES = { 'to-number', [ 'feature-state', - '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', + 'kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name', ], ], 12, From 7610ed72df7fd13f552d51476badad5d17fb6cfd Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 18 Mar 2020 15:08:49 -0400 Subject: [PATCH 17/25] remove unused method --- .../styles/vector/properties/dynamic_color_property.js | 10 ---------- .../vector/properties/dynamic_color_property.test.js | 2 +- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index 5498a799e3832..1aa205dbae36d 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -236,16 +236,6 @@ export class DynamicColorProperty extends DynamicStyleProperty { return ['match', ['to-string', ['get', this._field.getName()]], ...mbStops]; } - _getMbOrdinalColorStops() { - if (this._options.useCustomColorRamp) { - return this._options.customColorRamp.reduce((accumulatedStops, nextStop) => { - return [...accumulatedStops, nextStop.stop, nextStop.color]; - }, []); - } else { - return getOrdinalColorRampStops(this._options.color); - } - } - renderRangeLegendHeader() { if (this._options.color) { return ; diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js index 81cd490ff4b45..6a8cee4c65d9d 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js @@ -265,7 +265,7 @@ describe('get mapbox color expression', () => { expect(colorProperty._getMbColor()).toBeNull(); }); - test('Should set the correct mapbox style', async () => { + test('Should call .syncCircleColorWthMb correctly', async () => { const colorStyle = makeProperty({ color: 'Blues', type: undefined, From 276ae43014a2cf6534d783925a420790a9c65b4e Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 18 Mar 2020 15:13:17 -0400 Subject: [PATCH 18/25] create new test block --- .../properties/dynamic_color_property.test.js | 110 +++++++++--------- 1 file changed, 56 insertions(+), 54 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js index 6a8cee4c65d9d..6c63004eefb03 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js @@ -237,7 +237,62 @@ test('Should pluck the categorical style-meta from fieldmeta', async () => { }); }); -describe('get mapbox color expression', () => { +describe('sync mapbox color expression (via public API)', () => { + test('Should call .syncCircleColorWthMb with custom color ramp', async () => { + const colorStyle = makeProperty({ + color: 'Blues', + type: undefined, + }); + + const calls = []; + const mockMbMap = { + setPaintProperty(...args) { + calls.push(args); + }, + }; + + colorStyle.syncCircleColorWithMb('foobarid', mockMbMap, 1); + expect(calls[0]).toEqual([ + 'foobarid', + 'circle-color', + [ + 'interpolate', + ['linear'], + [ + 'coalesce', + [ + 'case', + ['==', ['feature-state', 'foobar'], null], + -1, + ['max', ['min', ['to-number', ['feature-state', 'foobar']], 100], 0], + ], + -1, + ], + -1, + 'rgba(0,0,0,0)', + 0, + '#f7faff', + 12.5, + '#ddeaf7', + 25, + '#c5daee', + 37.5, + '#9dc9e0', + 50, + '#6aadd5', + 62.5, + '#4191c5', + 75, + '#2070b4', + 87.5, + '#072f6b', + ], + ]); + expect(calls[1]).toEqual(['foobarid', 'circle-opacity', 1]); + }); +}); + +describe('get mapbox color expression (via internal _getMbColor)', () => { describe('ordinal color ramp', () => { test('should return null when field is not provided', async () => { const dynamicStyleOptions = { @@ -264,59 +319,6 @@ describe('get mapbox color expression', () => { const colorProperty = makeProperty(dynamicStyleOptions); expect(colorProperty._getMbColor()).toBeNull(); }); - - test('Should call .syncCircleColorWthMb correctly', async () => { - const colorStyle = makeProperty({ - color: 'Blues', - type: undefined, - }); - - const calls = []; - const mockMbMap = { - setPaintProperty(...args) { - calls.push(args); - }, - }; - - colorStyle.syncCircleColorWithMb('foobarid', mockMbMap, 1); - expect(calls[0]).toEqual([ - 'foobarid', - 'circle-color', - [ - 'interpolate', - ['linear'], - [ - 'coalesce', - [ - 'case', - ['==', ['feature-state', 'foobar'], null], - -1, - ['max', ['min', ['to-number', ['feature-state', 'foobar']], 100], 0], - ], - -1, - ], - -1, - 'rgba(0,0,0,0)', - 0, - '#f7faff', - 12.5, - '#ddeaf7', - 25, - '#c5daee', - 37.5, - '#9dc9e0', - 50, - '#6aadd5', - 62.5, - '#4191c5', - 75, - '#2070b4', - 87.5, - '#072f6b', - ], - ]); - expect(calls[1]).toEqual(['foobarid', 'circle-opacity', 1]); - }); }); describe('custom color ramp', () => { From c62b1e5b1eb00d37a72e80f79d8cc527112788a4 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 18 Mar 2020 23:10:05 -0400 Subject: [PATCH 19/25] functional test --- x-pack/test/functional/apps/maps/mapbox_styles.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/x-pack/test/functional/apps/maps/mapbox_styles.js b/x-pack/test/functional/apps/maps/mapbox_styles.js index f48733dcfe706..508a019db1764 100644 --- a/x-pack/test/functional/apps/maps/mapbox_styles.js +++ b/x-pack/test/functional/apps/maps/mapbox_styles.js @@ -27,7 +27,7 @@ export const MAPBOX_STYLES = { 'case', [ '==', - ['feature-state', 'kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name'], + ['feature-state', '__kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name'], null, ], 2, @@ -39,7 +39,7 @@ export const MAPBOX_STYLES = { 'to-number', [ 'feature-state', - 'kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name', + '__kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name', ], ], 12, @@ -97,10 +97,7 @@ export const MAPBOX_STYLES = { 'case', [ '==', - [ - 'feature-state', - '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', - ], + ['feature-state', '__kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name'], null, ], 2, @@ -112,7 +109,7 @@ export const MAPBOX_STYLES = { 'to-number', [ 'feature-state', - '__kbn__dynamic____kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name__fillColor', + '__kbnjoin__max_of_prop1_groupby_meta_for_geo_shapes*.shape_name', ], ], 12, From 7c94a0dd4eeed098daa8df90d2d9badbd0e6763b Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 18 Mar 2020 23:19:38 -0400 Subject: [PATCH 20/25] retain test --- .../properties/dynamic_color_property.test.js | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js index 6c63004eefb03..34c7b97d9fa66 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js @@ -319,6 +319,45 @@ describe('get mapbox color expression (via internal _getMbColor)', () => { const colorProperty = makeProperty(dynamicStyleOptions); expect(colorProperty._getMbColor()).toBeNull(); }); + test('should return mapbox expression for color ramp', async () => { + const dynamicStyleOptions = { + type: COLOR_MAP_TYPE.ORDINAL, + color: 'Blues', + }; + const colorProperty = makeProperty(dynamicStyleOptions); + expect(colorProperty._getMbColor()).toEqual([ + 'interpolate', + ['linear'], + [ + 'coalesce', + [ + 'case', + ['==', ['feature-state', 'foobar'], null], + -1, + ['max', ['min', ['to-number', ['feature-state', 'foobar']], 100], 0], + ], + -1, + ], + -1, + 'rgba(0,0,0,0)', + 0, + '#f7faff', + 12.5, + '#ddeaf7', + 25, + '#c5daee', + 37.5, + '#9dc9e0', + 50, + '#6aadd5', + 62.5, + '#4191c5', + 75, + '#2070b4', + 87.5, + '#072f6b', + ]); + }); }); describe('custom color ramp', () => { From aa49420e165988921b155241648cf72aa2274951 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Wed, 18 Mar 2020 23:24:21 -0400 Subject: [PATCH 21/25] remove crufty method --- .../styles/vector/properties/dynamic_size_property.js | 4 ++-- .../styles/vector/properties/dynamic_style_property.js | 7 ------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js index d3a87587c8d77..6a9d2b3967b6c 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_size_property.js @@ -77,7 +77,7 @@ export class DynamicSizeProperty extends DynamicStyleProperty { const rangeFieldMeta = this.getRangeFieldMeta(); if (this._isSizeDynamicConfigComplete(this._options) && rangeFieldMeta) { const halfIconPixels = this.getIconPixelSize() / 2; - const targetName = this.getCompletedFieldName(); + const targetName = this.getFieldName(); // Using property state instead of feature-state because layout properties do not support feature-state mbMap.setLayoutProperty(symbolLayerId, 'icon-size', [ 'interpolate', @@ -126,7 +126,7 @@ export class DynamicSizeProperty extends DynamicStyleProperty { } return this._getMbDataDrivenSize({ - targetName: this.getCompletedFieldName(), + targetName: this.getFieldName(), minSize: this._options.minSize, maxSize: this._options.maxSize, minValue: rangeFieldMeta.min, diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js index 239ee5a1b8e95..42238332978c0 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js @@ -108,13 +108,6 @@ export class DynamicStyleProperty extends AbstractStyleProperty { return this._field ? this._field.getName() : ''; } - getCompletedFieldName() { - if (!this.isComplete()) { - return null; - } - return this.getField().getName(); - } - isDynamic() { return true; } From 74147cf629c8a731fcc6581ff4dc100d8ed34b83 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 19 Mar 2020 17:19:30 -0400 Subject: [PATCH 22/25] ensure not fieldmeta is requested --- .../properties/dynamic_color_property.js | 15 +++++ .../properties/dynamic_color_property.test.js | 55 ------------------- .../properties/dynamic_style_property.js | 8 +-- 3 files changed, 16 insertions(+), 62 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index 1aa205dbae36d..b75b296f4f4b1 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -70,6 +70,17 @@ export class DynamicColorProperty extends DynamicStyleProperty { mbMap.setPaintProperty(mbLayerId, 'text-halo-color', color); } + supportsFieldMeta() { + if (!this.isComplete() || !this._field.supportsFieldMeta()) { + return false; + } + + return ( + (this.isCategorical() && !this.isCustomCategoricalPalette()) || + (this.isOrdinal() && !this.isCustomOrdinalColorRamp()) + ); + } + isOrdinal() { return ( typeof this._options.type === 'undefined' || this._options.type === COLOR_MAP_TYPE.ORDINAL @@ -84,6 +95,10 @@ export class DynamicColorProperty extends DynamicStyleProperty { return this._options.useCustomColorRamp; } + isCustomCategoricalPalette() { + return this._options.useCustomColorPalette; + } + supportsMbFeatureState() { return true; } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js index 34c7b97d9fa66..875b65308c15f 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js @@ -237,61 +237,6 @@ test('Should pluck the categorical style-meta from fieldmeta', async () => { }); }); -describe('sync mapbox color expression (via public API)', () => { - test('Should call .syncCircleColorWthMb with custom color ramp', async () => { - const colorStyle = makeProperty({ - color: 'Blues', - type: undefined, - }); - - const calls = []; - const mockMbMap = { - setPaintProperty(...args) { - calls.push(args); - }, - }; - - colorStyle.syncCircleColorWithMb('foobarid', mockMbMap, 1); - expect(calls[0]).toEqual([ - 'foobarid', - 'circle-color', - [ - 'interpolate', - ['linear'], - [ - 'coalesce', - [ - 'case', - ['==', ['feature-state', 'foobar'], null], - -1, - ['max', ['min', ['to-number', ['feature-state', 'foobar']], 100], 0], - ], - -1, - ], - -1, - 'rgba(0,0,0,0)', - 0, - '#f7faff', - 12.5, - '#ddeaf7', - 25, - '#c5daee', - 37.5, - '#9dc9e0', - 50, - '#6aadd5', - 62.5, - '#4191c5', - 75, - '#2070b4', - 87.5, - '#072f6b', - ], - ]); - expect(calls[1]).toEqual(['foobarid', 'circle-opacity', 1]); - }); -}); - describe('get mapbox color expression (via internal _getMbColor)', () => { describe('ordinal color ramp', () => { test('should return null when field is not provided', async () => { diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js index 42238332978c0..a5dadbcb6acbb 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js @@ -142,13 +142,7 @@ export class DynamicStyleProperty extends AbstractStyleProperty { } supportsFieldMeta() { - if (this.isOrdinal()) { - return this.isComplete() && this._field.supportsFieldMeta(); - } else if (this.isCategorical()) { - return this.isComplete() && this._field.supportsFieldMeta(); - } else { - return false; - } + return this.isComplete() && this._field.supportsFieldMeta(); } async getFieldMetaRequest() { From 55a4dfd842eec5b346b3bc0e944cf5b62780aa61 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Thu, 19 Mar 2020 17:20:49 -0400 Subject: [PATCH 23/25] simplify --- .../vector/properties/dynamic_color_property.js | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js index b75b296f4f4b1..146bc40aa8531 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.js @@ -76,8 +76,8 @@ export class DynamicColorProperty extends DynamicStyleProperty { } return ( - (this.isCategorical() && !this.isCustomCategoricalPalette()) || - (this.isOrdinal() && !this.isCustomOrdinalColorRamp()) + (this.isCategorical() && !this._options.useCustomColorPalette) || + (this.isOrdinal() && !this._options.useCustomColorRamp) ); } @@ -91,24 +91,16 @@ export class DynamicColorProperty extends DynamicStyleProperty { return this._options.type === COLOR_MAP_TYPE.CATEGORICAL; } - isCustomOrdinalColorRamp() { - return this._options.useCustomColorRamp; - } - - isCustomCategoricalPalette() { - return this._options.useCustomColorPalette; - } - supportsMbFeatureState() { return true; } isOrdinalRanged() { - return this.isOrdinal() && !this.isCustomOrdinalColorRamp(); + return this.isOrdinal() && !this._options.useCustomColorRamp; } hasOrdinalBreaks() { - return (this.isOrdinal() && this.isCustomOrdinalColorRamp()) || this.isCategorical(); + return (this.isOrdinal() && this._options.useCustomColorRamp) || this.isCategorical(); } _getMbColor() { From b2fb9555d2963a89c408ddcbb650b7ff7d3d4dc6 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Mon, 23 Mar 2020 12:11:19 -0400 Subject: [PATCH 24/25] add unit tests --- .../properties/dynamic_color_property.test.js | 79 ++++++++++++++++--- 1 file changed, 69 insertions(+), 10 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js index 60e13f5aac00e..b19c25b369848 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_color_property.test.js @@ -75,16 +75,10 @@ class MockLayer { } } -const makeProperty = options => { - return new DynamicColorProperty( - options, - VECTOR_STYLES.LINE_COLOR, - mockField, - new MockLayer(), - () => { - return x => x + '_format'; - } - ); +const makeProperty = (options, field = mockField) => { + return new DynamicColorProperty(options, VECTOR_STYLES.LINE_COLOR, field, new MockLayer(), () => { + return x => x + '_format'; + }); }; const defaultLegendParams = { @@ -236,6 +230,71 @@ test('Should pluck the categorical style-meta from fieldmeta', async () => { }); }); +describe('supportsFieldMeta', () => { + test('should support it when field does for ordinals', () => { + const dynamicStyleOptions = { + type: COLOR_MAP_TYPE.ORDINAL, + }; + const styleProp = makeProperty(dynamicStyleOptions); + + expect(styleProp.supportsFieldMeta()).toEqual(true); + }); + + test('should support it when field does for categories', () => { + const dynamicStyleOptions = { + type: COLOR_MAP_TYPE.CATEGORICAL, + }; + const styleProp = makeProperty(dynamicStyleOptions); + + expect(styleProp.supportsFieldMeta()).toEqual(true); + }); + + test('should not support it when field does not', () => { + const field = Object.create(mockField); + field.supportsFieldMeta = function() { + return false; + }; + + const dynamicStyleOptions = { + type: COLOR_MAP_TYPE.ORDINAL, + }; + const styleProp = makeProperty(dynamicStyleOptions, field); + + expect(styleProp.supportsFieldMeta()).toEqual(false); + }); + + test('should not support it when field config not complete', () => { + const dynamicStyleOptions = { + type: COLOR_MAP_TYPE.ORDINAL, + }; + const styleProp = makeProperty(dynamicStyleOptions, null); + + expect(styleProp.supportsFieldMeta()).toEqual(false); + }); + + test('should not support it when using custom ramp for ordinals', () => { + const dynamicStyleOptions = { + type: COLOR_MAP_TYPE.ORDINAL, + useCustomColorRamp: true, + customColorRamp: [], + }; + const styleProp = makeProperty(dynamicStyleOptions); + + expect(styleProp.supportsFieldMeta()).toEqual(false); + }); + + test('should not support it when using custom palette for categories', () => { + const dynamicStyleOptions = { + type: COLOR_MAP_TYPE.CATEGORICAL, + useCustomColorPalette: true, + customColorPalette: [], + }; + const styleProp = makeProperty(dynamicStyleOptions); + + expect(styleProp.supportsFieldMeta()).toEqual(false); + }); +}); + describe('get mapbox color expression (via internal _getMbColor)', () => { describe('ordinal color ramp', () => { test('should return null when field is not provided', async () => { From 0257429df4feb3923ca593d450fa404599c4cb67 Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Mon, 23 Mar 2020 13:49:07 -0400 Subject: [PATCH 25/25] improve readability --- .../styles/vector/properties/dynamic_style_property.js | 4 ---- .../maps/public/layers/styles/vector/vector_style.js | 7 ++++++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js index a5dadbcb6acbb..ea521f8749d80 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/properties/dynamic_style_property.js @@ -278,10 +278,6 @@ export class DynamicStyleProperty extends AbstractStyleProperty { } } - getFormattedValue(value) { - return this.isOrdinal() ? this.getNumericalMbFeatureStateValue(value) : this.formatField(value); - } - getNumericalMbFeatureStateValue(value) { const valueAsFloat = parseFloat(value); return isNaN(valueAsFloat) ? null : valueAsFloat; diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js index fc240f03e6f4a..ae5d148e43cfd 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector/vector_style.js @@ -510,7 +510,12 @@ export class VectorStyle extends AbstractStyle { //in practice, a new system property will only be created for: // - label text: this requires the value to be formatted first. // - icon orientation: this is a lay-out property which do not support feature-state (but we're still coercing to a number) - feature.properties[computedName] = dynamicStyleProp.getFormattedValue(rawValue); + + const formattedValue = dynamicStyleProp.isOrdinal() + ? dynamicStyleProp.getNumericalMbFeatureStateValue(rawValue) + : dynamicStyleProp.formatField(rawValue); + + feature.properties[computedName] = formattedValue; } } tmpFeatureIdentifier.source = mbSourceId;