Merge lp:~rharding/juju-gui/new-controls into lp:juju-gui/experimental

[Richard Harding]

Break up the search widget to work with updated UX.

- Split up the viewmode controls from the search widget itself.
- Update the viewmode controls widget to handle distinct fullscreen/sidebar events vs a toggle event
- Update the tests to work with both widgets properly.

This branch does not update the minimized search to work correctly in the grand scheme of things yet.

Break up the search widget to work with updated UX

- Split up the viewmode controls from the search widget itself.
- Update the viewmode controls widget to handle distinct fullscreen/sidebar events vs a toggle event
- Update the tests to work with both widgets properly.

This branch does not update the minimized search to work correctly in the grand scheme of things yet.

https://codereview.appspot.com/10877043/

[Richard Harding (rharding)]

Reviewers: mp+172631_code.launchpad.net,

Message:
Please take a look.

Description:
Break up the search widget to work with updated UX

- Split up the viewmode controls from the search widget itself.
- Update the viewmode controls widget to handle distinct
fullscreen/sidebar events vs a toggle event
- Update the tests to work with both widgets properly.

This branch does not update the minimized search to work correctly in
the grand scheme of things yet.

https://code.launchpad.net/~rharding/juju-gui/new-controls/+merge/172631

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/index.html
   M app/modules-debug.js
   M app/subapps/browser/templates/browser_charm.handlebars
   M app/subapps/browser/views/view.js
   M app/templates/browser-search.handlebars
   M app/templates/charm-token.handlebars
   M app/views/topology/importexport.js
   M app/views/topology/service.js
   M app/widgets/charm-search.js
   M app/widgets/charm-token.js
   A app/widgets/viewmode-controls.js
   M lib/views/stylesheet.less
   M test/data/browsercharm.json
   M test/data/related.json
   M test/index.html
   M test/test_browser_app.js
   M test/test_browser_charm_details.js
   M test/test_browser_search_widget.js
   M test/test_charm_token.js
   M test/test_charm_token_drag_and_drop.js
   M test/test_model.js
   A test/test_viewmode_controls_widget.js

[lp:~rharding/juju-gui/new-controls]

790. By Richard Harding on 2013-07-02

update per review

791. By Richard Harding on 2013-07-02

More @event docs

[Curtis Hovey (sinzui)]

Thank you Rick. These changes LGTM with the additional changes that I
can see are already added at Lp.

I see the discussion about widget vs view. I think widget is fine at
this time. This is clearly more than a view. The widget might be too
heavy for this use, but that may change in the next few weeks. That is
to say, We are making this late change to accommodate a late design.
Given the urgency to get the UI looking good, I think it is wiser to
review what we don't need at the end of July.

https://codereview.appspot.com/10877043/

[Richard Harding (rharding)]

Curtis and Jeff approved. Will wait to land as part of Huw's work since this branch depends on his.

review: Approve

[lp:~rharding/juju-gui/new-controls]

792. By Richard Harding on 2013-07-03

Update from trunk

793. By Richard Harding on 2013-07-03

remove block of css no longer needed

[Richard Harding (rharding)]

*** Submitted:

Break up the search widget to work with updated UX

- Split up the viewmode controls from the search widget itself.
- Update the viewmode controls widget to handle distinct
fullscreen/sidebar events vs a toggle event
- Update the tests to work with both widgets properly.

This branch does not update the minimized search to work correctly in
the grand scheme of things yet.

R=curtis
CC=
https://codereview.appspot.com/10877043

https://codereview.appspot.com/10877043/

=== modified file 'app/index.html'
--- app/index.html	2013-07-02 05:47:25 +0000
+++ app/index.html	2013-07-03 12:47:32 +0000
@@ -126,10 +126,12 @@
                   <i class="sprite juju_logo_dark" title="Juju GUI"></i>
                 </a>
               </div>
