Skip to content

Commit

Permalink
Fixes #3530. When updateStyles is called and an element is not atta…
Browse files Browse the repository at this point in the history
…ched, 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.
  • Loading branch information
Steven Orvell committed Jun 18, 2016
1 parent a54c1f2 commit ae4a07e
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 33 deletions.
7 changes: 4 additions & 3 deletions src/lib/style-properties.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,23 @@
// property names
decorateStyles: function(styles, scope) {
var self = this, props = {}, keyframes = [], ruleIndex = 0;
var scopeSelector = styleTransformer._calcHostScope(scope.is, scope.extends);
styleUtil.forRulesInStyles(styles, function(rule, style) {
self.decorateRule(rule);
// mark in-order position of ast rule in styles block, used for cache key
rule.index = ruleIndex++;
self.whenHostOrRootRule(scope, rule, style, function(info) {
// we can't cache styles with :host and :root props in @media rules
if (rule.parent.type === styleUtil.ruleTypes.MEDIA_RULE) {
scope._notCacheable = true;
scope.__notStyleScopeCacheable = true;
}
if (info.isHost) {
// check if the selector is in the form of `:host-context()` or `:host()`
// if so, this style is not cacheable
var hostContextOrFunction = info.selector.split(' ').some(function(s) {
return s.indexOf(scope.is) === 0 && s.length !== scope.is.length;
return s.indexOf(scopeSelector) === 0 && s.length !== scopeSelector.length;
});
scope._notCacheable = scope._notCacheable || hostContextOrFunction;
scope.__notStyleScopeCacheable = scope.__notStyleScopeCacheable || hostContextOrFunction;
}
});
self.collectPropertiesInCssText(rule.propertyInfo.cssText, props);
Expand Down
12 changes: 10 additions & 2 deletions src/standard/styling.html
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<link rel="import" href="../lib/style-transformer.html">
<link rel="import" href="../lib/style-extends.html">
<link rel="import" href="../lib/settings.html">
<link rel="import" href="../lib/apply-shim.html">

<script>

Expand All @@ -24,6 +25,7 @@
var styleUtil = Polymer.StyleUtil;
var styleTransformer = Polymer.StyleTransformer;
var styleExtends = Polymer.StyleExtends;
var applyShim = Polymer.ApplyShim;

var settings = Polymer.Settings;

Expand Down Expand Up @@ -66,13 +68,19 @@
return;
}
this._styles = this._styles || this._collectStyles();
// prepare to shim style properties.
this._prepStyleProperties();
// fixup usage of @apply. Note: this must be done before style
// css is calculated.
// css build takes care of apply shim, so avoid doing this work.
if (settings.useNativeCSSProperties && !this.__cssBuild) {
applyShim.transform(this._styles);
}
// calculate element static styling (with a targeted build and native
// properties, there's only 1 style and no need to parse it!
var cssText = settings.useNativeCSSProperties && hasTargetedCssBuild ?
(this._styles.length && this._styles[0].textContent.trim()) :
styleTransformer.elementStyles(this);
// prepare to shim style properties.
this._prepStyleProperties();
// apply static styles iff
// no custom properties are used (otherwise
// styles are applied via property shimming)
Expand Down
41 changes: 20 additions & 21 deletions src/standard/x-styling.html
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<link rel="import" href="../lib/settings.html">
<link rel="import" href="../lib/style-defaults.html">
<link rel="import" href="../lib/style-cache.html">
<link rel="import" href="../lib/apply-shim.html">

<script>
(function() {
'use strict';
Expand All @@ -22,7 +22,6 @@
var propertyUtils = Polymer.StyleProperties;
var styleTransformer = Polymer.StyleTransformer;
var styleDefaults = Polymer.StyleDefaults;
var applyShim = Polymer.ApplyShim;

var nativeShadow = Polymer.Settings.useNativeShadow;
var nativeVariables = Polymer.Settings.useNativeCSSProperties;
Expand All @@ -31,13 +30,8 @@

_prepStyleProperties: function() {
// note: an element should produce an x-scope stylesheet
// if it has any _stylePropertyNames
if (nativeVariables) {
// css build takes care of apply shim, so avoid doing this work.
if (!this.__cssBuild) {
applyShim.transform(this._styles);
}
} else {
// if it has any _ownStylePropertyNames
if (!nativeVariables) {
this._ownStylePropertyNames = this._styles && this._styles.length ?
propertyUtils.decorateStyles(this._styles, this) :
null;
Expand Down Expand Up @@ -109,7 +103,7 @@
.propertyDataFromStyles(scope._styles, this);
// the scope cache does not evaluate if @media rules, :host(), or :host-context() rules defined in this element have changed
// therefore, if we detect those rules, we opt-out of the scope cache
var scopeCacheable = !this._notCacheable;
var scopeCacheable = !this.__notStyleScopeCacheable;
// look in scope cache
if (scopeCacheable) {
scopeData.key.customStyle = this.customStyle;
Expand Down Expand Up @@ -261,13 +255,14 @@
* for setting `customStyle` and then calling `updateStyles`.
*/
updateStyles: function(properties) {
if (this.isAttached) {
if (properties) {
this.mixin(this.customStyle, properties);
}
if (nativeVariables) {
propertyUtils.updateNativeStyleProperties(this, this.customStyle);
} else {
if (properties) {
this.mixin(this.customStyle, properties);
}
if (nativeVariables) {
propertyUtils.updateNativeStyleProperties(this, this.customStyle);
} else {
// actually process styling changes iff attached
if (this.isAttached) {
// skip applying properties to self if not used
if (this._needsStyleProperties()) {
this._updateStyleProperties();
Expand All @@ -276,11 +271,15 @@
} else {
this._styleProperties = null;
}
if (this._styleCache) {
this._styleCache.clear();
}
// if called when an element is not attached, invalidate
// styling by unsetting scopeSelector.
} else {
this._scopeSelector = null;
}
if (this._styleCache) {
this._styleCache.clear();
}
// always apply properties to root
// go down...
this._updateRootStyles();
}
},
Expand Down
109 changes: 102 additions & 7 deletions test/unit/style-cache.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,43 @@
</script>
</dom-module>

<dom-module id="my-type-extension">
<template>
<style>
div {
height: 50px;
width: 50px;
color: var(--text-color);
}

:host {
display: block;
background: blue;
height: 100px;
--text-color: white;
}

:host([foo]) {
--text-color: black;
}

:host-context([bar]) {
--text-color: lightgray;
}
</style>

<div id="inner">text</div>
</template>
<script>
HTMLImports.whenReady(function() {
Polymer({
is: 'my-type-extension',
extends: 'div'
});
});
</script>
</dom-module>

<test-fixture id="test">
<template>
<div id="owner" bar>
Expand All @@ -60,6 +97,14 @@
</template>
</test-fixture>

<test-fixture id="test-type-extension">
<template>
<div id="owner" bar>
<div is="my-type-extension" id="element" foo></div>
</div>
</template>
</test-fixture>

<script>
suite('Style Cache', function() {

Expand All @@ -74,42 +119,92 @@
test(':host-context()', function() {
assert.equal(getComputedStyle(inner).color, 'rgb(211, 211, 211)');
owner.removeAttribute('bar');
Polymer.updateStyles();
element.updateStyles();
assert.equal(getComputedStyle(inner).color, 'rgb(0, 0, 0)');
});

test(':host()', function() {
owner.removeAttribute('bar');
Polymer.updateStyles();
element.updateStyles();
assert.equal(getComputedStyle(inner).color, 'rgb(0, 0, 0)');
element.removeAttribute('foo');
Polymer.updateStyles();
element.updateStyles();
assert.equal(getComputedStyle(inner).color, 'rgb(255, 255, 255)');
});
});

suite('global cache', function() {
test(':host-context', function() {
owner.removeAttribute('bar')
Polymer.updateStyles();
element.updateStyles();
assert.include(element.className, 'my-element-1');
owner.setAttribute('bar', '');
Polymer.updateStyles();
element.updateStyles();
assert.include(element.className, 'my-element-0');
});

test(':host()', function() {
assert.include(element.className, 'my-element-0');
owner.removeAttribute('bar');
element.removeAttribute('foo');
Polymer.updateStyles();
element.updateStyles();
assert.include(element.className, 'my-element-2');
element.setAttribute('foo', '');
Polymer.updateStyles();
element.updateStyles();
assert.include(element.className, 'my-element-1');
});
});
});

suite('Style Cache Type Extension', function() {

var owner, element, inner;
setup(function() {
owner = fixture('test-type-extension');
element = owner.querySelector('#element');
inner = element.$.inner;
});

suite('scope cache', function() {
test(':host-context()', function() {
assert.equal(getComputedStyle(inner).color, 'rgb(211, 211, 211)');
owner.removeAttribute('bar');
element.updateStyles();
assert.equal(getComputedStyle(inner).color, 'rgb(0, 0, 0)');
});

test(':host()', function() {
owner.removeAttribute('bar');
element.updateStyles();
assert.equal(getComputedStyle(inner).color, 'rgb(0, 0, 0)');
element.removeAttribute('foo');
element.updateStyles();
assert.equal(getComputedStyle(inner).color, 'rgb(255, 255, 255)');
});
});

suite('global cache', function() {
test(':host-context', function() {
owner.removeAttribute('bar')
element.updateStyles();
assert.include(element.className, 'my-type-extension-1');
owner.setAttribute('bar', '');
element.updateStyles();
assert.include(element.className, 'my-type-extension-0');
});

test(':host()', function() {
assert.include(element.className, 'my-type-extension-0');
owner.removeAttribute('bar');
element.removeAttribute('foo');
element.updateStyles();
assert.include(element.className, 'my-type-extension-2');
element.setAttribute('foo', '');
element.updateStyles();
assert.include(element.className, 'my-type-extension-1');
});
});
});
</script>
</body>
</html>
69 changes: 69 additions & 0 deletions test/unit/styling-cross-scope-var.html
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,43 @@
</script>
</dom-module>

<dom-module id="x-update-styles">
<template>
<style>
:host {
--border: 10px solid red;
}

:host([active]) {
--consumer: 6px solid orange;
}

:host {
display: block;
border: var(--border);
}
</style>
<x-variable-consumer id="child" class="foo"></x-variable-consumer>
</template>
<script>
HTMLImports.whenReady(function() {
Polymer({
is: 'x-update-styles',
properties: {
active: {
reflectToAttribute: true,
observer: '_activeChanged'
}
},

_activeChanged: function() {
this.updateStyles();
}
});
});
</script>
</dom-module>

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

Expand Down Expand Up @@ -920,6 +957,38 @@
assertComputed(styled.$.child.$.me, '2px');
});

test('updateStyles on when element when not attached', function() {
var x = document.createElement('x-update-styles');
document.body.appendChild(x);
CustomElements.takeRecords();
assertComputed(x, '10px');
document.body.removeChild(x);
CustomElements.takeRecords();
x.updateStyles({'--border': '8px solid orange'});
document.body.appendChild(x);
CustomElements.takeRecords();
assertComputed(x, '8px');
});

test('updateStyles called in observer when element not attached', function() {
var x = document.createElement('x-update-styles');
document.body.appendChild(x);
CustomElements.takeRecords();
assertComputed(x.$.child, '0px');
x.active = true;
assertComputed(x.$.child, '6px');
x.active = false;
assertComputed(x.$.child, '0px');
document.body.removeChild(x);
CustomElements.takeRecords();
x.active = true;
document.body.appendChild(x);
CustomElements.takeRecords();
assertComputed(x.$.child, '6px');
});



test('styles update based on root customStyle changes', function() {
assertComputed(styled.$.dynamic, '0px');
Polymer.StyleDefaults.customStyle['--dynamic'] = '4px solid navy';
Expand Down

0 comments on commit ae4a07e

Please sign in to comment.