Skip to content

Commit 6ace17f

Browse files
petermuessigmatz3
petermuessig
and
matz3
authored
[FIX] CSS Variables: Fix variable handling / relative urls (#172)
- Improve detection of library variables - Resolve url values in variables - Handle variables in the proper order - Enhance overall CSS Variables tests Co-authored-by: Matthias Osswald <mat.osswald@sap.com>
1 parent a6fea7c commit 6ace17f

22 files changed

+334
-53
lines changed

‎lib/index.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,10 @@ Builder.prototype.build = function(options) {
191191
// inject the library name as prefix (e.g. "sap.m" as "_sap_m")
192192
if (options.library && typeof options.library.name === "string") {
193193
const libName = config.libName = options.library.name;
194+
config.libPath = libName.replace(/\./g, "/");
194195
config.prefix = "_" + libName.replace(/\./g, "_") + "_";
196+
} else {
197+
config.prefix = ""; // no library, no prefix
195198
}
196199

197200
// Keep filename for later usage (see ImportCollectorPlugin) as less modifies the parserOptions.filename
@@ -287,10 +290,12 @@ Builder.prototype.build = function(options) {
287290
// generate the skeleton-css and the less-variables
288291
const CSSVariablesCollectorPlugin = require("./plugin/css-variables-collector");
289292
const oCSSVariablesCollector = new CSSVariablesCollectorPlugin(config);
293+
const oVariableCollector = new VariableCollectorPlugin(options.compiler);
290294
result.cssSkeleton = tree.toCSS(Object.assign({}, options.compiler, {
291-
plugins: [oCSSVariablesCollector]
295+
plugins: [oCSSVariablesCollector, oVariableCollector]
292296
}));
293-
result.cssVariablesSource = oCSSVariablesCollector.toLessVariables();
297+
const varsOverride = oVariableCollector.getAllVariables();
298+
result.cssVariablesSource = oCSSVariablesCollector.toLessVariables(varsOverride);
294299
if (oRTL) {
295300
const oCSSVariablesCollectorRTL = new CSSVariablesCollectorPlugin(config);
296301
result.cssSkeletonRtl = tree.toCSS(Object.assign({}, options.compiler, {

‎lib/plugin/css-variables-collector.js

Lines changed: 53 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@ const CSSVariablesCollectorPlugin = module.exports = function(config) {
1111
this.ruleStack = [];
1212
this.mixinStack = [];
1313
this.parenStack = [];
14-
this.importStack = [];
1514
};
1615

1716
CSSVariablesCollectorPlugin.prototype = {
1817

18+
// needed to keep the less variable references intact to use this info for the CSS variables references
1919
isPreEvalVisitor: true,
20+
2021
isReplacing: true,
2122

2223
_isInMixinOrParen() {
@@ -27,27 +28,44 @@ CSSVariablesCollectorPlugin.prototype = {
2728
return this.ruleStack.length > 0 && !this.ruleStack[this.ruleStack.length - 1].variable;
2829
},
2930

30-
_isInGlobalOrBaseImport() {
31-
return this.config.libName !== "sap.ui.core" && this.importStack.filter((importNode) => {
32-
return /\/(?:global|base)\.less$/.test(importNode.importedFilename);
33-
}).length > 0;
31+
_isVarInLibrary({filename} = {}) {
32+
// for libraries we check that the file is within the libraries theme path
33+
// in all other cases with no filename (indicates calculated variables)
34+
// or in case of variables in standalone less files we just include them!
35+
const regex = new RegExp(`(^|/)${this.config.libPath}/themes/`);
36+
const include = !filename ||
37+
(this.config.libPath ? regex.test(filename) : true);
38+
return include;
3439
},
3540

3641
_isRelevant() {
3742
return !this._isInMixinOrParen() && this._isVarInRule();
3843
},
3944

40-
toLessVariables() {
45+
toLessVariables(varsOverride) {
46+
const vars = {};
47+
Object.keys(this.vars).forEach((key) => {
48+
const override = this.vars[key].updateAfterEval && varsOverride[key] !== undefined;
49+
/*
50+
if (override) {
51+
console.log(`Override variable "${key}" from "${this.vars[key].css}" to "${varsOverride[key]}"`);
52+
}
53+
*/
54+
vars[key] = {
55+
css: override ? varsOverride[key] : this.vars[key].css,
56+
export: this.vars[key].export
57+
};
58+
});
4159
let lessVariables = "";
42-
Object.keys(this.vars).forEach((value, index) => {
43-
lessVariables += "@" + value + ": " + this.vars[value].css + ";\n";
60+
Object.keys(vars).forEach((value, index) => {
61+
lessVariables += "@" + value + ": " + vars[value].css + ";\n";
4462
});
4563
Object.keys(this.calcVars).forEach((value, index) => {
4664
lessVariables += "@" + value + ": " + this.calcVars[value].css + ";\n";
4765
});
4866
lessVariables += "\n:root {\n";
49-
Object.keys(this.vars).forEach((value, index) => {
50-
if (this.vars[value].export) {
67+
Object.keys(vars).forEach((value, index) => {
68+
if (vars[value].export) {
5169
lessVariables += "--" + value + ": @" + value + ";\n";
5270
}
5371
});
@@ -89,15 +107,17 @@ CSSVariablesCollectorPlugin.prototype = {
89107

90108
visitOperation(node, visitArgs) {
91109
if (this._isRelevant()) {
110+
// console.log("visitOperation", this.ruleStack[this.ruleStack.length - 1], this._getCSS(node));
92111
return new less.tree.Call("calc", [new less.tree.Expression([node.operands[0], new less.tree.Anonymous(node.op), node.operands[1]])]);
93112
}
94113
return node;
95114
},
96115

97116
visitCall(node, visitArgs) {
98-
// if variables are used inside rules, generate a new dynamic variable for it!
117+
// if variables are used inside rules, generate a new calculated variable for it!
99118
const isRelevantFunction = typeof less.tree.functions[node.name] === "function" && ["rgba"].indexOf(node.name) === -1;
100119
if (this._isRelevant() && isRelevantFunction) {
120+
// console.log("visitCall", this.ruleStack[this.ruleStack.length - 1], this._getCSS(node));
101121
const css = this._getCSS(node);
102122
let newName = this.config.prefix + "function_" + node.name + Object.keys(this.vars).length;
103123
// check for duplicate value in vars already
@@ -109,7 +129,7 @@ CSSVariablesCollectorPlugin.prototype = {
109129
}
110130
this.calcVars[newName] = {
111131
css: css,
112-
export: !this._isInGlobalOrBaseImport()
132+
export: this._isVarInLibrary()
113133
};
114134
return new less.tree.Call("var", [new less.tree.Anonymous("--" + newName, node.index, node.currentFileInfo, node.mapLines)]);
115135
}
@@ -119,6 +139,7 @@ CSSVariablesCollectorPlugin.prototype = {
119139
visitNegative(node, visitArgs) {
120140
// convert negative into calc function
121141
if (this._isRelevant()) {
142+
// console.log("visitNegative", this.ruleStack[this.ruleStack.length - 1], this._getCSS(node));
122143
return new less.tree.Call("calc", [new less.tree.Expression([new less.tree.Anonymous("-1"), new less.tree.Anonymous("*"), node.value])]);
123144
}
124145
return node;
@@ -133,6 +154,19 @@ CSSVariablesCollectorPlugin.prototype = {
133154
},
134155

135156
visitRule(node, visitArgs) {
157+
// check rule for being a variable declaration
158+
const isVarDeclaration = typeof node.name === "string" && node.name.startsWith("@");
159+
if (!this._isInMixinOrParen() && isVarDeclaration) {
160+
// add the variable declaration to the list of vars
161+
const varName = node.name.substr(1);
162+
const isVarInLib = this._isVarInLibrary({
163+
filename: node.currentFileInfo.filename
164+
});
165+
this.vars[varName] = {
166+
css: this._getCSS(node.value),
167+
export: isVarInLib
168+
};
169+
}
136170
// store the rule context for the call variable extraction
137171
this.ruleStack.push(node);
138172
return node;
@@ -168,29 +202,13 @@ CSSVariablesCollectorPlugin.prototype = {
168202
return node;
169203
},
170204

171-
visitImport(node, visitArgs) {
172-
// store the import context
173-
this.importStack.push(node);
174-
return node;
175-
},
176-
177-
visitImportOut(node) {
178-
// remove import context
179-
this.importStack.pop();
180-
return node;
181-
},
182-
183-
visitRuleset(node, visitArgs) {
184-
node.rules.forEach((value) => {
185-
const isVarDeclaration = value instanceof less.tree.Rule && typeof value.name === "string" && value.name.startsWith("@");
186-
if (!this._isInMixinOrParen() && isVarDeclaration) {
187-
// add the variable declaration to the list of vars
188-
this.vars[value.name.substr(1)] = {
189-
css: this._getCSS(value.value),
190-
export: !this._isInGlobalOrBaseImport()
191-
};
192-
}
193-
});
205+
visitUrl(node, visitArgs) {
206+
// we mark the less variables which should be updated after eval
207+
// => strangewise less variables with "none" values are also urls
208+
// after the less variables have been evaluated
209+
if (this.ruleStack.length > 0 && this.ruleStack[0].variable) {
210+
this.vars[this.ruleStack[0].name.substr(1)].updateAfterEval = true;
211+
}
194212
return node;
195213
}
196214

‎test/expected/complex/test-cssvars-skeleton.css

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,34 @@ a:hover {
99
color: #fff;
1010
background: var(--link-color);
1111
}
12+
.background {
13+
background-image: var(--backgroundImage);
14+
}
1215
.lazy-eval {
1316
width: var(--var);
1417
}
18+
.nested .calc-vars {
19+
height: var(--c);
20+
width: var(--d);
21+
color: var(--function_lighten14);
22+
top: calc(-1 * var(--top));
23+
line-height: calc(var(--lineHeight) / 2);
24+
background: #428bca;
25+
border-color: #92bce0;
26+
background: #5697d0;
27+
}
28+
.nested .calc-vars-duplicate {
29+
color: var(--function_lighten14);
30+
padding-left: calc(20px + 0.5rem);
31+
border-color: #ffffff;
32+
background: #ffffff;
33+
}
34+
.nested .calc-vars-duplicate somePrefix.somePadding {
35+
padding: 1rem;
36+
box-sizing: border-box;
37+
}
38+
@media (max-width: 599px) {
39+
.nested .calc-vars-duplicate somePrefix.somePadding {
40+
padding: 0;
41+
}
42+
}

‎test/expected/complex/test-cssvars-variables.css

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,15 @@
33
--link-color-hover: #3071a9;
44
--other-var: 0 0 0.25rem 0 rgba(0, 0, 0, 0.15), inset 0 -0.0625rem 0 0 #428bca;
55
--link-color-active: var(--link-color);
6+
--backgroundVariant: 1;
7+
--backgroundImage: url(img.png);
8+
--a: 100%;
69
--var: var(--a);
7-
--a: 9%;
10+
--b: #245682;
11+
--padLeft: 20px;
12+
--top: 5rem;
13+
--lineHeight: 2rem;
14+
--c: calc(20%);
15+
--d: -100%;
16+
--function_lighten14: #3071a9;
817
}

‎test/expected/complex/test-cssvars-variables.source.less

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
@link-color: #428bca;
2+
@link-color-hover: darken(@link-color, 10%);
3+
@other-var: 0 0 0.25rem 0 rgba(0, 0, 0, 0.15), inset 0 -0.0625rem 0 0 @link-color;
4+
@link-color-active: @link-color;
5+
@backgroundVariant: 1;
6+
@backgroundImage: url(img.png);
7+
@a: 100%;
8+
@var: @a;
9+
@b: darken(@link-color-active, 20%);
10+
@padLeft: 20px;
11+
@top: 5rem;
12+
@lineHeight: 2rem;
13+
@c: calc(100% - 80px);
14+
@d: -@a;
15+
@function_lighten14: lighten(@b, 10%);
16+
17+
:root {
18+
--link-color: @link-color;
19+
--link-color-hover: @link-color-hover;
20+
--other-var: @other-var;
21+
--link-color-active: @link-color-active;
22+
--backgroundVariant: @backgroundVariant;
23+
--backgroundImage: @backgroundImage;
24+
--a: @a;
25+
--var: @var;
26+
--b: @b;
27+
--padLeft: @padLeft;
28+
--top: @top;
29+
--lineHeight: @lineHeight;
30+
--c: @c;
31+
--d: @d;
32+
--function_lighten14: @function_lighten14;
33+
}

‎test/expected/complex/test.css

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,34 @@ a:hover {
99
color: #fff;
1010
background: #428bca;
1111
}
12+
.background {
13+
background-image: url(img.png);
14+
}
1215
.lazy-eval {
1316
width: 9%;
1417
}
18+
.nested .calc-vars {
19+
height: calc(20%);
20+
width: -100%;
21+
color: #3071a9;
22+
top: -5rem;
23+
line-height: 1rem;
24+
background: #428bca;
25+
border-color: #92bce0;
26+
background: #5697d0;
27+
}
28+
.nested .calc-vars-duplicate {
29+
color: #3071a9;
30+
padding-left: calc(20px + 0.5rem);
31+
border-color: #ffffff;
32+
background: #ffffff;
33+
}
34+
.nested .calc-vars-duplicate somePrefix.somePadding {
35+
padding: 1rem;
36+
box-sizing: border-box;
37+
}
38+
@media (max-width: 599px) {
39+
.nested .calc-vars-duplicate somePrefix.somePadding {
40+
padding: 0;
41+
}
42+
}

‎test/expected/libraries/lib1/my/ui/lib/themes/base/library-RTL.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
}
77

88
/* Inline theming parameters */
9-
#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22color1%22%3A%22%23fefefe%22%7D')}
9+
#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22color1%22%3A%22%23fefefe%22%2C%22url1%22%3A%22url(%27111%27)%22%7D')}

‎test/expected/libraries/lib1/my/ui/lib/themes/base/library.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
}
77

88
/* Inline theming parameters */
9-
#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22color1%22%3A%22%23fefefe%22%7D')}
9+
#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22color1%22%3A%22%23fefefe%22%2C%22url1%22%3A%22url(%27111%27)%22%7D')}

‎test/expected/libraries/lib1/my/ui/lib/themes/foo/css_variables.css

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
:root {
2+
--color1: #ffffff;
3+
--url1: url('../base/111');
4+
}
5+
.fooContrast {
6+
--color1: #000000;
7+
}
8+
9+
/* Inline theming parameters */
10+
#sap-ui-theme-my\.ui\.lib{background-image:url('data:text/plain;utf-8,%7B%22default%22%3A%7B%22color1%22%3A%22%23ffffff%22%2C%22url1%22%3A%22url(%27..%2Fbase%2F111%27)%22%7D%2C%22scopes%22%3A%7B%22fooContrast%22%3A%7B%22color1%22%3A%22%23000000%22%7D%7D%7D')}

‎test/expected/libraries/lib1/my/ui/lib/themes/foo/css_variables.source.less

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
@color1: #ffffff;
2+
@url1: url('../base/111');
3+
4+
:root {
5+
--color1: @color1;
6+
--url1: @url1;
7+
}

0 commit comments

Comments
 (0)