Skip to content

Conversation

@buren
Copy link
Contributor

@buren buren commented Sep 11, 2014

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7fcab15 on buren:font-awesome-icon-helper into d152f2e on Fullscreen:master.

@claudiofullscreen
Copy link
Contributor

@buren Wow, thanks for the PR!

Personally, I don't know about Font Awesome. Is it part of Bootstrap? I looked for it in getbootstrap.com and I couldn't find anything… I thought Bootstrap only supported Glyphicons by default.

@buren
Copy link
Contributor Author

buren commented Sep 11, 2014

@claudiofullscreen you're right, there seems to be no documentation of Font Awesome at http://getbootstrap.com, however they do reference Font Awesome at their CDN: http://www.bootstrapcdn.com/#fontawesome_tab

I could add //maxcdn.bootstrapcdn.com/font-awesome/4.2.0/css/font-awesome.min.css to the cdn helper. Or what do you think?

@buren
Copy link
Contributor Author

buren commented Sep 11, 2014

found a reference to it under the getboostrap.com domain name: http://glyphicons.getbootstrap.com/#tab_quickstart seems like they officially support it.

@claudiofullscreen
Copy link
Contributor

Mmmh… now I see, I just looked http://fortawesome.github.io/Font-Awesome/examples up and I understand.

The only difference with glyphicons is that you have to include a separate CSS file for "font awesome" icons to show up, the rest is very similar.

Let me spend some more time on this, I want to make sure that the two stylesheets do not clash 😆

Apart from that, having the CDN helper would be definitely beneficial, as well as probably coming up with a better helper name. For instance

<%= glyphicon :heart %>

is very legible, but

<%= font_awesome :heart %>

is a little "harsher".


Here's an idea… what if there was a new helper called icon that took as an optional parameter the class of icons? so you could write icon :heart (which would default to glyphicon, since that is Bootstrap's default), and you could write icon :heart, family: :font_awesome.

Just an idea… what do you think? Also, what names would you pick for the parameters?

@buren
Copy link
Contributor Author

buren commented Sep 11, 2014

I have another branch where I instead named the method icon_for.

Another option would be to have it as a config option to use glyphicons or font_awesome and it can default to glyphicons.

And I like your idea of being able to specify family: :font_awesome. This branch does not support all the different options that font awesome icons support. I made this pull-request to see what you thought. Will add support and tests for all those variations.

@claudiofullscreen
Copy link
Contributor

Mmmh… I'm not 100% sure about the config option. If Bh supported both, wouldn't it be nice to be able to mix-and-match them? For instance:

<%= icon :heart, library: :glyphicons %>
<%= icon :heart, library: :font_awesome %>

would result in

<span class="glyphicon glyphicon-heart"></span>
<span class="fa fa-heart"></span>

Then the method would be modular enough to add more libraries in the future (like another one I just discovered while looking for Font Awesome: elusive).


On a side note, I saw that you used <i> in the code, but I would still go with <span>, since the guys at Font Awesome actually say:

You can place Font Awesome icons just about anywhere using the CSS Prefix fa and the icon's name. Font Awesome is designed to be used with inline elements (we like the <i> tag for brevity, but using a <span> is more semantically correct).

@claudiofullscreen
Copy link
Contributor

One more note… about the name… could be icon, could be icon_tag, could be icon_for

I'm leaning more towards icon at this moment, since the other helpers also go by the name of the object they display (navbar, modal, panel)… can you think of anything wrong about it?

@buren
Copy link
Contributor Author

buren commented Sep 11, 2014

I agree with icon.

Will update this pull-request first with full support for all font-awesomes options and then look at moving to icon.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 7fccb0e on buren:font-awesome-icon-helper into d152f2e on Fullscreen:master.

@buren
Copy link
Contributor Author

buren commented Sep 11, 2014

Do you think the icon method should have a default library option? Or should it always be specified?

@buren buren force-pushed the font-awesome-icon-helper branch from 7fccb0e to 9494617 Compare September 11, 2014 23:07
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 9494617 on buren:font-awesome-icon-helper into d152f2e on Fullscreen:master.

@claudiofullscreen
Copy link
Contributor

Yes, :glyphicons should be the default library, since 200 of its icons come by default in the Bootstrap CSS.

Of course these are just HTML helpers, so it's up to you to load at the top of the page the corresponding CSS files. In other words, if I use Bh without loading any CSS, I can still write

<%= icon :heart %>

and obtain the following in my HTML:

<span class="glyphicon glyphicon-heart"></span>

but without the appropriate CSS, I wouldn't see anything (an empty span!).

The same thing would happen if someone type <%= icon :heart, library: :this_does_not_exist %>.

But the nice thing is that anyone with the full glyphicon library (not just the 200 icons that come with Bootstrap) would be able to keep the same syntax and write (for instance) <%= icon :snowflake %>.


My thought for now is to use :glyphicons as the default library for icon, and therefore to keep the existing method glyphicon as a public helper, but re-write it invoking the new icon method, passing the :glyphicons library.

I imagine there would be a method to identify what prefix to use based on the library, something on the line of:

case options[:library].to_s
  when 'font-awesome' then :fa
  else options[:library]
end

@buren
Copy link
Contributor Author

buren commented Sep 11, 2014

How should we support both libraries in BaseHelper#error_icon_tag?

@claudiofullscreen
Copy link
Contributor

I don't think we need; in that case it would just use the glyphicon, since glyphicon is included by default, and that's what the bootstrap example uses: http://getbootstrap.com/css/#forms-control-validation

@buren
Copy link
Contributor Author

buren commented Sep 11, 2014

But it would be a problem if you don't load glyphicons at all

See #14 for IconHelper

@claudiofullscreen
Copy link
Contributor

@buren Is there a way to load Bootstrap and not load the 200 glyphicons that come with it? If there is, I have missed it!

@buren
Copy link
Contributor Author

buren commented Sep 12, 2014

Closing this in favor of #14

@buren buren closed this Sep 12, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants