Code review comment for lp:~benji/juju-gui/no-foot-shooting

Revision history for this message
Benjamin Saller (bcsaller) wrote :

LGTM, a minor suggestion or two included.

https://codereview.appspot.com/7129049/diff/15001/app/views/service.js
File app/views/service.js (right):

https://codereview.appspot.com/7129049/diff/15001/app/views/service.js#newcode566
app/views/service.js:566: *
Agreed, each of these changes wrt gatherRenderData is nice.

https://codereview.appspot.com/7129049/diff/15001/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/7129049/diff/15001/app/views/topology/service.js#newcode769
app/views/topology/service.js:769:
menu.one('.destroy-service').addClass('disabled');
Does this actually work? .destroy-service still has the click handler
bound even if the class greys out the link or something, no?

Might we remove the link from the menu all together?

https://codereview.appspot.com/7129049/diff/15001/test/test_topology.js
File test/test_topology.js (right):

https://codereview.appspot.com/7129049/diff/15001/test/test_topology.js#newcode118
test/test_topology.js:118:
topo.events.ServiceModule.scene['.destroy-service'].click.callback(
a simulate('click') test might be better here if you leave this in the
menu.

https://codereview.appspot.com/7129049/

« Back to merge proposal