Code review comment for lp:~frankban/juju-gui/preserve-zoom

Reviewers: mp+130087_code.launchpad.net,

Message:
Please take a look.

Description:
Preserve zoom settings in the environment view.

The zoom scale and translate are preserved after a scene redraw
(e.g. when adding/removing relations, destroying services,
navigating to the service view and back to the front page).

https://code.launchpad.net/~frankban/juju-gui/preserve-zoom/+merge/130087

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/views/environment.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/environment.js
=== modified file 'app/views/environment.js'
--- app/views/environment.js 2012-10-15 19:22:40 +0000
+++ app/views/environment.js 2012-10-17 10:43:51 +0000
@@ -237,6 +237,13 @@
          .y(this.yscale)
          .scaleExtent([0.25, 2.0])
          .on('zoom', function() {
+ var sourceEvent = d3.event.sourceEvent;
+ // Avoid spurious zooming if the event is a double click
inside
+ // a service box.
+ if (sourceEvent.type === 'dblclick' &&
+ Y.one(sourceEvent.target).ancestor('.service')) {
+ return;
+ }
                  // Keep the slider up to date with the scale on other sorts
                  // of zoom interactions
                  var s = self.slider;
@@ -386,7 +393,7 @@
          },

          /*
- * Sync view models with curent db.models.
+ * Sync view models with current db.models.
           */
          updateData: function() {
            //model data
@@ -872,14 +879,17 @@
            });
          },
          renderSlider: function() {
- var self = this;
+ var self = this,
+ value = 100,
+ currentScale = this.get('scale');
            // Build a slider to control zoom level
- // TODO once we have a stored value in view models, use that
- // for the value property, but for now, zoom to 100%
+ if (currentScale) {
+ value = currentScale * 100;
+ }
            var slider = new Y.Slider({
              min: 25,
              max: 200,
- value: 100
+ value: value
            });
            slider.render('#slider-parent');
            slider.after('valueChange', function(evt) {
@@ -949,6 +959,22 @@
                .setAttribute('x', -width / 2);
            });

+ // Preserve zoom when the scene is updated.
+ var changed = false,
+ currentScale = this.get('scale'),
+ currentTranslate = this.get('translate');
+ if (currentTranslate && currentTranslate !==
this.zoom.translate()) {
+ this.zoom.translate(currentTranslate);
+ changed = true;
+ }
+ if (currentScale && currentScale !== this.zoom.scale()) {
+ this.zoom.scale(currentScale);
+ changed = true;
+ }
+ if (changed) {
+ this._fire_zoom(0);
+ }
+
            // Render the slider after the view is attached.
            // Although there is a .syncUI() method on sliders, it does not
            // seem to play well with the app framework: the slider will
render
@@ -1134,6 +1160,9 @@
              evt.scale = this.get('scale');
            }
            this.set('scale', evt.scale);
+ // Store the current value of translate by copying the event
array
+ // in order to avoid reference sharing.
+ this.set('translate', evt.translate.slice(0));
            vis.attr('transform', 'translate(' + evt.translate + ')' +
                ' scale(' + evt.scale + ')');
            this.updateServiceMenuLocation();