[Maps] do not show border color for icon in legend when border width …

…is zero (elastic#57501) * [Maps] do not show border color for icon in legend when border width is zero * fix jest tests * fix jest tests Co-authored-by: Elastic Machine <[email protected]>
nreese · Feb 13, 2020 · ac35279 · ac35279
1 parent 70fcc3d
commit ac35279
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 86 deletions.
diff --git a/...maps/public/layers/styles/vector/components/legend/__snapshots__/vector_icon.test.js.snap b/...maps/public/layers/styles/vector/components/legend/__snapshots__/vector_icon.test.js.snap
diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/components/legend/symbol_icon.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/components/legend/symbol_icon.js
@@ -12,62 +12,30 @@ import { getMakiSymbolSvg, styleSvg, buildSrcUrl } from '../../symbol_utils';
 export class SymbolIcon extends Component {
   state = {
     imgDataUrl: undefined,
-    prevSymbolId: undefined,
-    prevFill: undefined,
-    prevStroke: undefined,
-    prevStrokeWidth: undefined,
   };
 
   componentDidMount() {
     this._isMounted = true;
-    this._loadSymbol(
-      this.props.symbolId,
-      this.props.fill,
-      this.props.stroke,
-      this.props.strokeWidth
-    );
-  }
-
-  componentDidUpdate() {
-    this._loadSymbol(
-      this.props.symbolId,
-      this.props.fill,
-      this.props.stroke,
-      this.props.strokeWidth
-    );
+    this._loadSymbol();
   }
 
   componentWillUnmount() {
     this._isMounted = false;
   }
 
-  async _loadSymbol(nextSymbolId, nextFill, nextStroke, nextStrokeWidth) {
-    if (
-      nextSymbolId === this.state.prevSymbolId &&
-      nextFill === this.state.prevFill &&
-      nextStroke === this.state.prevStroke &&
-      nextStrokeWidth === this.state.prevStrokeWidth
-    ) {
-      return;
-    }
-
+  async _loadSymbol() {
     let imgDataUrl;
     try {
-      const svg = getMakiSymbolSvg(nextSymbolId);
-      const styledSvg = await styleSvg(svg, nextFill, nextStroke, nextStrokeWidth);
+      const svg = getMakiSymbolSvg(this.props.symbolId);
+      const styledSvg = await styleSvg(svg, this.props.fill, this.props.stroke);
       imgDataUrl = buildSrcUrl(styledSvg);
     } catch (error) {
       // ignore failures - component will just not display an icon
+      return;
     }
 
     if (this._isMounted) {
-      this.setState({
-        imgDataUrl,
-        prevSymbolId: nextSymbolId,
-        prevFill: nextFill,
-        prevStroke: nextStroke,
-        prevStrokeWidth: nextStrokeWidth,
-      });
+      this.setState({ imgDataUrl });
     }
   }
 
@@ -80,7 +48,6 @@ export class SymbolIcon extends Component {
       symbolId, // eslint-disable-line no-unused-vars
       fill, // eslint-disable-line no-unused-vars
       stroke, // eslint-disable-line no-unused-vars
-      strokeWidth, // eslint-disable-line no-unused-vars
       ...rest
     } = this.props;
 
@@ -98,7 +65,6 @@ export class SymbolIcon extends Component {
 
 SymbolIcon.propTypes = {
   symbolId: PropTypes.string.isRequired,
-  fill: PropTypes.string.isRequired,
-  stroke: PropTypes.string.isRequired,
-  strokeWidth: PropTypes.string.isRequired,
+  fill: PropTypes.string,
+  stroke: PropTypes.string,
 };
diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/components/legend/vector_icon.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/components/legend/vector_icon.js
@@ -37,10 +37,10 @@ export function VectorIcon({ fillColor, isPointsOnly, isLinesOnly, strokeColor,
 
   return (
     <SymbolIcon
+      key={`${symbolId}${fillColor}${strokeColor}`}
       symbolId={symbolId}
-      fill={style.fill}
-      stroke={style.stroke}
-      strokeWidth={style.strokeWidth}
+      fill={fillColor}
+      stroke={strokeColor}
     />
   );
 }
@@ -49,6 +49,6 @@ VectorIcon.propTypes = {
   fillColor: PropTypes.string,
   isPointsOnly: PropTypes.bool.isRequired,
   isLinesOnly: PropTypes.bool.isRequired,
-  strokeColor: PropTypes.string.isRequired,
+  strokeColor: PropTypes.string,
   symbolId: PropTypes.string,
 };
diff --git a/...maps/public/layers/styles/vector/components/symbol/__snapshots__/icon_select.test.js.snap b/...maps/public/layers/styles/vector/components/symbol/__snapshots__/icon_select.test.js.snap
diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/components/symbol/icon_select.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/components/symbol/icon_select.js
@@ -80,11 +80,10 @@ export class IconSelect extends Component {
           fullWidth
           prepend={
             <SymbolIcon
+              key={value}
               className="mapIconSelectSymbol__inputButton"
               symbolId={value}
               fill={isDarkMode ? 'rgb(223, 229, 239)' : 'rgb(52, 55, 65)'}
-              stroke={isDarkMode ? 'rgb(255, 255, 255)' : 'rgb(0, 0, 0)'}
-              strokeWidth={'1px'}
             />
           }
         />
@@ -100,10 +99,9 @@ export class IconSelect extends Component {
         label,
         prepend: (
           <SymbolIcon
+            key={value}
             symbolId={value}
             fill={isDarkMode ? 'rgb(223, 229, 239)' : 'rgb(52, 55, 65)'}
-            stroke={isDarkMode ? 'rgb(255, 255, 255)' : 'rgb(0, 0, 0)'}
-            strokeWidth={'1px'}
           />
         ),
       };

diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/components/vector_style_editor.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/components/vector_style_editor.js
@@ -146,11 +146,6 @@ export class VectorStyleEditor extends Component {
     this.props.handlePropertyChange(propertyName, styleDescriptor);
   };
 
-  _hasBorder() {
-    const width = this.props.styleProperties[VECTOR_STYLES.LINE_WIDTH];
-    return width.isDynamic() ? width.isComplete() : width.getOptions().size !== 0;
-  }
-
   _hasMarkerOrIcon() {
     const iconSize = this.props.styleProperties[VECTOR_STYLES.ICON_SIZE];
     return iconSize.isDynamic() || iconSize.getOptions().size > 0;
@@ -192,7 +187,7 @@ export class VectorStyleEditor extends Component {
     const disabledByIconSize = isPointBorderColor && !this._hasMarkerOrIcon();
     return (
       <VectorStyleColorEditor
-        disabled={disabledByIconSize || !this._hasBorder()}
+        disabled={disabledByIconSize || !this.props.hasBorder}
         disabledBy={disabledByIconSize ? VECTOR_STYLES.ICON_SIZE : VECTOR_STYLES.LINE_WIDTH}
         swatches={DEFAULT_LINE_COLORS}
         onStaticStyleChange={this._onStaticStyleChange}

diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/symbol_utils.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/symbol_utils.js
@@ -69,17 +69,15 @@ export function buildSrcUrl(svgString) {
   return domUrl.createObjectURL(svg);
 }
 
-export async function styleSvg(svgString, fill, stroke, strokeWidth) {
+export async function styleSvg(svgString, fill, stroke) {
   const svgXml = await parseXmlString(svgString);
   let style = '';
   if (fill) {
     style += `fill:${fill};`;
   }
   if (stroke) {
     style += `stroke:${stroke};`;
-  }
-  if (strokeWidth) {
-    style += `stroke-width:${strokeWidth};`;
+    style += `stroke-width:1;`;
   }
   if (style) svgXml.svg.$.style = style;
   const builder = new xml2js.Builder();
@@ -119,8 +117,6 @@ export function getIconPaletteOptions(isDarkMode) {
             className="mapIcon"
             symbolId={iconId}
             fill={isDarkMode ? 'rgb(223, 229, 239)' : 'rgb(52, 55, 65)'}
-            stroke={isDarkMode ? 'rgb(255, 255, 255)' : 'rgb(0, 0, 0)'}
-            strokeWidth={'1px'}
           />
         </div>
       );

diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector/symbol_utils.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector/symbol_utils.test.js
@@ -32,21 +32,12 @@ describe('styleSvg', () => {
     );
   });
 
-  it('Should add stroke style property to svg element', async () => {
+  it('Should add stroke and stroke-wdth style properties to svg element', async () => {
     const unstyledSvgString =
       '<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11"><path/></svg>';
     const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white');
     expect(styledSvg.split('\n')[1]).toBe(
-      '<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11" style="fill:red;stroke:white;">'
-    );
-  });
-
-  it('Should add stroke-width style property to svg element', async () => {
-    const unstyledSvgString =
-      '<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11"><path/></svg>';
-    const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white', '2px');
-    expect(styledSvg.split('\n')[1]).toBe(
-      '<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11" style="fill:red;stroke:white;stroke-width:2px;">'
+      '<svg version="1.1" width="11px" height="11px" viewBox="0 0 11 11" style="fill:red;stroke:white;stroke-width:1;">'
     );
   });
 });
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
@@ -138,6 +138,12 @@ export class VectorStyle extends AbstractStyle {
     ];
   }
 
+  _hasBorder() {
+    return this._lineWidthStyleProperty.isDynamic()
+      ? this._lineWidthStyleProperty.isComplete()
+      : this._lineWidthStyleProperty.getOptions().size !== 0;
+  }
+
   renderEditor({ layer, onStyleDescriptorChange }) {
     const rawProperties = this.getRawProperties();
     const handlePropertyChange = (propertyName, settings) => {
@@ -170,6 +176,7 @@ export class VectorStyle extends AbstractStyle {
         onIsTimeAwareChange={onIsTimeAwareChange}
         isTimeAware={this.isTimeAware()}
         showIsTimeAware={propertiesWithFieldMeta.length > 0}
+        hasBorder={this._hasBorder()}
       />
     );
   }
@@ -423,12 +430,18 @@ export class VectorStyle extends AbstractStyle {
 
   getIcon = () => {
     const isLinesOnly = this._getIsLinesOnly();
-    const strokeColor = isLinesOnly
-      ? extractColorFromStyleProperty(this._descriptor.properties[VECTOR_STYLES.LINE_COLOR], 'grey')
-      : extractColorFromStyleProperty(
-          this._descriptor.properties[VECTOR_STYLES.LINE_COLOR],
-          'none'
-        );
+    let strokeColor;
+    if (isLinesOnly) {
+      strokeColor = extractColorFromStyleProperty(
+        this._descriptor.properties[VECTOR_STYLES.LINE_COLOR],
+        'grey'
+      );
+    } else if (this._hasBorder()) {
+      strokeColor = extractColorFromStyleProperty(
+        this._descriptor.properties[VECTOR_STYLES.LINE_COLOR],
+        'none'
+      );
+    }
     const fillColor = isLinesOnly
       ? null
       : extractColorFromStyleProperty(