Deletes the databindings when they are detached

Deletes the databindings when they are detached

Because the left panel slot on the inspector gets destroyed
when switching between the unit/charm details views the databinding
would attempt to rebind to elements which didn't exist any longer.

This branch handles removing the databindings from the master list
of bindings when they are detached.

https://code.launchpad.net/~hatch/juju-gui/charm-details-link-fix/+merge/190489

(do not edit description out of merge proposal)

[jeff.pihach, 2013-10-10 21:12:55]

Please take a look.

[benjamin.saller, 2013-10-10 21:22:17]

Thanks for this, some comments below. My reading on the loop might be wrong but
I think it could work.

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

https://codereview.appspot.com/14489044/diff/1/app/views/databinding.js#newco...
app/views/databinding.js:670: if (binding.name.split('.')[0] === key) {
Do we need to iter keys here to do this? This will reset all change events bound
to the model so if viewlet.model.id === mID we splice it out, no? I think you
can lose the outer loop.

https://codereview.appspot.com/14489044/diff/1/app/views/viewlet-manager.js
File app/views/viewlet-manager.js (right):

https://codereview.appspot.com/14489044/diff/1/app/views/viewlet-manager.js#n...
app/views/viewlet-manager.js:424: existing.container.remove(true);
should viewlet.destroy do remove/container remove? when wouldn't that be the
case? pairs of methods you always call together and never apart isn't ideal

[jeff.pihach, 2013-10-11 03:26:27]

*** Submitted:

Deletes the databindings when they are detached

Because the left panel slot on the inspector gets destroyed
when switching between the unit/charm details views the databinding
would attempt to rebind to elements which didn't exist any longer.

This branch handles removing the databindings from the master list
of bindings when they are detached.

R=benjamin.saller
CC=
https://codereview.appspot.com/14489044

https://codereview.appspot.com/14489044/diff/1/app/views/viewlet-manager.js
File app/views/viewlet-manager.js (right):

https://codereview.appspot.com/14489044/diff/1/app/views/viewlet-manager.js#n...
app/views/viewlet-manager.js:424: existing.container.remove(true);
Right I was thinking the same actually, the destroy() method is an empty
function, and needs to be provided at the viewlet level - I can add that in this
branch.