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
LGTM, a minor suggestion or two included.
https:/ /codereview. appspot. com/7129049/ diff/15001/ app/views/ service. js service. js (right):
File app/views/
https:/ /codereview. appspot. com/7129049/ diff/15001/ app/views/ service. js#newcode566 service. js:566: *
app/views/
Agreed, each of these changes wrt gatherRenderData is nice.
https:/ /codereview. appspot. com/7129049/ diff/15001/ app/views/ topology/ service. js topology/ service. js (right):
File app/views/
https:/ /codereview. appspot. com/7129049/ diff/15001/ app/views/ topology/ service. js#newcode769 topology/ service. js:769: '.destroy- service' ).addClass( 'disabled' );
app/views/
menu.one(
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 topology. js (right):
File test/test_
https:/ /codereview. appspot. com/7129049/ diff/15001/ test/test_ topology. js#newcode118 topology. js:118: ServiceModule. scene[' .destroy- service' ].click. callback(
test/test_
topo.events.
a simulate('click') test might be better here if you leave this in the
menu.
https:/ /codereview. appspot. com/7129049/