yellow team mailing list archive

[Thiago Veronezi <thiago@xxxxxxxxxxxx>]

Thiago Veronezi has proposed merging lp:~tveronezi/juju-gui/generic-show-view-event into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1075962 in juju-ui: "Refactor per event views into a generic show view event"
  https://bugs.launchpad.net/juju-gui/+bug/1075962

For more details, see:
https://code.launchpad.net/~tveronezi/juju-gui/generic-show-view-event/+merge/133247

Generic show view event

Instead of wrapping the 'navigate' method call with multiple 'show' events, we want call the 'navigate' method from a single event.

So, instead of...

this.on('*:showService', this.navigate_to_service);
this.on('*:showUnit', this.navigate_to_unit);
this.on('*:showCharm', this.navigate_to_charm);
this.on('*:showEnvironment', this.navigate_to_environment);

... we will have...

this.on('*:navigateTo', function(e) {
 console.log('navigateTo', e);
 this.navigate(e.url);
}, this);

https://codereview.appspot.com/6819104/

-- 
https://code.launchpad.net/~tveronezi/juju-gui/generic-show-view-event/+merge/133247
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~tveronezi/juju-gui/generic-show-view-event into lp:juju-gui.

=== modified file 'app/app.js'
--- app/app.js	2012-11-01 13:30:58 +0000
+++ app/app.js	2012-11-07 13:53:20 +0000
@@ -151,15 +151,14 @@
 
       // Event subscriptions
 
+      this.on('*:navigateTo', function(e) {
+        console.log('navigateTo', e);
+        this.navigate(e.url);
+      }, this);
+
       // When the provider type becomes available, display it.
       this.env.after('providerTypeChange', this.onProviderTypeChange);
 
-      // TODO: refactor per event views into a generic show view event.
-      this.on('*:showService', this.navigate_to_service);
-      this.on('*:showUnit', this.navigate_to_unit);
-      this.on('*:showCharm', this.navigate_to_charm);
-      this.on('*:showEnvironment', this.navigate_to_environment);
-
       // Feed environment changes directly into the database.
       this.env.on('delta', this.db.on_delta, this.db);
 
@@ -260,42 +259,6 @@
       });
     },
 
