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

Update Font Awesome to 5.0.6 #129

Merged
merged 3 commits into from
Aug 10, 2018
Merged

Update Font Awesome to 5.0.6 #129

merged 3 commits into from
Aug 10, 2018

Conversation

matalo33
Copy link
Contributor

@matalo33 matalo33 commented Feb 19, 2018

I wanted a specific icon which was only made available in v5 of FontAwesome so I went ahead and upgraded the sitewide version.

With v5 there's no longer any need to keep the binary files locally, they are all loaded remotely, so this PR deletes all the local FA assets.

In layouts/partials/header.html I load the remote CSS. In v5 the icons have been split from fa into fas (fontawesome solid) and fab (fontawesome brands). There's a shim to ease transition to v5 but I just replaced all the selectors to point to the new icons so I am NOT loading the upgrade shim. Full v4 to v5 docs are here: https://fontawesome.com/how-to-use/upgrading-from-4

I also wrote a help page on how to include icons. As a user I was not aware the theme loaded fontawesome before I really dug into the source code. It's only in English, sorry i don't speak French :)

@matcornic
Copy link
Owner

Thank you, it's a very nice PR!

I would prefer local assets though. This kind of documentation theme can be used in private networks that do not have Internet access. So, we should avoid having assets loaded from Internet.

@matalo33
Copy link
Contributor Author

Ok I didn't consider offline use on private network. I'll bring assets back into local.

@matalo33
Copy link
Contributor Author

matalo33 commented Feb 19, 2018

Loading the new CSS file, the name of which changed to fontawesome-all.min.css.

The css loads the fonts from the path ../webfonts/ so rather than modify the upstream CSS I added the font files into webfonts. This does mean there is now a fonts/ and webfonts/.

@matalo33
Copy link
Contributor Author

Bump. Any possibility of getting this pushed through?

@asoltysik
Copy link

@matcornic Also bumping, it would be awesome if this PR was merged.

@matcornic matcornic merged commit efbd24b into matcornic:master Aug 10, 2018
@matcornic
Copy link
Owner

Merge, I updated to 5.2.0 too :)

@matalo33 matalo33 added the enhancement Improvements to existing features label Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvements to existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants