Automattic / gridicons

The WordPress.com icon set
http://automattic.github.io/gridicons/
GNU General Public License v2.0
112 stars 13 forks source link

Run svgmin on the svg folder in addition to the svg-min folder #206

Closed davewhitley closed 5 years ago

davewhitley commented 7 years ago

The svgs in the svg folder vary a lot in content and formatting, and ideally they should be pretty simple, like

<svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24">
    <path d="M19 13h-6v6h-2v-6H5v-2h6V5h2v6h6v2z"/>
</svg>

As raised in #205 we don't need the title element.

This causes a change in our workflow. Currently , we say to export the svg to the svg folder in AI by using the export script.

https://github.com/Automattic/gridicons/blob/master/README.md#add-a-proposed-icon-to-gridicons-admins-only

Perhaps we should also run svg min on the svg folder? Perhaps we could just get rid of the svg folder, and only have the svg-min folder?

I raise this issue because I think the sprite script uses the svg folder instead of svg-min folder. It doesn't seem wise to keep around a folder of messy svg files.

folletto commented 7 years ago

Perhaps we could just get rid of the svg folder, and only have the svg-min folder?

I'd probably go for this option, as I agree we shouldn't keep around a folder we don't use.

My only question would be if we have any use of the non-compressed folder... as it's just an output like svg-min, the sources are elsewhere, and svg-min seems preserving the invisible layers and shapes in graphic editing software... I think we can safely remove the svg only one.

davewhitley commented 7 years ago

Hm, perhaps svg could be the exact same as svg-min except with the invisible rect added. We don't need the invisible rect in svg-min since it's just for graphic editing software. Then, one folder would be truly minified, and the svg could be used for mockups.

folletto commented 7 years ago

This should be solved by the change in #217 right? Can we close?

davewhitley commented 7 years ago

Illustrator is now out of the process, which is good, but I still propose we:

The svgs in the svg folder are unnecessarily messy (AI comments, extra code, title, etc.)

I'll update the title of this PR.

Or perhaps, we don't even need 2 svg folders? The only benefit is the invisible rect I think.

folletto commented 7 years ago

Ok for me... even if I'd have opened a new PR as it would have better preserved history and would have been easier to read. But it's ok ;)

Or perhaps, we don't even need 2 svg folders?

Yeah, I agree with this bit: the svg folder is now a source folder, so we shouldn't touch it with automated scripts.

But as you said...

The svgs in the svg folder are unnecessarily messy (AI comments, extra code, title, etc.)

I'm 100% ok to clear the SVG files take care of their code: it's one of the benefits we have gained with the new process and SVG source. :)