Relations Topology Module

Relations Topology Module

Refactoring relations out of the mega topology module into its own module.  This is quite a big branch, and with undocumented methods and custom events, may get larger yet.  Requesting bcsaller as one reviewer.

https://code.launchpad.net/~makyo/juju-gui/topology-relations/+merge/141120

(do not edit description out of merge proposal)

[matthew.scott, 2012-12-21 19:34:33]

Please take a look.

[matthew.scott, 2012-12-21 21:31:57]

Please take a look.

[bcsaller, 2013-01-02 21:53:07]

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-co...
File app/assets/javascripts/d3-components.js (right):

https://codereview.appspot.com/6999047/diff/3001/app/assets/javascripts/d3-co...
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#n...
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....
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.

[bac, 2013-01-03 14:10:37]

Looks good Matt.

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#n...
app/views/topology/mega.js:37: //mouseleave: 'mousemove'
Delete commented out code.

[matthew.scott, 2013-01-03 20:44:16]

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-co...
File app/assets/javascripts/d3-components.js (right):

https://codereview.appspot.com/6999047/diff/3001/app/assets/javascripts/d3-co...
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#n...
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#n...
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.

[matthew.scott, 2013-01-03 20:48:55]

*** Submitted:

Relations Topology Module

Refactoring relations out of the mega topology module into its own module.  This
is quite a big branch, and with undocumented methods and custom events, may get
larger yet.  Requesting bcsaller as one reviewer.

R=
CC=
https://codereview.appspot.com/6999047