-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Icon component PropType update to FontAwesome 4.7.0 #1371
Comments
I just confirmed that this is also an issue in v0.66.0 of SUIR. |
@layershifter I can see that these strings are coming from our If there's no auto generating tool for this, I can take on this task and submit a PR on Friday. |
I did it ✌️ |
Moving forward, I think SUIR might benefit from having a way to pass other string arrays to the Icon component outside of the current FontAwesome icon offering. There are other icon libraries out there and it would be great to extend this component with other names. The CSS will of course be handled outside of SUIR, but the PropType validation I think would still be important. It seems to me that in the case of the Icon component we are coupled to those styles. |
Thanks for the quick response, @levithomason. I was thinking in terms of the string name validation... but at the end of the day your position on that in this comment makes the most sense for keeping 1:1 with SUI. I'm already using Do you think there would be any value to updating the docs with a note about doing this for an external/additional icon font library? I could do that. |
However, I thing that my pr with updated SUI icon names/aliases array is still good for this case, and for any other custom icon take @levithomason solution |
Quick clarification, SUI does not use FA classes: #1372 (comment)
Definitely value there, we're missing the top level comments that SUI core has. We should add those: |
@levithomason should I add it in my pr? |
@kamdz that would be great. |
@levithomason I can't :/ I tried to add something like that: /**
* An icon is a glyph used to represent something else.
* Icons serve a very similar function to text in a page. In Semantic icons receive a special tag <i> which allow for an abbreviated markup when sitting along-side text.
* Semantic includes a complete port of <a href="http://fontawesome.io/whats-new/">Font Awesome 4.7.0</a> designed by <a href="http://www.twitter.com/davegandy">Dave Gandy</a> for its standard icon set.
* @see Image
*/ but it's not render well, it's look like a mess, and without hyperlinks. I give up this time, however without this, my pr is complete :) |
Released in |
It appears the PropType validation for
name
on the Icon component needs to get updated to support the new FontAwesome 4.7.0 icons that were just added in the most recent SUI release.Steps
Set the
name
property to "address book outline" on an Icon component or on aicon
property for a List.Item componentExpected Result
The "address book outline" icon should show and there should be no console error for an invalid string passed.
Actual Result
The icon is rendered correctly but there is a console error:
Version
0.65.0 SUIR
2.2.9 SUI
Testcase
http://codepen.io/anon/pen/gmYGXZ
The text was updated successfully, but these errors were encountered: