Page MenuHomePhabricator

Settle on SVGO configuration for both the raw SVG files and the transformed (JSON/ES/CommonJS) ones
Closed, ResolvedPublic

Description

Goal

Provide best-in-class SVGO configuration for raw icon files and transformed SVGs

Background

We provide Wikimedia SVG coding guidelines that are partly enforced by help of SVGO

Questions

Reciting @Catrope from internal conversation leading up to this

The current icon system stores icons as .svg files that are relatively minimal, but still have a <title> tag (first image), but in the build output that gets minified to basically just one or more <path> tags (second image). The <title> tag is removed, as is the wrapping <svg> tag and the <?xml> tag. When the <cdx-icon> component uses this icon data, it adds its own <svg> and (optionally) <title> tags based on the props passed to it.
This minification step currently uses SVGO in its default configuration, plus a setting that prefixes IDs for uniqueness, plus a plugin I wrote that removes the <svg> tag. The main function of this step is to unwrap the SVG to just the <path> (or other shape) tags and deconflict the IDs, but since it uses SVGO it can do other optimizations too. And that could free us up to have the .svg files in the repo be slightly less minimal. For example:
IDs wouldn't need to be a, b, etc, they could be meaningful things like bracket, and that would still be transformed to cdx-icon-code-a in the output
We could have unused IDs to explain what different shapes are; they would be removed in the output
We could have <!-- comments --> in our SVGs
or anything else, if it would make our SVGs easier to read/edit
I'm not saying anything needs to change here, but I wanted to point out the opportunity and ask if it's something we want to do (now or in the future)

Criteria for done

  • Decide how much extra code additions (ids for readability, comments, etc.) we want to include in the raw SVG files
  • Add SVGO tailored config for raw SVG files, orienting on Wikimedia SVG coding conventions
  • Add SVGO tailored config for transformed (JSON/ES/CommonJS) ones
  • Document in library

Event Timeline

We actually have the icon sources in the public access Figma file, where we should put all non-code specific comments.

From my personal experience, I have not seen ids as specifically helpful as the creation overhead for them in code outweighs the benefits.
Mind, that our Codex icons all are exported by Figma, where we already rely on third party software for the code. We're (@bmartinezcalvo @Sarai-WMDE and myself) are currently attempting to add a Figma plugin following the shared Wikimedia SVG guidelines for initial export there. In any case most SVG code documentation would have to be correctly identified and added manually later on, would have to be re-added every time an icon changes. With all that, I wouldn't bother going down that route.

The other point about recent SVGO change resulting in slightly bigger (few bytes per icon) optimized output digged up by @thiemowmde has an actionable point from my perspective.
Even though we've got a great icons overview page, it's sometimes helpful for a quick design or icon amendment to start with Codex raw files, specifically for people who are not closely involved in design everyday.
The change is providing popular software like Inkscape or Adobe Illustrator with undistorted icons for their years old SVG render libs, therefore I'd suggest relying on default SVGO coding Wikimedia conventions including the slightly bigger raw files and add the plugins for the smaller path output back to the transform. (I'd also accept the slightly bigger defaults in OOUI and change only, if at all, the dist SVGO build script).

Change 755847 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] build, icons: Optimize raw file SVGO config further and re-crush icons

https://gerrit.wikimedia.org/r/755847

Sketch software beforeafter patch with better backwards-compatible SVGO output
image.png (264×260 px, 6 KB)
image.png (338×354 px, 7 KB)

(sorry, was only darkgrey background available)
Example: https://gerrit.wikimedia.org/r/c/design/codex/+/755847/1/packages/icons/src/images/alert.svg – 4 bytes more uncompressed

Change 755847 merged by jenkins-bot:

[design/codex@main] build, icons: Optimize raw file SVGO config further and re-crush icons

https://gerrit.wikimedia.org/r/755847

From my personal experience, I have not seen ids as specifically helpful as the creation overhead for them in code outweighs the benefits.

What do you think about situations where a path has to have an ID because it's reused? In that situation, would you want the ID to be something human-readable, or should it just be 'a', 'b', etc?

and add the plugins for the smaller path output back to the transform

Yes, let's do that. Can you identify what those plugins are, and how they change the output?

From my personal experience, I have not seen ids as specifically helpful as the creation overhead for them in code outweighs the benefits.

What do you think about situations where a path has to have an ID because it's reused? In that situation, would you want the ID to be something human-readable, or should it just be 'a', 'b', etc?

In our simple, small canvas icon SVGs, we don't have and won't in future need to deal a whole lot (currently 5 out of 200+ icons) with using transforms.
Among the most advanced examples is currently 'settings.svg':

<g transform="translate(10 10)">
	<path id="a" d="M1.5-10h-3l-1 6.5h5m0 7h-5l1 6.5h3"/>
	<use xlink:href="#a" transform="rotate(45)"/>
	<use xlink:href="#a" transform="rotate(90)"/>
	<use xlink:href="#a" transform="rotate(135)"/>
</g>

So only one id equal a is currently needed in all our icons collection for transform. I don't see a real win here for better human readability.

and add the plugins for the smaller path output back to the transform

Yes, let's do that. Can you identify what those plugins are, and how they change the output?

The main problem I've tried to clarify is use a DRY approach of reusing the default SVGO config in 'svgTransform.js' but loadConfig didn't provide in my own attempts.
I've been thinking of loading default config file and overriding several plugins, but maybe it's just a matter of turning the script loading order on the head (put general minification before 'svgTransform'). Inputs welcome!

Change 758547 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] build, icons: Amend Vite transform SVG build step to further optimize

https://gerrit.wikimedia.org/r/758547

Change 758547 merged by jenkins-bot:

[design/codex@main] build, icons: Amend Vite transform SVG build step to further optimize

https://gerrit.wikimedia.org/r/758547

Volker_E claimed this task.
Volker_E moved this task from Inbox to Done on the Codex board.
Volker_E removed a project: Patch-For-Review.