-              <ul id="browser-nav">
-                <li><a href="">Browse</a></li>
-                <li><a href="">Build</a></li>
-              </ul>
+              <div id="browser-nav">
+                <ul>
+                  <li><a class="fullscreen" href="">Browse</a></li>
+                   <li><a class="sidebar" href="">Build</a></li>
+                </ul>
+              </div>
               <div id="environment-switcher">
                 <i class="sprite environment_icon"> </i>
                 <span id="environment-name" draggable="true"></span>

=== modified file 'app/modules-debug.js'
--- app/modules-debug.js	2013-06-24 22:53:39 +0000
+++ app/modules-debug.js	2013-07-03 12:47:32 +0000
@@ -153,6 +153,10 @@
           fullpath: '/juju-ui/widgets/charm-search.js'
         },
 
+        'viewmode-controls': {
+          fullpath: '/juju-ui/widgets/viewmode-controls.js'
+        },
+
         'browser-tabview': {
           fullpath: '/juju-ui/widgets/browser-tabview.js'
         },

=== modified file 'app/subapps/browser/views/view.js'
--- app/subapps/browser/views/view.js	2013-06-10 15:04:39 +0000
+++ app/subapps/browser/views/view.js	2013-07-03 12:47:32 +0000
@@ -85,13 +85,17 @@
      */
     _bindSearchWidgetEvents: function() {
       this.addEvent(
-          this.search.on(
-              this.search.EVT_TOGGLE_VIEWABLE, this._toggleBrowser, this)
+          this.controls.on(
+              this.controls.EVT_TOGGLE_VIEWABLE, this._toggleBrowser, this)
       );
 
       this.addEvent(
-          this.search.on(
-              this.search.EVT_TOGGLE_FULLSCREEN, this._toggleFullscreen, this)
+          this.controls.on(
+              this.controls.EVT_FULLSCREEN, this._goFullscreen, this)
+      );
+      this.addEvent(
+          this.controls.on(
+              this.controls.EVT_SIDEBAR, this._goSidebar, this)
       );
 
       this.addEvent(
@@ -114,6 +118,9 @@
         fullscreenTarget: this._fullscreenTarget
       });
       this.search.render(node.one('.bws-header'));
+
+      this.controls = new widgets.ViewmodeControls();
+      this.controls.render();
     },
 
     /**
@@ -153,20 +160,36 @@
     },
 
     /**
-      Upon clicking the fullscreen toggle icon make sure we re-route to the
-      new form of the UX.
-
-      @method _toggleFullscreen
-      @param {Event} ev the click event handler on the button.
-
-     */
-    _toggleFullscreen: function(ev) {
-      ev.halt();
-      var change = {
-        viewmode: this.isFullscreen() ? 'sidebar' : 'fullscreen'
-      };
-      this.fire('viewNavigate', {
-        change: change
+      Upon clicking the browser icon make sure we re-route to the
+      new form of the UX.
+
+      @method _goFullscreen
+      @param {Event} ev the click event handler on the button.
+
+     */
+    _goFullscreen: function(ev) {
+      ev.halt();
+      this.fire('viewNavigate', {
+        change: {
+          viewmode: 'fullscreen'
+        }
+      });
+    },
+
+    /**
+      Upon clicking the build icon make sure we re-route to the
+      new form of the UX.
+
+      @method _goSidebar
+      @param {Event} ev the click event handler on the button.
+
+     */
+    _goSidebar: function(ev) {
+      ev.halt();
+      this.fire('viewNavigate', {
+        change: {
+          viewmode: 'sidebar'
+        }
       });
     },
 
@@ -263,6 +286,7 @@
     'juju-charm-store',
     'juju-models',
     'querystring-stringify',
-    'view'
+    'view',
+    'viewmode-controls'
   ]
 });

=== modified file 'app/templates/browser-search.handlebars'
--- app/templates/browser-search.handlebars	2013-07-02 05:47:25 +0000
+++ app/templates/browser-search.handlebars	2013-07-03 12:47:32 +0000
@@ -1,3 +1,2 @@
 <a href="" class="bws-icon"></a>