-    // Event handlers
-
-    /**
-     * @method navigate_to_unit
-     */
-    navigate_to_unit: function(e) {
-      console.log('Evt.Nav.Router unit target', e.unit_id);
-      this.navigate('/unit/' + e.unit_id.replace('/', '-') + '/');
-    },
-
-    /**
-     * @method navigate_to_service
-     */
-    navigate_to_service: function(e) {
-      var service = e.service;
-      console.log(service.get('id'), 'Evt.Nav.Router service target');
-      this.navigate('/service/' + service.get('id') + '/');
-    },
-
-    /**
-     * @method navigate_to_charm
-     */
-    navigate_to_charm: function(e) {
-      console.log('Evt.Nav.Router charm');
-      var charm_url = e.charm_data_url;
-      this.navigate('/charms/' + charm_url);
-    },
-
-    /**
-     * @method navigate_to_environment
-     */
-    navigate_to_environment: function(e) {
-      console.log('Evt.Nav.Router environment');
-      this.navigate('/');
-    },
-
     // Route handlers
 
     /**

=== modified file 'app/views/charm.js'
--- app/views/charm.js	2012-10-19 01:44:45 +0000
+++ app/views/charm.js	2012-11-07 13:53:20 +0000
@@ -87,7 +87,9 @@
       // The deploy call generates an event chain leading to a call to
       // `app.on_database_changed()`, which re-dispatches the current view.
       // For this reason we need to redirect to the root page right now.
-      this.fire('showEnvironment');
+      this.fire('navigateTo', {
+        url: '/'
+      });
       env.deploy(charmUrl, serviceName, config,
           Y.bind(this._deployCallback, this)
       );
@@ -130,7 +132,9 @@
       // TODO: Use view.events structure to attach this
       container.all('div.thumbnail').each(function(el) {
         el.on('click', function(evt) {
-          self.fire('showCharm', {charm_data_url: this.getData('charm-url')});
+          self.fire('navigateTo', {
+            url: '/charms/' + this.getData('charm-url')
+          });
         });
       });
 

=== modified file 'app/views/environment.js'
--- app/views/environment.js	2012-11-01 13:21:53 +0000
+++ app/views/environment.js	2012-11-07 13:53:20 +0000
@@ -203,7 +203,7 @@
 
         initializer: function() {
           console.log('View: Initialized: Env');
-          this.publish('showService', {preventable: false});
+          this.publish('navigateTo', {preventable: false});
 
           // Build a service.id -> BoundingBox map for services.
           this.service_boxes = {};
@@ -1335,7 +1335,7 @@
            * View a service
            */
           show_service: function(m, view) {
-            view.fire('showService', {service: m});
+            view.fire('navigateTo', {url: '/service/' + m.get('id') + '/'});
           },
 
           /*

=== modified file 'app/views/service.js'
--- app/views/service.js	2012-11-05 23:10:01 +0000
+++ app/views/service.js	2012-11-07 13:53:20 +0000
@@ -216,7 +216,9 @@
             ));
         this.panel.hide();
         this.panel.destroy();
-        this.fire('showEnvironment');
+        this.fire('navigateTo', {
+          url: '/'
+        });
       }
     }
   };
@@ -859,8 +861,11 @@
 
         events: {
           'div.unit': {click: function(ev) {
-            console.log('Unit clicked', ev.currentTarget.get('id'));
-            this.fire('showUnit', {unit_id: ev.currentTarget.get('id')});
+            var id = ev.currentTarget.get('id');
+            console.log('Unit clicked', id);
+            this.fire('navigateTo', {
+              url: '/unit/' + id.replace('/', '-') + '/'
+            });
           }}
         }
       });

=== modified file 'app/views/unit.js'
--- app/views/unit.js	2012-10-29 14:12:44 +0000
+++ app/views/unit.js	2012-11-07 13:53:20 +0000
@@ -208,7 +208,10 @@
         db.units.remove(db.units.getById(unit_name));
         service.set('unit_count', service.get('unit_count') - 1);
         this.remove_panel.destroy();
-        this.fire('showService', {service: service});
+        this.fire('navigateTo', {
+          service: service,
+          url: '/service/' + service.get('id') + '/'
+        });
       }
 
       btn.set('disabled', false);

=== modified file 'test/test_charm_view.js'
--- test/test_charm_view.js	2012-10-19 01:53:03 +0000
+++ test/test_charm_view.js	2012-11-07 13:53:20 +0000
@@ -87,7 +87,8 @@
         charm_store: localCharmStore,
         env: env});
       var redirected = false;
-      charmView.on('showEnvironment', function() {
+      charmView.on('navigateTo', function(e) {
+        assert.equal('/', e.url);
         redirected = true;
       });
       var deployInput = charmView.get('container').one('#charm-deploy');

=== modified file 'test/test_service_view.js'
--- test/test_service_view.js	2012-10-29 10:25:58 +0000
+++ test/test_service_view.js	2012-11-07 13:53:20 +0000
@@ -98,7 +98,8 @@
       var view = makeServiceView(),
           unit = container.one('ul.thumbnails').one('div.unit'),
           showUnitCalled = false;
-      view.on('*:showUnit', function() {
+      view.on('navigateTo', function(e) {
+        assert.equal('/unit/mysql-0/', e.url);
         showUnitCalled = true;
       });
       unit.simulate('click');
@@ -304,7 +305,8 @@
          var destroy = container.one('#destroy-modal-panel .btn-danger');
          destroy.simulate('click');
          var called = false;
-         view.on('showEnvironment', function(ev) {
+         view.on('navigateTo', function(ev) {
+           assert.equal('/', ev.url);
            called = true;
          });
          var callbacks = Y.Object.values(env._txn_callbacks);

=== modified file 'test/test_unit_view.js'
--- test/test_unit_view.js	2012-10-24 10:03:18 +0000
+++ test/test_unit_view.js	2012-11-07 13:53:20 +0000
@@ -189,7 +189,8 @@
       container.one('#remove-modal-panel .btn-danger')
          .simulate('click');
       var called_event = null;
-      view.on('showService', function(ev) {
+      view.on('navigateTo', function(ev) {
+        assert.equal('/service/mysql/', ev.url);
         called_event = ev;
       });
       var msg = conn.last_message();