Databinding reset support (to model values)

Databinding reset support (to model values)

Support for resetting a changed DOM to the values currently in the model.
This will most likely still need additional support to clear any conflict
state.

https://code.launchpad.net/~bcsaller/juju-gui/databinding-reset/+merge/184273

(do not edit description out of merge proposal)

[bcsaller, 2013-09-06 10:57:08]

Please take a look.

[benji, 2013-09-06 11:17:11]

Thanks for the branch, it LGTM.  I just had a couple of small things you'll
probably want to address.

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

https://codereview.appspot.com/13418046/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:214: @method deltaForViewlet
@param {Object} viewlet The viewlet for which a delta will be generated.

https://codereview.appspot.com/13418046/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:586: @param {String} viewletName (optional)
The parameter name is "name" instead of "viewletName".

Also, this @param needs a description.

https://codereview.appspot.com/13418046/diff/1/test/test_databinding.js
File test/test_databinding.js (right):

https://codereview.appspot.com/13418046/diff/1/test/test_databinding.js#newco...
test/test_databinding.js:106: // But then we reset
These tests are nicely commented.

https://codereview.appspot.com/13418046/diff/1/test/test_databinding.js#newco...
test/test_databinding.js:111: // Bindings continue to work after reset.
This needs one more space indent.

https://codereview.appspot.com/13418046/diff/1/test/test_databinding.js#newco...
test/test_databinding.js:122: name: 'testViewlet',
Is it intentional that the only difference between this test and the previous is
that this viewlet has a name?

If so, perhaps we need a comment calling this out so the reader sees it more
quickly or maybe there is a better way to test this since the previous test has
exercised most of the code that this test touches.

https://codereview.appspot.com/13418046/diff/1/test/test_databinding.js#newco...
test/test_databinding.js:142: // Bindings continue to work after reset.
This needs one more space indent.

[benjamin.saller, 2013-09-06 11:29:09]

Thanks for the review.

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

https://codereview.appspot.com/13418046/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:214: @method deltaForViewlet
On 2013/09/06 11:17:11, benji wrote:
> @param {Object} viewlet The viewlet for which a delta will be generated.

Done.

https://codereview.appspot.com/13418046/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:586: @param {String} viewletName (optional)
On 2013/09/06 11:17:11, benji wrote:
> The parameter name is "name" instead of "viewletName".
> 
> Also, this @param needs a description.

Done.

https://codereview.appspot.com/13418046/diff/1/test/test_databinding.js
File test/test_databinding.js (right):

https://codereview.appspot.com/13418046/diff/1/test/test_databinding.js#newco...
test/test_databinding.js:122: name: 'testViewlet',
On 2013/09/06 11:17:11, benji wrote:
> Is it intentional that the only difference between this test and the previous
is
> that this viewlet has a name?
> 
> If so, perhaps we need a comment calling this out so the reader sees it more
> quickly or maybe there is a better way to test this since the previous test
has
> exercised most of the code that this test touches.

Yes, this just tests the branch of code where we extract only those bindings
applying to a given viewlet. Added comment.

https://codereview.appspot.com/13418046/diff/1/test/test_databinding.js#newco...
test/test_databinding.js:142: // Bindings continue to work after reset.
On 2013/09/06 11:17:11, benji wrote:
> This needs one more space indent.

Done.

[bcsaller, 2013-09-06 12:45:24]

*** Submitted:

Databinding reset support (to model values)

Support for resetting a changed DOM to the values currently in the model.
This will most likely still need additional support to clear any conflict
state.

R=benji, benjamin.saller
CC=
https://codereview.appspot.com/13418046