-
Notifications
You must be signed in to change notification settings - Fork 156
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
Material icons node dependency #127
Conversation
@DeepDiver1975 can you help move the copy process to a nice method in the Makefile? … Pleeeeeeeease 😁 |
you need to think in terms of "what is the target file" and "what are the source files" and "what is the command":
for compile-themes:
Makefile rule:
See https://www.gnu.org/software/make/manual/html_node/Automatic-Variables.html for compile-icons part 1:
Makefile rule:
Where for compile-icons part 2:
Makefile rule:
Then you need to make sure all these run when running regular make, so add more alias rules:
|
next, you need to split
then change core rule to just depend on the other rules
|
@felixheidecke conflicting |
8d96068
to
62c5a18
Compare
Fixes #228 |
62c5a18
to
0b22d6e
Compare
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.
see comments
Makefile
Outdated
@@ -19,18 +19,28 @@ node_modules: package.json package-lock.json | |||
core/css/uikit.%.css: src/themes/%.less node_modules | |||
node_modules/less/bin/lessc src/themes/$*.less core/css/uikit.$*.css --relative-urls | |||
|
|||
core/css/material-icons.css: src/themes/icons.scss node_modules | |||
node node_modules/.bin/sass src/themes/icons.scss core/css/material-icons.css --no-source-map |
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.
is the node
in front of it required ? I thought one can just use directly whatever is in .bin
(it might have a shebang)
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.
you can replace "core/css/material-icons.css" with $@
which is an alias for the target rule if you like
mkdir core/fonts | ||
|
||
core/fonts/MaterialIcons-Regular.%: core/fonts node_modules | ||
cp node_modules/material-icons/iconfont/MaterialIcons-Regular.* core/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.
if you use the placeholder %
in the rule name, you need to also use it somewhere here in this command as $*
.
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.
well - I basically want all MaterialIcons-Regular.* to be copied over
what would be the best rule to write this?
still asking myself if this is the right approach - can't webpack just grab that?
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.
yeah, I suspect webpack should be able to grab and even compile some stuff directly without needing extra Makefile rules.
@@ -26,11 +36,11 @@ core/js/core.bundle.js: node_modules | |||
# core | |||
# | |||
.PHONY: core | |||
core: core/js/core.bundle.js core/css/uikit.owncloud.css | |||
core: core/js/core.bundle.js core/css/uikit.owncloud.css core/css/material-icons.css core/fonts/MaterialIcons-Regular.% |
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.
this might work only by chance
the "%" placeholder here will never have a value
I suggest just using "core/fonts" as dependency and have all the copying stuff done inside the "core/fonts" rule
why not use https://www.npmjs.com/package/vue-material-design-icons ? |
has hint for webpack - https://gitlab.com/robcresswell/vue-material-design-icons#tips |
https://www.npmjs.com/package/vue-material-design-icons is MIT, but they're using https://github.com/Templarian/MaterialDesign , which is using SIL OPEN FONT LICENSE. Does this work for us? |
|
The same reason why I don't fancy vueify right now. These would have to be compiled into each app instead of the theme. |
Can't we Deal with this by some intelligent bundling? |
Remove link to fonts.googleapis.com in favor of
material-icons
node dependency