Merge lp://staging/~bcsaller/juju-gui/scrollwheel into lp://staging/juju-gui/experimental

Reviewers: mp+143003_code.launchpad.net,

Message:
Please take a look.

Description:
Restore Mouse Wheel In PanZoom

The default zoom behavior bound to mouse wheel was doing scaled
translation
that didn't play well with the rest of the pan/zoom math. This overrides
the
default behavior and fires a standard YUI zoom synthetic event. We are
then
able to manually apply the delta and manage scale/translate ourselves.

https://code.launchpad.net/~bcsaller/juju-gui/scrollwheel/+merge/143003

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7099046/

Affected files:
   A [revision details]
   M app/views/login.js
   M app/views/topology/panzoom.js
   M app/views/topology/topology.js

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision: <email address hidden>

Index: app/views/login.js
=== modified file 'app/views/login.js'
--- app/views/login.js 2013-01-09 21:28:04 +0000
+++ app/views/login.js 2013-01-11 23:51:26 +0000
@@ -71,6 +71,7 @@
              env.failedAuthentication ? 'Unknown user or password.' : ''),
          help_text: this.get('help_text')
        }));
+ this.get('container').one('input[type=password]').focus();
        return this;
      }

Index: app/views/topology/panzoom.js
=== modified file 'app/views/topology/panzoom.js'
--- app/views/topology/panzoom.js 2013-01-09 16:59:19 +0000
+++ app/views/topology/panzoom.js 2013-01-11 23:51:26 +0000
@@ -84,8 +84,23 @@
        if (!this.slider) {
          return;
        }
- slider.set('value', this.toSlider(evt.scale));
- this.rescale(d3.event);
+
+ // If this is a scroll wheel event translate
+ // delta and apply to scale.
+ if (evt.sourceEvent) {
+ // If we have a wrapped event facade extract the raw data,
+ // this is needed to properly handle wheel delta events below.
+ evt.stopPropogation();
+ evt.preventDefault();
+ evt = evt.sourceEvent;
+ }
+ if (evt.type === 'mousewheel') {
+ var delta = (evt.wheelDelta > 0) ? 0.1 : -0.1;
+ evt.scale = (topo.get('scale') + delta);
+ evt.translate = topo.get('translate');
+ }
+ slider._set('value', this.toSlider(evt.scale));
+ this.rescale(evt);
      },

      /*

Index: app/views/topology/topology.js
=== modified file 'app/views/topology/topology.js'
--- app/views/topology/topology.js 2013-01-09 17:21:35 +0000
+++ app/views/topology/topology.js 2013-01-11 23:51:26 +0000
@@ -94,9 +94,11 @@
                            .attr('width', width)
                            .attr('height', height)
                            .call(this.zoom)
- .on('mousewheel.zoom', null)
- .on('DOMMouseScroll.zoom', null)
- .on('dblclick.zoom', null);
+ .on('dblclick.zoom', null)
+ .on('DOMMouseScroll', function(evt) {
+ ...

The code looks good, but there are a few interaction problems that I'd
at least like input on from Ben+others.

1 - as below, the focus call is happening before the container is placed
in the DOM, so it's not having any effect. Can either get rid of it, or
move it to after line 550 in app.js, after showView('login'), or within
showView's callback function.

2 - Scrollwheel doesn't work if the mouse is over a service or relation.
I can't remember if this was the case previously or not, too, so it may
be safe to ignore this).

3 - Zooming with the scroll-wheel zooms from the origin (that is,
translate is not updated in the transform). While I don't like this, it
may be acceptable, depending on what London/others say. Two ways around
it are a) the d3.behavior.zoom way, which is to scroll from the mouse's
location (which d3.mouse(<rect element> can provide) or b) scroll from
the center of the rect, which is visible in the old _fire_zoom method on
line 1210 back in
http://bazaar.launchpad.net/~juju-gui/juju-gui/trunk/revision/275#app/views/topology/mega.js

What does everyone think? I don't really have any issues other than
these, and if they're non-issues, that's fine too.

https://codereview.appspot.com/7099046/diff/1/app/views/login.js
File app/views/login.js (right):

https://codereview.appspot.com/7099046/diff/1/app/views/login.js#newcode74
app/views/login.js:74:
this.get('container').one('input[type=password]').focus();
At this point, the container has not been added to the DOM, so this line
is not having the desired effect. Tested with chrome debugger with

Y.one('#login-column') === null

https://codereview.appspot.com/7099046/

Additionally, it appears that panning is no longer working. This is one
I don't think we can live without.

https://codereview.appspot.com/7099046/

Pan, wow, didn't even notice that I was so focused on getting the zoom
working. Thanks, I'll look into it.

https://codereview.appspot.com/7099046/

*** Submitted:

Restore Mouse Wheel In PanZoom

The default zoom behavior bound to mouse wheel was doing scaled
translation
that didn't play well with the rest of the pan/zoom math. This overrides
the
default behavior and fires a standard YUI zoom synthetic event. We are
then
able to manually apply the delta and manage scale/translate ourselves.

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

https://codereview.appspot.com/7099046/