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

Feature request: Support font-awesome v.5 #7516

Closed
1 task done
gomesp opened this issue Apr 21, 2018 · 15 comments
Closed
1 task done

Feature request: Support font-awesome v.5 #7516

gomesp opened this issue Apr 21, 2018 · 15 comments

Comments

@gomesp
Copy link
Contributor

gomesp commented Apr 21, 2018

Overview of the issue

JHipster is currently supporting font-awesome 4.7.0. The project has now released version 5.0.10. Could we please add support to it?

Unfortunately it is not a simple change of the package version, as the icon names have changed slightly.

This could even trigger another option for the generator, e.g. "What font-awesome icon set would you like to include in your project? 1. Solid, 2. Regular, 3. Light".

I am keen to have a go and open a pull-request at some point if the project is in agreement for the change.

Motivation for or Use Case

To keep up with the font-awesome project development.

  • Checking this box is mandatory (this is just to show you read everything)
@gmarziou
Copy link
Contributor

gmarziou commented Apr 21, 2018

+1
Yes we should upgrade, there are so many new useful icons.

This could even trigger another option for the generator, e.g. "What font-awesome icon set would you like to include in your project? 1. Solid, 2. Regular, 3. Light".

Currently we have 63 places where we use icons in our templates.
It's very easy for a user to do a search/replace on class="fa to select another variant.
So I'm not sure a new question is really needed.

@pascalgrimaud
Copy link
Member

+1 too

@deepu105
Copy link
Member

deepu105 commented Apr 21, 2018 via email

@gomesp
Copy link
Contributor Author

gomesp commented Apr 22, 2018

@deepu105 if you give me 1 week I can do a PR. I imagine that the agreement is to simply replace the package with v.5 and test the generator again, right?

I will keep the icon set with the solid family (fas or fa) to avoid a lot of regression testing. I may have change the css class for any icons that are now under the "font-awesome brands" (e.g. github, etc).

@jdubois
Copy link
Member

jdubois commented Apr 22, 2018

Yes it should not be very hard but that's quite time consuming to test... oh and please have a look at Angular and React - for this part it should be very similar.
Anyway big big +1 if you do that PR!
(At work we are still with v4 and I'd love to have some of the new v5 icons)

@gomesp
Copy link
Contributor Author

gomesp commented Apr 22, 2018

I just opened the pull request #7519.

I decided to open it sooner so that you guys could give feedback faster. I'm not sure if my approach for including the SVG/JS icon set is according to guidelines. If not I can change it and resubmit the PR.

@jdubois I only worked in the angular codebase. But that was mostly because I could not find any instance of the FA icons in the react codebase! That may have to do with me not knowing the first thing about react. ;)

I can look into the react codebase again tomorrow to figure out where FA is being used and open another PR just for react.

@jdubois
Copy link
Member

jdubois commented Apr 22, 2018

Oh I know the Angular codebase much better - maybe it's not included in React.

@agaspardcilia
Copy link
Member

The React client is using a lib for that. It's probably already updated or with a bit of luck, it's just going to be a matter of package version change.

@deepu105
Copy link
Member

@agaspardcilia @gomesp FA now has an official binding for React so maybe we could switch to that as well

@gomesp
Copy link
Contributor Author

gomesp commented Apr 23, 2018

@deepu105 I will have a look at the react codebase later this week to see what's involved.

@agaspardcilia
Copy link
Member

@gomesp I won't mind doing the React part if you don't have the time.

@gomesp
Copy link
Contributor Author

gomesp commented Apr 25, 2018

@agaspardcilia By all means go ahead. I am new to React, so I'd have to spend some time exploring until I'd do something useful. Tag me on the PR so I can read what you've done and learn something too. 👍

@agaspardcilia
Copy link
Member

@gomesp Alright then.

@maznag
Copy link
Contributor

maznag commented Apr 26, 2018

here is a PR to fix sort icon names for fontawesome v5 jhipster/ng-jhipster#70

fa-sort-up and fa-sort-down instead of fa-sort-asc and fa-sort-desc

@jdubois
Copy link
Member

jdubois commented Apr 30, 2018

Closing as this is done for Angular and React!

@jdubois jdubois closed this as completed Apr 30, 2018
@jdubois jdubois added this to the 5.0.0-beta.1 milestone May 3, 2018
kaidohallik pushed a commit to kaidohallik/generator-jhipster that referenced this issue Nov 5, 2020
this PR is to fix sort icon for fontawesome v5

related issue jhipster#7516
kaidohallik pushed a commit to kaidohallik/generator-jhipster that referenced this issue Nov 6, 2020
this PR is to fix sort icon for fontawesome v5

related issue jhipster#7516
kaidohallik pushed a commit to kaidohallik/generator-jhipster that referenced this issue Nov 7, 2020
this PR is to fix sort icon for fontawesome v5

related issue jhipster#7516
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants