Enable every app to generate their scss file #3197
Conversation
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
skjnldsv
requested review from
rullzer,
MorrisJobke,
jancborchardt,
ChristophWurst,
eppfel and
blizzz
| $style = substr($style, strpos($style, '/')+1); | ||
| $app_path = \OC_App::getAppPath($app); | ||
| $app_url = \OC_App::getAppWebPath($app); | ||
| $this->append($app_path, $style.'.css', $app_url); | ||
| if(!$this->cacheAndAppendScssIfExist($this->serverroot.'/apps', $app.'/'.$style.'.scss', $app)) { |
There was a problem hiding this comment.
Use
if(!$this->cacheAndAppendScssIfExist($app_path, $style.'.scss', $app)) {instead to make sure things work when using alternative app directories.
There was a problem hiding this comment.
if(!$this->cacheAndAppendScssIfExist($app_path, $style.'.scss', null, $app)) {to pass the correct app parameter
There was a problem hiding this comment.
How did I missed that!
Thanks!
There was a problem hiding this comment.
Hum, after a quick thought, it seems that we never used the webroot here.
So I'm not sure we need it :p
There was a problem hiding this comment.
Webroot isn't needed with scss since we load it trough a route anyway, for css it's still needed afaik
edit: ignore this, was talking about different webroot
There was a problem hiding this comment.
I fixed it.
We had the webroot var, but it wasn't used by anything nor passed through a function.
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
|
@skjnldsv still need to use |
|
@icewind1991 what do you mean? |
|
An admin can configure multiple folder that apps can be stored in besides
|
|
Oh nice! |
|
Conveniently we already have the variable 3 lines above it 😄 |
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
|
@icewind1991 you're amazing ! 🚀 |
Current coverage is 54.08% (diff: 0.00%)@@ master #3197 diff @@
==========================================
Files 1299 1299
Lines 80145 81069 +924
Methods 7909 8111 +202
Messages 0 0
Branches 1248 1248
==========================================
+ Hits 43228 43845 +617
- Misses 36917 37224 +307
Partials 0 0
|
|
I would have loved a little feedback from @nextcloud/security . |
With this pr, every app can use the core scss compile&cache system.
You just have to change your css file to a .scss and the server will handle the rest :)
ex:
/core/css/styles.scsswill be stored underappdata/css/core/styles.cssor:
/settings/css/settings.scssinappdata/css/settings/settings.cssor:
/apps/mail/css/mail.scssinappdata/mail/mail.css@eppfel, it will be helpful for your new app store design #3194 and #3195 :)
@nextcloud/security Can I have a validation from you? What are the possible outcome from such implementation?