DmitryEfimenko / TwitterBootstrapMvc

Fluent implementation of ASP.NET-MVC HTML helpers for Twitter Bootstrap.
Apache License 2.0
222 stars 79 forks source link

Suggestion: Add support for stacked icons in font-awesome #39

Closed hades200082 closed 8 years ago

hades200082 commented 11 years ago

Font Awesome is a Bootstrap extension that replaces the default glyphicons with a font-based icon system including hundreds of additional icons.

It also has the ability to stack multiple icons on top of each other to create combination icons among other things.

Take a look at the examples page: http://fortawesome.github.io/Font-Awesome/examples/

Could we add these abilities to this extension IF font awesome is also installed?

DmitryEfimenko commented 11 years ago

I like the idea of adding support for FontAwesome. The way I see it is that there need to be a class Icon that allows for fluent syntax. This class would have the following methods:

new Icon(FaIcons.some-icon-name) // FaIcons is an Enum with all available 
    .FaSize(FaSize.Large) // FaSize is an enu, with available font-awesome sizes
    .Bordered() // adds icon-border class
    .PullLeft() // adds pull-left class
    .PullRight() // adds pull-right class
    .FixedWidth() // adds icon-fixed-width class
    .BulletedLists() // adds icon-li class
    .Spin() // adds icon-spin class
    .Rotate(FaRotate.Rotate90) // FaRotate is an enum with rotation options
    .Stack(Icon icon) // icon is the Icon class. Wraps whole thing into <span class="icon-stack"></span>. Adds specified icon to the end of the span. Applies class icon-stack-base to it.

Does this look reasonable? The only problem I see with it is maintaining FaIcons enum since Font-Awesome always comes up with new icons. Perhaps there should be no FaIcons enum. Instead one would have to put the name of the Icon as a string.

Let me know your thoughts on all of this.

JustinPierce commented 11 years ago

I think you're pretty well covered by keeping a constructor that takes a custom icon name (that's how I use FontAwesome today). The enum is nice, but I still have to look up my icons every time anyway, just to see what they look like.

hades200082 commented 11 years ago

Just using a string value for the icon name should be fine.

Remember that IconAppend and IconPrepend should both allow for the new Icon() syntax too :)

DmitryEfimenko commented 11 years ago

Great! Sounds like a plan for the new feature is complete! What's now left is to implement it. It would be great if you could do that and submit a pull request as I'm a bit swamped with other things.

hades200082 commented 11 years ago

lol.. you and me both.

No big rush for this though.... I'm sure one of us (or even someone else) will get to it sooner or later :+1:

DmitryEfimenko commented 11 years ago

hey guys, release to support TB3 is pretty much ready. I've been going through issues submitted by people and getting these taken care of. This is the last issue that need to be done. Reading over it again I saw something that I didn't notice before:

Could we add these abilities to this extension IF font awesome is also installed?

The word IF...

Since FontAwesome (FA) is just a combination of files added to the project, there is no way to determine programmatically whether FA is installed (at least no clean way).

There are two options:

Let me know your thoughts.

hades200082 commented 11 years ago

Could you put the FA related stuff in a separate nuget package? It could be named as per your namespace idea - TwitterBootstrapMVC.FontAwesome

This way developers only need to install the extra features if they use FA and if not they won't get confused by it.

Also.... bear in mind that with Bootstrap3 on the horizon will FA still be supported anyway? Are FA going to update for BS3?

So far they don't seem too bothered about updating to include new icons any more so they may not be updating to BS3.

DmitryEfimenko commented 11 years ago

Not sure placing it in the separate package makes sense. From the perspective of keeping BMVC library small, FA-related code will be unnoticeable. However if we go that route, in addition to the need of adding the namespace, a developer will also have to find and install a separate package. So basically it will require one more action from the developer to get the functionality.

As for the FA supporting BS3, based on this issue they will figure it out eventually. Though whatever they decide, I don't think it will affect functionality of BMVC since all that matters is slapping a correct class on the icon.

hades200082 commented 11 years ago

A separate library makes sense purely from the point of view that many developers like to only include things they are actually going to use within their project. Hence if they are not using FA they would prefer not to include FA support in this.

I admit I am one such developer, but I know many others also.

Having the option to not include FA is good. An extra Install-Package TwitterBootstrapMVC.FontAwesome it's not that big of a deal for a developer that wants to use it and at the same time you cater to those that don't, but who still want to use the rest of TwitterBootstrapMVC.

I understand what you say about it not being that much extra but the point is that many developers like to keep their projects as lean as possible rather than including extraneous code/libraries.

Also, releasing it as a separate package would serve as an example of how your TwitterBootstrapMVC package can be extended and could lead other developers to create their own extensions for other bootstrap addons and build an ecosystem around your package.

MarcoGaribaldi commented 11 years ago

I find both options valid but if it was up to me I'd prefer a separate namespace in the same package.

DmitryEfimenko commented 11 years ago

I'm gonna go with my gut on this one and put it in the same package.

DmitryEfimenko commented 11 years ago

Actually I'm gonna wait with FA support till FA starts supporting B3. Who knows how they will implement this...

johnwc commented 8 years ago

Did this ever get anywhere? I see the namespace in the library, but I don't see anything in it.

DmitryEfimenko commented 8 years ago

It just did. Add the following namespace in the web.config under Views folder.

<add namespace="TwitterBootstrapMVC.FontAwesome4" />

You'll get new extension methods that have Fa prefix.