Skip to content
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

fix(Icon): Icon component PropType update to FontAwesome 4.7.0 #1372

Merged
merged 5 commits into from
Feb 26, 2017
Merged

Conversation

kamdz
Copy link
Contributor

@kamdz kamdz commented Feb 22, 2017

fixes #1371

I took names and aliases from changelog on here http://fontawesome.io/whats-new/

@levithomason
Copy link
Member

SUI doesn't use font-awesome css classes directly, they are aliased to more "semantic" names. Example:

FA

image

SUI

image

See the line comment at the top of the SUI icons arrays which indicates where the names should be retrieved from in SUI core, namely, icon.css:

See https://github.com/Semantic-Org/Semantic-UI/blob/master/dist/components/icon.css#L486

Sidenote, looks like this file was updated 23hrs ago :) Thanks for the quick response to the change!

@levithomason
Copy link
Member

This actually now has me wondering if/how we can achieve this programmatically. Perhaps that is for another PR, but it would be handy to auto parse this data from the installed semantic-ui-css package.

@kamdz
Copy link
Contributor Author

kamdz commented Feb 22, 2017

Ok, I will change it. For now, manually...

@codecov-io
Copy link

codecov-io commented Feb 22, 2017

Codecov Report

Merging #1372 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1372   +/-   ##
=======================================
  Coverage   99.74%   99.74%           
=======================================
  Files         140      140           
  Lines        2345     2345           
=======================================
  Hits         2339     2339           
  Misses          6        6

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76dfe9b...c4ae8e5. Read the comment docs.

@kamdz
Copy link
Contributor Author

kamdz commented Feb 22, 2017

class names are now 1:1 to this from SUI-css

@levithomason
Copy link
Member

Just one lint issue to fix

/home/ubuntu/Semantic-UI-React/src/lib/SUI.js
  59:1  error  Line 59 exceeds the maximum line length of 120  max-len

@levithomason
Copy link
Member

Also could you please put fixes #1371 in the PR description in order to auto close the issue on merge? Thanks!

https://github.com/blog/1506-closing-issues-via-pull-requests

@layershifter
Copy link
Member

Also, please update Icon typings file.

lint fix
@kamdz
Copy link
Contributor Author

kamdz commented Feb 23, 2017

@levithomason Done
@layershifter But why and how? Now it is:

/** Name of the icon. */
  name?: string;

and for me it's enough. If we change it to SUI.ICONS_AND_ALIASES then users can't set their own custom names for Icon component

@layershifter
Copy link
Member

@kamdz Nevermind, I forgot that we don't have icons list in typings.

@levithomason
Copy link
Member

/home/ubuntu/Semantic-UI-React/src/lib/SUI.js
  59:106  error  Trailing spaces not allowed  no-trailing-spaces

@kamdz
Copy link
Contributor Author

kamdz commented Feb 24, 2017

done

@levithomason levithomason merged commit 9b927c4 into Semantic-Org:master Feb 26, 2017
@levithomason
Copy link
Member

Thanks much!

@levithomason
Copy link
Member

Released in [email protected].

harel pushed a commit to harel/Semantic-UI-React that referenced this pull request Mar 8, 2017
…tic-Org#1372)

* Icon component PropType update to FontAwesome 4.7.0

* changed names from FA to SUI

* Update SUI.js

* Update SUI.js

lint fix

* remove trailing space
harel pushed a commit to harel/Semantic-UI-React that referenced this pull request Mar 8, 2017
…tic-Org#1372)

* Icon component PropType update to FontAwesome 4.7.0

* changed names from FA to SUI

* Update SUI.js

* Update SUI.js

lint fix

* remove trailing space
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.

Icon component PropType update to FontAwesome 4.7.0
4 participants