yellow team mailing list archive

[Matthew Scott <matthew.scott@xxxxxxxxxxxxx>]

Thanks for the reviews.  Discovered
https://bugs.launchpad.net/juju-gui/+bug/1095761 in the process, which
was causing my regression.  Will investigate this before getting to the
service topology module.


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']});
On 2013/01/02 21:53:07, bcsaller wrote:
> surprised this needed to be here. Isn't only modules that need this?

It's needed where events are bound.  Chances are, unless event are bound
using the YUI framework within the modules, they won't need either
event-resize or event.  Perhaps another card, or I can add it to this
branch.

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#newcode37
app/views/topology/mega.js:37: //mouseleave: 'mousemove'
On 2013/01/03 14:10:37, bac wrote:
> Delete commented out code.

Done.

https://codereview.appspot.com/6999047/diff/3001/app/views/topology/mega.js#newcode66
app/views/topology/mega.js:66: topo.fire('hideSubordinateRelations');
On 2013/01/02 21:53:07, bcsaller wrote:
> 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.

This makes sense, and I even had an appropriate event set up in
relations, just not set up properly.  Done.

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.