Merge lp:~hatch/juju-gui/bundle-tabs-1240973 into lp:juju-gui/experimental

[Jeff Pihach]

Bundle tabview now reacts to the url hash

https://codereview.appspot.com/14702045/

[Jeff Pihach (hatch)]

Reviewers: mp+191680_code.launchpad.net,

Message:
Please take a look.

Description:
Bundle tabview now reacts to the url hash

https://code.launchpad.net/~hatch/juju-gui/bundle-tabs-1240973/+merge/191680

(do not edit description out of merge proposal)

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

Affected files (+51, -20 lines):
   A [revision details]
   M app/subapps/browser/views/bundle.js
   M app/subapps/browser/views/charm.js
   M app/subapps/browser/views/entity-base.js
   M test/test_bundle_details_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_bundle_details_view.js
=== modified file 'test/test_bundle_details_view.js'
--- test/test_bundle_details_view.js 2013-10-17 14:11:43 +0000
+++ test/test_bundle_details_view.js 2013-10-17 16:55:19 +0000
@@ -51,24 +51,31 @@
    });

    beforeEach(function() {
+ view = generateBundleView();
+ view._setupLocalFakebackend = function() {
+ this.fakebackend = utils.makeFakeBackend();
+ };
+ });
+
+ afterEach(function() {
+ container.remove().destroy(true);
+ view.destroy();
+ });
+
+ function generateBundleView(options) {
      data = Y.clone(origData);
      container = utils.makeContainer();
      container.append('<div class="bws-view-data"></div>');
- view = new Y.juju.browser.views.BrowserBundleView({
+ var defaults = {
        store: modifyFakeStore(),
        db: {},
        entityId: data.id,
        renderTo: container
- });
- view._setupLocalFakebackend = function() {
- this.fakebackend = utils.makeFakeBackend();
      };
- });
-
- afterEach(function() {
- container.remove().destroy(true);
- view.destroy();
- });
+ var bundleView = Y.mix(defaults, options, true);
+ view = new Y.juju.browser.views.BrowserBundleView(bundleView);
+ return view;
+ }

    function modifyFakeStore(options) {
      var defaults = {
@@ -297,6 +304,19 @@
      view.render();
    });

-
+ it('selects the proper tab when given one', function(done) {
+ view = generateBundleView({
+ activeTab: '#bws-charms'
+ });
+ view._parseData = function() {
+ return new Y.Promise(function(resolve) { resolve(); });
+ };
+ view.render();
+ view.after('renderedChange', function(e) {
+ var selected = view.get('container').one('.yui3-tab-selected a');
+ assert.equal(selected.getAttribute('href'), '#bws-charms');
+ done();
+ });
+ });

  });

Index: app/subapps/browser/views/bundle.js
=== modified file 'app/subapps/browser/views/bundle.js'
--- app/subapps/browser/views/bundle.js 2013-10-17 14:11:43 +0000
+++ app/subapps/browser/views/bundle.js 2013-10-17 16:55:19 +0000
@@ -204,6 +204,7 @@
          this.tabview.selectChild(2);
        }
        this._dispatchTabEvents(this.tabview);
+ this._showActiveTab();
        this._renderCharmListing();

        this.set('rendered', true);

Index: app/subapps/brow...

[Jeff Pihach (hatch)]

Review notes

https://codereview.appspot.com/14702045/diff/1/app/subapps/browser/views/charm.js
File app/subapps/browser/views/charm.js (left):

https://codereview.appspot.com/14702045/diff/1/app/subapps/browser/views/charm.js#oldcode364
app/subapps/browser/views/charm.js:364: if (this.get('activeTab')) {
Moved to entity-base.js

https://codereview.appspot.com/14702045/

[Madison Scott-Clary (makyo)]

LGTM, QA okay

https://codereview.appspot.com/14702045/

[Jeff Pihach (hatch)]

Thanks for the review!

https://codereview.appspot.com/14702045/

[Jeff Pihach (hatch)]

*** Submitted:

Bundle tabview now reacts to the url hash

R=matthew.scott
CC=
https://codereview.appspot.com/14702045

https://codereview.appspot.com/14702045/

=== modified file 'app/subapps/browser/views/bundle.js'
--- app/subapps/browser/views/bundle.js	2013-10-17 14:11:43 +0000
+++ app/subapps/browser/views/bundle.js	2013-10-17 17:02:04 +0000
@@ -204,6 +204,7 @@
         this.tabview.selectChild(2);
       }
       this._dispatchTabEvents(this.tabview);
