Code review comment for lp://staging/~gary/juju-gui/databindingChangedValues

Revision history for this message
Gary Poster (gary) wrote :

Comments for reviewers.

https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js
File app/views/databinding.js (left):

https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js#oldcode69
app/views/databinding.js:69: function _getNodeHandler(node) {
I moved this down into the class, because I needed it for tests and it
seemed reasonable and easy there.

https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js
File app/views/databinding.js (right):

https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js#newcode38
app/views/databinding.js:38: 'input[type=checkbox]': Object.create({
Using Object.create is an easy way to let the other functions refer to
each other.

https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js#newcode51
app/views/databinding.js:51: 'eq': function(node, value) {
I added 'eq' to all of the field handlers so that the _storeChanged
method (below) could use it.

https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js#newcode272
app/views/databinding.js:272: @method getFieldHandler
This should be "getNodeHandler". I adjusted it in my branch. Same for
similar bits in the docstring.

https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js#newcode678
app/views/databinding.js:678: BindingEngine.prototype._storeChanged =
function(e, viewlet) {
These changes are the real heart of my branch.

https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js#newcode841
app/views/databinding.js:841: delete viewlet.changedValues[key];
Switching the changedValues to a hash just seemed to make so much code
simpler.

https://codereview.appspot.com/13385045/

« Back to merge proposal