rapideditor / temaki

🐡 An icon expansion pack for Maki
https://rapideditor.github.io/temaki/docs
Other
75 stars 21 forks source link

Scale scottdejonge/map-icons to 15px by 15px #60

Closed plepe closed 4 years ago

plepe commented 4 years ago

The icons which were imported in #2 have a size of 50x50, which is against the standard which is 15x15 (see my issue #59). This pull request scales all these icons to 15x15 (by only changing the width/height attributes, without changing the viewbox).

bhousel commented 4 years ago

@plepe, are you working on something where the inconsistent size is causing an issue?

My preference is really to redraw them to match the 15px grid. I’ve done this with a few egregiously blurry ones already (like the slot machine).

plepe commented 4 years ago

No, it's not totally necessary, I have to adapt my code though to accommodate these icons.

Also, I like it, when things work in a consistent way. With this very small pull request the icons behave like the others.

quincylvania commented 4 years ago

From my view, technical consistency seems useful even if visually some of these icons shouldn't be rendered at 15x15. But I don't feel too strongly about it and I'll defer to @bhousel here.

bhousel commented 4 years ago

Thinking about it more, really none of the svg files should have width and height attributes.

We should probably just remove them all and leave it up to the viewBox to define the coordinate system, and then the icons will be responsive within whatever element contains them. (This is how the fontawesome icons are).

quincylvania commented 4 years ago

Thinking about it more, really none of the svg files should have width and height attributes.

We should probably just remove them all and leave it up to the viewBox to define the coordinate system, and then the icons will be responsive within whatever element contains them.

Sounds okay to me. People expect SVGs to be easily scalable. But we should continue to design for a 15x15 minimum size.

bhousel commented 4 years ago

But we should continue to design for a 15x15 minimum size.

I agree - I'll put a note about this in the README to make it more clear.

bhousel commented 4 years ago

Ok I added a linting script that runs on build to cleanup the icon files and warn about anything that looks suspicious. It's pretty chatty right now with the warnings, but we can adjust that if we want to.

bhousel commented 4 years ago

Oh also with width and height gone we should do a major semver bump whenever the next release is. This is minor but may affect anyone who uses the icons.

plepe commented 4 years ago

Thanks for the discussion! I think that is the way to go forward :-)

quincylvania commented 4 years ago

@plepe I'm glad this change works for you! Thanks for kicking off the discussion.