From 406c49a18eea9c836e4583b62734f0fa4d9b5c7d Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Fri, 1 Apr 2022 16:10:03 -0400 Subject: [PATCH 1/5] Apply styles to icon SVGs via descendent selectors The wildcard descendent selector to applies fill and stroke to all elements in the SVG. This was not necessary before because Maki icons always had a single path element. But we don't have control over the number of elements with custom icons, so we must override any style settings on sibling and nested elements. The `non-scaling-stroke` property fixes the issue where the stroke was barely visible in the legend with large custom icons. --- .../__snapshots__/symbol_utils.test.js.snap | 46 +++++++++++++++++++ .../vector/components/legend/symbol_icon.tsx | 4 +- .../classes/styles/vector/symbol_utils.js | 20 ++++---- .../styles/vector/symbol_utils.test.js | 18 +++----- 4 files changed, 64 insertions(+), 24 deletions(-) create mode 100644 x-pack/plugins/maps/public/classes/styles/vector/__snapshots__/symbol_utils.test.js.snap diff --git a/x-pack/plugins/maps/public/classes/styles/vector/__snapshots__/symbol_utils.test.js.snap b/x-pack/plugins/maps/public/classes/styles/vector/__snapshots__/symbol_utils.test.js.snap new file mode 100644 index 0000000000000..947d176a3fcf9 --- /dev/null +++ b/x-pack/plugins/maps/public/classes/styles/vector/__snapshots__/symbol_utils.test.js.snap @@ -0,0 +1,46 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`styleSvg Should add fill style property to svg element 1`] = ` +" + + + +" +`; + +exports[`styleSvg Should add stroke and stroke-wdth style properties to svg element 1`] = ` +" + + + +" +`; + +exports[`styleSvg Should not add style property when style not provided 1`] = ` +" + + + +" +`; diff --git a/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx b/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx index 4cc4d4169d7e0..836849dcf75bf 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx +++ b/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx @@ -7,7 +7,7 @@ import React, { Component, CSSProperties } from 'react'; // @ts-expect-error -import { CUSTOM_ICON_PREFIX_SDF, getSymbolSvg, styleSvg, buildSrcUrl } from '../../symbol_utils'; +import { styleSvg, buildSrcUrl } from '../../symbol_utils'; interface Props { symbolId: string; @@ -40,7 +40,7 @@ export class SymbolIcon extends Component { async _loadSymbol() { let imgDataUrl; try { - const styledSvg = await styleSvg(this.props.svg, this.props.fill, this.props.stroke); + const styledSvg = await styleSvg(this.props.svg, this.props.symbolId, this.props.fill, this.props.stroke); imgDataUrl = buildSrcUrl(styledSvg); } catch (error) { // ignore failures - component will just not display an icon diff --git a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js index af165863ffc9c..37dbe0ebb9cce 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js @@ -104,17 +104,17 @@ export function buildSrcUrl(svgString) { return domUrl.createObjectURL(svg); } -export async function styleSvg(svgString, fill, stroke) { +export async function styleSvg(svgString, symbolId, fill, stroke) { const svgXml = await parseXmlString(svgString); - let style = ''; - if (fill) { - style += `fill:${fill};`; - } - if (stroke) { - style += `stroke:${stroke};`; - style += `stroke-width:1;`; - } - if (style) svgXml.svg.$.style = style; + svgXml.svg.$.id = symbolId; + svgXml.svg.style = ` + #${symbolId} * { + ${fill ? `fill: ${fill}` : '#000'}; + ${stroke ? `stroke: ${stroke}` : '#000'}; + stroke-width: 1; + vector-effect: non-scaling-stroke; + } + `; const builder = new xml2js.Builder(); return builder.buildObject(svgXml); } diff --git a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js index 8c85702b19579..090e527643029 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js @@ -19,27 +19,21 @@ describe('styleSvg', () => { it('Should not add style property when style not provided', async () => { const unstyledSvgString = ''; - const styledSvg = await styleSvg(unstyledSvgString); - expect(styledSvg.split('\n')[1]).toBe( - '' - ); + const styledSvg = await styleSvg(unstyledSvgString, 'unstyled-symbol'); + expect(styledSvg).toMatchSnapshot(); }); it('Should add fill style property to svg element', async () => { const unstyledSvgString = ''; - const styledSvg = await styleSvg(unstyledSvgString, 'red'); - expect(styledSvg.split('\n')[1]).toBe( - '' - ); + const styledSvg = await styleSvg(unstyledSvgString, 'filled-symbol', 'red'); + expect(styledSvg).toMatchSnapshot(); }); it('Should add stroke and stroke-wdth style properties to svg element', async () => { const unstyledSvgString = ''; - const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white'); - expect(styledSvg.split('\n')[1]).toBe( - '' - ); + const styledSvg = await styleSvg(unstyledSvgString, 'filled-stroked-symbol', 'red', 'white'); + expect(styledSvg).toMatchSnapshot(); }); }); From 01b0b9e168cd7fd32755c54a2cd52df6e9de442e Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Fri, 1 Apr 2022 16:11:18 -0400 Subject: [PATCH 2/5] Move custom icons to top of map settings panel --- .../map_settings_panel/map_settings_panel.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/maps/public/connected_components/map_settings_panel/map_settings_panel.tsx b/x-pack/plugins/maps/public/connected_components/map_settings_panel/map_settings_panel.tsx index 1efa07e280039..005c40f48cce6 100644 --- a/x-pack/plugins/maps/public/connected_components/map_settings_panel/map_settings_panel.tsx +++ b/x-pack/plugins/maps/public/connected_components/map_settings_panel/map_settings_panel.tsx @@ -74,6 +74,12 @@ export function MapSettingsPanel({
+ + - -
From 2a6283bb8159e0b360d73593e3e8581793dd71c0 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Fri, 1 Apr 2022 21:02:09 +0000 Subject: [PATCH 3/5] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../styles/vector/components/legend/symbol_icon.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx b/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx index 836849dcf75bf..10cdd9d118365 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx +++ b/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx @@ -40,7 +40,12 @@ export class SymbolIcon extends Component { async _loadSymbol() { let imgDataUrl; try { - const styledSvg = await styleSvg(this.props.svg, this.props.symbolId, this.props.fill, this.props.stroke); + const styledSvg = await styleSvg( + this.props.svg, + this.props.symbolId, + this.props.fill, + this.props.stroke + ); imgDataUrl = buildSrcUrl(styledSvg); } catch (error) { // ignore failures - component will just not display an icon From 8cb8a114064e48e1a49a71b012616de4e4044618 Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Tue, 5 Apr 2022 10:37:06 -0400 Subject: [PATCH 4/5] Override fill and stroke styles on descendants I incorrectly assumed the style element would override style attributes on descendants. This changes the specificity to the svg element and all descendents. It appears the scope of the style element is limited to the SVG and does not affect other SVGs. Additionally, the !important property force overrides any style attributes in descendant elements. It is still possible that an element may have a style attribute with !important that we can not override. Hopefully that is a rare case though. --- .../__snapshots__/symbol_utils.test.js.snap | 53 ++++++++++++------- .../vector/components/legend/symbol_icon.tsx | 2 +- .../classes/styles/vector/symbol_utils.js | 16 +++--- .../styles/vector/symbol_utils.test.js | 20 +++++-- 4 files changed, 62 insertions(+), 29 deletions(-) diff --git a/x-pack/plugins/maps/public/classes/styles/vector/__snapshots__/symbol_utils.test.js.snap b/x-pack/plugins/maps/public/classes/styles/vector/__snapshots__/symbol_utils.test.js.snap index 947d176a3fcf9..d4936639523c3 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/__snapshots__/symbol_utils.test.js.snap +++ b/x-pack/plugins/maps/public/classes/styles/vector/__snapshots__/symbol_utils.test.js.snap @@ -2,14 +2,14 @@ exports[`styleSvg Should add fill style property to svg element 1`] = ` " - + " @@ -17,14 +17,14 @@ exports[`styleSvg Should add fill style property to svg element 1`] = ` exports[`styleSvg Should add stroke and stroke-wdth style properties to svg element 1`] = ` " - + " @@ -32,14 +32,31 @@ exports[`styleSvg Should add stroke and stroke-wdth style properties to svg elem exports[`styleSvg Should not add style property when style not provided 1`] = ` " - + +" +`; + +exports[`styleSvg Should override any inherent fill and stroke styles in SVGs 1`] = ` +" + + + + + " diff --git a/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx b/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx index 836849dcf75bf..fd9b952dbbdea 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx +++ b/x-pack/plugins/maps/public/classes/styles/vector/components/legend/symbol_icon.tsx @@ -40,7 +40,7 @@ export class SymbolIcon extends Component { async _loadSymbol() { let imgDataUrl; try { - const styledSvg = await styleSvg(this.props.svg, this.props.symbolId, this.props.fill, this.props.stroke); + const styledSvg = await styleSvg(this.props.svg, this.props.fill, this.props.stroke); imgDataUrl = buildSrcUrl(styledSvg); } catch (error) { // ignore failures - component will just not display an icon diff --git a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js index 37dbe0ebb9cce..108a2eb686dcf 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js @@ -104,15 +104,17 @@ export function buildSrcUrl(svgString) { return domUrl.createObjectURL(svg); } -export async function styleSvg(svgString, symbolId, fill, stroke) { +export async function styleSvg(svgString, fill, stroke) { const svgXml = await parseXmlString(svgString); - svgXml.svg.$.id = symbolId; + + // Elements nested under svg root may define style attribute + // Wildcard descendent selector provides more specificity to ensure root svg style attribute is applied instead of children style attributes svgXml.svg.style = ` - #${symbolId} * { - ${fill ? `fill: ${fill}` : '#000'}; - ${stroke ? `stroke: ${stroke}` : '#000'}; - stroke-width: 1; - vector-effect: non-scaling-stroke; + svg * { + ${fill ? `fill: ${fill}` : '#000'} !important; + ${stroke ? `stroke: ${stroke}` : '#000'} !important; + stroke-width: 1 !important; + vector-effect: non-scaling-stroke !important; } `; const builder = new xml2js.Builder(); diff --git a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js index 090e527643029..e5ee0bba6f7d1 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js @@ -19,21 +19,35 @@ describe('styleSvg', () => { it('Should not add style property when style not provided', async () => { const unstyledSvgString = ''; - const styledSvg = await styleSvg(unstyledSvgString, 'unstyled-symbol'); + const styledSvg = await styleSvg(unstyledSvgString); expect(styledSvg).toMatchSnapshot(); }); it('Should add fill style property to svg element', async () => { const unstyledSvgString = ''; - const styledSvg = await styleSvg(unstyledSvgString, 'filled-symbol', 'red'); + const styledSvg = await styleSvg(unstyledSvgString, 'red'); expect(styledSvg).toMatchSnapshot(); }); it('Should add stroke and stroke-wdth style properties to svg element', async () => { const unstyledSvgString = ''; - const styledSvg = await styleSvg(unstyledSvgString, 'filled-stroked-symbol', 'red', 'white'); + const styledSvg = await styleSvg(unstyledSvgString, 'red', 'white'); expect(styledSvg).toMatchSnapshot(); }); + + it('Should override any inherent fill and stroke styles in SVGs', async () => { + const unstyledSvgString = ` + + + + + + `; + + const styledSvg = await styleSvg(unstyledSvgString, 'blue', 'black'); + expect(styledSvg).toMatchSnapshot(); + }) }); + From c53ed15ab132675b21a3a1b3dee1517330a86994 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 5 Apr 2022 15:18:24 +0000 Subject: [PATCH 5/5] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../maps/public/classes/styles/vector/symbol_utils.test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js index e5ee0bba6f7d1..f6d40bd70dbea 100644 --- a/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js +++ b/x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.test.js @@ -48,6 +48,5 @@ describe('styleSvg', () => { const styledSvg = await styleSvg(unstyledSvgString, 'blue', 'black'); expect(styledSvg).toMatchSnapshot(); - }) + }); }); -