-<a href="" class="toggle-fullscreen"></a>
 {{> bws-searchbox }}

=== modified file 'app/widgets/charm-search.js'
--- app/widgets/charm-search.js	2013-07-02 05:47:25 +0000
+++ app/widgets/charm-search.js	2013-07-03 12:47:32 +0000
@@ -38,14 +38,15 @@
    *
    * @class Search
    * @extends {Y.Widget}
+   * @event EV_CLEAR_SEARCH the widget requests all search reset.
+   * @event EV_SEARCH_CHANGED the widgets notifies that the search input has
+   * changed.
    *
    */
   ns.Search = Y.Base.create('search-widget', Y.Widget, [
     Y.Event.EventTracker
   ], {
     EVT_CLEAR_SEARCH: 'clear_search',
-    EVT_TOGGLE_VIEWABLE: 'toggle_viewable',
-    EVT_TOGGLE_FULLSCREEN: 'toggle_fullscreen',
     EVT_SEARCH_CHANGED: 'search_changed',
 
     TEMPLATE: templates['browser-search'],
@@ -68,33 +69,6 @@
     },
 
     /**
-     * Expose to the outside world that we've got a request to go fullscreen.
-     *
-     * @method _toggleFullScreen
-     * @param {Event} ev the click event from the control.
-     * @private
-     *
-     */
-    _toggleFullScreen: function(ev) {
-      ev.halt();
-      this.fire(this.EVT_TOGGLE_FULLSCREEN);
-    },
-
-    /**
-     * Expose to the outside world that we've got a request to hide from
-     * sight.
-     *
-     * @method _toggleViewable
-     * @param {Event} ev the click event from the control.
-     * @private
-     *
-     */
-    _toggleViewable: function(ev) {
-      ev.halt();
-      this.fire(this.EVT_TOGGLE_VIEWABLE);
-    },
-
-    /**
      * Set the form to active so that we can change the search appearance.
      *
      * @method _setActive
@@ -134,14 +108,6 @@
       var container = this.get('boundingBox');
 
       this.addEvent(
-          container.one('.bws-icon').on(
-              'click', this._toggleViewable, this)
-      );
-      this.addEvent(
-          container.one('.toggle-fullscreen').on(
-              'click', this._toggleFullScreen, this)
-      );
-      this.addEvent(
           container.one('form').on(
               'submit', this._handleSubmit, this)
       );
@@ -170,8 +136,6 @@
        * widget only needs to clear the search input box.
        *
        */
-      this.publish(this.EVT_TOGGLE_VIEWABLE);
-      this.publish(this.EVT_TOGGLE_FULLSCREEN);
       this.publish(this.EVT_SEARCH_CHANGED);
     },
 
@@ -220,16 +184,6 @@
         value: {
           text: ''
         }
-      },
-      /**
-         @attribute fullscreeTarget
-         @default undefined
-         @type {Node}
-         @required true
-
-       */
-      fullscreenTarget: {
-        required: true
       }
     }
   });

