yoast-components icon indicating copy to clipboard operation
yoast-components copied to clipboard

Make SVGicon.js work better

Open hedgefield opened this issue 6 years ago • 3 comments

We've run into some issues with the way we handle SVGs lately.

SVG icons are expected in a certain formatting

What is expected is something like: <svg><path d="M13.75 7q0 1.375-0.535 2.625t-1.438 2.152-2.152 1.438-2.625 0.535-2.625-0.535-2.152-1.438-1.438-2.152-0.535-2.625q0-1.664 0.758-3.113t2.070-2.387 2.922-1.18v1.781q-1.727 0.352-2.863 1.727t-1.137 3.172q0 1.016 0.398 1.941t1.066 1.594 1.594 1.066 1.941 0.398 1.941-0.398 1.594-1.066 1.066-1.594 0.398-1.941q0-1.797-1.137-3.172t-2.863-1.727v-1.781q1.609 0.242 2.922 1.18t2.070 2.387 0.758 3.113z"></path></svg>

While a cleaner and more readable version cannot be used: <svg><circle class="path" fill="none" stroke-width="8" stroke-linecap="round" cx="33" cy="33" r="28"></circle></svg>

Which means instead of this spinner we were forced to use this spinner...

SVG icons cannot have more than one path

Icons are listed in the file by their path contents, but only one can be specified per icon. Usually not a problem, but when the OK score smiley needed black eyes and mouth to pass WCAG contrast, we could not add it to the SVG, because those were extra paths. Instead a dirty CSS hack had to be applied, which is not what you want.

SVG colors and shape are separated

The path contents define the shape of the SVG, but the colors are managed somewhere else because they are CSS. It makes it harder to make small adjustments. For the design team it would be more ideal if there was a folder that actually contained the SVG icon files, so that we don't have to pass them through a dev each time a new one is added or an existing one needs to be tweaked.

Viewboxes need to be the same

It's always good practice to have icons be the same size, but sometimes an inconsistent viewbox can slip through, and now there has been some code built to allow an override, that shouldn't have had to be necessary.

hedgefield avatar Aug 09 '18 14:08 hedgefield

it would be more ideal if there was a folder that actually contained the SVG icon files

WIth webpack, react, and styled-components we can't have physical SVG files. Instead, the SVG icons are rendered as react components. So I'm afraid this part can't be done. /Cc @atimmer

Instead, starting with https://github.com/Yoast/yoast-components/pull/704 we can now have more than one element within an SVG or one that is not a <path>. The initial implementation assumed the SVG were always made by one single <path> and so it was a bit limited. Now we can pass, for example, a <circle> or even more than one element.

CSS: there's no good way to style the details of our SVG icons with CSS so far. This would require some investigation.

Viewbox: it would be great to have always the same viewbox but this means the SVG icon must be shaped based on that viewbox when it gets created. It' snot possible to use an existing shape and expecting it works properly on a different viewport.

afercia avatar Aug 09 '18 17:08 afercia

WIth webpack, react, and styled-components we can't have physical SVG files. Instead, the SVG icons are rendered as react components. So I'm afraid this part can't be done. /Cc @atimmer

I am sure it can be done. We can always write a custom webpack loader that loads the SVG file in the way we need it to.

That being said, it is probably more of a nice-to-have than a must-have.

atimmer avatar Aug 20 '18 11:08 atimmer

For reference, the whole purpose of the SvgIcon component was to "start using svg react component instead of svg files", discussed on Slack on January 31st (2018).

afercia avatar Sep 05 '18 08:09 afercia