From d04e99941999fe29c9c959cdc6d888051b43d87f Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 4 Aug 2020 13:30:09 -0400 Subject: [PATCH 1/2] [Maps] Custom color ramps should show correctly on the map for mvt layers (#74169) --- .../public/classes/fields/es_agg_field.ts | 4 +++ .../maps/public/classes/fields/field.ts | 9 +++++ .../maps/public/classes/fields/mvt_field.ts | 4 +++ .../fields/top_term_percentage_field.ts | 4 +++ .../properties/dynamic_color_property.js | 10 ++---- .../properties/dynamic_color_property.test.js | 36 ++++++++++++++----- .../properties/dynamic_size_property.js | 8 ++--- .../properties/dynamic_style_property.js | 14 ++++++-- 8 files changed, 64 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/maps/public/classes/fields/es_agg_field.ts b/x-pack/plugins/maps/public/classes/fields/es_agg_field.ts index 15779d22681c0..7b184819b839b 100644 --- a/x-pack/plugins/maps/public/classes/fields/es_agg_field.ts +++ b/x-pack/plugins/maps/public/classes/fields/es_agg_field.ts @@ -134,6 +134,10 @@ export class ESAggField implements IESAggField { supportsAutoDomain(): boolean { return true; } + + canReadFromGeoJson(): boolean { + return true; + } } export function esAggFieldsFactory( diff --git a/x-pack/plugins/maps/public/classes/fields/field.ts b/x-pack/plugins/maps/public/classes/fields/field.ts index 410b38e79ffe4..2c190d54f0265 100644 --- a/x-pack/plugins/maps/public/classes/fields/field.ts +++ b/x-pack/plugins/maps/public/classes/fields/field.ts @@ -26,7 +26,12 @@ export interface IField { // then styling properties that require the domain to be known cannot use this property. supportsAutoDomain(): boolean; + // Determinse wheter Maps-app can automatically deterime the domain of the field-values + // _without_ having to retrieve the data as GeoJson + // e.g. for ES-sources, this would use the extended_stats API supportsFieldMeta(): boolean; + + canReadFromGeoJson(): boolean; } export class AbstractField implements IField { @@ -90,4 +95,8 @@ export class AbstractField implements IField { supportsAutoDomain(): boolean { return true; } + + canReadFromGeoJson(): boolean { + return true; + } } diff --git a/x-pack/plugins/maps/public/classes/fields/mvt_field.ts b/x-pack/plugins/maps/public/classes/fields/mvt_field.ts index eb2bb94b36a69..7c8d08bacdb51 100644 --- a/x-pack/plugins/maps/public/classes/fields/mvt_field.ts +++ b/x-pack/plugins/maps/public/classes/fields/mvt_field.ts @@ -56,4 +56,8 @@ export class MVTField extends AbstractField implements IField { supportsAutoDomain() { return false; } + + canReadFromGeoJson(): boolean { + return false; + } } diff --git a/x-pack/plugins/maps/public/classes/fields/top_term_percentage_field.ts b/x-pack/plugins/maps/public/classes/fields/top_term_percentage_field.ts index f4625e42ab5de..fc931b13619ef 100644 --- a/x-pack/plugins/maps/public/classes/fields/top_term_percentage_field.ts +++ b/x-pack/plugins/maps/public/classes/fields/top_term_percentage_field.ts @@ -79,4 +79,8 @@ export class TopTermPercentageField implements IESAggField { canValueBeFormatted(): boolean { return false; } + + canReadFromGeoJson(): boolean { + return true; + } } diff --git a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.js b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.js index cde4d1f201199..e643abcaf8d54 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.js @@ -8,7 +8,7 @@ import { DynamicStyleProperty } from './dynamic_style_property'; import { makeMbClampedNumberExpression, dynamicRound } from '../style_util'; import { getOrdinalMbColorRampStops, getColorPalette } from '../../color_palettes'; import React from 'react'; -import { COLOR_MAP_TYPE, MB_LOOKUP_FUNCTION } from '../../../../../common/constants'; +import { COLOR_MAP_TYPE } from '../../../../../common/constants'; import { isCategoricalStopsInvalid, getOtherCategoryLabel, @@ -91,10 +91,6 @@ export class DynamicColorProperty extends DynamicStyleProperty { return colors ? colors.length : 0; } - supportsMbFeatureState() { - return true; - } - _getMbColor() { if (!this._field || !this._field.getName()) { return null; @@ -120,7 +116,7 @@ export class DynamicColorProperty extends DynamicStyleProperty { const lessThanFirstStopValue = firstStopValue - 1; return [ 'step', - ['coalesce', ['feature-state', targetName], lessThanFirstStopValue], + ['coalesce', [this.getMbLookupFunction(), targetName], lessThanFirstStopValue], RGBA_0000, // MB will assign the base value to any features that is below the first stop value ...colorStops, ]; @@ -146,7 +142,7 @@ export class DynamicColorProperty extends DynamicStyleProperty { makeMbClampedNumberExpression({ minValue: rangeFieldMeta.min, maxValue: rangeFieldMeta.max, - lookupFunction: MB_LOOKUP_FUNCTION.FEATURE_STATE, + lookupFunction: this.getMbLookupFunction(), fallback: lessThanFirstStopValue, fieldName: targetName, }), diff --git a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.test.js b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.test.js index 7992ee5b3aeaf..b47a3c69650ca 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.test.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_color_property.test.js @@ -343,6 +343,15 @@ describe('get mapbox color expression (via internal _getMbColor)', () => { }); describe('custom color ramp', () => { + const dynamicStyleOptions = { + type: COLOR_MAP_TYPE.ORDINAL, + useCustomColorRamp: true, + customColorRamp: [ + { stop: 10, color: '#f7faff' }, + { stop: 100, color: '#072f6b' }, + ], + }; + test('should return null when customColorRamp is not provided', async () => { const dynamicStyleOptions = { type: COLOR_MAP_TYPE.ORDINAL, @@ -362,15 +371,7 @@ describe('get mapbox color expression (via internal _getMbColor)', () => { expect(colorProperty._getMbColor()).toBeNull(); }); - test('should return mapbox expression for custom color ramp', async () => { - const dynamicStyleOptions = { - type: COLOR_MAP_TYPE.ORDINAL, - useCustomColorRamp: true, - customColorRamp: [ - { stop: 10, color: '#f7faff' }, - { stop: 100, color: '#072f6b' }, - ], - }; + test('should use `feature-state` by default', async () => { const colorProperty = makeProperty(dynamicStyleOptions); expect(colorProperty._getMbColor()).toEqual([ 'step', @@ -382,6 +383,23 @@ describe('get mapbox color expression (via internal _getMbColor)', () => { '#072f6b', ]); }); + + test('should use `get` when source cannot return raw geojson', async () => { + const field = Object.create(mockField); + field.canReadFromGeoJson = function () { + return false; + }; + const colorProperty = makeProperty(dynamicStyleOptions, undefined, field); + expect(colorProperty._getMbColor()).toEqual([ + 'step', + ['coalesce', ['get', 'foobar'], 9], + 'rgba(0,0,0,0)', + 10, + '#f7faff', + 100, + '#072f6b', + ]); + }); }); }); diff --git a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_size_property.js b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_size_property.js index 662d1ccf33b95..83bd4b70ba5c3 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_size_property.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_size_property.js @@ -33,7 +33,7 @@ export class DynamicSizeProperty extends DynamicStyleProperty { return false; } - return true; + return super.supportsMbFeatureState(); } syncHaloWidthWithMb(mbLayerId, mbMap) { @@ -109,17 +109,13 @@ export class DynamicSizeProperty extends DynamicStyleProperty { } _getMbDataDrivenSize({ targetName, minSize, maxSize, minValue, maxValue }) { - const lookup = this.supportsMbFeatureState() - ? MB_LOOKUP_FUNCTION.FEATURE_STATE - : MB_LOOKUP_FUNCTION.GET; - const stops = minValue === maxValue ? [maxValue, maxSize] : [minValue, minSize, maxValue, maxSize]; return [ 'interpolate', ['linear'], makeMbClampedNumberExpression({ - lookupFunction: lookup, + lookupFunction: this.getMbLookupFunction(), maxValue, minValue, fieldName: targetName, diff --git a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_style_property.js b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_style_property.js index 15d0b3c4bf913..a3e96e9a9dd18 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_style_property.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_style_property.js @@ -8,9 +8,10 @@ import _ from 'lodash'; import { AbstractStyleProperty } from './style_property'; import { DEFAULT_SIGMA } from '../vector_style_defaults'; import { - STYLE_TYPE, - SOURCE_META_DATA_REQUEST_ID, FIELD_ORIGIN, + MB_LOOKUP_FUNCTION, + SOURCE_META_DATA_REQUEST_ID, + STYLE_TYPE, } from '../../../../../common/constants'; import React from 'react'; import { OrdinalFieldMetaPopover } from '../components/field_meta/ordinal_field_meta_popover'; @@ -20,6 +21,7 @@ export class DynamicStyleProperty extends AbstractStyleProperty { static type = STYLE_TYPE.DYNAMIC; constructor(options, styleName, field, vectorLayer, getFieldFormatter) { + super(options, styleName); super(options, styleName); this._field = field; this._layer = vectorLayer; @@ -150,7 +152,13 @@ export class DynamicStyleProperty extends AbstractStyleProperty { } supportsMbFeatureState() { - return true; + return !!this._field && this._field.canReadFromGeoJson(); + } + + getMbLookupFunction() { + return this.supportsMbFeatureState() + ? MB_LOOKUP_FUNCTION.FEATURE_STATE + : MB_LOOKUP_FUNCTION.GET; } getFieldMetaOptions() { From 9fac8ace6c6ddc77d41ef5788d7c199d288f5afc Mon Sep 17 00:00:00 2001 From: Thomas Neirynck Date: Tue, 4 Aug 2020 15:23:22 -0400 Subject: [PATCH 2/2] fix typo --- .../classes/styles/vector/properties/dynamic_style_property.js | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_style_property.js b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_style_property.js index a3e96e9a9dd18..038f8da2835d8 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_style_property.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/properties/dynamic_style_property.js @@ -21,7 +21,6 @@ export class DynamicStyleProperty extends AbstractStyleProperty { static type = STYLE_TYPE.DYNAMIC; constructor(options, styleName, field, vectorLayer, getFieldFormatter) { - super(options, styleName); super(options, styleName); this._field = field; this._layer = vectorLayer;