LGTM with notes below. Will QA in a sec.
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#newcode62 app/views/databinding.js:62: return (normalizedValue === currentValue); should we refactor this out into any _stringEq or something since it's duped in the three eq methods?
https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js#newcode688 app/views/databinding.js:688: if (nodeHandler.eq(e.target, binding.get(model))) { will this blow up if no binding was found matching key?
https://codereview.appspot.com/13385045/diff/1/app/views/databinding.js#newcode875 app/views/databinding.js:875: 'event-valuechange'] is this required in this module? It's used in the inspector I believe, but not here.
https://codereview.appspot.com/13385045/diff/1/test/test_databinding.js File test/test_databinding.js (right):
https://codereview.appspot.com/13385045/diff/1/test/test_databinding.js#newcode26 test/test_databinding.js:26: 'model', 'model-list', 'node-event-simulate']; this needs the event-valueChange since you are binding with that event below.
https://codereview.appspot.com/13385045/
« Back to merge proposal
LGTM with notes below. Will QA in a sec.
https:/ /codereview. appspot. com/13385045/ diff/1/ app/views/ databinding. js databinding. js (right):
File app/views/
https:/ /codereview. appspot. com/13385045/ diff/1/ app/views/ databinding. js#newcode62 databinding. js:62: return (normalizedValue === currentValue);
app/views/
should we refactor this out into any _stringEq or something since it's
duped in the three eq methods?
https:/ /codereview. appspot. com/13385045/ diff/1/ app/views/ databinding. js#newcode688 databinding. js:688: if (nodeHandler. eq(e.target, get(model) )) {
app/views/
binding.
will this blow up if no binding was found matching key?
https:/ /codereview. appspot. com/13385045/ diff/1/ app/views/ databinding. js#newcode875 databinding. js:875: 'event- valuechange' ]
app/views/
is this required in this module? It's used in the inspector I believe,
but not here.
https:/ /codereview. appspot. com/13385045/ diff/1/ test/test_ databinding. js databinding. js (right):
File test/test_
https:/ /codereview. appspot. com/13385045/ diff/1/ test/test_ databinding. js#newcode26 databinding. js:26: 'model', 'model-list', simulate' ];
test/test_
'node-event-
this needs the event-valueChange since you are binding with that event
below.
https:/ /codereview. appspot. com/13385045/