Pull Bulma, FA, OpenSans from NPM repository instead of including from CDN #286
Conversation
rabejens
changed the title
|
For some reason, this builds perfectly on Windows. I am currently setting up an environment in WSL Ubuntu and trying to reproduce the error. |
|
When running the tests in WSL Ubuntu, I get this: I don't know what to do against it. @theimowski can you have a look at it? |
|
|
||
| //#if (layout != "none") | ||
| // FontAwesome | ||
| $fa_font_path: "~@fortawesome/fontawesome-free/webfonts"; |
There was a problem hiding this comment.
Is it normal to load both FontAwesome and OpenSans icons at the same time?
There was a problem hiding this comment.
OpenSans is not for icons, it is for the overall fonts, at least that's what I understood.
There was a problem hiding this comment.
I believe I added open sans there as some of the fulma templates used it
There was a problem hiding this comment.
Then we could remove it as well.
There was a problem hiding this comment.
Sorry I meant we still use it when you specify --layout fulma-hero for example. So don't remove it for now
There was a problem hiding this comment.
But here you include it for all the layouts because it's (layout != "none") no?
There was a problem hiding this comment.
The condition is not perfectly valid here, currently layout can be one of:
- none
- fulma-basic
- fulma-hero
- ... (other fulma templates)
Not sure here but probably fulma-basic doesn't need the font, so it would be better to have layout != "none" && layout != "fulma-basic" but I was too lazy for that
There was a problem hiding this comment.
End game is we want to eventually leave just none and fulma-basic so we could remove the font then
|
When testing the PR locally I get following error: never seen that before |
|
Interestingly, removing final newline from *.scss fixed the issue for me |
Content/package.json
Outdated
| @@ -24,5 +30,13 @@ | |||
| "webpack-cli": "^3.1.2", | |||
| "webpack-dev-server": "^3.1.14", | |||
| "whatwg-fetch": "^3.0.0" | |||
| //#if (layout != "none") | |||
| }, | |||
| "dependencies": { | |||
There was a problem hiding this comment.
do these need to be in dependencies, or can we move them to devDependencies?
There was a problem hiding this comment.
I don't know, I'm sorry, but I can try if it works with devDependencies when I return home from work.
Content/package.json
Outdated
| @@ -8,16 +8,16 @@ | |||
| "@babel/runtime": "^7.4.4", | |||
| //#if (layout != "none") | |||
| "@fortawesome/fontawesome-free": "^5.9.0", | |||
| "file-loader": "^4.0.0", | |||
| "bulma": "^0.7.1", | |||
| "open-sans-fonts": "^1.5.1", | |||
There was a problem hiding this comment.
Making all dependencies devDependencies seems to work.
|
This looks good now - let me first fix the Travis build and then rerun this PR to see all works as expected |
|
I decided to remove the if condition for package.json - it adds another layer of complexity and makes it hard to update package.json.lock and yarn.lock. |
|
The change results in following warnings: Anyone knows what the SVG files are for? Related issue: #185 |
|
Thanks! released in v1.11 |
This PR causes Bulma, FontAwesome Free and OpenSans to be pulled from NPM repository instead of CDN. This makes it possible to use SAFE applications on machines without any connection to the Internet.
Related discussion: #281