LGTM, with the notes below. I'd suggest moving to single event fires where indicated below.
https://codereview.appspot.com/6999047/diff/3001/app/assets/javascripts/d3-components.js File app/assets/javascripts/d3-components.js (right): https://codereview.appspot.com/6999047/diff/3001/app/assets/javascripts/d3-components.js#newcode512 app/assets/javascripts/d3-components.js:512: 'event-resize']}); surprised this needed to be here. Isn't only modules that need this? https://codereview.appspot.com/6999047/diff/3001/app/views/topology/mega.js File app/views/topology/mega.js (right): https://codereview.appspot.com/6999047/diff/3001/app/views/topology/mega.js#newcode66 app/views/topology/mega.js:66: topo.fire('hideSubordinateRelations'); This is a place the patterns need some thought from all of us. In this case its not that we want to fire an event specifically coupled to the 'cancelRelBuild' et al. Rather we indicate what has happened and respond to that in the proper place. Naming around these will take some time to get right across the board. In this case the single 'clickX' can be listened to and trigger both actions from a single fire. https://codereview.appspot.com/6999047/diff/3001/app/views/topology/relation.js File app/views/topology/relation.js (right): https://codereview.appspot.com/6999047/diff/3001/app/views/topology/relation.js#newcode8 app/views/topology/relation.js:8: Templates = views.Templates; Hard to identify what changed in porting things to this module from the diff but it looks good. https://codereview.appspot.com/6999047/ -- https://code.launchpad.net/~makyo/juju-gui/topology-relations/+merge/141120 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~makyo/juju-gui/topology-relations into lp:juju-gui. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
