Skip to content

Commit

Permalink
Configure attr's with property effects. More robust fix for #3288.
Browse files Browse the repository at this point in the history
  • Loading branch information
kevinpschaaf committed Feb 5, 2016
1 parent 9968266 commit 0f55d1d
Show file tree
Hide file tree
Showing 6 changed files with 50 additions and 15 deletions.
6 changes: 4 additions & 2 deletions src/lib/annotations/annotations.html
Original file line number Diff line number Diff line change
Expand Up @@ -351,12 +351,14 @@
// (properties) are case sensitive. Gambit is to map dash-case to
// camel-case: `foo-bar` becomes `fooBar`.
// Attribute bindings are excepted.
var propertyName = Polymer.CaseMap.dashToCamelCase(name);
if (kind === 'property') {
name = Polymer.CaseMap.dashToCamelCase(name);
name = propertyName;
}
return {
kind: kind,
name: name,
propertyName: propertyName,
parts: parts,
literal: literal,
isCompound: parts.length !== 1
Expand All @@ -372,7 +374,7 @@
Polymer.Annotations.findAnnotatedNode(root, annote.parent);
// unwind the stack, returning the indexed node at each level
if (parent) {
// note: marginally faster than indexing via childNodes
// note: marginally faster than indexing via childNodes
// (http://jsperf.com/childnodes-lookup)
for (var n=parent.firstChild, i=0; n; n=n.nextSibling) {
if (annote.index === i++) {
Expand Down
13 changes: 8 additions & 5 deletions src/standard/configure.html
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,18 @@
for (var i=0, l=fx.length, x; (i<l) && (x=fx[i]); i++) {
// TODO(kschaaf): compound bindings (as well as computed effects)
// are excluded from top-down configure for now; to be revisited
if (x.kind === 'annotation' &&
x.effect.kind !== 'attribute' &&
!x.isCompound) {
if (x.kind === 'annotation' && !x.isCompound) {
var node = this._nodes[x.effect.index];
var name = x.effect.propertyName;
// seeding configuration only
if (node._configValue) {
if (node._propertyEffects && node._propertyEffects[name]) {
var value = (p === x.effect.value) ? config[p] :
this._get(x.effect.value, config);
node._configValue(x.effect.name, value);
if (x.effect.kind == 'attribute') {
value = this.deserialize(value,
node._propertyInfo[name].type);
}
node._configValue(name, value);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/standard/effectBuilder.html
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@
kind: note.kind,
index: index,
name: note.name,
propertyName: note.propertyName,
value: part.value,
isCompound: note.isCompound,
compoundIndex: part.compoundIndex,
Expand Down
6 changes: 1 addition & 5 deletions test/unit/attributes.html
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,7 @@
// applied to property with effect
assert.strictEqual(compose.$.basic.prop, 'compose');
assert.equal(compose.$.basic.propChangedCount, 1);
// Note: Attribute binding does not participate in efficient configuration
// per #3288. As such, a property bound with an attribute binding will
// see its default value first, then be overwritten when the attribute
// binding runs and re-deserializes to the property, hence 2 observer calls
assert.equal(compose.$.basic.attr1ChangedCount, 2);
assert.equal(compose.$.basic.attr1ChangedCount, 1);
assert.equal(compose.prop2, 'hi');
});

Expand Down
21 changes: 19 additions & 2 deletions test/unit/configure-elements.html
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,21 @@
},
stomp: {
value: 5
},
attrDash: {
observer: 'attrDashChanged',
value: 'default'
},
attrNumber: {
type: Number,
observer: 'attrNumberChanged',
value: 0
}
},

created: function() {
this.attrDashChanged = sinon.spy();
this.attrNumberChanged = sinon.spy();
}

});
Expand All @@ -128,7 +142,7 @@

<dom-module id="x-configure-host">
<template>
<x-configure-child id="child" content="{{content}}" object="{{object.goo}}" attr$="{{attrValue}}"></x-configure-child>
<x-configure-child id="child" content="{{content}}" object="{{object.goo}}" attr$="{{attrValue}}" attr-dash$="{{attrValue}}" attr-number$="{{attrNumber}}"></x-configure-child>
</template>
<script>
Polymer({
Expand Down Expand Up @@ -158,7 +172,10 @@
value: 5
},
attrValue: {
value: 'attrValue'
value: 'attrValue',
},
attrNumber: {
value: '42'
}
}

Expand Down
18 changes: 17 additions & 1 deletion test/unit/configure.html
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,28 @@
assert.equal(e.readOnly, 'default');
});

test('properties for attribute bindings not configured', function() {
test('attribute bindings to properties without effects not configured', function() {
var e = document.querySelector('x-configure-host');
assert.equal(e.$.child.getAttribute('attr'), 'attrValue');
assert.equal(e.$.child.attr, undefined);
});

test('attribute bindings to properties with effects configured', function() {
var e = document.createElement('x-configure-host');

assert.equal(e.$.child.getAttribute('attr-dash'), 'attrValue');
assert.notProperty(e.$.child, 'attr-dash');
assert.equal(e.$.child.attrDash, 'attrValue');
assert.isTrue(e.$.child.attrDashChanged.calledOnce);
assert.equal(e.$.child.attrDashChanged.getCall(0).args[0], 'attrValue');

assert.equal(e.$.child.getAttribute('attr-number'), '42');
assert.notProperty(e.$.child, 'attr-number');
assert.strictEqual(e.$.child.attrNumber, 42);
assert.isTrue(e.$.child.attrNumberChanged.calledOnce);
assert.equal(e.$.child.attrNumberChanged.getCall(0).args[0], 42);
});

});

</script>
Expand Down

0 comments on commit 0f55d1d

Please sign in to comment.