+      this._showActiveTab();
       this._renderCharmListing();
 
       this.set('rendered', true);

=== modified file 'app/subapps/browser/views/charm.js'
--- app/subapps/browser/views/charm.js	2013-10-11 17:51:10 +0000
+++ app/subapps/browser/views/charm.js	2013-10-17 17:02:04 +0000
@@ -349,7 +349,7 @@
 
       this._setupTabview();
       this._dispatchTabEvents(this.tabview);
-
+      this._showActiveTab();
 
       if (isFullscreen) {
         if (!this.get('entity').get('relatedCharms')) {
@@ -361,14 +361,6 @@
         }
       }
 
-      if (this.get('activeTab')) {
-        var tab = this.get('container').one(
-            '.tabs a[href="' + this.get('activeTab') + '"]');
-        if (tab) {
-          tab.get('parentNode').simulate('click');
-        }
-      }
-
       // XXX: Ideally we shouldn't have to do this; resetting the container
       // with .empty or something before rendering the charm view should work.
       // But it doesn't so we scroll the nav bar into view, load the charm

=== modified file 'app/subapps/browser/views/entity-base.js'
--- app/subapps/browser/views/entity-base.js	2013-10-09 23:37:12 +0000
+++ app/subapps/browser/views/entity-base.js	2013-10-17 17:02:04 +0000
@@ -435,6 +435,22 @@
         render: true,
         srcNode: this.get('container').one('.tabs')
       });
+    },
+
+    /**
+      Shows the active tabview tab set via the browser subapp.
+
+      @method _showActiveTab
+    */
+    _showActiveTab: function() {
+      var activeTab = this.get('activeTab');
+      if (activeTab) {
+        var tab = this.get('container')
+                      .one('.tabs a[href="' + activeTab + '"]');
+        if (tab) {
+          tab.get('parentNode').simulate('click');
+        }
+      }
     }
   };
 

=== modified file 'test/test_bundle_details_view.js'
--- test/test_bundle_details_view.js	2013-10-17 14:11:43 +0000
+++ test/test_bundle_details_view.js	2013-10-17 17:02:04 +0000
@@ -51,24 +51,31 @@
   });
 
   beforeEach(function() {
+    view = generateBundleView();
+    view._setupLocalFakebackend = function() {
+      this.fakebackend = utils.makeFakeBackend();
+    };
+  });
+
+  afterEach(function() {
+    container.remove().destroy(true);
+    view.destroy();
+  });
+
+  function generateBundleView(options) {
     data = Y.clone(origData);
     container = utils.makeContainer();
     container.append('<div class="bws-view-data"></div>');
-    view = new Y.juju.browser.views.BrowserBundleView({
+    var defaults = {
       store: modifyFakeStore(),
       db: {},
       entityId: data.id,
       renderTo: container
-    });
-    view._setupLocalFakebackend = function() {
-      this.fakebackend = utils.makeFakeBackend();
     };
-  });
-
-  afterEach(function() {
-    container.remove().destroy(true);
-    view.destroy();
-  });
+    var bundleView = Y.mix(defaults, options, true);
+    view = new Y.juju.browser.views.BrowserBundleView(bundleView);
+    return view;
+  }
 
   function modifyFakeStore(options) {
     var defaults = {
@@ -297,6 +304,19 @@
     view.render();
   });
 
-
+  it('selects the proper tab when given one', function(done) {
+    view = generateBundleView({
+      activeTab: '#bws-charms'
+    });
+    view._parseData = function() {
+      return new Y.Promise(function(resolve) { resolve(); });
+    };
+    view.render();
+    view.after('renderedChange', function(e) {
+      var selected = view.get('container').one('.yui3-tab-selected a');
+      assert.equal(selected.getAttribute('href'), '#bws-charms');
+      done();
+    });
+  });
 
 });