Merge lp:~rharding/juju-gui/charm-details-1172050 into lp:juju-gui/experimental

[Richard Harding]

Fixes #1172050 correct browser state tracking.

- The browser viewstate wasn't tracking the query string correctly.
- The searchresults view didn't track charm-token clicks and navigate so that
you always ended up to the link sidebar/charmid.
- Drive by fix the editorial as it shouldn't have had to specify the viewmode.
- Update the routing to handle search/charm/id urls better and not double
route through the state.

[lp:~rharding/juju-gui/charm-details-1172050]

605. By Richard Harding on 2013-04-24

Add failing test to start the day

606. By Richard Harding on 2013-04-24

Add test that query string is in the state and checked

607. By Richard Harding on 2013-04-24

Fix the tests for the cases around search dispatch

=== modified file 'app/subapps/browser/browser.js'
--- app/subapps/browser/browser.js	2013-04-23 16:20:42 +0000
+++ app/subapps/browser/browser.js	2013-04-24 12:51:26 +0000
@@ -92,9 +92,10 @@
      */
     _initState: function() {
       this._oldState = {
-        viewmode: null,
+        charmID: null,
+        querystring: null,
         search: null,
-        charmID: null
+        viewmode: null
       };
       this._viewState = Y.merge(this._oldState, {});
     },
