search

WordPress / dashicons

Dashicons, the WordPress admin icon font. For the official resource, please see the WordPress Developer Hub.
https://developer.wordpress.org/resource/dashicons/
GNU General Public License v3.0
564 stars 185 forks source link

Try adding code-standards icon #353

Closed jasmussen closed 5 years ago

jasmussen commented 5 years ago

This PR is courtesy of @desrosj, his work, his icon as far as I know. There were some issues so I helped out. Don't merge this quite yet, for now it's a PR for further discussion. Here's what was wrong with the initial SVG:

1: Dashicons are 20x20. There’s been discussion on moving to 24x24 to be compatibile with potential future material icons, but this requires a new build process and a separate sprite sheet, and won’t work for the existing setup. 2: The build script we have is a bit picky and hacky, it has a regex that looks for viewBox="0 0 20 20" (I think) — it’s been a while. In any case it’s prone to error an expects somewhat clean and expected svg syntax 3: There can only be a single path inside the SVG. This one had two, one for the magnifying glass, one for the brackets inside. In Illustrator, merging those and flattening makes it one path. This is not for it to work in the font, but for it to work in the React component. This is also something that can be improved if the right programmer manages to improve the build process

What I did to fix this was scale down the icon to 16x16 and center that in a NEW 20x20 canvas (so the viewbox is correct when the SVG is generated), then combine and unify the shapes, then save and build.

Worth noting that the icon is blurry even at its initial larger size, and likely to be illegible at 20x20. This icon might be better off as an SVG uploaded somewhere in a larger size and referenced when necessary, but that's beside the point of this PR :) — just an observation.

Please take it from here, Jonathan!

jasmussen commented 5 years ago

This is 24x24:

Screenshot 2019-03-27 at 18 32 11

It's scaled down further from that, so very hard to see the brackets. But if used in a large font face (i.e. 2x or 3x multiples of 20px), it may be fine.

desrosj commented 5 years ago

Thanks, @jasmussen! I believe the intent is for this to be used in larger contexts across WordPress.org (badges, etc.).

I made one change, I moved the icon to 61754 (f13a). .dashicons-email-alt2 was previously set to f10a for a short period of time (and currently is that way in 5.2-alpha). This will eliminate the very slim chance of someone using the wrong icon.

jasmussen commented 5 years ago

I'll trust you for the context! Take it away! Edit: in case that sentence was ambiguous, I meant it as "run with it" or "blaze a trail" 😅

desrosj commented 5 years ago

@ianbelanger79 did some Windows testing to verify this for me. Let's merge it.