Ckeditor5: Clean up the .svg icon files in the project

Created on 19 Jun 2017  Â·  20Comments  Â·  Source: ckeditor/ckeditor5

The icons contain tons of garbage code like comments, unnecessary <title>, other tags, attributes etc..

image

We got to get rid of this code and possibly use some external utility to optimize the files. 4KB for a bold icon feels like an excess. Some icons like object-center.svg are just ~300 bytes and that feels more like the right size.

A follow–up of https://github.com/ckeditor/ckeditor5-ui/issues/245.

task

Most helpful comment

Now everything looks fine. I've merged all the branches and removed them. Please update Wiki and we will be able to close this issue.

All 20 comments

https://github.com/svg/svgo feels like the right tool to use. Perhaps we should use it during the build phase?

Yeah... I wanted to comment on that. It'd be perfect if we could do the cleanup as late as on the build phase. But it means changing dozen Webpack configs. This is awful. We are yet to understand how much can our single Webpack plugin configure. It'd be perfect if it was able to set all those things so developers wouldn't have to repeat that.

@ma2ciek, be prepared :D

I checked the svgo. It saves 20KB in the ckeditor5-core/theme/icons alone 😮

(t/90 8c6cd2f) oleq@MBP ~/...ckeditor5-core/theme/icons> svgo -i . -o out/
Processing directory '.':

align-center.svg:
Done in 34 ms!
1.081 KiB - 78.6% = 0.231 KiB

align-left.svg:
Done in 10 ms!
0.982 KiB - 76.9% = 0.227 KiB

align-right.svg:
Done in 7 ms!
1.076 KiB - 78.8% = 0.229 KiB

image.svg:
Done in 25 ms!
1.658 KiB - 64.8% = 0.583 KiB

input.svg:
Done in 43 ms!
3.722 KiB - 32.9% = 2.499 KiB

low-vision.svg:
Done in 34 ms!
3.878 KiB - 46% = 2.096 KiB

object-center.svg:
Done in 5 ms!
0.338 KiB - 60.1% = 0.135 KiB

object-left.svg:
Done in 2 ms!
0.403 KiB - 56.4% = 0.176 KiB

object-right.svg:
Done in 4 ms!
0.461 KiB - 57.4% = 0.196 KiB

picker.svg:
Done in 7 ms!
1.628 KiB - 71.7% = 0.461 KiB

quote.svg:
Done in 24 ms!
2.852 KiB - 66% = 0.97 KiB

source.svg:
Done in 7 ms!
1.767 KiB - 71% = 0.513 KiB

table.svg:
Done in 28 ms!
1.671 KiB - 75.5% = 0.409 KiB

underline.svg:
Done in 26 ms!
3.248 KiB - 65.6% = 1.118 KiB

and

(t/ckeditor5-core/90 1f314e9) oleq@MBP ~/...ckeditor5-basic-styles/theme/icons> svgo -i .
Processing directory '.':

bold.svg:
Done in 60 ms!
3.447 KiB - 67.1% = 1.133 KiB

italic.svg:
Done in 24 ms!
2.28 KiB - 70.7% = 0.668 KiB

But adding svg minifying process (which is needed only once per svg) to each build would extend the build time. Or maybe I misunderstand something.

I'm not sure about running shell commands from Webpack. The JS lib that could process the file content would be the best, as it could be just a simple Webpack loader.

But adding svg minifying process (which is needed only once per svg) to each build would extend the build time. Or maybe I misunderstand something.

Yes, I got your point. We may repeat it every time an editor is built or we can make people remember about it during the review. I wonder which approach is easier/safer 🤔

BTW: https://github.com/tcoopman/image-webpack-loader supports svgo (found it in https://github.com/svg/svgo/blob/master/README.md).

But adding svg minifying process (which is needed only once per svg) to each build would extend the build time. Or maybe I misunderstand something.

I wouldn't worry about it too much. After all, that's what build process is for – to optimise stuff.

The other option, which is optimising this on commit will require some code in all our repos and will slow down the dev env boostrap time.

The other option, which is optimising this on commit will require some code in all our repos and will slow down the dev env boostrap time.

In my mind, the other option was more like "make sure you check it when reviewing any new icon" ;-)

BTW: https://github.com/tcoopman/image-webpack-loader supports svgo (found it in https://github.com/svg/svgo/blob/master/README.md).

This package looks really promising, so probably we can add the loader with our custom plugin, if we don't want to force people to insert the loader manually into their weback.config.js file.

I wouldn't worry about it too much. After all, that's what build process is for – to optimise stuff.

Probably you're right, as it doesn't take too much time.

In my mind, the other option was more like "make sure you check it when reviewing any new icon" ;-)

But "check" and what? Someone needs to optimise it anyway. Unless this is easy to run manually by anyone in the team, that would be an inconvenient option. But if it is easy to run (e.g. svgo --file=icon.svg), then it may be the best option for now. We'll just need to notify the entire team.

Interesting fact is that Safari displays svg title

image

That's all you need to do:

npm install -g svgo
svgo -i icon.svg

I suppose we're not gonna introduce many new icons so there's no point in making it an automated process.

@oskarwrobel https://github.com/ckeditor/ckeditor5-ui/issues/245 ;-)

This is funny because it will be a PR-less issue. Changes awaiting the R are in t/ckeditor5/474 branches in the following repos:

The uncompressed (no gzip, raw XML) icons weighted 41.5kB before the optimization. Now it's 15.5kB, which is quite impressive TBH (~37% of the original size).

When reviewing, make sure no distortion appeared visually and the icon properly responds to the parent color change (e.g. apply color: red to the .ck-button CSS class).

I think that what this ticket misses is explaining somewhere how to deal with the new icons. Since we don't have any other place right now it can be somewhere in https://github.com/ckeditor/ckeditor5/wiki/Development-environment as "Additional stuff to remember when contributing". We'll later extract it to "Contributing" guide or something.

It looks like the tool is leaving <title> in icon files: https://github.com/ckeditor/ckeditor5-basic-styles/commit/b8580a129228d3e817d9e09b64ae6821654ef829#diff-4e876a53975446929ce59efb641136e5R1 so Safari is still displaying tooltips.

I just noticed that removeTitle is disabled by default in SVGO. I'll enable it and update branches soon.

Updated. Let's see how does it work now.

Now everything looks fine. I've merged all the branches and removed them. Please update Wiki and we will be able to close this issue.

Updated the wiki. The issue can be closed safely now.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jodator picture jodator  Â·  3Comments

devaptas picture devaptas  Â·  3Comments

hybridpicker picture hybridpicker  Â·  3Comments

pomek picture pomek  Â·  3Comments

MCMicS picture MCMicS  Â·  3Comments