@@ -151,7 +152,8 @@
           (
            this._hasStateChanged('search') ||
            this._hasStateChanged('viewmode') ||
-           this._hasStateChanged('querystring')
+           this._hasStateChanged('querystring') ||
+           (this._hasStateChanged('charmID') && !this._viewState.charmID)
           )
       ) {
         return true;
@@ -197,13 +199,15 @@
        @param {Object} params the params from the request payload.
 
      */
-    _updateState: function(path, params) {
+    _updateState: function(path, params, query) {
       // Update the viewmode. Every request has a viewmode.
       this._viewState.viewmode = params.viewmode;
 
       // Check for a charm id in the request.
       if (params.id && params.id !== 'search') {
-        this._viewState.charmID = params.id;
+        // Make sure we clear out any accidental matching of search/ in the
+        // url.
+        this._viewState.charmID = params.id.replace(/^search\//, '');
       } else {
         this._viewState.charmID = null;
       }
@@ -214,6 +218,14 @@
       } else {
         this._viewState.search = false;
       }
+
+      // Check if there's a query string to set.
+      if (query) {
+        // Store it as a straight string.
+        this._viewState.querystring = Y.QueryString.stringify(query);
+      } else {
+        this._viewState.querystring = null;
+      }
     },
 
     /**
@@ -278,7 +290,7 @@
      *
      */
     renderCharmDetails: function(req, res, next) {
-      var charmID = req.params.id;
+      var charmID = this._viewState.charmID;
       var extraCfg = {
         charmID: charmID,
         container: Y.Node.create('<div class="charmview"/>'),
@@ -402,6 +414,20 @@
         this._fullscreen = this.showView('fullscreen', this._getViewCfg());
       }
 
+      // Regardless of the results below we need to clear out the old
+      // subviews.
+      if (this._editorial) {
+        this._editorial.destroy();
+      }
+
+      if (this._details) {
+        this._details.destroy();
+      }
+
+      if (this._search) {
+        this._search.destroy();
+      }
+
       // If we've changed the charmID or the viewmode has changed and we have
       // a charmID, render charmDetails.
       if (this._shouldShowCharm()) {
@@ -411,7 +437,7 @@
         // Render search results if search is in the url and the viewmode or
         // the search has been changed in the state.
         this.renderSearchResults(req, res, next);
-      } else if (!this._viewState.charmID) {
+      } else if (!this._viewState.search && !this._viewState.charmID) {
         // Render the editorial in fullscreen only if we don't have a charmid
         this.renderEditorial(req, res, next);
       }
@@ -440,10 +466,20 @@
       // Render search results if search is in the url and the viewmode or the
       // search has been changed in the state.
       if (this._shouldShowSearch()) {
+        // Showing search implies that other sidebar content is destroyed.
+        if (this._editorial) {
+          this._editorial.destroy();
+        }
+
         this.renderSearchResults(req, res, next);
       }
 
       if (this._shouldShowEditorial()) {
+        // Showing editorial implies that other sidebar content is destroyed.
+        if (this._search) {
+          this._search.destroy();
+        }
+
         this.renderEditorial(req, res, next);
       }
 
@@ -454,7 +490,7 @@
         this.renderCharmDetails(req, res, next);
       }
 
-      // If the sidebar is the final part of the route, then hide the div for
+      // If no details in the route then hide the div for
       // viewing the charm details.
       if (!this._viewState.charmID) {
         this._detailsVisible(false);
@@ -484,7 +520,7 @@
      */
     routeView: function(req, res, next) {
       // Update the state for the rest of things to figure out what to do.
-      this._updateState(req.path, req.params);
+      this._updateState(req.path, req.params, req.query);
       this[req.params.viewmode](req, res, next);
     }
 
@@ -574,6 +610,7 @@
     'juju-charm-store',
     'juju-models',
     'querystring-parse',
+    'querystring-stringify',
     'sub-app',
     'subapp-browser-charmview',
     'subapp-browser-editorial',

=== modified file 'app/subapps/browser/views/editorial.js'
--- app/subapps/browser/views/editorial.js	2013-04-23 18:52:28 +0000
+++ app/subapps/browser/views/editorial.js	2013-04-24 12:51:26 +0000
@@ -60,17 +60,14 @@
       var charmID = charm.getData('charmid');
 
       // Update the UI for the active one.
-      this._updateActive(ev.currentTarget);
+      if (!this.get('isFullscreen')) {
+        this._updateActive(ev.currentTarget);
+      }
+
       var change = {
         charmID: charmID
       };
 
-      if (this.get('isFullscreen')) {
-        change.viewmode = 'fullscreen';
-      } else {
-        change.viewmode = 'sidebar';
-      }
-
       this.fire('viewNavigate', {change: change});
     },
 

=== modified file 'app/subapps/browser/views/search.js'
--- app/subapps/browser/views/search.js	2013-04-23 13:31:29 +0000
+++ app/subapps/browser/views/search.js	2013-04-24 12:51:26 +0000
@@ -17,7 +17,52 @@
   ns.BrowserSearchView = Y.Base.create('browser-view-searchview', Y.View, [
     views.utils.apiFailingView
   ], {
+    events: {
+      '.charm-token': {
+        click: '_handleCharmSelection'
+      }
+    },
+
     template: views.Templates.search,
+
+    /**
+        When selecting a charm from the list make sure we re-route the app to
+        the details view with that charm selected.
+
+        @method _handleCharmSelection
+        @param {Event} ev the click event handler for the charm selected.
+
+     */
+    _handleCharmSelection: function(ev) {
+      ev.halt();
+      var charm = ev.currentTarget;
+      var charmID = charm.getData('charmid');
+
+      // Update the UI for the active one.
+      if (!this.get('isFullscreen')) {
+        this._updateActive(ev.currentTarget);
+      }
+      var change = {
+        charmID: charmID
+      };
+      this.fire('viewNavigate', {change: change});
+    },
+
+    /**
+      Update the node in the editorial list marked as 'active'.
+
+      @method _updateActive
+      @param {Node} clickTarget the charm-token clicked on to activate.
+
+    */
+    _updateActive: function(clickTarget) {
+      // Remove the active class from any nodes that have it.
+      Y.all('.yui3-charmtoken.active').removeClass('active');
+
+      // Add it to the current node.
+      clickTarget.ancestor('.yui3-charmtoken').addClass('active');
+    },
+
     /**
      * Renders the search results from the the store query.
      *
@@ -30,6 +75,9 @@
           tplNode = Y.Node.create(tpl),
           container = tplNode.one('.search-results');
 
+      // Set the container so that our events will delegate based off of it.
+      this.set('container', container);
+
       results.map(function(charm) {
         var ct = new widgets.browser.CharmToken(charm.getAttrs());
         ct.render(container);

=== modified file 'test/test_browser_app.js'
--- test/test_browser_app.js	2013-04-23 19:36:04 +0000
+++ test/test_browser_app.js	2013-04-24 12:51:26 +0000
@@ -468,6 +468,66 @@
       assert.deepEqual(hits, expected);
     });
 
+    it('changing the query string dispatches correctly', function() {
+      var req = {
+        path: '/bws/fullscreen/search/',
+        params: {
+          viewmode: 'fullscreen'
+        },
+        query: {
+          text: 'test'
+        }
+      };
+      browser.routeView(req, undefined, function() {});
+
+      // Reset the hits and we should not redraw anything to update the view.
+      resetHits();
+      req.query.text = 'test2';
+
+      var expected = Y.merge(hits, {
+        renderSearchResults: true
+      });
+      browser.routeView(req, undefined, function() {});
+      assert.deepEqual(hits, expected);
+    });
+
+    it('no change to query string does not redraw', function() {
+      var req = {
+        path: '/bws/fullscreen/search/',
+        params: {
+          viewmode: 'fullscreen'
+        },
+        query: {
+          text: 'test'
+        }
+      };
+      browser.routeView(req, undefined, function() {});
+
+      // Reset the hits and we should not redraw anything to update the view.
+      resetHits();
+
+      var expected = Y.merge(hits);
+      browser.routeView(req, undefined, function() {});
+      assert.deepEqual(hits, expected);
+    });
+
+    it('no querystring still searched', function() {
+      var req = {
+        path: '/bws/fullscreen/search/',
+        params: {
+          viewmode: 'fullscreen'
+        }
+      };
+
+      var expected = Y.merge(hits, {
+        fullscreen: true,
+        renderSearchResults: true
+      });
+
+      browser.routeView(req, undefined, function() {});
+      assert.deepEqual(hits, expected);
+    });
+
   });
 })();
 

=== modified file 'test/test_browser_editorial.js'
--- test/test_browser_editorial.js	2013-04-23 19:36:04 +0000
+++ test/test_browser_editorial.js	2013-04-24 12:51:26 +0000
@@ -106,7 +106,6 @@
 
       view.on('viewNavigate', function(ev) {
         ev.halt();
-        assert(ev.change.viewmode === 'fullscreen');
         assert(ev.change.charmID === 'precise/ceph-7');
         done();
       });
@@ -134,7 +133,6 @@
 
       view.on('viewNavigate', function(ev) {
         ev.halt();
-        assert(ev.change.viewmode === 'sidebar');
         assert(ev.change.charmID === 'precise/ceph-7');
         done();
       });

=== modified file 'test/test_browser_search_view.js'
--- test/test_browser_search_view.js	2013-04-22 21:04:27 +0000
+++ test/test_browser_search_view.js	2013-04-24 12:51:26 +0000
@@ -12,6 +12,7 @@
         'json',
         'juju-charm-store',
         'node',
+        'node-event-simulate',
         'subapp-browser-searchview',
         function(Y) {
           done();
@@ -75,4 +76,27 @@
     view.render();
     assert.equal('charms?text=', apiURL);
   });
+
+  it('clicking a charm navigates for fullscreen', function(done) {
+    view.render();
+    view.on('viewNavigate', function(ev) {
+      ev.halt();
+      assert(ev.change.charmID === 'foo/bar-2');
+      done();
+    });
+
+    container.one('.charm-token').simulate('click');
+  });
+
+  it('clicking a charm navigates for sidebar', function(done) {
+    view.render();
+    view.on('viewNavigate', function(ev) {
+      ev.halt();
+      assert(ev.change.charmID === 'foo/bar-2');
+      done();
+    });
+
+    container.one('.charm-token').simulate('click');
+  });
+
 });