Panzoom Module

Panzoom Module

This branch breaks out the first module
from Mega into a more module unit. While this
module is small a number of changes occur to make
this happen. The framework underwent some improvemnts,
changes around interaction with App and view replacement
occured, event bindings had to be updated. A pattern for 
cross module event firing was established.

In future modules, the topo/component fires the events and
modules are bubble targets.

https://code.launchpad.net/~bcsaller/juju-gui/topology-panzoom/+merge/140671

(do not edit description out of merge proposal)

[bcsaller, 2012-12-19 13:50:18]

Please take a look.

[bcsaller, 2012-12-20 13:39:21]

Please take a look.

[bcsaller, 2012-12-20 14:49:03]

Please take a look.

[gary.poster, 2012-12-20 15:23:26]

Thanks for helping me understand this.  As I expected, I'm +1 on the code.  I
would like a bug/card to fix the prod tests on start.  I'd like to see if we can
fix the viewport resize issue one way or another before we land.

Thank you

Gary

https://codereview.appspot.com/6971045/diff/17/app/assets/javascripts/d3-comp...
File app/assets/javascripts/d3-components.js (right):

https://codereview.appspot.com/6971045/diff/17/app/assets/javascripts/d3-comp...
app/assets/javascripts/d3-components.js:328: bindAllD3Events: function() {
Comment describing behavior and saying that the expected client of this is the
component might be nice.

https://codereview.appspot.com/6971045/diff/17/app/views/topology/panzoom.js
File app/views/topology/panzoom.js (right):

https://codereview.appspot.com/6971045/diff/17/app/views/topology/panzoom.js#...
app/views/topology/panzoom.js:107: * Wraper around the actual rescale method for
zoom buttons.
Wrapper

[hazmat, 2012-12-20 15:43:29]

quick items

https://codereview.appspot.com/6971045/diff/7002/app/assets/javascripts/d3-co...
File app/assets/javascripts/d3-components.js (right):

https://codereview.appspot.com/6971045/diff/7002/app/assets/javascripts/d3-co...
app/assets/javascripts/d3-components.js:255: console.log('Bind', target,
eventPhase, name);
s/console.log // console.debug .. 
s/'Bind'/'d3 components bind'

[bcsaller, 2012-12-20 17:23:55]

*** Submitted:

Panzoom Module

This branch breaks out the first module
from Mega into a more module unit. While this
module is small a number of changes occur to make
this happen. The framework underwent some improvemnts,
changes around interaction with App and view replacement
occured, event bindings had to be updated. A pattern for 
cross module event firing was established.

In future modules, the topo/component fires the events and
modules are bubble targets.

R=gary.poster, hazmat
CC=
https://codereview.appspot.com/6971045

https://codereview.appspot.com/6971045/diff/17/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/6971045/diff/17/app/views/environment.js#newco...
app/views/environment.js:27: broadcast: true,
Test again w/o broadcast.

https://codereview.appspot.com/6971045/diff/17/app/views/topology/mega.js
File app/views/topology/mega.js (right):

https://codereview.appspot.com/6971045/diff/17/app/views/topology/mega.js#new...
app/views/topology/mega.js:872: self.cursorBox = new views.BoundingBox();
need new here?

[bac, 2012-12-20 17:51:40]

I make several suggestions but they are all straight-forward so no re-look is
necessary before landing.

Recently I *think* we agreed to the 'one var per variable' idea which you didn't
follow for the newly added code.  Perhaps it was "When in Rome" or you just
forgot.  I'm just mentioning as a point of discussion not a suggestion to change
as it would delay this important work from landing.

https://codereview.appspot.com/6971045/diff/17/app/assets/javascripts/d3-comp...
File app/assets/javascripts/d3-components.js (right):

https://codereview.appspot.com/6971045/diff/17/app/assets/javascripts/d3-comp...
app/assets/javascripts/d3-components.js:155: 
These two cases are mutually exclusive, right, so 'else if' would be clearer.

https://codereview.appspot.com/6971045/diff/17/app/assets/javascripts/d3-comp...
app/assets/javascripts/d3-components.js:248: if
(Y.Array.indexOf(['windowresize'], name) !== -1) {
I don't understand why the indexOf is used instead of just a comparison.

https://codereview.appspot.com/6971045/diff/17/app/assets/javascripts/d3-comp...
app/assets/javascripts/d3-components.js:315: // Create an adaptor
s/adaptor/adapter

https://codereview.appspot.com/6971045/diff/17/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/6971045/diff/17/app/views/environment.js#newco...
app/views/environment.js:66: // Bind d3 events (manually)
This line is a complete thought and needs a period to be more readable.

https://codereview.appspot.com/6971045/diff/17/app/views/environment.js#newco...
app/views/environment.js:69: // the existing (from change to showView)
I'm unclear what you're saying here.

https://codereview.appspot.com/6971045/diff/17/app/views/topology/panzoom.js
File app/views/topology/panzoom.js (right):

https://codereview.appspot.com/6971045/diff/17/app/views/topology/panzoom.js#...
app/views/topology/panzoom.js:9: * Handle PanZoom within the a Topology.
typo: the a

Pick one?  :)

https://codereview.appspot.com/6971045/diff/17/app/views/topology/panzoom.js#...
app/views/topology/panzoom.js:51: contianer = topo.get('container'),
typo: container

Is that variable even used?

https://codereview.appspot.com/6971045/diff/17/app/views/topology/panzoom.js#...
app/views/topology/panzoom.js:127: evt.translate[1] -=
parseInt(rect.attr('height'), 10) / 2 * delta;
I wish we'd made a wrapper early on for parseInt that defaulted to base 10.  So
annoying to see everywhere.

https://codereview.appspot.com/6971045/diff/17/app/views/topology/topology.js
File app/views/topology/topology.js (right):

https://codereview.appspot.com/6971045/diff/17/app/views/topology/topology.js...
app/views/topology/topology.js:17: * Emmitted Events:
Emitted