Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix Node.bind to pass the property name to reflectPropertyToAttribute #319

Merged
merged 1 commit into from
Oct 15, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/instance/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
return value;
}
},
relectPropertyToAttribute: function(name) {
reflectPropertyToAttribute: function(name) {
//if (Object.keys(this[PUBLISHED]).indexOf(name) >= 0) {
// TODO(sjmiles): consider memoizing this
var inferredType = typeof this.__proto__[name];
Expand All @@ -77,10 +77,10 @@
// boolean properties must reflect as boolean attributes
if (serializedValue !== undefined) {
this.setAttribute(name, serializedValue);
// TODO(sorvell): we should remove attr for all properties
// that have undefined serialization; however, we will need to
// TODO(sorvell): we should remove attr for all properties
// that have undefined serialization; however, we will need to
// refine the attr reflection system to achieve this; pica, for example,
// relies on having inferredType object properties not removed as
// relies on having inferredType object properties not removed as
// attrs.
} else if (inferredType === 'boolean') {
this.removeAttribute(name);
Expand All @@ -92,5 +92,5 @@
// exports

scope.api.instance.attributes = attributes;

})(Polymer);
16 changes: 8 additions & 8 deletions src/instance/mdv.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
var log = window.logFlags || 0;

// use an MDV syntax

var mdv_syntax = new PolymerExpressions();

// element api supporting mdv
Expand All @@ -35,7 +35,7 @@
// reflect bound property to attribute when binding
// to ensure binding is not left on attribute if property
// does not update due to not changing.
this.relectPropertyToAttribute(name);
this.reflectPropertyToAttribute(property);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the actual bug fix. figuring out how to reproduce it in the test case was the hard part, because observeAttributeProperty+observe-js polyfills means it doesn't happen in practice for a typical case :)

return this.bindings[name] = observer;
} else {
return this.super(arguments);
Expand All @@ -50,7 +50,7 @@
unbindAll: function() {
if (!this._unbound) {
this.unbindAllProperties();
this.super();
this.super();
// unbind shadowRoot
var root = this.shadowRoot;
while (root) {
Expand Down Expand Up @@ -84,11 +84,11 @@
function unbindNodeTree(node) {
forNodeTree(node, _nodeUnbindAll);
}

function _nodeUnbindAll(node) {
node.unbindAll();
}

function forNodeTree(node, callback) {
if (node) {
callback(node);
Expand All @@ -97,12 +97,12 @@
}
}
}

var mustachePattern = /\{\{([^{}]*)}}/;

// exports

scope.bindPattern = mustachePattern;
scope.api.instance.mdv = mdv;

})(Polymer);
6 changes: 3 additions & 3 deletions src/instance/properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
// construct an observer on 'name' that ...
this._observe(name, function() {
// reflects the value to an attribute
self.relectPropertyToAttribute(name);
self.reflectPropertyToAttribute(name);
});
},
observeProperty: function(name, methodName) {
Expand All @@ -72,7 +72,7 @@
// construct an observer on 'name' that ...
this._observe(name, function(value, old) {
// reflects the value to an attribute
self.relectPropertyToAttribute(name);
self.reflectPropertyToAttribute(name);
// observes the value if it is an array
self.observeArrayValue(name, value, old);
// invokes user's side-effect method
Expand Down Expand Up @@ -150,7 +150,7 @@
}
};


// property binding

// bind a property in A to a path in B by converting A[property] to a
Expand Down
54 changes: 54 additions & 0 deletions test/html/prop-attr-bind-reflection.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<!DOCTYPE html>
<html>
<head>
<title>property to attribute reflection with bind</title>
<script src="../../polymer.js"></script>
<script src="../../../tools/test/htmltest.js"></script>
<script src="../../../tools/test/chai/chai.js"></script>
</head>

<body>
<polymer-element name="my-child-element">
<template>
<h1>Hello from the child</h1>
<p>The camelCase is {{camelCase}}, attr is {{attributes.camelCase.value}}</p>
<p>The lowercase is {{lowercase}}, attr is {{attributes.lowercase.value}}</p>
</template>
<script>
Polymer('my-child-element', {
publish: { camelCase: 0, lowercase: 0 },
// Make this a no-op, so we can verify the initial
// reflectPropertyToAttribute works.
observeAttributeProperty: function(name) { }
});
</script>
</polymer-element>

<polymer-element name="my-element">
<template>
<h1>Hello from the custom element. The volume is {{volume}}</h1>
<p>
<my-child-element id="child"
camelCase="{{volume}}" lowercase="{{volume}}"></my-child-element>
</p>
</template>
<script>
Polymer('my-element', {
publish: { volume: 11 },
ready: function() {
var child = this.$.child;
chai.assert.equal(child.lowercase, 11);
chai.assert.equal(child.camelCase, 11);

chai.assert.equal('' + child.lowercase, child.getAttribute('lowercase'));
chai.assert.equal('' + child.camelCase, child.getAttribute('camelcase'));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test was failing on this line, because camelcase attribute was left as "{{volume}}"


done();
}
});
</script>
</polymer-element>

<my-element></my-element>
</body>
</html>
9 changes: 5 additions & 4 deletions test/js/bindMDV.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ suite('bindMDV', function() {
t.innerHTML = html;
return t.createInstance(model);
}

test('bindModel bindModel', function(done) {
var test = document.createElement('div');
var fragment = parseAndBindHTML('<div id="a" foo="{{bar}}"></div>',
test);
test.appendChild(fragment);
var a = test.querySelector('#a');

test.bar = 5;
Platform.flush();
var mutation = 0;
Expand All @@ -35,7 +35,7 @@ suite('bindMDV', function() {
}
}).observe(a, {attributes: true});
});

test('bindModel bind input', function(done) {
var test = document.createElement('div');
var fragment = parseAndBindHTML('<input value="{{bar}}" />', test);
Expand All @@ -50,12 +50,13 @@ suite('bindMDV', function() {
done();
});
});

});


htmlSuite('bind', function() {
htmlTest('html/template-distribute-dynamic.html');
htmlTest('html/bind.html');
htmlTest('html/unbind.html');
htmlTest('html/prop-attr-bind-reflection.html');
});