=== added file 'app/widgets/viewmode-controls.js'
--- app/widgets/viewmode-controls.js	1970-01-01 00:00:00 +0000
+++ app/widgets/viewmode-controls.js	2013-07-03 12:47:32 +0000
@@ -0,0 +1,170 @@
+/*
+This file is part of the Juju GUI, which lets users view and manage Juju
+environments within a graphical interface (https://launchpad.net/juju-gui).
+Copyright (C) 2012-2013 Canonical Ltd.
+
+This program is free software: you can redistribute it and/or modify it under
+the terms of the GNU Affero General Public License version 3, as published by
+the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful, but WITHOUT
+ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Affero
+General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License along
+with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+'use strict';
+
+
+/**
+ * The widget used across Browser view to manage the search box and the
+ * controls for selecting which view you're in.
+ *
+ * @module widgets
+ * @submodule browser
+ *
+ */
+YUI.add('viewmode-controls', function(Y) {
+  var ns = Y.namespace('juju.widgets');
+
+
+  /**
+   * Search widget present in the Charm browser across both fullscreen and
+   * sidebar views.
+   *
+   * @class ViewmodeControls
+   * @extends {Y.Widget}
+   * @event EV_TOGGLE_VIEWABLE toggle if the browser is visible.
+   * @event EV_FULLSCREEN force fullscreen viewmode.
+   * @event EV_SIDEBAR force the sidebar viewmode.
+   *
+   */
+  ns.ViewmodeControls = Y.Base.create('viewmode-controls', Y.Widget, [
+    Y.Event.EventTracker
+  ], {
+    CONTENT_TEMPLATE: null,
+    EVT_TOGGLE_VIEWABLE: 'toggle_viewable',
+    EVT_FULLSCREEN: 'fullscreen',
+    EVT_SIDEBAR: 'sidebar',
+
+    /**
+     * Expose to the outside world that we've got a request to go fullscreen.
+     *
+     * @method _goFullscreen
+     * @param {Event} ev the click event from the control.
+     * @private
+     *
+     */
+    _goFullscreen: function(ev) {
+      ev.halt();
+      this.fire(this.EVT_FULLSCREEN);
+    },
+
+    /**
+     * Expose to the outside world that we've got a request to go sidebar.
+     *
+     * @method _goSidebar
+     * @param {Event} ev the click event from the control.
+     * @private
+     *
+     */
+    _goSidebar: function(ev) {
+      ev.halt();
+      this.fire(this.EVT_SIDEBAR);
+    },
+
+    /**
+     * Expose to the outside world that we've got a request to hide from
+     * sight.
+     *
+     * @method _toggleViewable
+     * @param {Event} ev the click event from the control.
+     * @private
+     *
+     */
+    _toggleViewable: function(ev) {
+      ev.halt();
+      this.fire(this.EVT_TOGGLE_VIEWABLE);
+    },
+
+    /**
+     * bind the UI events to the DOM making up the widget control.
+     *
+     * @method bindUI
+     *
+     */
+    bindUI: function() {
+      var container = this.get('boundingBox');
+
+      this.addEvent(
+          Y.one('#content').delegate(
+              'click',
+              this._toggleViewable,
+              '.bws-icon',
+              this
+          )
+      );
+      this.addEvent(
+          container.one('.fullscreen').on(
+              'click', this._goFullscreen, this)
+      );
+      this.addEvent(
+          container.one('.sidebar').on(
+              'click', this._goSidebar, this)
+      );
+
+    },
+
+    /**
+     * Generic initializer for the widget. Publish events we expose for
+     * outside use.
+     *
+     * @method initializer
+     * @param {Object} cfg configuration override object.
+     *
+     */
+    initializer: function(cfg) {
+      /*
+       * Fires when the "Charm Browser" link is checked. Needs to communicate
+       * with the parent view so that it can handle filters and the like. This
+       * widget only needs to clear the search input box.
+       *
+       */
+      this.publish(this.EVT_TOGGLE_VIEWABLE);
+      this.publish(this.EVT_SIDEBAR);
+      this.publish(this.EVT_FULLSCREEN);
+    }
+
+  }, {
+    ATTRS: {
+      /**
+       * @attribute boundingBox
+       * @default {Node} the .browser-nav ul
+       * @type {Node}
+       *
+       */
+      boundingBox: {
+        /**
+         * @method boundingBox.valueFn
+         * @return {Node} The already present browser-nav node.
+         *
+         */
+        valueFn: function() {
+          return Y.one('#browser-nav');
+        }
+      }
+
+    }
+  });
+
+}, '0.1.0', {
+  requires: [
+    'base',
+    'event',
+    'event-tracker',
+    'widget'
+  ]
+});

