[Maps] Fix cannot select Solid fill-color when removing fields (#70621)

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elastic · Jul 2, 2020 · 23ea7ac · 23ea7ac
1 parent f5b2800
commit 23ea7ac
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 74 deletions.
diff --git a/x-pack/plugins/maps/public/actions/layer_actions.ts b/x-pack/plugins/maps/public/actions/layer_actions.ts
@@ -13,6 +13,7 @@ import {
   getLayerListRaw,
   getSelectedLayerId,
   getMapReady,
+  getMapColors,
 } from '../selectors/map_selectors';
 import { FLYOUT_STATE } from '../reducers/ui';
 import { cancelRequest } from '../reducers/non_serializable_instances';
@@ -384,7 +385,8 @@ export function clearMissingStyleProperties(layerId: string) {
 
     const nextFields = await (targetLayer as IVectorLayer).getFields(); // take into account all fields, since labels can be driven by any field (source or join)
     const { hasChanges, nextStyleDescriptor } = style.getDescriptorWithMissingStylePropsRemoved(
-      nextFields
+      nextFields,
+      getMapColors(getState())
     );
     if (hasChanges && nextStyleDescriptor) {
       dispatch<any>(updateLayerStyle(layerId, nextStyleDescriptor));

diff --git a/x-pack/plugins/maps/public/classes/styles/style.ts b/x-pack/plugins/maps/public/classes/styles/style.ts
@@ -13,7 +13,8 @@ import { DataRequest } from '../util/data_request';
 export interface IStyle {
   getDescriptor(): StyleDescriptor | null;
   getDescriptorWithMissingStylePropsRemoved(
-    nextFields: IField[]
+    nextFields: IField[],
+    mapColors: string[]
   ): { hasChanges: boolean; nextStyleDescriptor?: StyleDescriptor };
   pluckStyleMetaFromSourceDataRequest(sourceDataRequest: DataRequest): StyleMetaDescriptor;
   renderEditor({
@@ -34,7 +35,8 @@ export class AbstractStyle implements IStyle {
   }
 
   getDescriptorWithMissingStylePropsRemoved(
-    nextFields: IField[]
+    nextFields: IField[],
+    mapColors: string[]
   ): { hasChanges: boolean; nextStyleDescriptor?: StyleDescriptor } {
     return {
       hasChanges: false,

diff --git a/x-pack/plugins/maps/public/classes/styles/vector/vector_style.js b/x-pack/plugins/maps/public/classes/styles/vector/vector_style.js
@@ -7,7 +7,12 @@
 import _ from 'lodash';
 import React from 'react';
 import { VectorStyleEditor } from './components/vector_style_editor';
-import { getDefaultProperties, LINE_STYLES, POLYGON_STYLES } from './vector_style_defaults';
+import {
+  getDefaultProperties,
+  getDefaultStaticProperties,
+  LINE_STYLES,
+  POLYGON_STYLES,
+} from './vector_style_defaults';
 import { AbstractStyle } from '../style';
 import {
   GEO_JSON_TYPE,
@@ -191,7 +196,7 @@ export class VectorStyle extends AbstractStyle {
    * This method does not update its descriptor. It just returns a new descriptor that the caller
    * can then use to update store state via dispatch.
    */
-  getDescriptorWithMissingStylePropsRemoved(nextFields) {
+  getDescriptorWithMissingStylePropsRemoved(nextFields, mapColors) {
     const originalProperties = this.getRawProperties();
     const updatedProperties = {};
 
@@ -201,6 +206,13 @@ export class VectorStyle extends AbstractStyle {
     });
 
     dynamicProperties.forEach((key) => {
+      // Convert dynamic styling to static stying when there are no nextFields
+      if (nextFields.length === 0) {
+        const staticProperties = getDefaultStaticProperties(mapColors);
+        updatedProperties[key] = staticProperties[key];
+        return;
+      }
+
       const dynamicProperty = originalProperties[key];
       const fieldName =
         dynamicProperty && dynamicProperty.options.field && dynamicProperty.options.field.name;

diff --git a/x-pack/plugins/maps/public/classes/styles/vector/vector_style.test.js b/x-pack/plugins/maps/public/classes/styles/vector/vector_style.test.js
@@ -6,7 +6,12 @@
 
 import { VectorStyle } from './vector_style';
 import { DataRequest } from '../../util/data_request';
-import { FIELD_ORIGIN, STYLE_TYPE, VECTOR_SHAPE_TYPE } from '../../../../common/constants';
+import {
+  FIELD_ORIGIN,
+  STYLE_TYPE,
+  VECTOR_SHAPE_TYPE,
+  VECTOR_STYLES,
+} from '../../../../common/constants';
 
 jest.mock('../../../kibana_services');
 jest.mock('ui/new_platform');
@@ -42,6 +47,7 @@ class MockSource {
 
 describe('getDescriptorWithMissingStylePropsRemoved', () => {
   const fieldName = 'doIStillExist';
+  const mapColors = [];
   const properties = {
     fillColor: {
       type: STYLE_TYPE.STATIC,
@@ -59,7 +65,8 @@ describe('getDescriptorWithMissingStylePropsRemoved', () => {
     iconSize: {
       type: STYLE_TYPE.DYNAMIC,
       options: {
-        color: 'a color',
+        minSize: 1,
+        maxSize: 10,
         field: { name: fieldName, origin: FIELD_ORIGIN.SOURCE },
       },
     },
@@ -75,86 +82,55 @@ describe('getDescriptorWithMissingStylePropsRemoved', () => {
     const vectorStyle = new VectorStyle({ properties }, new MockSource());
 
     const nextFields = [new MockField({ fieldName })];
-    const { hasChanges } = vectorStyle.getDescriptorWithMissingStylePropsRemoved(nextFields);
+    const { hasChanges } = vectorStyle.getDescriptorWithMissingStylePropsRemoved(
+      nextFields,
+      mapColors
+    );
     expect(hasChanges).toBe(false);
   });
 
   it('Should clear missing fields when next ordinal fields do not contain existing style property fields', () => {
     const vectorStyle = new VectorStyle({ properties }, new MockSource());
 
-    const nextFields = [];
+    const nextFields = [new MockField({ fieldName: 'someOtherField' })];
     const {
       hasChanges,
       nextStyleDescriptor,
-    } = vectorStyle.getDescriptorWithMissingStylePropsRemoved(nextFields);
+    } = vectorStyle.getDescriptorWithMissingStylePropsRemoved(nextFields, mapColors);
     expect(hasChanges).toBe(true);
-    expect(nextStyleDescriptor.properties).toEqual({
-      fillColor: {
-        options: {},
-        type: 'STATIC',
-      },
-      icon: {
-        options: {
-          value: 'marker',
-        },
-        type: 'STATIC',
-      },
-      iconOrientation: {
-        options: {
-          orientation: 0,
-        },
-        type: 'STATIC',
-      },
-      iconSize: {
-        options: {
-          color: 'a color',
-        },
-        type: 'DYNAMIC',
-      },
-      labelText: {
-        options: {
-          value: '',
-        },
-        type: 'STATIC',
-      },
-      labelBorderColor: {
-        options: {
-          color: '#FFFFFF',
-        },
-        type: 'STATIC',
-      },
-      labelBorderSize: {
-        options: {
-          size: 'SMALL',
-        },
-      },
-      labelColor: {
-        options: {
-          color: '#000000',
-        },
-        type: 'STATIC',
-      },
-      labelSize: {
-        options: {
-          size: 14,
-        },
-        type: 'STATIC',
-      },
-      lineColor: {
-        options: {},
-        type: 'DYNAMIC',
+    expect(nextStyleDescriptor.properties[VECTOR_STYLES.LINE_COLOR]).toEqual({
+      options: {},
+      type: 'DYNAMIC',
+    });
+    expect(nextStyleDescriptor.properties[VECTOR_STYLES.ICON_SIZE]).toEqual({
+      options: {
+        minSize: 1,
+        maxSize: 10,
       },
-      lineWidth: {
-        options: {
-          size: 1,
-        },
-        type: 'STATIC',
+      type: 'DYNAMIC',
+    });
+  });
+
+  it('Should convert dynamic styles to static styles when there are no next fields', () => {
+    const vectorStyle = new VectorStyle({ properties }, new MockSource());
+
+    const nextFields = [];
+    const {
+      hasChanges,
+      nextStyleDescriptor,
+    } = vectorStyle.getDescriptorWithMissingStylePropsRemoved(nextFields, mapColors);
+    expect(hasChanges).toBe(true);
+    expect(nextStyleDescriptor.properties[VECTOR_STYLES.LINE_COLOR]).toEqual({
+      options: {
+        color: '#41937c',
       },
-      symbolizeAs: {
-        options: {
-          value: 'circle',
-        },
+      type: 'STATIC',
+    });
+    expect(nextStyleDescriptor.properties[VECTOR_STYLES.ICON_SIZE]).toEqual({
+      options: {
+        size: 6,
       },
+      type: 'STATIC',
     });
   });
 });