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
560 stars 182 forks source link

dashicons.css default values #13

Closed steveush closed 10 years ago

steveush commented 10 years ago

Hey Guys,

Straight into the issue I'm having, you have a fixed font-size, width and height set on your .dashicons class. This makes actually getting the icon to fall inline with text not so straight forward if using a font-size different to the standard baseline size of 20px.

Here is a what I propose the class should look like:

.dashicons {
  font-family: "dashicons";
  display: inline-block;
  speak: none;
  font-style: normal;
  font-weight: normal;
  font-variant: normal;
  text-transform: none;
  line-height: 1;
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
}

The original 20px set on the width and height has no affect on the :before element that is actually showing the icon as that grows to it's content based on font-size, it simply alters the default inline layout.

As for the font-size that is a decision that should be left up to the developer making use of the icons. Having to override the default of 20px is out of line with the majority of font's out there like FontAwesome or anything created with IcoMoon that simply inherit the parents font-size.

There are also some transitions included in the .dashicons class which should rather be refactored out into the #iconlist div class in the styles.css file.

Overall it seems as though some style properties used to create the index.html page have crept into the core dashicons.css file when they should be in the styles.css file. This makes simply switching from other font's to dashicons in plugins etc. more complicated as we have to go and override these properties to get the icons to display correctly inline.

This issue is even evident on http://melchoyce.github.io/dashicons/. Simply open dev tools and inspect any of the icons and you will notice the rather strange layout. A small 20px div (.dashicons) padded to increase it's size to the required size to get the layout to work as expected. In dev tools untick the padding on the #iconlist div class and all the icons overlap each other, this is not how the default layout should behave.

With the above changes to the .dashicons class the #iconlist div class in styles.css could be changed from the current one using paddings to force the layout to one that simply uses the font-size and margins so the layout behaves as one would expect from an inline element adjusting itself according to the font-size. I also added in the transitions that I removed from the above to this class. Not everyone will want transitions and this is simply something else we would need to override.

#iconlist div {
  margin: 10px;
  font-size: 40px;
  line-height: 1;
  -moz-transition: color .1s ease-in 0;
  -webkit-transition: color .1s ease-in 0;
}

Please let me know what you think as currently the hard coded defaults seem more restricting than anything else.

Thanks

jasmussen commented 10 years ago

You make a lot of good points here. Your suggestions on cleaning up the CSS and separating out some of the stuff that was intended for the example page from the helper CSS is stuff we should do. Removing the baked in width and height from the helper CSS also makes good sense.

I disagree about removing the font-size: 20px; though, it's there for a very good reason. Dashicons has been designed according to a specific 20x20px pixel grid. Just because it's vector graphics doesn't mean it'll actually look good at any size, especially when they're shown small.

Here's an example showing the same icon in 20px and 21px sizes, zoomed way in. On the left you'll see that the hairlines inside the sticky note are exactly 1px high. On the right, the increased size blurs that all up due to the antialiasing. As such, Dashicons with its 20px grid will look perfect in only that size, and 2x multiples thereof (i.e. 40px, 80px etc.)

The issue becomes less prevalent as you pick larger sizes due to the increased fidelity. 60px and anything above will look fine probably. But I doubt many use them in those sizes. As such, removing font-size: 20px; does a disservice to those using the icons. But if you still want a non-pixel-grid size, you can easily override with font-size: inherit; in your own stylesheet.

steveush commented 10 years ago

Thanks for the reply, however after attempting to integrate the icons into my plugins I eventually decided against it due to the responses to other issues opened here on GitHub. Primarily that this icon set has been designed specifically only for use within the admin.

The baseline, alignment and even sizes of the icons differ to such a degree that added to the layout issues it simply wasn't feasible or clean to incorporate them. This was very disappointing as on various blogs this is being pushed quite heavily for use by plugin and even theme developers and I simply do not think that it should be. I'm not even 100% sure why this repo was made public as if this is solely for use within the admin, and has been customized so heavily for it, public use and input is limited.

Finally on the font-size issue, I agree with you that the icons should be displayed at 20px or multiples thereof, however the issue of having to display a font in it's baseline size or appropriate sizes is the same with all font icons. For example FontAwesome has a baseline size of 14px, however no where do they force this in the CSS. The same goes for IcoMoon fonts which have a baseline size of 16px, this size is also not forced in the CSS. The baseline font-size is something developers should be aware of when using a font and should be documented for the font but should not be forced by the font.

Again thanks for the response but the customization's made to fit the admin make these icons difficult to incorporate and generally leads to messy CSS to try fix the alignment, sizing and layout issues.

field2 commented 10 years ago

16px is the default font size of most browsers. That's why icons set in a 16px grid look sharp without any font-size applied to them. FontAwesome does in fact force the font-size to 14px. If you go to http://fontawesome.io/icons/ untick all 14px values in the inspector, the icons lose their sharpness because their vectors fall between pixels. Can you post an example of your attempt to integrate a Dashicon into one of your plugins, and describe the problem? It would help to see how they aren't working for you so we can adjust if necessary.

steveush commented 10 years ago

@field2 sorry I think you misunderstood what I meant when I said they do not force a font-size. If you download FontAwesome right now from the site and search the .css file, no where in that file is the size of 14px set. The 14px on the FontAwesome site comes from the site.css stylesheet, not the font stylesheet. As I said above you will have to provide a size for the icons but that should be left up to the developer and is set in their own stylesheets as can be seen with FontAwesome.

As for the example of issues integrating I have scraped all code with dashicons so I cannot provide it however the issues are the ones mentioned above. The baseline, alignment and size of the icons differs to much to provide a consistent look for icons. A pixel off makes a world of difference and as can be seen in issue#4 (marked as wontfix) the icons are drastically different in baseline, alignment and size.

field2 commented 10 years ago

They may leave the size up to the developer, but if the developer chooses a value other than a multiple of 14 (or nothing at all), the icons will not display correctly. I still don't see your issue here. Without concrete evidence (a url, screenshot, etc.) of the icons not working correctly, I"m not sure what you're asking us to do here. Just saying they're not working isn't enough. Also, icon fonts don't have a baseline or alignment; they are not letters and should generally center horizontally and vertically within their bounding box, which Dashicons do.

steveush commented 10 years ago

Sorry @field2 but all the evidence you need for the alignment issues are provided in issue#4, the fact that you're not willing to acknowledge the problem is a big issue.

As you have just stated yourself they should all have the same center point for each icon, this is not the case with dashicons, just take a look at this image http://wpimpact.com/wp-content/uploads/2013/12/Untitled-8.png, notice how the plus icon is no where near centered? Compare it to the cross right next to it, it is a couple of pixels off center. Even in the comments in that issue you state you have nudged them around to fit the admin perfectly, so by your own words they are not aligned correctly.

Anyway I am closing this issue as I have already stated I am no longer going to make use of this font due to it's numerous problems and I don't feel like banging my head against a wall.

field2 commented 10 years ago

A few are slightly nudged. These are the core icons that are used by default. The icons we've designed for plugin authors to use are assuredly centered.