=== modified file 'lib/views/browser/main.less'
--- lib/views/browser/main.less	2013-07-02 05:47:25 +0000
+++ lib/views/browser/main.less	2013-07-03 12:47:32 +0000
@@ -41,21 +41,6 @@
   .bws-header {
       padding: 20px;
       border-bottom: 1px solid @bws-border;
-
-    //XXX: this can be removed once the control has been moved to the header.
-    .toggle-fullscreen {
-        position: absolute;
-        display: block;
-        right: -40px;
-        top: 20px;
-        width: 40px;
-        height: 40px;
-        background: @bws-dark url(/juju-ui/assets/images/browser_min_max.png)
-        no-repeat 0 0;
-        box-shadow: 3px 0 2px rgba(0, 0, 0, 0.1);
-        border-top-right-radius: 4px;
-        border-bottom-right-radius: 4px;
-    }
   }
   #bws-search {
     .search-title {

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2013-07-03 12:38:35 +0000
+++ lib/views/stylesheet.less	2013-07-03 12:47:32 +0000
@@ -255,6 +255,26 @@
                 }
             }
         }
+        #browser-nav {
+            float: left;
+
+            ul {
+              margin: 0 20px 0 0;
+              padding: 0;
+              list-style: none;
+            }
+
+            li {
+                float: left;
+
+                a {
+                    display: block;
+                    padding: 25px 10px 0 10px;
+                    font-size: 12pt;
+                    color: #9b9b9b;
+                }
+            }
+        }
         #environment-switcher {
             float: left;
             height: 47px;

=== modified file 'test/index.html'
--- test/index.html	2013-06-28 13:54:34 +0000
+++ test/index.html	2013-07-03 12:47:32 +0000
@@ -131,6 +131,7 @@
 
   <script src="test_utils.js"></script>
   <script src="test_view_container.js"></script>
+  <script src="test_viewmode_controls_widget.js"></script>
   <script src="test_viewport_module.js"></script>
   <script src="test_websocket_logging.js"></script>
   <script>

=== modified file 'test/test_browser_app.js'
--- test/test_browser_app.js	2013-06-27 17:40:21 +0000
+++ test/test_browser_app.js	2013-07-03 12:47:32 +0000
@@ -47,8 +47,15 @@
     });
 
     var addBrowserContainer = function(Y) {
-      Y.Node.create('<div id="subapp-browser">' +
-                    '</div>').appendTo(container);
+      Y.Node.create([
+        '<div id="content">',
+        '<div id="browser-nav">',
+        '<div class="sidebar"></div>',
+        '<div class="fullscreen"</div>',
+        '</div>',
+        '<div id="subapp-browser"></div>',
+        '</div>'
+      ].join('')).appendTo(container);
     };
 
     afterEach(function() {
@@ -128,11 +135,17 @@
     });
 
     var addBrowserContainer = function(Y) {
-      Y.Node.create('<div id="subapp-browser">' +
-          '</div>').appendTo(container);
+      Y.Node.create([
+        '<div id="content">',
+        '<div id="browser-nav">',
+        '<div class="sidebar"></div>',
+        '<div class="fullscreen"</div>',
+        '</div>',
+        '<div id="subapp-browser"></div>',
+        '</div>'
+      ].join('')).appendTo(container);
     };
 
