[Maps] Apply styles to icon SVGs in legend via descendent selectors #129255

nickpeihl · 2022-04-01T20:18:15Z

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 apply and override any fill and stroke settings on sibling and nested elements.

The vector-effect: non-scaling-stroke property fixes the issue where the stroke was barely visible in the legend with large custom icons.

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.

To test this:

Open the [Flights] Origin Time Delayed map from the Sample flight data
In Map settings, add a custom icon using an SVG with a width/height or viewBox much greater than 15px that also has style attributes applied to the descendent elements. This Shoes walking is a good example.
Change the Flight Origin Location layer Symbol type to icon and select the custom icon
Change the Fill and Border colors to solid colors.
Verify the color in the Legend matches the selected colors.

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.

elasticmachine · 2022-04-01T20:18:17Z

Pinging @elastic/kibana-gis (Team:Geo)

…-fix'

nreese · 2022-04-04T14:29:47Z

x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js

-    style += `stroke-width:1;`;
-  }
-  if (style) svgXml.svg.$.style = style;
+  svgXml.svg.$.id = symbolId;


Can you make the id a uuid so there can be no conflict.

nreese · 2022-04-04T14:44:48Z

x-pack/plugins/maps/public/classes/styles/vector/symbol_utils.js

-  if (style) svgXml.svg.$.style = style;
+  svgXml.svg.$.id = symbolId;
+  svgXml.svg.style = `
+    #${symbolId} * {


Can you add a comment about why css selector is needed, maybe something like
// 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

nreese

LGTM. Thanks for fixing. Thanks for also moving custom icons to top of map settings and fixing issue where custom icon includes style attributes in children elements.

code review, tested in chrome

nreese · 2022-04-04T14:56:34Z

Actually, this creates problems for icons styled by a single color. In the screen shot below, notice how the shoe icon is black and not fill color of the layer

…icons-legend-fix

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.

…l/kibana into custom-icons-legend-fix

nickpeihl · 2022-04-05T14:48:39Z

Actually, this creates problems for icons styled by a single color. In the screen shot below, notice how the shoe icon is black and not fill color of the layer

8cb8a11 should fix this.

…-fix'

nreese

LGTM - Thanks for all the fixes.
code review, tested in chrome

nickpeihl · 2022-04-08T19:48:04Z

@elasticmachine merge upstream

kibana-ci · 2022-04-08T21:42:40Z

💚 Build Succeeded

Test Failures

[job] [logs] OSS CI Group #11 / visualize app visualize ciGroup11 visual builder Time Series basics Clicking on the chart should create a filter for series with multiple split by terms fields one of which has formatting

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`maps`	2.7MB	2.7MB	+122.0B

History

💚 Build #36203 succeeded c53ed15
💔 Build #35659 failed 2a6283b
💔 Build #35650 failed b6216ac
💔 Build #35649 failed 01b0b9e

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…129255) (cherry picked from commit 7b64a54)

kibanamachine · 2022-04-08T21:46:07Z

💚 All backports created successfully

Status	Branch	Result
✅	8.2

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…129255) (#129853) (cherry picked from commit 7b64a54) Co-authored-by: Nick Peihl <[email protected]>

nickpeihl added 2 commits April 1, 2022 16:10

Move custom icons to top of map settings panel

01b0b9e

nickpeihl added [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed v8.2.0 v8.3.0 labels Apr 1, 2022

nickpeihl requested a review from a team as a code owner April 1, 2022 20:18

nickpeihl and others added 2 commits April 1, 2022 16:18

Merge branch 'main' into custom-icons-legend-fix

b6216ac

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

2a6283b

…-fix'

nreese reviewed Apr 4, 2022

View reviewed changes

nreese approved these changes Apr 4, 2022

View reviewed changes

nreese self-requested a review April 4, 2022 14:56

nickpeihl added 3 commits April 5, 2022 10:24

Merge branch 'main' of https://github.com/elastic/kibana into custom-…

526d01e

…icons-legend-fix

Merge branch 'custom-icons-legend-fix' of https://github.com/nickpeih…

7617d9b

…l/kibana into custom-icons-legend-fix

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

c53ed15

…-fix'

nreese approved these changes Apr 6, 2022

View reviewed changes

nickpeihl enabled auto-merge (squash) April 8, 2022 19:47

Merge branch 'main' into custom-icons-legend-fix

a80af08

nickpeihl merged commit 7b64a54 into elastic:main Apr 8, 2022

kibanamachine mentioned this pull request Apr 8, 2022

[8.2] [Maps] Apply styles to icon SVGs in legend via descendent selectors (#129255) #129853

Merged

kibanamachine pushed a commit that referenced this pull request Apr 8, 2022

[Maps] Apply styles to icon SVGs in legend via descendent selectors (#…

c069a28

…129255) (cherry picked from commit 7b64a54)

kibanamachine added a commit that referenced this pull request Apr 8, 2022

[Maps] Apply styles to icon SVGs in legend via descendent selectors (#…

dac04e4

…129255) (#129853) (cherry picked from commit 7b64a54) Co-authored-by: Nick Peihl <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Maps] Apply styles to icon SVGs in legend via descendent selectors #129255

[Maps] Apply styles to icon SVGs in legend via descendent selectors #129255

nickpeihl commented Apr 1, 2022 •

edited

Loading

elasticmachine commented Apr 1, 2022

nreese Apr 4, 2022

nreese Apr 4, 2022 •

edited

Loading

nreese left a comment •

edited

Loading

nreese commented Apr 4, 2022

nickpeihl commented Apr 5, 2022

nreese left a comment

nickpeihl commented Apr 8, 2022

kibana-ci commented Apr 8, 2022

kibanamachine commented Apr 8, 2022

[Maps] Apply styles to icon SVGs in legend via descendent selectors #129255

[Maps] Apply styles to icon SVGs in legend via descendent selectors #129255

Conversation

nickpeihl commented Apr 1, 2022 • edited Loading

elasticmachine commented Apr 1, 2022

nreese Apr 4, 2022

Choose a reason for hiding this comment

nreese Apr 4, 2022 • edited Loading

Choose a reason for hiding this comment

nreese left a comment • edited Loading

Choose a reason for hiding this comment

nreese commented Apr 4, 2022

nickpeihl commented Apr 5, 2022

nreese left a comment

Choose a reason for hiding this comment

nickpeihl commented Apr 8, 2022

kibana-ci commented Apr 8, 2022

💚 Build Succeeded

Test Failures

Metrics [docs]

Async chunks

History

kibanamachine commented Apr 8, 2022

💚 All backports created successfully

Questions ?

nickpeihl commented Apr 1, 2022 •

edited

Loading

nreese Apr 4, 2022 •

edited

Loading

nreese left a comment •

edited

Loading