Skip to content

Commit

Permalink
Fixes #1851: Include customStyle in the style host cache key so tha…
Browse files Browse the repository at this point in the history
…t changes to customStyle are recognized.

Fixes #1915: Include a `customStyleProperties` argument to `updateStyles` for convenience.
  • Loading branch information
Steven Orvell committed Jun 19, 2015
1 parent 279d381 commit 6bc360d
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 13 deletions.
10 changes: 9 additions & 1 deletion src/lib/style-cache.html
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,23 @@
// right now. The objects we're checking here are either objects that must
// always have the same keys OR arrays.
_objectsEqual: function(target, source) {
var t, s;
for (var i in target) {
if (target[i] !== source[i]) {
t = target[i], s = source[i];
if (!(typeof t === 'object' && t ? this._objectsStrictlyEqual(t, s) :
t === s)) {
return false;
}
}
if (Array.isArray(target)) {
return target.length === source.length;
}
return true;
},

_objectsStrictlyEqual: function(target, source) {
return this._objectsEqual(target, source) &&
this._objectsEqual(source, target);
}

};
Expand Down
23 changes: 22 additions & 1 deletion src/lib/style-defaults.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

_styles: [],
_properties: null,
customStyle: {},

addStyle: function(style) {
this._styles.push(style);
Expand All @@ -36,6 +37,8 @@
this._styles._scopeStyleProperties = null;
this._properties = styleProperties
.scopePropertiesFromStyles(this._styles);
// mixin customStyle
styleProperties.mixinCustomStyle(this._properties, this.customStyle);
styleProperties.reify(this._properties);
}
return this._properties;
Expand All @@ -47,7 +50,25 @@
return this._styleProperties;
},

