-
Notifications
You must be signed in to change notification settings - Fork 13
[FIX] CSS Variables: Fix variable handling / relative urls #172
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
Changes from 12 commits
c8f3595
77ca02c
6217f5a
1fbb1a6
b2ec78d
64c00e9
c99433f
1f41cc1
9e52bc9
6169b39
76edeed
15c6fb8
a24aa11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,12 +11,13 @@ const CSSVariablesCollectorPlugin = module.exports = function(config) { | |
this.ruleStack = []; | ||
this.mixinStack = []; | ||
this.parenStack = []; | ||
this.importStack = []; | ||
}; | ||
|
||
CSSVariablesCollectorPlugin.prototype = { | ||
|
||
// needed to keep the less variable references intact to use this info for the CSS variables references | ||
isPreEvalVisitor: true, | ||
|
||
isReplacing: true, | ||
|
||
_isInMixinOrParen() { | ||
|
@@ -27,27 +28,44 @@ CSSVariablesCollectorPlugin.prototype = { | |
return this.ruleStack.length > 0 && !this.ruleStack[this.ruleStack.length - 1].variable; | ||
}, | ||
|
||
_isInGlobalOrBaseImport() { | ||
return this.config.libName !== "sap.ui.core" && this.importStack.filter((importNode) => { | ||
return /\/(?:global|base)\.less$/.test(importNode.importedFilename); | ||
}).length > 0; | ||
_isVarInLibrary({varName, filename}) { | ||
matz3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// for libraries we check that the file is within the libraries theme path | ||
// in all other cases with no filename (indicates calculated variables) | ||
// or in case of variables in standalone less files we just include them! | ||
const regex = new RegExp(`(^|/)${this.config.libPath}/themes/`); | ||
const include = !filename || | ||
(this.config.libPath ? regex.test(filename) : true); | ||
return include; | ||
}, | ||
|
||
_isRelevant() { | ||
return !this._isInMixinOrParen() && this._isVarInRule(); | ||
}, | ||
|
||
toLessVariables() { | ||
toLessVariables(varsOverride) { | ||
const vars = {}; | ||
Object.keys(this.vars).forEach((key) => { | ||
const override = this.vars[key].updateAfterEval && varsOverride[key] !== undefined; | ||
/* | ||
if (override) { | ||
console.log(`Override variable "${key}" from "${this.vars[key].css}" to "${varsOverride[key]}"`); | ||
} | ||
*/ | ||
vars[key] = { | ||
css: override ? varsOverride[key] : this.vars[key].css, | ||
export: this.vars[key].export | ||
}; | ||
}); | ||
let lessVariables = ""; | ||
Object.keys(this.vars).forEach((value, index) => { | ||
lessVariables += "@" + value + ": " + this.vars[value].css + ";\n"; | ||
Object.keys(vars).forEach((value, index) => { | ||
lessVariables += "@" + value + ": " + vars[value].css + ";\n"; | ||
}); | ||
Object.keys(this.calcVars).forEach((value, index) => { | ||
lessVariables += "@" + value + ": " + this.calcVars[value].css + ";\n"; | ||
}); | ||
lessVariables += "\n:root {\n"; | ||
Object.keys(this.vars).forEach((value, index) => { | ||
if (this.vars[value].export) { | ||
Object.keys(vars).forEach((value, index) => { | ||
if (vars[value].export) { | ||
lessVariables += "--" + value + ": @" + value + ";\n"; | ||
} | ||
}); | ||
|
@@ -89,15 +107,17 @@ CSSVariablesCollectorPlugin.prototype = { | |
|
||
visitOperation(node, visitArgs) { | ||
if (this._isRelevant()) { | ||
// console.log("visitOperation", this.ruleStack[this.ruleStack.length - 1], this._getCSS(node)); | ||
return new less.tree.Call("calc", [new less.tree.Expression([node.operands[0], new less.tree.Anonymous(node.op), node.operands[1]])]); | ||
} | ||
return node; | ||
}, | ||
|
||
visitCall(node, visitArgs) { | ||
// if variables are used inside rules, generate a new dynamic variable for it! | ||
// if variables are used inside rules, generate a new calculated variable for it! | ||
const isRelevantFunction = typeof less.tree.functions[node.name] === "function" && ["rgba"].indexOf(node.name) === -1; | ||
if (this._isRelevant() && isRelevantFunction) { | ||
// console.log("visitCall", this.ruleStack[this.ruleStack.length - 1], this._getCSS(node)); | ||
const css = this._getCSS(node); | ||
let newName = this.config.prefix + "function_" + node.name + Object.keys(this.vars).length; | ||
// check for duplicate value in vars already | ||
|
@@ -109,7 +129,9 @@ CSSVariablesCollectorPlugin.prototype = { | |
} | ||
this.calcVars[newName] = { | ||
css: css, | ||
export: !this._isInGlobalOrBaseImport() | ||
export: this._isVarInLibrary({ | ||
varName: newName | ||
}) | ||
}; | ||
return new less.tree.Call("var", [new less.tree.Anonymous("--" + newName, node.index, node.currentFileInfo, node.mapLines)]); | ||
} | ||
|
@@ -119,6 +141,7 @@ CSSVariablesCollectorPlugin.prototype = { | |
visitNegative(node, visitArgs) { | ||
// convert negative into calc function | ||
if (this._isRelevant()) { | ||
// console.log("visitNegative", this.ruleStack[this.ruleStack.length - 1], this._getCSS(node)); | ||
return new less.tree.Call("calc", [new less.tree.Expression([new less.tree.Anonymous("-1"), new less.tree.Anonymous("*"), node.value])]); | ||
} | ||
return node; | ||
|
@@ -133,6 +156,20 @@ CSSVariablesCollectorPlugin.prototype = { | |
}, | ||
|
||
visitRule(node, visitArgs) { | ||
// check rule for being a variable declaration | ||
const isVarDeclaration = typeof node.name === "string" && node.name.startsWith("@"); | ||
if (!this._isInMixinOrParen() && isVarDeclaration) { | ||
// add the variable declaration to the list of vars | ||
const varName = node.name.substr(1); | ||
const isVarInLib = this._isVarInLibrary({ | ||
varName, | ||
filename: node.currentFileInfo.filename | ||
}); | ||
this.vars[varName] = { | ||
css: this._getCSS(node.value), | ||
export: isVarInLib | ||
}; | ||
} | ||
// store the rule context for the call variable extraction | ||
this.ruleStack.push(node); | ||
return node; | ||
|
@@ -168,29 +205,13 @@ CSSVariablesCollectorPlugin.prototype = { | |
return node; | ||
}, | ||
|
||
visitImport(node, visitArgs) { | ||
// store the import context | ||
this.importStack.push(node); | ||
return node; | ||
}, | ||
|
||
visitImportOut(node) { | ||
// remove import context | ||
this.importStack.pop(); | ||
return node; | ||
}, | ||
|
||
visitRuleset(node, visitArgs) { | ||
node.rules.forEach((value) => { | ||
const isVarDeclaration = value instanceof less.tree.Rule && typeof value.name === "string" && value.name.startsWith("@"); | ||
if (!this._isInMixinOrParen() && isVarDeclaration) { | ||
// add the variable declaration to the list of vars | ||
this.vars[value.name.substr(1)] = { | ||
css: this._getCSS(value.value), | ||
export: !this._isInGlobalOrBaseImport() | ||
}; | ||
} | ||
}); | ||
visitUrl(node, visitArgs) { | ||
// we mark the less variables which should be updated after eval | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My expectation was actually the other way round. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are issues within the isPreEval phase where less variables with the value none also have the URL content - so doing it this way is the best to determine properly the variables and apply only the needed later! All other options require much more code! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. Would be great to have those specialities documented somewhere. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is described in my comments |
||
// => strangewise less variables with "none" values are also urls | ||
// after the less variables have been evaluated | ||
if (this.ruleStack.length > 0 && this.ruleStack[0].variable) { | ||
this.vars[this.ruleStack[0].name.substr(1)].updateAfterEval = true; | ||
} | ||
return node; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
@link-color: #428bca; | ||
@link-color-hover: darken(@link-color, 10%); | ||
@other-var: 0 0 0.25rem 0 rgba(0, 0, 0, 0.15), inset 0 -0.0625rem 0 0 @link-color; | ||
@link-color-active: @link-color; | ||
@backgroundVariant: 1; | ||
@backgroundImage: url(img.png); | ||
@a: 100%; | ||
@var: @a; | ||
@b: darken(@link-color-active, 20%); | ||
@padLeft: 20px; | ||
@top: 5rem; | ||
@lineHeight: 2rem; | ||
@c: calc(100% - 80px); | ||
@d: -@a; | ||
@function_lighten14: lighten(@b, 10%); | ||
|
||
:root { | ||
--link-color: @link-color; | ||
--link-color-hover: @link-color-hover; | ||
--other-var: @other-var; | ||
--link-color-active: @link-color-active; | ||
--backgroundVariant: @backgroundVariant; | ||
--backgroundImage: @backgroundImage; | ||
--a: @a; | ||
--var: @var; | ||
--b: @b; | ||
--padLeft: @padLeft; | ||
--top: @top; | ||
--lineHeight: @lineHeight; | ||
--c: @c; | ||
--d: @d; | ||
--function_lighten14: @function_lighten14; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
:root { | ||
--color1: #ffffff; | ||
--url1: url('../base/111'); | ||
} | ||
.fooContrast { | ||
--color1: #000000; | ||
} | ||
|
||
/* Inline theming parameters */ | ||
#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')} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
@color1: #ffffff; | ||
@url1: url('../base/111'); | ||
|
||
:root { | ||
--color1: @color1; | ||
--url1: @url1; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
.myUiLibRule1 { | ||
color: var(--color1); | ||
} | ||
.myUiLibRule2 { | ||
padding: 1px 4px 3px 2px; | ||
} | ||
.fooContrast.myUiLibRule1, | ||
.fooContrast .myUiLibRule1 { | ||
color: var(--color1); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
.myUiLibRule1 { | ||
color: var(--color1); | ||
} | ||
.myUiLibRule2 { | ||
padding: 1px 2px 3px 4px; | ||
} | ||
.fooContrast.myUiLibRule1, | ||
.fooContrast .myUiLibRule1 { | ||
color: var(--color1); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
@myColor: #ff0000; | ||
|
||
:root { | ||
--myColor: @myColor; | ||
} |
Uh oh!
There was an error while loading. Please reload this page.