When deploying the new header we shipped a couple of non-optimised SVGS (see T159979).
To protect against this we should get Jenkins to check if svgo can optimise the SVGs and if so complain when they have not been optimised.
We do something similar to ensure compiled assets are checked in inside Popups.
Thoughts?
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | None | T162319 Tooling: Should not be possible to ship un-optimised SVGs | |||
| Resolved | ABorbaWMF | T162827 Tooling: MobileFrontend should check SVGS committed have been optimised. | |||
| Duplicate | Jdlrobson | T179361 Move MobileFrontend/Minerva's svg_check.sh script into a reusable, separate library |
I've run into the same problem on various occassions and products. That seems reasonable. One problem remaining is a threshold.
We've had a similar discussion on OOUI and it turned out that the SVGO optimization (chosen back then) caused visual regressions on some icons.
Yeah, unfortunately svgo isn't perfect so can't run in unsupervised mode, and I'd not be shocked if svgo would find an icon "compressible" when doing so would lead to rendering errors; having to force-V+2 the repo forever wouldn't be a good outcome. :-) Complain is not the same as vote-against, though we don't really have non-error dev/merge CI that people listen to right now…
You can maintain a whitelist of svgs that shouldn't be compressed. In MobileFrontend with the --pretty parameter we have not run into any issues.
pretty just makes the output formatted (pretty-printed, inserting some EOLs and indentation).
I've found from my very low threshold of acceptable visual impact (=no impact preferred) when optimizing images, that those SVGO options might cause trouble and should be disabled:
My goto tool to try the different options out is SVGOMG by Jake Archibald.
@bmansurov Put it on the agenda for this Wednesday's meeting. What are the specifics that you want to have as outcome and do you want to join the meeting?
@Volker_E done. Yes, I'd like to participate. I'm mainly interested in who'll own this work and how our team can support the owner.
We used to have various issues with that (I could dig up the relevant tasks/commits), but that was ~2 years ago. I'm not following SVGO development, but it seems to be actively developed and it seems to have caught more traction since then. These breaking bugs might very well have been fixed since then.
We talked about this in the frontend standards group and the general advice is to add a grunt tool that runs svgo and complains if the diff is different (similar to how Page Previews checks compiled assets have been checked in). Any project can do this now without any need for a change.
We could update Extension:BoilerPlate or Manual:Assets to be the canonical place for new setups.
I've created T162827 for the MobileFrontend use case.
We do this in MobileFrontend now. The tooling is generic and can be used by other extensions if they need to.
On further tests, above recommendations for SVGO have been proven mostly fail-safe so far, with one addition, disabling 'cleanupIDs' is necessary on more complex SVGs, in our case exported SVGs from Sketch.
Also, I've added https://gerrit.wikimedia.org/r/#/c/395931/ for adding SVGO to OOUI's dist SVG files.
And started SVG coding conventions capturing the learnings from last couple of months.