-
Notifications
You must be signed in to change notification settings - Fork 680
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
Remove all font files except WOFF #682
Conversation
@@ -3,11 +3,8 @@ | |||
var fs = require("fs-extra"); | |||
|
|||
var srcDir = "./node_modules/font-awesome/fonts/"; | |||
var destDir = "./client/fonts/"; | |||
var destDir = "./client/css/fonts/"; |
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.
@xPaw, this was pulled to client/fonts/
on purpose. I believe all fonts should be there instead of client/css/fonts/
, but this is also a way to locate the fonts installed by our (up and coming) asset pipeline differently.
What do you think?
@@ -5,7 +5,8 @@ coverage/ | |||
|
|||
# Built assets created at npm install/prepublish time | |||
# See https://docs.npmjs.com/misc/scripts | |||
client/fonts/ | |||
client/css/fonts/fontawesome-webfont.woff2 | |||
client/css/fonts/fontawesome-webfont.woff |
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.
Another benefit of putting the installed fonts in client/fonts/
is that we do not have to keep track of the specific file in different configuration/ignore files
This is great! A couple comments before approving. |
For the record:
|
Remove all font files except WOFF
WOFF is supported 92% globally (basically everywhere, except for Opera Mini which simply does not support any fonts). WOFF2 is supported 64% globally, pretty much in all latest browsers.