updateStyles: function() {
/**
* Re-evaluates and applies custom CSS properties to all elements in the
* document based on dynamic changes, such as adding or removing classes.
*
* For performance reasons, Polymer's custom CSS property shim relies
* on this explicit signal from the user to indicate when changes have
* been made that affect the values of custom properties.
*
* @method updateStyles
* @param {Object=} properties Properties object which is mixed into
* the document root `customStyle` property. This argument provides a
* shortcut for setting `customStyle` and then calling `updateStyles`.
*/
updateStyles: function(properties) {
// force properties update.
this._properties = null;
if (properties) {
Polymer.Base.mixin(this.customStyle, properties);
}
// invalidate the cache
this._styleCache.clear();
// update any custom-styles we are tracking
Expand Down
16 changes: 15 additions & 1 deletion src/lib/style-properties.html
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@
var s = element._customStyle;
if (s && !nativeShadow && (s !== style)) {
s._useCount--;
if (s._useCount <= 0) {
if (s._useCount <= 0 && s.parentNode) {
s.parentNode.removeChild(s);
}
}
Expand Down Expand Up @@ -330,6 +330,20 @@
return style;
},

// customStyle treats null property values as deletes. This enables
// properties to be reset to inherited values.
mixinCustomStyle: function(props, customStyle) {
var v;
for (var i in customStyle) {
v = customStyle[i];
if (v === null) {
delete props[i];
} else {
props[i] = v;
}
}
},

rx: {
VAR_ASSIGN: /(?:^|;\s*)(--[^\:;]*?):\s*?(?:([^;{]*?)|{([^}]*)})(?=;)/gim,
MIXIN_MATCH: /(?:^|\W+)@apply[\s]*\(([^)]*)\);?/im,
Expand Down
28 changes: 18 additions & 10 deletions src/standard/x-styling.html
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@
var scopeData = propertyUtils
.propertyDataFromStyles(scope._styles, this);
// look in scope cache
info = scope._styleCache.retrieve(this.is, scopeData.key,
this._styles);
scopeData.key.customStyle = this.customStyle;
info = scope._styleCache.retrieve(this.is, scopeData.key, this._styles);
// compute style properties (fast path, if cache hit)
var scopeCached = Boolean(info);
if (scopeCached) {
Expand All @@ -84,7 +84,7 @@
// is available.
var style = this._applyStyleProperties(info);
// no cache so store in cache
//console.warn(this, scopeCached, globalCached, info && info._scopeSelector);
//console.warn(this.is, scopeCached, globalCached, info && info._scopeSelector);
if (!scopeCached) {
// create an info object for caching
// TODO(sorvell): clone style node when using native Shadow DOM
Expand All @@ -94,13 +94,15 @@
var cacheableStyle = style;
if (nativeShadow) {
cacheableStyle = style.cloneNode ?
style.cloneNode(true) : Object.create(style || null);
style.cloneNode(true) : null;
}
info = {
style: cacheableStyle,
_scopeSelector: this._scopeSelector,
_styleProperties: this._styleProperties
}
scopeData.key.customStyle = {};
this.mixin(scopeData.key.customStyle, this.customStyle);
scope._styleCache.store(this.is, info, scopeData.key, this._styles);
if (!globalCached) {
// save in global cache
Expand Down Expand Up @@ -129,7 +131,7 @@
// finally mixin properties inherent to this element
this.mixin(props,
propertyUtils.scopePropertiesFromStyles(this._styles));
this.mixin(props, this.customStyle);
propertyUtils.mixinCustomStyle(props, this.customStyle);
// reify properties (note: only does own properties)
propertyUtils.reify(props);
this._styleProperties = props;
Expand All @@ -154,8 +156,8 @@
this.is + '-' + this.__proto__._scopeCount++;
var style = propertyUtils.applyElementStyle(this,
this._styleProperties, this._scopeSelector, info && info.style);
// apply scope selector (iff we have some scoped css)
if ((style || oldScopeSelector) && !nativeShadow) {
// apply scope selector
if (!nativeShadow) {
propertyUtils.applyElementScopeSelector(this, this._scopeSelector,
oldScopeSelector, this._scopeCssViaAttr);
}
Expand Down Expand Up @@ -197,9 +199,15 @@
* been made that affect the values of custom properties.
*
* @method updateStyles
* @param {Object=} properties Properties object which is mixed into
* the element's `customStyle` property. This argument provides a shortcut
* for setting `customStyle` and then calling `updateStyles`.
*/
updateStyles: function() {
updateStyles: function(properties) {
if (this.isAttached) {
if (properties) {
this.mixin(this.customStyle, properties);
}
// skip applying properties to self if not used
if (this._needsStyleProperties()) {
this._updateStyleProperties();
Expand Down Expand Up @@ -234,9 +242,9 @@
* Force all custom elements using cross scope custom properties,
* to update styling.
*/
Polymer.updateStyles = function() {
Polymer.updateStyles = function(properties) {
// update default/custom styles
styleDefaults.updateStyles();
styleDefaults.updateStyles(properties);
// search the document for elements to update
Polymer.Base._updateRootStyles(document);
};
Expand Down
58 changes: 58 additions & 0 deletions test/unit/styling-cross-scope-var.html
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,26 @@
});
</script>
</dom-module>

<dom-module id="x-dynamic">
<style>
:host {
display: block;
margin: 20px;
border: var(--dynamic);
}
</style>
<template>
Dynamic
</template>
<script>
HTMLImports.whenReady(function() {
Polymer({
is: 'x-dynamic'
});
});
</script>
</dom-module>

<dom-module id="x-scope">
<style>
Expand Down Expand Up @@ -376,6 +396,7 @@
<x-host-property id="hostProp"></x-host-property>
<x-has-if id="iffy"></x-has-if>
<div id="after"></div>
<x-dynamic id="dynamic"></x-dynamic>
</template>
<script>
HTMLImports.whenReady(function() {
Expand Down Expand Up @@ -491,6 +512,43 @@
assertComputed(styled.$.child.$.me, '2px');
});

test('updateStyles changes own properties based on customStyle changes', function() {
styled.$.child.customStyle['--child-scope-var'] = '30px solid orange';
styled.$.child.updateStyles();
assertComputed(styled.$.child.$.me, '30px');
styled.$.child.customStyle = {};
styled.$.child.updateStyles();
assertComputed(styled.$.child.$.me, '2px');
});

test('updateStyles with properties argument changes styles', function() {
styled.$.child.updateStyles({'--child-scope-var': '26px solid seagreen'});
assertComputed(styled.$.child.$.me, '26px');
styled.$.child.customStyle = {};
styled.$.child.updateStyles();
assertComputed(styled.$.child.$.me, '2px');
});

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

test('null customStyles unapply', function() {
styled.$.dynamic.updateStyles({'--dynamic': '8px solid black'});
assertComputed(styled.$.dynamic, '8px');
styled.$.dynamic.updateStyles({'--dynamic': null});
assertComputed(styled.$.dynamic, '6px');
styled.$.dynamic.updateStyles({'--dynamic': '8px solid black'});
assertComputed(styled.$.dynamic, '8px');
styled.$.dynamic.updateStyles({'--dynamic': null});
assertComputed(styled.$.dynamic, '6px');
});

test('style properties with dom-if', function() {
var e = styled.$.iffy;
var c = Polymer.dom(e.root).querySelector('.iffy');
Expand Down

0 comments on commit 6bc360d

Please sign in to comment.