-
     it('knows that it is not fullscreen', function() {
       view = new Sidebar();
       view.isFullscreen().should.equal(false);

=== modified file 'test/test_browser_search_widget.js'
--- test/test_browser_search_widget.js	2013-07-02 05:47:25 +0000
+++ test/test_browser_search_widget.js	2013-07-03 12:47:32 +0000
@@ -24,7 +24,7 @@
 
   before(function(done) {
     Y = YUI(GlobalConfig).use(['browser-search-widget',
-      'juju-tests-utils',
+                               'juju-tests-utils',
                                'event-simulate',
                                'node-event-simulate',
                                'node'], function(Y) {
@@ -56,31 +56,4 @@
     container.one('input').get('value').should.eql('test');
   });
 
-  it('should fire a toggle fullscreen event when expand clicked', function() {
-    var search = new Search(),
-        triggered = false;
-    search.render(container);
-
-    search.on(search.EVT_TOGGLE_FULLSCREEN, function(ev) {
-      triggered = true;
-    });
-
-    var toggle = container.one('.toggle-fullscreen');
-    toggle.simulate('click');
-    triggered.should.eql(true);
-  });
-
-  it('should fire a toggle viewable event when icon clicked', function() {
-    var search = new Search(),
-        triggered = false;
-    search.render(container);
-
-    search.on(search.EVT_TOGGLE_VIEWABLE, function(ev) {
-      triggered = true;
-    });
-
-    var toggle = container.one('.bws-icon');
-    toggle.simulate('click');
-    triggered.should.eql(true);
-  });
 });

=== added file 'test/test_viewmode_controls_widget.js'
--- test/test_viewmode_controls_widget.js	1970-01-01 00:00:00 +0000
+++ test/test_viewmode_controls_widget.js	2013-07-03 12:47:32 +0000
@@ -0,0 +1,99 @@
+/*
+This file is part of the Juju GUI, which lets users view and manage Juju
+environments within a graphical interface (https://launchpad.net/juju-gui).
+Copyright (C) 2012-2013 Canonical Ltd.
+
+This program is free software: you can redistribute it and/or modify it under
+the terms of the GNU Affero General Public License version 3, as published by
+the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful, but WITHOUT
+ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Affero
+General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License along
+with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+'use strict';
+
+
+describe('viewmode controls widgets', function() {
+  var Y, container, controls, ViewmodeControls;
+
+  before(function(done) {
+    Y = YUI(GlobalConfig).use(['viewmode-controls',
+                               'juju-tests-utils',
+                               'event-simulate',
+                               'node-event-simulate',
+                               'node'], function(Y) {
+      ViewmodeControls = Y.juju.widgets.ViewmodeControls;
+      done();
+    });
+  });
+
+  beforeEach(function() {
+    container = Y.namespace('juju-tests.utils').makeContainer('container');
+    Y.Node.create([
+      '<div id="content">',
+      '<div id="browser-nav">',
+      '<div class="sidebar"></div>',
+      '<div class="fullscreen"</div>',
+      '</div>'
+    ].join('')).appendTo(container);
+  });
+
+  afterEach(function() {
+    container.remove(true);
+    if (controls) {
+      controls.destroy();
+    }
+  });
+
+  it('should fire a fullscreen event when expand clicked', function() {
+    var triggered = false;
+    controls = new ViewmodeControls();
+    controls.render();
+
+    controls.on(controls.EVT_FULLSCREEN, function(ev) {
+      triggered = true;
+    });
+
+    var toggle = container.one('.fullscreen');
+    toggle.simulate('click');
+    triggered.should.eql(true);
+  });
+
+  it('should fire a sidebar event when expand clicked', function() {
+    var triggered = false;
+    controls = new ViewmodeControls();
+    controls.render();
+
+    controls.on(controls.EVT_SIDEBAR, function(ev) {
+      triggered = true;
+    });
+
+    var toggle = container.one('.sidebar');
+    toggle.simulate('click');
+    triggered.should.eql(true);
+  });
+
+  it('should fire a toggle viewable event when icon clicked', function() {
+    var triggered = false;
+    controls = new ViewmodeControls();
+    controls.render();
+
+    controls.on(controls.EVT_TOGGLE_VIEWABLE, function(ev) {
+      triggered = true;
+    });
+
+    // The toggle button is a delegate, so verify it works for a node inserted
+    // after the instance of the widget is created.
+    container.one('#content').append(Y.Node.create('<a class="bws-icon"/>'));
+    var toggle = container.one('.bws-icon');
+
+    toggle.simulate('click');
+    triggered.should.eql(true);
+  });
+});