search

Automattic / gridicons

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

Add icon for the Stories feature #309

Closed bjtitus closed 3 years ago

bjtitus commented 3 years ago

New icon:

gridicon-story

Stories are a new post type shown in the mobile apps (they are implemented as as a Gutenberg block).

bjtitus commented 3 years ago

@folletto @davewhitley I was hoping one of you could point me in the right direction on generating the assets for these. When I run the script, it produces mangled PDFs and makes modifications to the svg files in the docs and sprites folders. Perhaps one of the dependencies has changed?

Here's an example of the pdf it produces, which won't open in any PDF editor I try: gridicons-add.pdf

I also tried updating the pdfkit and svg-to-pdfkit dependencies to their latest but then it produces completely empty PDF files.

I am running on macOS Big Sur, so I created a Docker image using the Node base image but still see the same thing.

Maybe my package-lock file has some clues.

folletto commented 3 years ago

Heya! I can confirm that... something changed. I get also all the PDFs being now updated, which shouldn't be the case. I feel some package has changed its internals at some point and... it broke... again.

It seems that ALL PDFs are generated broken now — all file sizes are 437 bytes.

Also... we are requiring a specific version of both "svg-to-pdfkit" and "pdfkit" so... I'm really wondering what broke given the version has to be exactly that one.

folletto commented 3 years ago

I created https://github.com/Automattic/gridicons/pull/310 to look into that.

folletto commented 3 years ago

Ok #310 helped me find out the problem, #311 should fix it.

Could you please @bjtitus review #311 so we can merge that? Once it's done, you should be able to add the icon as expected.

davewhitley commented 3 years ago

Might be relevant to choosing a specific pdfkit version https://github.com/Automattic/gridicons/pull/304

bjtitus commented 3 years ago

@folletto Thanks! This seems to have fixed the generated PDF and it's not touching the other files. 🎊

folletto commented 3 years ago

Nice. I checked the commit, and it seems alright.

I think we can merge — are you ok doing it? :)

bjtitus commented 3 years ago

Yep! Thanks checking it over!