Skip to content

Commit

Permalink
Fixes #3326. Changes inspired by #3276 and #3344
Browse files Browse the repository at this point in the history
  • Loading branch information
Steven Orvell committed Feb 5, 2016
1 parent 9968266 commit b5ba9a8
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 28 deletions.
14 changes: 2 additions & 12 deletions src/lib/style-properties.html
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,7 @@
// but not production, so strip out {...}
cssText = cssText.replace(this.rx.BRACKETED, '')
.replace(this.rx.VAR_ASSIGN, '');
var parts = cssText.split(';');
for (var i=0, p; i<parts.length; i++) {
p = parts[i];
if (p.match(this.rx.MIXIN_MATCH) ||
p.match(this.rx.VAR_MATCH) ||
this.rx.ANIMATION_MATCH.test(p) ||
styleUtil.isKeyframesSelector(rule)) {
customCssText += p + ';\n';
}
}
return customCssText;
return cssText;
},

collectPropertiesInCssText: function(cssText, props) {
Expand Down Expand Up @@ -369,7 +359,7 @@
var parts = selector.split(',');
for (var i=0, l=parts.length, p; (i<l) && (p=parts[i]); i++) {
parts[i] = p.match(hostRx) ?
p.replace(hostSelector, hostSelector + scope) :
p.replace(hostSelector, scope) :
scope + ' ' + p;
}
rule.selector = parts.join(',');
Expand Down
11 changes: 10 additions & 1 deletion src/standard/styling.html
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,17 @@
}
if (this._template) {
this._styles = this._collectStyles();
// calculate shimmed styles (we must always do this as it
// stores shimmed style data in the css rules for later use)
var cssText = styleTransformer.elementStyles(this);
if (cssText) {
// do we really need to output shimmed styles
var needsStatic = this._needsStaticStyles(this._styles);
cssText = needsStatic ? cssText : '';
// under shady dom we always output a shimmed style (which may be
// empty) so that other dynamic stylesheets can always be placed
// after the element's main stylesheet.
// This helps ensure element styles are always in registration order.
if (needsStatic || !nativeShadow) {
var style = styleUtil.applyCss(cssText, this.is,
nativeShadow ? this._template.content : null);
// keep track of style when in document scope (polyfill) so we can
Expand Down
16 changes: 16 additions & 0 deletions src/standard/x-styling.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,22 @@

Polymer.Base._addFeature({

// Skip applying CSS if there are some mixins or variables used
// since styles with mixins and variables will be added on later stages anyway,
// and will include styles applied here, no need to do this twice
_needsStaticStyles: function(styles) {
var needsStatic;
for (var i=0, l=styles.length, css; i < l; i++) {
css = styles[i].textContent;
needsStatic = needsStatic || Boolean(css);
if (css.match(propertyUtils.rx.MIXIN_MATCH) ||
css.match(propertyUtils.rx.VAR_MATCH)) {
return false;
}
}
return needsStatic;
},

_prepStyleProperties: function() {
// note: an element should produce an x-scope stylesheet
// if it has any _stylePropertyNames
Expand Down
3 changes: 2 additions & 1 deletion test/unit/styling-cross-scope-var.html
Original file line number Diff line number Diff line change
Expand Up @@ -795,10 +795,11 @@
});

// skip for now, until #3326 is fixed
test.skip('custom style class overrides css variable', function() {
test('custom style class overrides css variable', function() {
var d = document.createElement('x-variable-override');
d.classList.add('variable-override');
document.body.appendChild(d);
CustomElements.takeRecords();
assertComputed(d, '10px');
});

Expand Down
20 changes: 15 additions & 5 deletions test/unit/styling-remote.html
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,23 @@
});

test('styles shimmed in registration order', function() {
var regList = Polymer.telemetry.registrations.map(function(reg) {
return reg.is;
});
function regIndex(styleScope) {
for (var i=0; (r=regList[i]); i++) {
if (styleScope.match(new RegExp(r + '\\-?\\d*$'))) {
return i;
}
}
}
var s$ = document.head.querySelectorAll('style[scope]');
var expected = ['x-order', 'x-child2', 'x-styled', 'x-styled-0', 'x-button', 'x-dynamic-scope'];
var actual = [];
for (var i=0; i<s$.length; i++) {
actual.push(s$[i].getAttribute('scope'));
for (var i=0, scope, n=0, o=0; i<s$.length; i++) {
scope = s$[i].getAttribute('scope');
n = regIndex(scope);
assert.isTrue(n >= o, 'style not in registration order: ' + scope);
o = n;
}
assert.deepEqual(actual, expected);
});
});
}
Expand Down
33 changes: 24 additions & 9 deletions test/unit/styling-scoped.html
Original file line number Diff line number Diff line change
Expand Up @@ -249,17 +249,23 @@
});

test('styles shimmed in registration order', function() {
var regList = Polymer.telemetry.registrations.map(function(reg) {
return reg.is;
});
function regIndex(styleScope) {
for (var i=0; (r=regList[i]); i++) {
if (styleScope.match(new RegExp(r + '\\-?\\d*$'))) {
return i;
}
}
}
var s$ = document.head.querySelectorAll('style[scope]');
var expected = ['x-keyframes', 'x-keyframes-1', 'x-keyframes-0', 'x-gchild', 'x-child2',
'x-styled', 'x-button', 'x-mixed-case', 'x-mixed-case-button',
'x-dynamic-scope', 'x-dynamic-template', 'x-dynamic-svg',
'x-specificity', 'x-overriding', 'x-overriding-0',
'x-specificity-parent-0', 'x-specificity-nested-0'];
var actual = [];
for (var i=0; i<s$.length; i++) {
actual.push(s$[i].getAttribute('scope'));
for (var i=0, scope, n=0, o=0; i<s$.length; i++) {
scope = s$[i].getAttribute('scope');
n = regIndex(scope);
assert.isTrue(n >= o, 'style not in registration order: ' + scope);
o = n;
}
assert.deepEqual(actual, expected);
});

test('svg elements properly scoped', function() {
Expand Down Expand Up @@ -306,6 +312,15 @@
test('specificity of ::content > :not(template) selector', function() {
assertComputed(document.querySelector('[is=x-specificity-nested]'), '10px');
});

test('overwriting mixin properties', function() {
var root = document.querySelector('x-overriding');
assertComputed(root.querySelector('.red'), '1px');
assertComputed(root.querySelector('.green'), '2px');
assertComputed(root.querySelector('.red-2'), '1px');
assertComputed(root.querySelector('.blue'), '3px');
});

});

test('svg classes are dynamically scoped correctly', function() {
Expand Down

0 comments on commit b5ba9a8

Please sign in to comment.