Css4 variables migration

[skjnldsv]

Why?

• Reactivity: css4 variables can be changed on-the-fly and provide a theming possibility preview far easier and smooth
• External css: others apps/modules can use our variables without scss
• Vue components: we can't process our scoped scss from the vue components and exclude the variables compilation from the process. If we want to allow the use of variables, this is the only way.

Fixes #6727

How

• Convert our scss vars to css4
• Cleanup our multiple darken/lighten functions (who are incompatible with css4 vars)
• rgba fixes
• theming migration
• Implement IE11 ponyfill (which works perfectly only a small downside, see comments)

[codecov]

Codecov Report

Merging #9723 into master will decrease coverage by 0.02%.
The diff coverage is 90.9%.

@@             Coverage Diff              @@
##             master    #9723      +/-   ##
============================================
- Coverage     52.08%   52.06%   -0.03%     
+ Complexity    25915    25901      -14     
============================================
  Files          1642     1642              
  Lines         95747    95876     +129     
  Branches       1289     1319      +30     
============================================
+ Hits          49874    49920      +46     
- Misses        45873    45956      +83

Impacted Files	Coverage Δ	Complexity Δ
...ings/templates/settings/personal/personal.info.php	0% <ø> (ø)	0 <0> (ø)	⬇️
settings/templates/settings/frame.php	0% <ø> (ø)	0 <0> (ø)	⬇️
apps/theming/lib/Controller/ThemingController.php	69.23% <100%> (ø)	35 <0> (ø)	⬇️
apps/theming/lib/ThemingDefaults.php	97.29% <100%> (ø)	52 <0> (ø)	⬇️
lib/private/legacy/template.php	29.87% <100%> (-0.46%)	42 <0> (ø)
core/js/js.js	65.53% <50%> (-0.04%)	0 <0> (ø)
apps/oauth2/lib/Settings/Admin.php	92.3% <0%> (-7.7%)	4% <0%> (+1%)
apps/oauth2/lib/Controller/SettingsController.php	72.41% <0%> (-5.85%)	3% <0%> (-2%)
...B/QueryBuilder/FunctionBuilder/FunctionBuilder.php	77.77% <0%> (-2.23%)	9% <0%> (-1%)
lib/private/User/Database.php	79.47% <0%> (-1.78%)	43% <0%> (+1%)
... and 12 more

[rullzer]

Question time!

• This works in all supported browsers?
• Would this mean we could eventually move to generated CSS (not on the fly like we do now but in a make file?) This just shipping plain files?

[skjnldsv]

This works in all supported browsers?

Yes, except IE11 (but the ponyfill is working fine and will be added into this pr as well)

Would this mean we could eventually move to generated CSS (not on the fly like we do now but in a make file?) This just shipping plain files?

No because scss also add more stuff like the colour manipulation functions, calculation and most importantly, our nesting css properties :)

[rullzer]

No because scss also add more stuff like the colour manipulation functions, calculation and most importantly, our nesting css properties :)

I wasn't suggesting killing SCSS. More like if we could do it only once when creating tarballs basically. But I guess not if we need colour manipulating functions etc.

[skjnldsv]

@rullzer sorry, I misunderstood :)
Since scss are pre-processed scripts, we can already do that.
So yes, in the end the only colours manipulations functions are used (and can only be used for this exact purpose) in the variables.scss function. Since the vars are going to be forwarded to css4 variables, we could probably ship fully compiled css files within tarballs. It will still require to migrate the thming app to css4 vars before doing so though :)

[skjnldsv]

@juliushaertl would you take a look at the theming part?
I guess you just need to redefine the variables you need to change into the css :root ?

[juliusknorr]

@skjnldsv Sure, I'll have a look.

[juliusknorr]

@skjnldsv I adjusted the theming preview code to just reload the css variables and the theming.scss file (which has some additional customizations) It is indeed a lot faster than rebuilding the whole server.scss file.

I didn't move the whole theming to css vars for now, since that would actually be something i would tackle in a separate PR. Would that be ok for you?

[skjnldsv]

I didn't move the whole theming to css vars for now, since that would actually be something i would tackle in a separate PR. Would that be ok for you?

Perfect!

@juliushaertl For 14, we'll keep them both since theming is designed to be instance-wide, I like it a lot that we have both available
For 15 I'll say we remove the variables.scss from the compile import to ensure no apps use them anymore. Like that we force the css4 variables everywhere and they're still managed by our scss variables and the neat colours functions sass provides. What do you think?

[skjnldsv]

@danxuliu There is some strange behaviour with some acceptance tests. Sometimes it passes, sometimes not, maybe a timeout issue? Could you take a quick look please? <3

[skjnldsv]

@nextcloud/designers I know we all hate IE11, but I need your opinion.
The css4 ponyfill script works fine but have a little downside to it. I'll let you see it in action. Is this still acceptable ? If not we'll have to go for something else and theming per-user won't be compatible with ie11.

[MorrisJobke]

@nextcloud/designers I know we all hate IE11, but I need your opinion.
The css4 ponyfill script works fine but have a little downside to it. I'll let you see it in action. Is this still acceptable ? If not we'll have to go for something else and theming per-user won't be compatible with ie11.

Is the flickering the downside? Then it should be okay.

[skjnldsv]

@MorrisJobke Yes, the flickering! :)

[danxuliu]

@skjnldsv

There is some strange behaviour with some acceptance tests. Sometimes it passes, sometimes not, maybe a timeout issue? Could you take a quick look please? <3

Sure, here is the summary:

• failures in app-theming were caused by the new transition added to the header colour; while I really love it, in the future please add those type of enhancements in a different pull request, specially with pull requests as big as this one ;-)
• step I close the details view in app-files is a known race condition that I have to fix
• step I click the New user button in app-users is expected to fail sometimes with its current implementation (remember my pending clean up? ;-) )
• although I have a hunch, I need to take a deeper look to why step I open the XXX section in app fails; it should not be related to the changes in this pull request, though

As they are not related to this one, the pending changes to the acceptance tests will be done in other pull requests ;-)

[skjnldsv]

@danxuliu thanks!! <3

[skjnldsv]

Failure not related

sh: 1: kill: No such process