yellow team mailing list archive

[Benjamin Saller <benjamin.saller@xxxxxxxxxxxxx>]

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.