Skip to content

Commit ae4a07e

Browse files
author
Steven Orvell
committed
Fixes #3530. When updateStyles is called and an element is not attached, invalidate its styling so that when it is attached, its custom properties will be updated.
Also fixed a related issue with scope style cache disablement when :host function rules are used: * "property prepping" and decoration are now after static shimming so that transformed selectors are available to calculate :host function, * the detection of :host function properly accounts for type extension, * since "property prepping" was moved after shimming, apply shim was separated from this and moved before shimming so that its output can be used there.
1 parent a54c1f2 commit ae4a07e

5 files changed

Lines changed: 205 additions & 33 deletions

File tree

‎src/lib/style-properties.html‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,22 +29,23 @@
2929
// property names
3030
decorateStyles: function(styles, scope) {
3131
var self = this, props = {}, keyframes = [], ruleIndex = 0;
32+
var scopeSelector = styleTransformer._calcHostScope(scope.is, scope.extends);
3233
styleUtil.forRulesInStyles(styles, function(rule, style) {
3334
self.decorateRule(rule);
3435
// mark in-order position of ast rule in styles block, used for cache key
3536
rule.index = ruleIndex++;
3637
self.whenHostOrRootRule(scope, rule, style, function(info) {
3738
// we can't cache styles with :host and :root props in @media rules
3839
if (rule.parent.type === styleUtil.ruleTypes.MEDIA_RULE) {
39-
scope._notCacheable = true;
40+
scope.__notStyleScopeCacheable = true;
4041
}
4142
if (info.isHost) {
4243
// check if the selector is in the form of `:host-context()` or `:host()`
4344
// if so, this style is not cacheable
4445
var hostContextOrFunction = info.selector.split(' ').some(function(s) {
45-
return s.indexOf(scope.is) === 0 && s.length !== scope.is.length;
46+
return s.indexOf(scopeSelector) === 0 && s.length !== scopeSelector.length;
4647
});
47-
scope._notCacheable = scope._notCacheable || hostContextOrFunction;
48+
scope.__notStyleScopeCacheable = scope.__notStyleScopeCacheable || hostContextOrFunction;
4849
}
4950
});
5051
self.collectPropertiesInCssText(rule.propertyInfo.cssText, props);

‎src/standard/styling.html‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
<link rel="import" href="../lib/style-transformer.html">
1414
<link rel="import" href="../lib/style-extends.html">
1515
<link rel="import" href="../lib/settings.html">
16+
<link rel="import" href="../lib/apply-shim.html">
1617

1718
<script>
1819

@@ -24,6 +25,7 @@
2425
var styleUtil = Polymer.StyleUtil;
2526
var styleTransformer = Polymer.StyleTransformer;
2627
var styleExtends = Polymer.StyleExtends;
28+
var applyShim = Polymer.ApplyShim;
2729

2830
var settings = Polymer.Settings;
2931

@@ -66,13 +68,19 @@
6668
return;
6769
}
6870
this._styles = this._styles || this._collectStyles();
69-
// prepare to shim style properties.
70-
this._prepStyleProperties();
71+
// fixup usage of @apply. Note: this must be done before style
72+
// css is calculated.
73+
// css build takes care of apply shim, so avoid doing this work.
74+
if (settings.useNativeCSSProperties && !this.__cssBuild) {
75+
applyShim.transform(this._styles);
76+
}
7177
// calculate element static styling (with a targeted build and native
7278
// properties, there's only 1 style and no need to parse it!
7379
var cssText = settings.useNativeCSSProperties && hasTargetedCssBuild ?
7480
(this._styles.length && this._styles[0].textContent.trim()) :
7581
styleTransformer.elementStyles(this);
82+
// prepare to shim style properties.
83+
this._prepStyleProperties();
7684
// apply static styles iff
7785
// no custom properties are used (otherwise
7886
// styles are applied via property shimming)

‎src/standard/x-styling.html‎

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
<link rel="import" href="../lib/settings.html">
1313
<link rel="import" href="../lib/style-defaults.html">
1414
<link rel="import" href="../lib/style-cache.html">
15-
<link rel="import" href="../lib/apply-shim.html">
15+
1616
<script>
1717
(function() {
1818
'use strict';
@@ -22,7 +22,6 @@
2222
var propertyUtils = Polymer.StyleProperties;
2323
var styleTransformer = Polymer.StyleTransformer;
2424
var styleDefaults = Polymer.StyleDefaults;
25-
var applyShim = Polymer.ApplyShim;
2625

2726
var nativeShadow = Polymer.Settings.useNativeShadow;
2827
var nativeVariables = Polymer.Settings.useNativeCSSProperties;
@@ -31,13 +30,8 @@
3130

3231
_prepStyleProperties: function() {
3332
// note: an element should produce an x-scope stylesheet
34-
// if it has any _stylePropertyNames
35-
if (nativeVariables) {
36-
// css build takes care of apply shim, so avoid doing this work.
37-
if (!this.__cssBuild) {
38-
applyShim.transform(this._styles);
39-
}
40-
} else {
33+
// if it has any _ownStylePropertyNames
34+
if (!nativeVariables) {
4135
this._ownStylePropertyNames = this._styles && this._styles.length ?
4236
propertyUtils.decorateStyles(this._styles, this) :
4337
null;
@@ -109,7 +103,7 @@
109103
.propertyDataFromStyles(scope._styles, this);
110104
// the scope cache does not evaluate if @media rules, :host(), or :host-context() rules defined in this element have changed
111105
// therefore, if we detect those rules, we opt-out of the scope cache
112-
var scopeCacheable = !this._notCacheable;
106+
var scopeCacheable = !this.__notStyleScopeCacheable;
113107
// look in scope cache
114108
if (scopeCacheable) {
115109
scopeData.key.customStyle = this.customStyle;
@@ -261,13 +255,14 @@
261255
* for setting `customStyle` and then calling `updateStyles`.
262256
*/
263257
updateStyles: function(properties) {
264-
if (this.isAttached) {
265-
if (properties) {
266-
this.mixin(this.customStyle, properties);
267-
}
268-
if (nativeVariables) {
269-
propertyUtils.updateNativeStyleProperties(this, this.customStyle);
270-
} else {
258+
if (properties) {
259+
this.mixin(this.customStyle, properties);
260+
}
261+
if (nativeVariables) {
262+
propertyUtils.updateNativeStyleProperties(this, this.customStyle);
263+
} else {
264+
// actually process styling changes iff attached
265+
if (this.isAttached) {
271266
// skip applying properties to self if not used
272267
if (this._needsStyleProperties()) {
273268
this._updateStyleProperties();
@@ -276,11 +271,15 @@
276271
} else {
277272
this._styleProperties = null;
278273
}
279-
if (this._styleCache) {
280-
this._styleCache.clear();
281-
}
274+
// if called when an element is not attached, invalidate
275+
// styling by unsetting scopeSelector.
276+
} else {
277+
this._scopeSelector = null;
278+
}
279+
if (this._styleCache) {
280+
this._styleCache.clear();
282281
}
283-
// always apply properties to root
282+
// go down...
284283
this._updateRootStyles();
285284
}
286285
},

‎test/unit/style-cache.html‎

Lines changed: 102 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,43 @@
5252
</script>
5353
</dom-module>
5454

55+
<dom-module id="my-type-extension">
56+
<template>
57+
<style>
58+
div {
59+
height: 50px;
60+
width: 50px;
61+
color: var(--text-color);
62+
}
63+
64+
:host {
65+
display: block;
66+
background: blue;
67+
height: 100px;
68+
--text-color: white;
69+
}
70+
71+
:host([foo]) {
72+
--text-color: black;
73+
}
74+
75+
:host-context([bar]) {
76+
--text-color: lightgray;
77+
}
78+
</style>
79+
80+
<div id="inner">text</div>
81+
</template>
82+
<script>
83+
HTMLImports.whenReady(function() {
84+
Polymer({
85+
is: 'my-type-extension',
86+
extends: 'div'
87+
});
88+
});
89+
</script>
90+
</dom-module>
91+
5592
<test-fixture id="test">
5693
<template>
5794
<div id="owner" bar>
@@ -60,6 +97,14 @@
6097
</template>
6198
</test-fixture>
6299

100+
<test-fixture id="test-type-extension">
101+
<template>
102+
<div id="owner" bar>
103+
<div is="my-type-extension" id="element" foo></div>
104+
</div>
105+
</template>
106+
</test-fixture>
107+
63108
<script>
64109
suite('Style Cache', function() {
65110

@@ -74,42 +119,92 @@
74119
test(':host-context()', function() {
75120
assert.equal(getComputedStyle(inner).color, 'rgb(211, 211, 211)');
76121
owner.removeAttribute('bar');
77-
Polymer.updateStyles();
122+
element.updateStyles();
78123
assert.equal(getComputedStyle(inner).color, 'rgb(0, 0, 0)');
79124
});
80125

81126
test(':host()', function() {
82127
owner.removeAttribute('bar');
83-
Polymer.updateStyles();
128+
element.updateStyles();
84129
assert.equal(getComputedStyle(inner).color, 'rgb(0, 0, 0)');
85130
element.removeAttribute('foo');
86-
Polymer.updateStyles();
131+
element.updateStyles();
87132
assert.equal(getComputedStyle(inner).color, 'rgb(255, 255, 255)');
88133
});
89134
});
90135

91136
suite('global cache', function() {
92137
test(':host-context', function() {
93138
owner.removeAttribute('bar')
94-
Polymer.updateStyles();
139+
element.updateStyles();
95140
assert.include(element.className, 'my-element-1');
96141
owner.setAttribute('bar', '');
97-
Polymer.updateStyles();
142+
element.updateStyles();
98143
assert.include(element.className, 'my-element-0');
99144
});
100145

101146
test(':host()', function() {
102147
assert.include(element.className, 'my-element-0');
103148
owner.removeAttribute('bar');
104149
element.removeAttribute('foo');
105-
Polymer.updateStyles();
150+
element.updateStyles();
106151
assert.include(element.className, 'my-element-2');
107152
element.setAttribute('foo', '');
108-
Polymer.updateStyles();
153+
element.updateStyles();
109154
assert.include(element.className, 'my-element-1');
110155
});
111156
});
112157
});
158+
159+
suite('Style Cache Type Extension', function() {
160+
161+
var owner, element, inner;
162+
setup(function() {
163+
owner = fixture('test-type-extension');
164+
element = owner.querySelector('#element');
165+
inner = element.$.inner;
166+
});
167+
168+
suite('scope cache', function() {
169+
test(':host-context()', function() {
170+
assert.equal(getComputedStyle(inner).color, 'rgb(211, 211, 211)');
171+
owner.removeAttribute('bar');
172+
element.updateStyles();
173+
assert.equal(getComputedStyle(inner).color, 'rgb(0, 0, 0)');
174+
});
175+
176+
test(':host()', function() {
177+
owner.removeAttribute('bar');
178+
element.updateStyles();
179+
assert.equal(getComputedStyle(inner).color, 'rgb(0, 0, 0)');
180+
element.removeAttribute('foo');
181+
element.updateStyles();
182+
assert.equal(getComputedStyle(inner).color, 'rgb(255, 255, 255)');
183+
});
184+
});
185+
186+
suite('global cache', function() {
187+
test(':host-context', function() {
188+
owner.removeAttribute('bar')
189+
element.updateStyles();
190+
assert.include(element.className, 'my-type-extension-1');
191+
owner.setAttribute('bar', '');
192+
element.updateStyles();
193+
assert.include(element.className, 'my-type-extension-0');
194+
});
195+
196+
test(':host()', function() {
197+
assert.include(element.className, 'my-type-extension-0');
198+
owner.removeAttribute('bar');
199+
element.removeAttribute('foo');
200+
element.updateStyles();
201+
assert.include(element.className, 'my-type-extension-2');
202+
element.setAttribute('foo', '');
203+
element.updateStyles();
204+
assert.include(element.className, 'my-type-extension-1');
205+
});
206+
});
207+
});
113208
</script>
114209
</body>
115210
</html>

‎test/unit/styling-cross-scope-var.html‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -714,6 +714,43 @@
714714
</script>
715715
</dom-module>
716716

717+
<dom-module id="x-update-styles">
718+
<template>
719+
<style>
720+
:host {
721+
--border: 10px solid red;
722+
}
723+
724+
:host([active]) {
725+
--consumer: 6px solid orange;
726+
}
727+
728+
:host {
729+
display: block;
730+
border: var(--border);
731+
}
732+
</style>
733+
<x-variable-consumer id="child" class="foo"></x-variable-consumer>
734+
</template>
735+
<script>
736+
HTMLImports.whenReady(function() {
737+
Polymer({
738+
is: 'x-update-styles',
739+
properties: {
740+
active: {
741+
reflectToAttribute: true,
742+
observer: '_activeChanged'
743+
}
744+
},
745+
746+
_activeChanged: function() {
747+
this.updateStyles();
748+
}
749+
});
750+
});
751+
</script>
752+
</dom-module>
753+
717754
<script>
718755
suite('scoped-styling-var', function() {
719756

@@ -920,6 +957,38 @@
920957
assertComputed(styled.$.child.$.me, '2px');
921958
});
922959

960+
test('updateStyles on when element when not attached', function() {
961+
var x = document.createElement('x-update-styles');
962+
document.body.appendChild(x);
963+
CustomElements.takeRecords();
964+
assertComputed(x, '10px');
965+
document.body.removeChild(x);
966+
CustomElements.takeRecords();
967+
x.updateStyles({'--border': '8px solid orange'});
968+
document.body.appendChild(x);
969+
CustomElements.takeRecords();
970+
assertComputed(x, '8px');
971+
});
972+
973+
test('updateStyles called in observer when element not attached', function() {
974+
var x = document.createElement('x-update-styles');
975+
document.body.appendChild(x);
976+
CustomElements.takeRecords();
977+
assertComputed(x.$.child, '0px');
978+
x.active = true;
979+
assertComputed(x.$.child, '6px');
980+
x.active = false;
981+
assertComputed(x.$.child, '0px');
982+
document.body.removeChild(x);
983+
CustomElements.takeRecords();
984+
x.active = true;
985+
document.body.appendChild(x);
986+
CustomElements.takeRecords();
987+
assertComputed(x.$.child, '6px');
988+
});
989+
990+
991+
923992
test('styles update based on root customStyle changes', function() {
924993
assertComputed(styled.$.dynamic, '0px');
925994
Polymer.StyleDefaults.customStyle['--dynamic'] = '4px solid navy';

0 commit comments

Comments
 (0)