Code Review of Icon Menu about govpress

[devinsays]

The css classes for item count aren't working correct.

[devinsays]

I simplified this by using filters rather than a walker:
https://github.com/govfresh/govpress/blob/master/inc/icon-menu-extras.php

Still not completely happy with it as someone might intentionally want to use icon-* on another menu.

Perhaps the best solution is to just add an additional rule to all the icon css:

.menu-icon .icon-* a:before { content: '\icon' }

But it's also annoying to add all that css.

[lukefretwell]

Looks like you're using an older version of FA. Do we want to use the newer one?

http://fontawesome.io/

[lukefretwell]

Added icon CSS into menu, and it's breaking the spacing, causing each to block.

See here: http://civicmeet.govfresh.com/

[devinsays]

I'm using version 4.0.3 of FontAwesome- which looks like the latest. Is there something that makes you think it's not?

[lukefretwell]

I tried using the newsest FA code is the CSS and it didn't work so I went
with the old form and that did.

On Saturday, February 15, 2014, Devin Price wrote:

I'm using version 4.0.3 of FontAwesome- which looks like the latest. Is
there something that makes you think it's not?

Reply to this email directly or view it on GitHubhttps://github.com//issues/21#issuecomment-35159807
.

Luke Fretwell
415.722.8678
[email protected]
http://lukefretwell.com

[devinsays]

Could you give me an example of the new form and old form?

I did alter the CSS slightly. I'm using "icon" rather than "fa".
https://github.com/govfresh/govpress/blob/master/fonts/font-awesome/font-awesome.css

[lukefretwell]

I see. I had to use 'icon-rocket' which is similar to the markup from 3.2.1 to get it to work so assumed you were pulling from those.

[devinsays]

I'm leaning towards changing the markup back to "fa" from "icon". That could help prevent conflicts with other icon libraries. Also allows us to link straight to the font-awesome page so folks can find the class they need. This will cause issues on some of the staging sites- but should be done before release if it is going to happen. Thoughts @lukefretwell?

[lukefretwell]

+1

On Saturday, February 22, 2014, Devin Price wrote:

I'm leaning towards changing the markup back to "fa" from "icon". That
could help prevent conflicts with other icon libraries. Also allows us to
link straight to the font-awesome page so folks can find the class they
need. This will cause issues on some of the staging sites- but should be
done before release if it is going to happen. Thoughts @lukefretwellhttps://github.com/lukefretwell
?

Reply to this email directly or view it on GitHubhttps://github.com//issues/21#issuecomment-35819698
.

Luke Fretwell
415.722.8678
[email protected]
http://lukefretwell.com

[devinsays]

Icon prefix has been changed back to "fa". You'll need to update any icon menus you've already set.