vaadin / web-components

A set of high-quality standards based web components for enterprise web applications. Part of Vaadin 20+
https://vaadin.com/docs/latest/components
426 stars 81 forks source link

[icon] Support for using strokes in addition to fills in icon paths #4312

Closed jouni closed 1 year ago

jouni commented 1 year ago

Describe your motivation

Many 3rd party icon sets use stroke="currentColor" and stroke-width="2" when defining icon paths/shapes, instead of solid paths that use fill="currentColor".

Currently it’s not easy to use such icons together with <vaadin-icon>, because it assumes all paths are filled.

Describe the solution you'd like

Respect the fill, stroke, and stroke-width attributes defined on the <g> elements in the <vaadin-iconset> definitions.

For example, if I have the following iconset defined:

<vaadin-iconset name="myapp-outline" size="24">
  <svg><defs>
    <g id="myapp-outline:academic-cap" fill="none" stroke-width="2" stroke="currentColor"><path d="M12 14l9-5-9-5-9 5 9 5z"/><path d="M12 14l6.16-3.422a12.083 12.083 0 01.665 6.479A11.952 11.952 0 0012 20.055a11.952 11.952 0 00-6.824-2.998 12.078 12.078 0 01.665-6.479L12 14z"/><path stroke-linecap="round" stroke-linejoin="round" d="M12 14l9-5-9-5-9 5 9 5zm0 0l6.16-3.422a12.083 12.083 0 01.665 6.479A11.952 11.952 0 0012 20.055a11.952 11.952 0 00-6.824-2.998 12.078 12.078 0 01.665-6.479L12 14zm-4 6v-7.5l4-2.222"/></g>
  </defs></svg>
</vaadin-iconset>

When I use that icon with <vaadin-icon icon="myapp-outline:academic-cap">, the <svg> element that gets inserted to the DOM should include the attributes:

<svg fill="none" stroke-width="2" stroke="currentColor">

Describe alternatives you've considered

Allow defining the same attributes on the <vaadin-iconset fill="none" stroke="currentColor" stroke-width="2"> element and copy those over to the <svg> elements.

That would be a bit more limiting, as it assumes all icons in the set want the same strokes and that they are not filled.

Perhaps both could be used, and that attributes on the <g> elements would override the ones on the <vaadin-iconset> element.

Additional context

An icon set that uses strokes instead of filled paths: https://github.com/tailwindlabs/heroicons/tree/master/optimized/outline

jouni commented 1 year ago

If you want to use such stoke icons today, you will have to convert those strokes to filled paths using an SVG editor (e.g. “outline strokes”), and then export the SVG code again.

web-padawan commented 1 year ago

The logic was changed in #4164 so that the original SVG element is fully cloned, only the id attribute is removed. So this should no longer be an issue with the latest Vaadin 23.1 (as the fix was also backported to that version).

jouni commented 1 year ago

Oh, right, so it is possible to just put a bunch of <svg> elements inside the <defs> element, without the <g> element. Great, that does solve all the issues I had. Thanks!

jouni commented 1 year ago

Actually, need to wrap the original <svg> inside a <g id="..."> element. Then it works.