-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Helper#icon prepends fa- to given fa class string #106
base: master
Are you sure you want to change the base?
Conversation
ex: icon('flag 2x') rather than icon('flag', class: 'fa-2x')
@@ -5,7 +5,8 @@ module ViewHelpers | |||
def icon(icon, text = nil, html_options = {}) | |||
text, html_options = nil, text if text.is_a?(Hash) | |||
|
|||
content_class = "fa fa-#{icon}" | |||
icons = icon.split(" ").map { |icon| "fa-#{icon}" }.join(" ") |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean?
f5ea06f
to
09d0c7d
Compare
@@ -4,8 +4,8 @@ module Rails | |||
module ViewHelpers | |||
def icon(icon, text = nil, html_options = {}) | |||
text, html_options = nil, text if text.is_a?(Hash) | |||
|
|||
content_class = "fa fa-#{icon}" | |||
icon = icon.split(' ').map { |icon| "fa-#{icon}" }.join(' ') |
There was a problem hiding this comment.
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.
c15208c
to
18dec73
Compare
@@ -5,7 +5,7 @@ module ViewHelpers | |||
def icon(icon, text = nil, html_options = {}) | |||
text, html_options = nil, text if text.is_a?(Hash) | |||
|
|||
content_class = "fa fa-#{icon}" | |||
content_class = "fa #{icon.split(' ').map { |i| "fa-#{i}" }.join(' ')}" |
There was a problem hiding this comment.
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]
There was a problem hiding this comment.
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 :)
You should probably call |
How about optional array instead?
|
18dec73
to
6acfa88
Compare
@@ -5,7 +5,8 @@ module ViewHelpers | |||
def icon(icon, text = nil, html_options = {}) | |||
text, html_options = nil, text if text.is_a?(Hash) | |||
|
|||
content_class = "fa fa-#{icon}" | |||
icons = icon.is_a?(Array) ? icon : icon.to_s.split(' ') |
There was a problem hiding this comment.
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.
@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'])
6acfa88
to
825bbac
Compare
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 useicon('flag', class='fa-2x')
which make more sense to use it as the following.