global .fa command is breaking custom icon sizes

[rob-clicktripz]

The following two commands appear in _core.scss and _mixins.scss, and seem to be added in the latest version (4.2.0):

.fa { font: normal normal normal 14px/1 FontAwesome; // shortening font declaration font-size: inherit; // can't have font-size inherit on line above, so need to override }

This combination of commands makes it impossible to override the font size without using the !important symbol. For instance, I have the following in my application:

i { font-size: 22px; }

which gets invalidated by the initial .fa code since they have the same CSS specificity, and the FontAwesome SASS is included at the top of the file.

To overcome this, I have to change it to:

.fa { font-family: FontAwesome; }

which allows me to set the custom font size without using the !important symbol.

Since I'm using bower to install FontAwesome, this change can't be made on our production server, which breaks all of our icon sizes.

Could you please replace the commands with the one that I suggested, so that you're not hard-coding the font size?

[tagliala]

Hi,

sorry if I didn't get at 100% your issue...

You want to change base icon size one for good, am I right?

And if you make something like

.fa {
  font-size: 22px;
}

you will break fa-2x, fa-3x..., right?

And this is why you are suggesting to go back to the old font declaration.

Unfortunately we can't do this in a minor release because it will break backward compatibility. You could say "you already broke backward compatibility with this stuff!" and you are right, sorry, but that was not intended.

The actual improved font declaration has some reasons and it is also used by octicons here: https://github.com/github/octicons/blob/master/octicons/octicons.less#L17

BTW I hacked with the development console to following fiddle: http://jsfiddle.net/tagliala/77re29ct/33/

As far as i see, overriding the <i> tag will break the fa-2x feature the same:

So my suggestion is to add a variable to customize base font size, something like

.fa {
 font: normal normal normal #{$fa-font-size-base}/1 FontAwesome; /* ... */
}

and set it to 14px. This will not break backward compatibility and will allow to customize the base font size, given that fa-2x, fa-3x will be related to container's font size and not base icon font size.

At least we can multiply the base font size when generating fa-2x classes and this makes sense, but this need to be done in 5.0.0

[rob-clicktripz]

I'm glad you worked on this bug, but I don't think you completely understood the issue.

"You want to change base icon size one for good, am I right?" No, that is not what we're doing. We don't want to set a base font size in FontAwesome for our entire project, we want to specify a font size for individual icons on a case-by-case basis. As an example, here's what we're doing, which has worked for many previous versions of FontAwesome:

(I replaced "<" with "[" because GitHub won't render the HTML otherwise)

HTML: [button class="select" onclick="..."] [i class="fa fa-icon-glass"][/i] [/button]

CSS: button { &.select {

i { &.fa-icon-glass { &:before { font-size: 18px; } } } } }

With the addition of the following line in v4.2, the custom font size specification as shown above no longer works:

.fa { font: normal normal normal 14px/1 FontAwesome; // shortening font declaration font-size: inherit; // can't have font-size inherit on line above, so need to override }

The effect of this is that the previously working CSS commands shown above are ineffective, and all of the FontAwesome icons remain the same size, breaking backwards compatibility.

The reason that this causes problems is, that since you included the font size inside a single-line "font" command, you have to issue a new "font" command in order to override it, which means that everyone who upgrades to 4.2 has to inspect thousands of lines of SASS code and change all of the "font-size" commands to "font" commands, which is very time consuming, and highly redundant (since FontAwesome needs to be specified over and over).

I suggest changing it back to the v4.1 version, which has the same effect of setting FontAwesome as the default font without hard-coding a font size into the "font" command. It's not helpful to anyone that you're setting the base font size to 14px or any other size. This should be determined by the natural CSS inheritance flow of the page, not by a special FontAwesome variable.

.fa { font-family: FontAwesome; // \ delete the font-size: inherit command, it's unnecessary }

Does that explain our issue better?

[tagliala]

The effect of this is that the previously working CSS commands shown above are ineffective, and all of the FontAwesome icons remain the same size, breaking backwards compatibility.

This was not an intended change and we can't intentionally break backward compatibility.

It could be done in 5.0.0

Could you please provide a couple of working fiddle that show the issue? You can start with these

http://jsfiddle.net/tagliala/77re29ct/33/ <-- 4.2 http://jsfiddle.net/tagliala/1eerp46s/ <-- 4.1

[rob-clicktripz]

The actual issue is when CSS specificity is, in some circumstances, being overridden by the global .fa command. For instance, when using the following CSS command to change font size in 4.1, it works, but in 4.2, it doesn't:

body > section > header > div > aside > i { font-size: 22px; }

Here are the fiddles to demonstrate the breakage:

http://jsfiddle.net/77re29ct/72/ <-- 4.2 http://jsfiddle.net/1eerp46s/6/ <-- 4.1

[davegandy]

We've moved to a shortened syntax, which I'm very happy with.

For your particular issue, is there a reason you can't just attach those sizes to .fa instead of i? Or even attach it to i.fa? That would be a more correct way to override classes in CSS and would solve your issue (I just tested this on your jsFiddle).

Closing for now, but happy to continue the discussion.

[rob-clicktripz]

It's a question of backward compatibility. 4.1 and below didn't require the use of a class to override, and we have frozen at 4.1 because 4.2 would require us to go through thousands of lines of CSS in order to add an "&.fa { }" section. We use object-oriented CSS which discourages the use of classes and instead relies on the inherent HTML structure for styling.

Although the shortened syntax might appeal to someone who looks at the code and enjoys a minimal structure for the .fa { } section with a condensed "font" command, in reality it breaks any overrides that were used in 4.1 and below for no real gain.

[tagliala]

@rob-clicktripz sorry but as I said this was an unintentional breaking change.

I would suggest you to use the less/scss version and override the .fa class to meet your needs

We use object-oriented CSS which discourages the use of classes and instead relies on the inherent HTML structure for styling.

I'm a little bit confused about this one... you said that OOCSS discourages the use of classes, but in your example you have an <i> tag with three classes plus one css rule to override the font size set on the tag itself

http://jsfiddle.net/77re29ct/72/ <-- I would write this in the following way --> http://jsfiddle.net/tagliala/77re29ct/73/ (no classes involved at all)

Refers to #5198

[rob-clicktripz]

"I'm a little bit confused about this one... you said that OOCSS discourages the use of classes, but in your example you have an [i] tag with three classes plus one css rule to override the font size set on the tag itself"

I think there's a misunderstanding here: OOCSS discourages the use of extraneous and unnecessary classes, and instead relies on the inherent structure of the HTML for pinpointing commands. So, the only classes that should exist on the [i] element are the ones related to font-awesome, which are "fa fa-history" (fa-44px was included for demonstration). In order to override the global font command that you added in 4.2, it becomes necessary to add another class to the [i] since the "body div > span > i" command is too weak to override the global "font" command set by Font Awesome.

What we're actually doing, which worked with 4.1 and below, is setting a custom font size on each [i] element in our application using the "font-size" command, which can be different sizes according to its use on the page. Because you're including this condensed "font" command with a hard-coded 14px, along with a redundant "font-size: inherit" command (it's already inherited!), it forces us to add a class to the [i] in order to compete with the specificity of the ".fa" command.

The actual issue is that you haven't improved anything in 4.2 by using this condensed font command (except that it looks cool), you've only made it difficult to override the font-size. If you look back to 4.1 (and below), the font command does not include a "font-size" command, which allows the user to set their own font size without competing with the global ".fa" command.

Here's the change again:

4.2: .fa { font: normal normal normal 14px/1 FontAwesome; // shortening font declaration font-size: inherit; // can't have font-size inherit on line above, so need to override }

replace with:

4.1: .fa { font-family: FontAwesome; font-style: normal; font-weight: normal; }

TLDR: Font Awesome shouldn't be setting a base font-size at all. That should be left up to the web page, the web browsers settings, and the user's stylesheets. Your condensed font command, along with the "font-size: inherit;" command, causes specificity problems with user's stylesheets that have to compete with your "14px" and "inherit" font size commands. Remove the font-size commands from the ".fa" section and all is well again (meaning that we can actually upgrade to 4.2 without breaking our application).

[Hagith]

Some time passed, but I just step in here because of similar (or even the same) problem.

I'm using SCSS and overwriting $fa-font-size-base. The value is properly set in generated CSS. BUT it looks that font-size: inherit in .#{$fa-css-prefix} is blocking to actually change icon size. Font size defined in parent element have precedence and my icons are still small :(

Maybe I'm doing something wrong, but check this fiddle: http://jsfiddle.net/modernweb/QKDmF/22/ Comment out font-size: inherit and run again.

So as @rob-clicktripz said: this declaration is redundant, it's already inherited! You are just forcing to skip declaration in .fa and get value inherited from parents.

@tagliala

The actual improved font declaration has some reasons and it is also used by octicons here: https://github.com/github/octicons/blob/master/octicons/octicons.less#L17

The main difference is that they didn't added font-size: inherited declaration.

[tagliala]

removing font-size: inherit will not solve @rob-clicktripz 's issue, because .fa will still have priority over i

The solution is to go back to the old declaration and we can't consider this until 5.0.0.

Since this is an edge use case, I would advise for a custom build

[Hagith]

So IMO this issue should be reopened and scheduled for 5.x release.

[tagliala]

@rafalgalka legit, but I don't know if dave wants to go back to the old font declaration, as per https://github.com/FortAwesome/Font-Awesome/issues/5128#issuecomment-70882874

[tagliala]

Hi, the font declaration has been changed.

https://github.com/FortAwesome/Font-Awesome/blob/master/web-fonts-with-css/css/fontawesome.css#L10

I'm going to close this, please comment if this is still an issue