Skip to content

Commit

Permalink
Fixes #3461: Only avoid creating a statically scoped stylesheet when …
Browse files Browse the repository at this point in the history
…properties are consumed in an element, properly excluding properties produced as a result of consumption.
  • Loading branch information
Steven Orvell committed Feb 25, 2016
1 parent 809352d commit e26a806
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 9 deletions.
14 changes: 8 additions & 6 deletions src/lib/style-properties.html
Expand Up @@ -82,12 +82,14 @@

// returns cssText of properties that consume variables/mixins
collectCssText: function(rule) {
var cssText = rule.parsedCssText;
// NOTE: we support consumption inside mixin assignment
// but not production, so strip out {...}
cssText = cssText.replace(this.rx.BRACKETED, '')
return this.collectConsumingCssText(rule.parsedCssText);
},

// NOTE: we support consumption inside mixin assignment
// but not production, so strip out {...}
collectConsumingCssText: function(cssText) {
return cssText.replace(this.rx.BRACKETED, '')
.replace(this.rx.VAR_ASSIGN, '');
return cssText;
},

collectPropertiesInCssText: function(cssText, props) {
Expand Down Expand Up @@ -312,7 +314,7 @@
_elementKeyframeTransforms: function(element, scopeSelector) {
var keyframesRules = element._styles._keyframes;
var keyframeTransforms = {};
if (!nativeShadow) {
if (!nativeShadow && keyframesRules) {
// For non-ShadowDOM, we transform all known keyframes rules in
// advance for the current scope. This allows us to catch keyframes
// rules that appear anywhere in the stylesheet:
Expand Down
1 change: 1 addition & 0 deletions src/standard/x-styling.html
Expand Up @@ -33,6 +33,7 @@
var needsStatic;
for (var i=0, l=styles.length, css; i < l; i++) {
css = styleUtil.parser._clean(styles[i].textContent);
css = propertyUtils.collectConsumingCssText(css);
needsStatic = needsStatic || Boolean(css);
if (css.match(propertyUtils.rx.MIXIN_MATCH) ||
css.match(propertyUtils.rx.VAR_MATCH)) {
Expand Down
34 changes: 31 additions & 3 deletions test/unit/styling-cross-scope-apply.html
Expand Up @@ -269,6 +269,27 @@
</script>
</dom-module>

<dom-module id="x-var-produce-via-consume">
<template>
<style>
:host {
display: block;
border: 10px solid orange;
--foo: {
color: var(--bar);
}
}
</style>
</template>
<script>
HTMLImports.whenReady(function() {
Polymer({
is: 'x-var-produce-via-consume'
});
});
</script>
</dom-module>


<script>
suite('scoped-styling-apply', function() {
Expand Down Expand Up @@ -361,10 +382,17 @@
});
});

test('producing a var that consumes another var preserves static styling', function() {
var d = document.createElement('x-var-produce-via-consume');
document.body.appendChild(d);
CustomElements.takeRecords();
assertComputed(d, '10px');
});

// TODO(sorvell): fix for #1761 was reverted; include test once this issue is addressed
// test('mixin values can be overridden by subsequent concrete properties', function() {
// assertComputed(styled.$.override, '19px');
// });
test('mixin values can be overridden by subsequent concrete properties', function() {
assertComputed(styled.$.override, '19px');
});
});

</script>
Expand Down
26 changes: 26 additions & 0 deletions test/unit/styling-cross-scope-var.html
Expand Up @@ -599,6 +599,25 @@
</script>
</dom-module>

<dom-module id="x-var-produce-via-consume">
<template>
<style>
:host {
display: block;
border: 10px solid orange;
--foo: var(--bar);
}
</style>
</template>
<script>
HTMLImports.whenReady(function() {
Polymer({
is: 'x-var-produce-via-consume'
});
});
</script>
</dom-module>

<script>
suite('scoped-styling-var', function() {

Expand Down Expand Up @@ -864,6 +883,13 @@
assertComputed(styled.$.overridesConcrete, '4px');
});

test('producing a var that consumes another var preserves static styling', function() {
var d = document.createElement('x-var-produce-via-consume');
document.body.appendChild(d);
CustomElements.takeRecords();
assertComputed(d, '10px');
});

});

</script>
Expand Down

0 comments on commit e26a806

Please sign in to comment.