Merge lp:~makyo/juju-gui/center into lp:juju-gui/experimental

[Madison Scott-Clary]

Fix initial centering of canvas in real env.

In a real environment with positioned services, this centers the services within the viewport, an artifact that was lost in a recent refactor. Also tests that the panToPoint event was fired when a positioned service is added (future designs will complicate this, as the inspector will have an arrow pointing to a service, if it's open; this will be prevented in app/views/topology/service.js:panToCenter() once that's implemented). Other tests in place already test finding and setting the centroid.

https://codereview.appspot.com/14771044/

[Madison Scott-Clary (makyo)]

Reviewers: mp+191681_code.launchpad.net,

Message:
Please take a look.

Description:
Fix initial centering of canvas in real env.

In a real environment with positioned services, this centers the
services within the viewport, an artifact that was lost in a recent
refactor. Also tests that the panToPoint event was fired when a
positioned service is added (future designs will complicate this, as the
inspector will have an arrow pointing to a service, if it's open; this
will be prevented in app/views/topology/service.js:panToCenter() once
that's implemented). Other tests in place already test finding and
setting the centroid.

https://code.launchpad.net/~makyo/juju-gui/center/+merge/191681

(do not edit description out of merge proposal)

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

Affected files (+30, -28 lines):
   A [revision details]
   M app/views/topology/service.js
   M test/test_environment_view.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: test/test_environment_view.js
=== modified file 'test/test_environment_view.js'
--- test/test_environment_view.js 2013-10-15 22:24:17 +0000
+++ test/test_environment_view.js 2013-10-17 16:54:10 +0000
@@ -586,7 +586,7 @@
            view.topo.service_boxes.wordpressb.center);
      });

- it('must be able to use position annotations', function() {
+ it('must be able to use position annotations', function(done) {
        var tmp_data = {
          op: 'delta',
          result: [
@@ -621,7 +621,13 @@
        match[1].should.eql('374.1');
        match[2].should.eql('211.2');

- // A positioned service will never be auto-positioned.
+ // A positioned service will never be auto-positioned. It will also
+ // center the canvas on itself.
+ view.topo.on('panToPoint', function() {
+ // Once we reach here, the view has been updated and the canvas
panned
+ // to the newly added/annotated service.
+ done();
+ });
        tmp_data = {
          op: 'delta',
          result: [
@@ -636,20 +642,6 @@
        };
        db.onDelta({ data: tmp_data });
        view.update();
-
- tmp_data = {
- op: 'delta',
- result: [
- ['service', 'add',
- {
- 'subordinate': false,
- 'charm': 'cs:precise/wordpress-6',
- 'id': 'wordpressb'
- }
- ]]
- };
- db.onDelta({ data: tmp_data });
- view.update();
      });

      it('must be able to use Landscape annotations', function() {

Index: app/views/topology/service.js
=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js 2013-10-15 19:39:05 +0000
+++ app/views/topology/service.js 2013-10-17 17:01:32 +0000
@@ -714,7 +714,7 @@
                    level: 'important'
                  });
                } else {
- console.log('import failed', file, result);
+ ...

[Madison Scott-Clary (makyo)]

*** Submitted:

Fix initial centering of canvas in real env.

In a real environment with positioned services, this centers the
services within the viewport, an artifact that was lost in a recent
refactor. Also tests that the panToPoint event was fired when a
positioned service is added (future designs will complicate this, as the
inspector will have an arrow pointing to a service, if it's open; this
will be prevented in app/views/topology/service.js:panToCenter() once
that's implemented). Other tests in place already test finding and
setting the centroid.

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

https://codereview.appspot.com/14771044/

=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js	2013-10-15 19:39:05 +0000
+++ app/views/topology/service.js	2013-10-17 17:08:07 +0000
@@ -714,7 +714,7 @@
                   level: 'important'
                 });
               } else {
-                console.log('import failed', file, result);
+                console.warn('import failed', file, result);
                 notifications.add({
                   title: 'Import Environment Failed',
                   message: 'Import from "' + file.name +
@@ -1000,12 +1000,20 @@
       // and drop, or those who don't have position attributes/annotations.
       var vertices = [];
       Y.each(topo.service_boxes, function(boundingBox) {
-        var annotations = boundingBox.annotations;
-        if (annotations['gui-x'] && boundingBox.x === undefined) {
-          boundingBox.x = annotations['gui-x'];
-        }
-        if (annotations['gui-y'] && boundingBox.y === undefined) {
-          boundingBox.y = annotations['gui-y'];
+        if (!boundingBox.model.get('pending')) {
+          var annotations = boundingBox.annotations,
+              addToVertices = 0;
+          if (annotations['gui-x'] && boundingBox.x === undefined) {
+            boundingBox.x = annotations['gui-x'];
+            addToVertices += 1;
+          }
+          if (annotations['gui-y'] && boundingBox.y === undefined) {
+            boundingBox.y = annotations['gui-y'];
+            addToVertices += 1;
+          }
+          if (addToVertices === 2) {
+            vertices.push([boundingBox.x, boundingBox.y]);
+          }
         }
       });
 
@@ -1073,11 +1081,11 @@
             }
           });
         }
-        // Find the centroid of our hull of services and inform the
-        // topology.
-        if (vertices.length) {
-          this.findCentroid(vertices);
-        }
+      }
+      // Find the centroid of our hull of services and inform the
+      // topology.
+      if (vertices.length) {
+        this.findCentroid(vertices);
       }
 
       // enter

=== modified file 'test/test_environment_view.js'
--- test/test_environment_view.js	2013-10-15 22:24:17 +0000
+++ test/test_environment_view.js	2013-10-17 17:08:07 +0000
@@ -586,7 +586,7 @@
           view.topo.service_boxes.wordpressb.center);
     });
 
-    it('must be able to use position annotations', function() {
+    it('must be able to use position annotations', function(done) {
       var tmp_data = {
         op: 'delta',
         result: [
@@ -621,7 +621,13 @@
       match[1].should.eql('374.1');
       match[2].should.eql('211.2');
 
-      // A positioned service will never be auto-positioned.
+      // A positioned service will never be auto-positioned. It will also
+      // center the canvas on itself.
+      view.topo.on('panToPoint', function() {
+        // Once we reach here, the view has been updated and the canvas panned
+        // to the newly added/annotated service.
+        done();
+      });
       tmp_data = {
         op: 'delta',
         result: [
@@ -636,20 +642,6 @@
       };
       db.onDelta({ data: tmp_data });
       view.update();
-
-      tmp_data = {
-        op: 'delta',
-        result: [
-          ['service', 'add',
-            {
-              'subordinate': false,
-              'charm': 'cs:precise/wordpress-6',
-              'id': 'wordpressb'
-            }
-          ]]
-      };
-      db.onDelta({ data: tmp_data });
-      view.update();
     });
 
     it('must be able to use Landscape annotations', function() {