Skip to content

Helper#icon prepends fa- to given fa class string#106

Open
nnattawat wants to merge 2 commits intoFortAwesome:mainfrom
nnattawat:handle-icon-array
Open

Helper#icon prepends fa- to given fa class string#106
nnattawat wants to merge 2 commits intoFortAwesome:mainfrom
nnattawat:handle-icon-array

Conversation

@nnattawat
Copy link

I made the improvement on the helper method. Now it can handle multiple fa- classes.

For example, to add fa-2x class we need to use icon('flag', class='fa-2x') which make more sense to use it as the following.

icon('flag 2x')
# => <i class="fa fa-flag fa-2x"></i>

ex: icon('flag 2x') rather than icon('flag', class: 'fa-2x')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shadowing outer local variable - icon.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shadowing outer local variable - icon.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@nnattawat nnattawat force-pushed the handle-icon-array branch 2 times, most recently from c15208c to 18dec73 Compare November 12, 2015 02:23

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [81/80]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's not that long haha :)

@nickpearson
Copy link

You should probably call to_s on icon before trying to split it. Before this change, a Symbol could be passed in, but now it requires a String, potentially causing existing usages to break.

@andreykul
Copy link

How about optional array instead?

icon([:flag, '2x', :fw], "Big Flag")
# => <i class="fa fa-flag fa-2x fa-fw"></i>

icon(:flag, 'Normal flag')
# => <i class="fa fa-flag"></i>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@nnattawat
Copy link
Author

@nickpearson and @andreykul good points guys. I have made it to support more formats.

icon(:flag)
# => <i class="fa fa-flag">
icon('flag')
# => <i class="fa fa-flag">
icon([:flag, :fw, '2x'])
# => <i class="fa fa-flag fa-fw fa-2x">
icon("flag fw 2x")
# => <i class="fa fa-flag fa-fw fa-2x">

…flag fa-2x'> now we can do:

- icon('flag 2x')
- icon([:flag, '2x'])
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.

4 participants