From cee4186aea7670ee2eda91eaddcd27481331c3ca Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Fri, 9 Aug 2019 15:25:09 -0700 Subject: [PATCH 1/4] [Maps] Enable borders on icons --- .../__snapshots__/vector_icon.test.js.snap | 2 ++ .../components/vector/legend/symbol_icon.js | 18 +++++++---- .../components/vector/legend/vector_icon.js | 2 ++ .../components/vector/vector_style_editor.js | 31 +++++++++---------- .../vector/vector_style_symbol_editor.js | 2 ++ .../maps/public/layers/styles/symbol_utils.js | 12 +++++-- .../public/layers/styles/symbol_utils.test.js | 20 ++++++++++-- .../maps/public/layers/styles/vector_style.js | 4 +++ 8 files changed, 63 insertions(+), 28 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/__snapshots__/vector_icon.test.js.snap b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/__snapshots__/vector_icon.test.js.snap index 553e1471b61bc..57368b52a2bce 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/__snapshots__/vector_icon.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/__snapshots__/vector_icon.test.js.snap @@ -38,6 +38,8 @@ exports[`Renders PolygonIcon with correct styles when not line only or not point exports[`Renders SymbolIcon with correct styles when isPointOnly and symbolId provided 1`] = ` `; diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js index 1cff6003e291a..7f16746138de1 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js @@ -15,31 +15,35 @@ export class SymbolIcon extends Component { imgDataUrl: undefined, prevSymbolId: undefined, prevFill: undefined, + prevStroke: undefined, + prevStrokeWidth: undefined, } componentDidMount() { this._isMounted = true; - this._loadSymbol(this.props.symbolId, this.props.fill); + this._loadSymbol(this.props.symbolId, this.props.fill, this.props.stroke, this.props.strokeWidth); } componentDidUpdate() { - this._loadSymbol(this.props.symbolId, this.props.fill); + this._loadSymbol(this.props.symbolId, this.props.fill, this.props.stroke, this.props.strokeWidth); } componentWillUnmount() { this._isMounted = false; } - async _loadSymbol(nextSymbolId, nextFill) { + async _loadSymbol(nextSymbolId, nextFill, nextStroke, nextStrokeWidth) { if (nextSymbolId === this.state.prevSymbolId - && nextFill === this.state.prevFill) { + && nextFill === this.state.prevFill + && nextStroke === this.state.prevStroke + && nextStrokeWidth === this.state.prevStrokeWidth) { return; } let imgDataUrl; try { const svg = getMakiSymbolSvg(nextSymbolId); - const styledSvg = await styleSvg(svg, nextFill); + const styledSvg = await styleSvg(svg, nextFill, nextStroke, nextStrokeWidth); imgDataUrl = buildSrcUrl(styledSvg); } catch (error) { // ignore failures - component will just not display an icon @@ -49,7 +53,9 @@ export class SymbolIcon extends Component { this.setState({ imgDataUrl, prevSymbolId: nextSymbolId, - prevFill: nextFill + prevFill: nextFill, + prevStroke: nextStroke, + prevStrokeWidth: nextStrokeWidth }); } } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/vector_icon.js b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/vector_icon.js index 64f97722c4df2..9659914562f2c 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/vector_icon.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/vector_icon.js @@ -75,6 +75,8 @@ export class VectorIcon extends Component { ); } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js index 44977f5898990..1941d0b2d8d4c 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js @@ -14,7 +14,7 @@ import { VectorStyleSymbolEditor } from './vector_style_symbol_editor'; import { OrientationEditor } from './orientation/orientation_editor'; import { getDefaultDynamicProperties, getDefaultStaticProperties } from '../../vector_style_defaults'; import { VECTOR_SHAPE_TYPES } from '../../../sources/vector_feature_types'; -import { SYMBOLIZE_AS_CIRCLE } from '../../vector_constants'; +import { SYMBOLIZE_AS_ICON } from '../../vector_constants'; import { i18n } from '@kbn/i18n'; import { SYMBOL_OPTIONS } from '../../symbol_utils'; @@ -140,23 +140,20 @@ export class VectorStyleEditor extends Component { } _renderPointProperties() { - let lineColor; - let lineWidth; let iconOrientation; - if (this.props.styleProperties.symbol.options.symbolizeAs === SYMBOLIZE_AS_CIRCLE) { - lineColor = ( - - {this._renderLineColor()} - - - ); - lineWidth = ( - - {this._renderLineWidth()} - - - ); - } else { + const lineColor = ( + + {this._renderLineColor()} + + + ); + const lineWidth = ( + + {this._renderLineWidth()} + + + ); + if (this.props.styleProperties.symbol.options.symbolizeAs === SYMBOLIZE_AS_ICON) { iconOrientation = ( diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.js b/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.js index a32ae8d414b46..22e1a6aea6a72 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.js @@ -62,11 +62,19 @@ export function buildSrcUrl(svgString) { return domUrl.createObjectURL(svg); } -export async function styleSvg(svgString, fill) { +export async function styleSvg(svgString, fill, stroke, strokeWidth) { const svgXml = await parseXmlString(svgString); + let style = ''; if (fill) { - svgXml.svg.$.style = `fill: ${fill};`; + style += `fill:${fill};`; } + if (stroke) { + style += `stroke:${stroke};`; + } + if (strokeWidth) { + style += `stroke-width:${strokeWidth};`; + } + if (style) svgXml.svg.$.style = style; const builder = new xml2js.Builder(); return builder.buildObject(svgXml); } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.test.js index b62f385680daa..66752460331c9 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.test.js @@ -14,16 +14,30 @@ describe('getMakiSymbolSvg', () => { }); describe('styleSvg', () => { - it('Should not add style property when fill not provided', async () => { + it('Should not add style property when style not provided', async () => { const unstyledSvgString = ''; const styledSvg = await styleSvg(unstyledSvgString); expect(styledSvg.split('\n')[1]).toBe(''); }); - it('Should add style property to svg element', async () => { + it('Should add fill style property to svg element', async () => { const unstyledSvgString = ''; const styledSvg = await styleSvg(unstyledSvgString, 'red'); // eslint-disable-next-line max-len - expect(styledSvg.split('\n')[1]).toBe(''); + expect(styledSvg.split('\n')[1]).toBe(''); + }); + + it('Should add stroke style property to svg element', async () => { + const unstyledSvgString = ''; + const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white'); + // eslint-disable-next-line max-len + expect(styledSvg.split('\n')[1]).toBe(''); + }); + + it('Should add stroke-width style property to svg element', async () => { + const unstyledSvgString = ''; + const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white', '2px'); + // eslint-disable-next-line max-len + expect(styledSvg.split('\n')[1]).toBe(''); }); }); diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector_style.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector_style.js index 44989ef133b36..46832240e5922 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector_style.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector_style.js @@ -546,8 +546,12 @@ export class VectorStyle extends AbstractStyle { const symbolId = this._descriptor.properties.symbol.options.symbolId; mbMap.setLayoutProperty(symbolLayerId, 'icon-anchor', getMakiSymbolAnchor(symbolId)); const color = this._getMBColor(this._descriptor.properties.fillColor); + const haloColor = this._getMBColor(this._descriptor.properties.lineColor); + const haloWidth = this._getMbSize(this._descriptor.properties.lineWidth); // icon-color is only supported on SDF icons. mbMap.setPaintProperty(symbolLayerId, 'icon-color', color); + mbMap.setPaintProperty(symbolLayerId, 'icon-halo-color', haloColor); + mbMap.setPaintProperty(symbolLayerId, 'icon-halo-width', haloWidth); mbMap.setPaintProperty(symbolLayerId, 'icon-opacity', alpha); // circle sizing is by radius From 6f7e3e8bede0aae14dae8beb1cedc731dff7eed7 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Fri, 9 Aug 2019 15:25:09 -0700 Subject: [PATCH 2/4] [Maps] Enable borders on icons --- .../__snapshots__/vector_icon.test.js.snap | 2 ++ .../components/vector/legend/symbol_icon.js | 18 +++++++---- .../components/vector/legend/vector_icon.js | 2 ++ .../components/vector/vector_style_editor.js | 31 +++++++++---------- .../vector/vector_style_symbol_editor.js | 2 ++ .../maps/public/layers/styles/symbol_utils.js | 12 +++++-- .../public/layers/styles/symbol_utils.test.js | 20 ++++++++++-- .../maps/public/layers/styles/vector_style.js | 4 +++ 8 files changed, 63 insertions(+), 28 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/__snapshots__/vector_icon.test.js.snap b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/__snapshots__/vector_icon.test.js.snap index 553e1471b61bc..57368b52a2bce 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/__snapshots__/vector_icon.test.js.snap +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/__snapshots__/vector_icon.test.js.snap @@ -38,6 +38,8 @@ exports[`Renders PolygonIcon with correct styles when not line only or not point exports[`Renders SymbolIcon with correct styles when isPointOnly and symbolId provided 1`] = ` `; diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js index 1cff6003e291a..7f16746138de1 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js @@ -15,31 +15,35 @@ export class SymbolIcon extends Component { imgDataUrl: undefined, prevSymbolId: undefined, prevFill: undefined, + prevStroke: undefined, + prevStrokeWidth: undefined, } componentDidMount() { this._isMounted = true; - this._loadSymbol(this.props.symbolId, this.props.fill); + this._loadSymbol(this.props.symbolId, this.props.fill, this.props.stroke, this.props.strokeWidth); } componentDidUpdate() { - this._loadSymbol(this.props.symbolId, this.props.fill); + this._loadSymbol(this.props.symbolId, this.props.fill, this.props.stroke, this.props.strokeWidth); } componentWillUnmount() { this._isMounted = false; } - async _loadSymbol(nextSymbolId, nextFill) { + async _loadSymbol(nextSymbolId, nextFill, nextStroke, nextStrokeWidth) { if (nextSymbolId === this.state.prevSymbolId - && nextFill === this.state.prevFill) { + && nextFill === this.state.prevFill + && nextStroke === this.state.prevStroke + && nextStrokeWidth === this.state.prevStrokeWidth) { return; } let imgDataUrl; try { const svg = getMakiSymbolSvg(nextSymbolId); - const styledSvg = await styleSvg(svg, nextFill); + const styledSvg = await styleSvg(svg, nextFill, nextStroke, nextStrokeWidth); imgDataUrl = buildSrcUrl(styledSvg); } catch (error) { // ignore failures - component will just not display an icon @@ -49,7 +53,9 @@ export class SymbolIcon extends Component { this.setState({ imgDataUrl, prevSymbolId: nextSymbolId, - prevFill: nextFill + prevFill: nextFill, + prevStroke: nextStroke, + prevStrokeWidth: nextStrokeWidth }); } } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/vector_icon.js b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/vector_icon.js index 64f97722c4df2..9659914562f2c 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/vector_icon.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/vector_icon.js @@ -75,6 +75,8 @@ export class VectorIcon extends Component { ); } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js index 44977f5898990..1941d0b2d8d4c 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js @@ -14,7 +14,7 @@ import { VectorStyleSymbolEditor } from './vector_style_symbol_editor'; import { OrientationEditor } from './orientation/orientation_editor'; import { getDefaultDynamicProperties, getDefaultStaticProperties } from '../../vector_style_defaults'; import { VECTOR_SHAPE_TYPES } from '../../../sources/vector_feature_types'; -import { SYMBOLIZE_AS_CIRCLE } from '../../vector_constants'; +import { SYMBOLIZE_AS_ICON } from '../../vector_constants'; import { i18n } from '@kbn/i18n'; import { SYMBOL_OPTIONS } from '../../symbol_utils'; @@ -140,23 +140,20 @@ export class VectorStyleEditor extends Component { } _renderPointProperties() { - let lineColor; - let lineWidth; let iconOrientation; - if (this.props.styleProperties.symbol.options.symbolizeAs === SYMBOLIZE_AS_CIRCLE) { - lineColor = ( - - {this._renderLineColor()} - - - ); - lineWidth = ( - - {this._renderLineWidth()} - - - ); - } else { + const lineColor = ( + + {this._renderLineColor()} + + + ); + const lineWidth = ( + + {this._renderLineWidth()} + + + ); + if (this.props.styleProperties.symbol.options.symbolizeAs === SYMBOLIZE_AS_ICON) { iconOrientation = ( diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.js b/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.js index a32ae8d414b46..22e1a6aea6a72 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.js @@ -62,11 +62,19 @@ export function buildSrcUrl(svgString) { return domUrl.createObjectURL(svg); } -export async function styleSvg(svgString, fill) { +export async function styleSvg(svgString, fill, stroke, strokeWidth) { const svgXml = await parseXmlString(svgString); + let style = ''; if (fill) { - svgXml.svg.$.style = `fill: ${fill};`; + style += `fill:${fill};`; } + if (stroke) { + style += `stroke:${stroke};`; + } + if (strokeWidth) { + style += `stroke-width:${strokeWidth};`; + } + if (style) svgXml.svg.$.style = style; const builder = new xml2js.Builder(); return builder.buildObject(svgXml); } diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.test.js b/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.test.js index b62f385680daa..66752460331c9 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.test.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/symbol_utils.test.js @@ -14,16 +14,30 @@ describe('getMakiSymbolSvg', () => { }); describe('styleSvg', () => { - it('Should not add style property when fill not provided', async () => { + it('Should not add style property when style not provided', async () => { const unstyledSvgString = ''; const styledSvg = await styleSvg(unstyledSvgString); expect(styledSvg.split('\n')[1]).toBe(''); }); - it('Should add style property to svg element', async () => { + it('Should add fill style property to svg element', async () => { const unstyledSvgString = ''; const styledSvg = await styleSvg(unstyledSvgString, 'red'); // eslint-disable-next-line max-len - expect(styledSvg.split('\n')[1]).toBe(''); + expect(styledSvg.split('\n')[1]).toBe(''); + }); + + it('Should add stroke style property to svg element', async () => { + const unstyledSvgString = ''; + const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white'); + // eslint-disable-next-line max-len + expect(styledSvg.split('\n')[1]).toBe(''); + }); + + it('Should add stroke-width style property to svg element', async () => { + const unstyledSvgString = ''; + const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white', '2px'); + // eslint-disable-next-line max-len + expect(styledSvg.split('\n')[1]).toBe(''); }); }); diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/vector_style.js b/x-pack/legacy/plugins/maps/public/layers/styles/vector_style.js index 44989ef133b36..46832240e5922 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/vector_style.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/vector_style.js @@ -546,8 +546,12 @@ export class VectorStyle extends AbstractStyle { const symbolId = this._descriptor.properties.symbol.options.symbolId; mbMap.setLayoutProperty(symbolLayerId, 'icon-anchor', getMakiSymbolAnchor(symbolId)); const color = this._getMBColor(this._descriptor.properties.fillColor); + const haloColor = this._getMBColor(this._descriptor.properties.lineColor); + const haloWidth = this._getMbSize(this._descriptor.properties.lineWidth); // icon-color is only supported on SDF icons. mbMap.setPaintProperty(symbolLayerId, 'icon-color', color); + mbMap.setPaintProperty(symbolLayerId, 'icon-halo-color', haloColor); + mbMap.setPaintProperty(symbolLayerId, 'icon-halo-width', haloWidth); mbMap.setPaintProperty(symbolLayerId, 'icon-opacity', alpha); // circle sizing is by radius From 048837f73274dc97007f8b5abeb1ee1b3e2fe4f4 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Mon, 12 Aug 2019 09:06:12 -0700 Subject: [PATCH 3/4] Review feedback --- docs/maps/vector-style-properties.asciidoc | 4 ++++ .../components/vector/legend/symbol_icon.js | 2 ++ .../components/vector/vector_style_editor.js | 22 +++++++------------ 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/docs/maps/vector-style-properties.asciidoc b/docs/maps/vector-style-properties.asciidoc index 9cfd59f967df5..4d8d0fd6603af 100644 --- a/docs/maps/vector-style-properties.asciidoc +++ b/docs/maps/vector-style-properties.asciidoc @@ -24,6 +24,10 @@ Use *Icon* to symbolize Points as icons. *Fill color*:: The fill color of the point features. +*Border color*:: The border color of the point features. + +*Border width*:: The border width of the point features. + *Symbol orientation*:: The symbol orientation rotating the icon clockwise. *Symbol size*:: The radius of the symbol size, in pixels. diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js index 7f16746138de1..a2f8d44604a0a 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/legend/symbol_icon.js @@ -74,4 +74,6 @@ export class SymbolIcon extends Component { SymbolIcon.propTypes = { symbolId: PropTypes.string.isRequired, fill: PropTypes.string.isRequired, + stroke: PropTypes.string.isRequired, + strokeWidth: PropTypes.string.isRequired }; diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js index 1941d0b2d8d4c..d9a808876f33d 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js @@ -141,18 +141,6 @@ export class VectorStyleEditor extends Component { _renderPointProperties() { let iconOrientation; - const lineColor = ( - - {this._renderLineColor()} - - - ); - const lineWidth = ( - - {this._renderLineWidth()} - - - ); if (this.props.styleProperties.symbol.options.symbolizeAs === SYMBOLIZE_AS_ICON) { iconOrientation = ( @@ -182,9 +170,15 @@ export class VectorStyleEditor extends Component { {this._renderFillColor()} - {lineColor} + + {this._renderLineColor()} + + - {lineWidth} + + {this._renderLineWidth()} + + {iconOrientation} From 27cbed2f348b9e3509a29dce6d7926d48f4959a7 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Mon, 12 Aug 2019 09:45:02 -0700 Subject: [PATCH 4/4] Remove unnecessary Fragment --- .../styles/components/vector/vector_style_editor.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js index d9a808876f33d..f5394f6b63567 100644 --- a/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js +++ b/x-pack/legacy/plugins/maps/public/layers/styles/components/vector/vector_style_editor.js @@ -170,15 +170,11 @@ export class VectorStyleEditor extends Component { {this._renderFillColor()} - - {this._renderLineColor()} - - + {this._renderLineColor()} + - - {this._renderLineWidth()} - - + {this._renderLineWidth()} + {iconOrientation}