Skip to content

Commit

Permalink
Compute and use correct annotation value during config
Browse files Browse the repository at this point in the history
* Refactor _applyEffectValue

... to get a new function that computes the final value an annotation
effect will propagate downwards.

* Compute correct annotation value during config.

Due to a typo compound annotations were never actually excluded from
config, they only received the wrong value; remove that false conditional
b/c with this change the compounds don't have to be excluded at all.
  • Loading branch information
kaste authored and dfreedm committed Apr 5, 2016
1 parent d9c03a4 commit 1b02e96
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 28 deletions.
11 changes: 1 addition & 10 deletions src/lib/bind/effects.html
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,7 @@
value = this._get(effect.value);
this.__data__[effect.value] = value;
}
var calc = effect.negate ? !value : value;
// For better interop, dirty check before setting when custom events
// are used, since the target element may not dirty check (e.g. <input>)
if (!effect.customEvent ||
this._nodes[effect.index][effect.name] !== calc) {
return this._applyEffectValue(effect, calc);
}
this._applyEffectValue(effect, value);
},

_reflectEffect: function(source, value, effect) {
Expand Down Expand Up @@ -101,9 +95,6 @@
var args = Polymer.Bind._marshalArgs(this.__data__, effect, source, value);
if (args) {
var computedvalue = fn.apply(computedHost, args);
if (effect.negate) {
computedvalue = !computedvalue;
}
this._applyEffectValue(effect, computedvalue);
}
} else if (effect.dynamicFn) {
Expand Down
10 changes: 6 additions & 4 deletions src/standard/configure.html
Original file line number Diff line number Diff line change
Expand Up @@ -143,18 +143,20 @@
var fx = fx$[p];
if (fx) {
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.isCompound) {
// TODO(kschaaf): computed annotations are excluded from top-down
// configure for now; to be revisited
if (x.kind === 'annotation') {
var node = this._nodes[x.effect.index];
var name = x.effect.propertyName;
// seeding configuration only
var isAttr = (x.effect.kind == 'attribute');
var hasEffect = (node._propertyEffects &&
var hasEffect = (node._propertyEffects &&
node._propertyEffects[name]);
if (node._configValue && (hasEffect || !isAttr)) {
var value = (p === x.effect.value) ? config[p] :
this._get(x.effect.value, config);
value = this._computeFinalAnnotationValue(node, name, value,
x.effect);
if (isAttr) {
// For attribute bindings, flow through the same ser/deser
// process to ensure the value is the same as if it were
Expand Down
44 changes: 31 additions & 13 deletions src/standard/effectBuilder.html
Original file line number Diff line number Diff line change
Expand Up @@ -322,16 +322,41 @@
_applyEffectValue: function(info, value) {
var node = this._nodes[info.index];
var property = info.name;

value = this._computeFinalAnnotationValue(node, property, value, info);

// For better interop, dirty check before setting when custom events
// are used, since the target element may not dirty check (e.g. <input>)
if (info.customEvent && node[property] === value) {
return;
}

if (info.kind == 'attribute') {
this.serializeValueToAttribute(value, property, node);
} else {
var pinfo = node._propertyInfo && node._propertyInfo[property];
if (pinfo && pinfo.readOnly) {
return;
}
// Ideally we would call setProperty using fromAbove: true to avoid
// spinning the wheel needlessly, but we found that users were listening
// for change events outside of bindings
this.__setProperty(property, value, false, node);
}
},

_computeFinalAnnotationValue: function(node, property, value, info) {
if (info.negate) {
value = !value;
}

if (info.isCompound) {
var storage = node.__compoundStorage__[property];
storage[info.compoundIndex] = value;
value = storage.join('');
}
// special processing for 'class' and 'className'; 'class' handled
// when attr is serialized.
if (info.kind == 'attribute') {
this.serializeValueToAttribute(value, property, node);
} else {

if (info.kind !== 'attribute') {
// TODO(sorvell): consider pre-processing the following two string
// comparisons in the hot path so this can be a boolean check
if (property === 'className') {
Expand All @@ -342,15 +367,8 @@
(node.localName == 'input' && property == 'value')) {
value = value == undefined ? '' : value;
}
// Ideally we would call setProperty using fromAbove: true to avoid
// spinning the wheel needlessly, but we found that users were listening
// for change events outside of bindings
var pinfo;
if (!node._propertyInfo || !(pinfo = node._propertyInfo[property]) ||
!pinfo.readOnly) {
this.__setProperty(property, value, false, node);
}
}
return value;
},

_executeStaticEffects: function() {
Expand Down
23 changes: 22 additions & 1 deletion test/unit/configure-elements.html
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,16 @@
observer: 'contentChanged',
value: 'child'
},
negatedContent: {
type: Boolean,
observer: 'negatedContentChanged',
value: true
},
compoundInput: {
type: String,
observer: 'compoundInputChanged',
value: 'default'
},
object: {
type: Object,
notify: true,
Expand Down Expand Up @@ -140,6 +150,8 @@
this.attrDashChanged = sinon.spy();
this.attrNumberChanged = sinon.spy();
this.attrBooleanChanged = sinon.spy();
this.negatedContentChanged = sinon.spy();
this.compoundInputChanged = sinon.spy();
}

});
Expand All @@ -166,7 +178,16 @@

<dom-module id="x-configure-host">
<template>
<x-configure-child id="child" content="{{content}}" object="{{object.goo}}" attr$="{{attrValue}}" attr-dash$="{{attrValue}}" attr-number$="{{attrNumber}}" attr-boolean$="{{attrBoolean}}"></x-configure-child>
<x-configure-child id="child"
content="{{content}}"
negated-content="[[!content]]"
compound-input="a [[simple]] [[content]]"
object="{{object.goo}}"
attr$="{{attrValue}}"
attr-dash$="{{attrValue}}"
attr-number$="{{attrNumber}}"
attr-boolean$="{{attrBoolean}}"
></x-configure-child>
<x-configure-simple-child id="simple" noeffect="{{simple}}"></x-configure-simple-child>
</template>
<script>
Expand Down
13 changes: 13 additions & 0 deletions test/unit/configure.html
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,19 @@
testConfigureHost(e, 'host');
});

test('negated value configured correctly', function() {
var e = document.querySelector('x-configure-host');
assert.equal(e.$.child.negatedContent, false);
assert.isTrue(e.$.child.negatedContentChanged.calledOnce, 'negated content not changed exactly once');
});

test('compound effect resulting value set once', function() {
var e = document.querySelector('x-configure-host');

assert.equal(e.$.child.compoundInput, 'a simple host');
assert.isTrue(e.$.child.compoundInputChanged.calledOnce, 'compound content not changed exactly once');
});

test('attribute overrides configured values and propagates', function() {
var e = document.querySelector('x-configure-host[content]');
testConfigureHost(e, 'attr');
Expand Down

0 comments on commit 1b02e96

Please sign in to comment.