Code review comment for lp:~makyo/juju-gui/center-on-load

Revision history for this message
Madison Scott-Clary (makyo) wrote :

Reviewers: mp+183606_code.launchpad.net,

Message:
Please take a look.

Description:
Zoom to center on dimensions set

The dimensions of the canvas default to 800x600px, and so on smaller
screens, centering the services in the environment centered them beneath
the charm store. This centers them *after* the sizes are calculated,
which centers them in the center of the screen itself.

https://code.launchpad.net/~makyo/juju-gui/center-on-load/+merge/183606

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/views/topology/viewport.js
   M test/test_service_module.js
   M test/test_viewport_module.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_service_module.js
=== modified file 'test/test_service_module.js'
--- test/test_service_module.js 2013-08-09 17:58:47 +0000
+++ test/test_service_module.js 2013-09-03 09:41:49 +0000
@@ -393,9 +393,9 @@
      var eventHandle = Y.on('initiateDeploy', function(charm,
ghostAttributes) {
        eventHandle.detach();
        // After the translation and calculations the above x and y coords
should
- // place the element at 305, 157
+ // place the element at 320, 392
        assert.deepEqual(ghostAttributes, {
- coordinates: [305, 157],
+ coordinates: [320, 392],
          icon: src
        });
        // Make sure that the drag and drop was properly prevented.

Index: test/test_viewport_module.js
=== modified file 'test/test_viewport_module.js'
--- test/test_viewport_module.js 2013-05-17 14:51:05 +0000
+++ test/test_viewport_module.js 2013-09-03 09:41:49 +0000
@@ -94,7 +94,8 @@
  });

  describe('views.ViewportModule.setAllTheDimensions', function() {
- var views, Y, testUtils, view, width, height, canvas, svg, topo,
zoomPlane;
+ var views, Y, testUtils, view, width, height, canvas, svg, topo,
zoomPlane,
+ eventFired;
    before(function(done) {
      Y = YUI(GlobalConfig).use(['node', 'juju-views', 'juju-tests-utils'],
          function(Y) {
@@ -108,7 +109,12 @@
      height = Math.floor(Math.random() * 1000);
      width = Math.floor(Math.random() * 1000);
      // Build test doubles that record height and width settings.
- topo = {vis: {}};
+ topo = {
+ vis: {},
+ fire: function(evt) {
+ eventFired = evt;
+ }
+ };
      topo.set = testUtils.setter(topo);
      topo.vis.attr = testUtils.setter(topo.vis);
      view = new views.ViewportModule();
@@ -153,4 +159,7 @@
      assert.equal(topo.vis.height, height);
    });

+ it('should center canvas', function() {
+ assert.equal(eventFired, 'panToCenter');
+ });
  });

Index: app/views/topology/viewport.js
=== modified file 'app/views/topology/viewport.js'
--- app/views/topology/viewport.js 2013-06-05 16:58:11 +0000
+++ app/views/topology/viewport.js 2013-09-03 09:32:59 +0000
@@ -93,6 +93,7 @@
          height: dimensions.height + 'px'});
        // Reset the scale parameters
        topo.set('size', [dimensions.width, dimensions.height]);
+ topo.fire('panToCenter');
      },

      /**

« Back to merge proposal