← Back to team overview

yellow team mailing list archive

[Merge] lp:~bac/juju-gui/cs-filter into lp:juju-gui

 

Brad Crittenden has proposed merging lp:~bac/juju-gui/cs-filter into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #107296 in p7zip (Ubuntu): "description inaccurately describes RAR support"
  https://bugs.launchpad.net/ubuntu/+source/p7zip/+bug/107296

For more details, see:
https://code.launchpad.net/~bac/juju-gui/cs-filter/+merge/137050

Add the required charm filter dropdown.

The original design for the charm search results panel had a dropdown
that was to be used to filter the results.  After some consideration
the three sets were chosen to be 'all', 'deployed', and 'subordinate'.

This branch implements the dropdown and filter mechanism.  The widget
was based on the graph/list picker on the bottom of the page.  It was
refactored into a generic LESS function and supports both uses.

The design for the existing picker had a 1px wide design element to
provide the background.  It was not tall enough for a picker with
three items.  As a work-around an 'outset' mitred HTML border is
used.  It will be easy to replace with the proper asset when it is
produced.

If the picker is open and a click outside closes the charm panel then
the picker remains open.  That is a wart that needs to be fixed.
-- 
https://code.launchpad.net/~bac/juju-gui/cs-filter/+merge/137050
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~bac/juju-gui/cs-filter into lp:juju-gui.
=== added file 'app/assets/images/picker_view_right_button_down.png'
Binary files app/assets/images/picker_view_right_button_down.png	1970-01-01 00:00:00 +0000 and app/assets/images/picker_view_right_button_down.png	2012-11-29 21:07:22 +0000 differ
=== modified file 'app/models/charm.js'
--- app/models/charm.js	2012-11-20 16:55:43 +0000
+++ app/models/charm.js	2012-11-29 21:07:22 +0000
@@ -215,7 +215,7 @@
         }
       });
   models.Charm = Charm;
-
+  models.charmIdRe = charmIdRe;
   var CharmList = Y.Base.create('charmList', Y.ModelList, [], {
     model: Charm
   }, {

=== modified file 'app/store/charm.js'
--- app/store/charm.js	2012-11-20 16:55:43 +0000
+++ app/store/charm.js	2012-11-29 21:07:22 +0000
@@ -70,7 +70,8 @@
         var charm = list.getById(result.store_url);
         if (!charm) {
           charm = list.add(
-              { id: result.store_url, summary: result.summary });
+              { id: result.store_url, summary: result.summary,
+                is_subordinate: result.subordinate});
         }
         var series = charm.get('series');
         if (!Y.Lang.isValue(hash[series])) {

=== modified file 'app/templates/charm-search-result.handlebars'
--- app/templates/charm-search-result.handlebars	2012-11-19 16:29:35 +0000
+++ app/templates/charm-search-result.handlebars	2012-11-29 21:07:22 +0000
@@ -1,4 +1,21 @@
+<div class="picker charm-filter-picker">
+  <i class="picker-edge sprite picker_left_edge"></i>
+  <div class="picker-expanded">
+    <div class="picker-list">
+      <ul>
+        <li class="picker-item all" data-filter="all">All charms ({{all_charms_count}})</li>
+        <li class="picker-item subordinates" data-filter="subordinates">Subordinates ({{subordinate_charms_count}})</li>
+        <li class="picker-item deployed" data-filter="deployed">Deployed ({{deployed_charms_count}})</li>
+      </ul>
+    </div>
+    <i class="picker-edge picker-edge-right sprite view_right_edge"></i>
+  </div>
+  <div class="picker-body"></div>
+  <i class="picker-button sprite picker_view_right_button_down"></i>
+</div>
+
 <div class="search-result-div">
+
   <div>
     {{#charms}}
     <div class="series-charms">

=== modified file 'app/templates/overview.handlebars'
--- app/templates/overview.handlebars	2012-11-15 15:55:06 +0000
+++ app/templates/overview.handlebars	2012-11-29 21:07:22 +0000
@@ -20,23 +20,23 @@
             <img src="/juju-ui/assets/images/cmd_icon.png" />
             Command line
             <img class="divider" src="/juju-ui/assets/images/bottom_bar_small_div.png" />
-            <img src="/juju-ui/assets/images/view_icon.png" />
+            <img src="/juju-ui/assets/images/view_icon.png"></i>
             <div class="picker graph-list-picker">
-                <img class="picker-edge" src="/juju-ui/assets/images/picker_left_edge.png" />
+                <i class="picker-edge sprite picker_left_edge"></i>
                 <div class="picker-expanded">
-                    <img class="picker-edge" src="/juju-ui/assets/images/view_left_edge.png" />
+                    <i class="picker-edge sprite view_left_edge"></i>
                     <div class="picker-list">
                         <ul>
                             <li class="active">Graph view<div class="activetick"></div></li>
                             <li>List view</li>
                         </ul>
                     </div>
-                    <img class="picker-edge picker-edge-right" src="/juju-ui/assets/images/view_right_edge.png" />
+                    <i class="picker-edge picker-edge-right sprite view_right_edge"></i>
                 </div>
                 <div class="picker-body">
                     Graph view
                 </div>
-                <img class="picker-button" src="/juju-ui/assets/images/picker_view_right_button.png" />
+                <i class="picker-button sprite picker_view_right_button"></i>
             </div>
             <img class="divider" src="/juju-ui/assets/images/bottom_bar_small_div.png" />
             <img id="zoom-out-btn" src="/juju-ui/assets/images/zoom_minus.png" />

=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2012-11-26 22:58:56 +0000
+++ app/views/charm-panel.js	2012-11-29 21:07:22 +0000
@@ -68,6 +68,79 @@
         }
       },
       /**
+       * Given a set of entries as returned by the charm store "find"
+       * method (charms grouped by series), return the list filtered
+       * by 'filter'.
+       *
+       * @method filterEntries
+       * @static
+       * @private
+       * @param {Array} entries An ordered collection of groups of charms, as
+       *   returned by the charm store "find" method.
+       * @param {String} filter Either 'all', 'subordinates', or 'deployed'.
+       * @param {Object} services The db.services model list.
+       * @return {Array} A filtered, grouped set of filtered entries.
+       */
+      filterEntries = function(entries, filter, services) {
+        var deployed_charms,
+            /**
+             * Filter to determine if a charm is a subordinate.
+             *
+             * @method is_sub_filter
+             * @param {Object} charm The charm to test.
+             * @return {Boolean} True if the charm is a subordinate.
+             */
+            is_sub_filter = function(charm) {
+              return !!charm.get('is_subordinate');
+            },
+            /**
+             * Filter to determine if a charm is the same as any
+             * deployed services.
+             *
+             * @method is_deployed_filter
+             * @param {Object} charm The charm to test.
+             * @return {Boolean} True if the charm matches a deployed service.
+             */
+            is_deployed_filter = function(charm) {
+              return deployed_charms.indexOf(charm.get('package_name')) !== -1;
+            },
+            filter_fcn;
+
+        if (filter === 'all') {
+          return entries;
+        } else if (filter === 'subordinates') {
+          filter_fcn = is_sub_filter;
+        } else if (filter === 'deployed') {
+          filter_fcn = is_deployed_filter;
+          if (!Y.Lang.isValue(services)) {
+            deployed_charms = [];
+          } else {
+            var charm_ids = services.get('charm'),
+                package_names = Y.Array.map(charm_ids, function(id) {
+                  var parts = models.charmIdRe.exec(id);
+                  return parts[4];
+                });
+            deployed_charms = Y.Array.dedupe(package_names);
+          }
+        } else {
+          // This case should not happen.
+          return entries;
+        }
+
+        var filtered = Y.clone(entries);
+        /* Filter the charms based on the filter function. */
+        filtered.forEach(function(series_group) {
+          var sub_charms = series_group.charms.filter(filter_fcn);
+          series_group.charms = sub_charms;
+        });
+        /* Filter the series group based on the existence of any
+           filtered charms. */
+        return filtered.filter(function(series_group) {
+          return series_group.charms.length > 0;
+        });
+      },
+
+      /**
        * Given a set of grouped entries as returned by the charm store "find"
        * method, return the same data but with the charms converted into data
        * objects that are more amenable to rendering with handlebars.
@@ -128,6 +201,12 @@
         mouseleave: function(ev) {
           ev.currentTarget.all('.btn').transition({opacity: 0, duration: 0.25});
         }
+      },
+      '.charm-filter-picker .picker-button': {
+        click: 'showCharmFilterPicker'
+      },
+      '.charm-filter-picker .picker-item': {
+        click: 'hideCharmFilterPicker'
       }
     },
     // Set searchText to cause the results to be found and rendered.
@@ -135,6 +214,7 @@
     // found and rendered.
     initializer: function() {
       var self = this;
+      this.set('filter', 'all');
       this.after('searchTextChange', function(ev) {
         this.set('resultEntries', null);
         if (ev.newVal) {
@@ -172,6 +252,10 @@
         this.render();
       });
       this.after('heightChange', this._setScroll);
+      this.after('filterChange', function() {
+        console.log('filterChange', this.get('filter'));
+        //this.render();
+      });
     },
     render: function() {
       var container = this.get('container'),
@@ -179,8 +263,46 @@
           defaultEntries = this.get('defaultEntries'),
           resultEntries = this.get('resultEntries'),
           raw_entries = searchText ? resultEntries : defaultEntries,
-          entries = raw_entries && makeRenderableResults(raw_entries);
-      container.setHTML(this.template({ charms: entries }));
+          entries,
+          db = this.get('db') || undefined,
+          services = db && db.services || undefined,
+          filtered = {},
+          filters = ['all', 'subordinates', 'deployed'];
+
+      if (!raw_entries) {
+        return this;
+      }
+      for (var sel in filters) {
+        if (true) {             // Avoid lint warning.
+          filtered[filters[sel]] = filterEntries(
+              raw_entries, filters[sel], services);
+        }
+      }
+
+      entries = makeRenderableResults(filtered[this.get('filter')]);
+      var countEntries = function(entries) {
+        if (!entries) {return 0;}
+        var lengths = entries.map(function(e) {return e.charms.length;});
+        // Initial value of 0 required since the array may be empty.
+        return lengths.reduce(function(pv, cv) {return pv + cv;}, 0);
+      };
+
+      container.setHTML(this.template(
+          { charms: entries,
+            all_charms_count: countEntries(filtered.all),
+            subordinate_charms_count: countEntries(filtered.subordinates),
+            deployed_charms_count: countEntries(filtered.deployed)
+          }));
+
+      // The picker has now been rendered generically.  Based on the
+      // filter add the decorations.
+      var selected = container.one('.' + this.get('filter')),
+          picker = container.one('.charm-filter-picker');
+      selected.addClass('activetick');
+      picker.one('.picker-body').set('text', selected.get('text'));
+      // The charm details and summary are user-supplied and may be
+      // way too big for the fixed height cells.  Sadly the best we
+      // can do is truncate them with elllipses.
       container.all('.charm-detail').ellipsis();
       container.all('.charm-summary').ellipsis({'lines': 2});
       this._setScroll();
@@ -311,7 +433,37 @@
             level: 'error'
           })
       );
+    },
+
+    /**
+     * Event handler to show the charm filter picker.
+     *
+     * @method showCharmFilterPicker
+     * @param {Object} evt The event.
+     * @return {undefined} nothing.
+     */
+    showCharmFilterPicker: function(evt) {
+      var container = this.get('container'),
+          picker = container.one('.charm-filter-picker');
+      picker.addClass('inactive');
+      picker.one('.picker-expanded').addClass('active');
+    },
+
+    /**
+     * Event handler to hide the charm filter picker
+     *
+     * @method hideCharmFilterPicker
+     * @param {Object} evt The event.
+     * @return {undefined} nothing.
+     */
+    hideCharmFilterPicker: function(evt) {
+      // Set the filter and re-render the control.
+      var selected = evt.currentTarget;
+      this.set('filter', selected.getData('filter'));
+      this.render();
+      evt.halt();
     }
+
   });
   views.CharmCollectionView = CharmCollectionView;
 
@@ -1043,6 +1195,9 @@
     }
   };
 
+  // Exposed for testing.
+  views.filterEntries = filterEntries;
+
 }, '0.1.0', {
   requires: [
     'view',

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2012-11-27 15:48:09 +0000
+++ lib/views/stylesheet.less	2012-11-29 21:07:22 +0000
@@ -642,105 +642,71 @@
     }
 }
 
-.picker {
-    position: relative;
-    display: inline-block;
-    margin: 0;
-    padding: 0;
-    height: 24px;
-    &.graph-list-picker {
-        width: 116px;
-        .picker-body {
-            width: 91px;
-        }
-
-
-        .picker-expanded {
-            display: none;
-
-            &.active {
-                z-index: 100;
-                display: block;
-            }
-            .picker-edge {
-                display: block;
-                top: auto;
-                bottom: -10px;
-                margin-left: -3px;
-            }
-            .picker-edge-right {
-                left: auto;
-                right: 0;
-                margin-left: 0;
-                margin-right: -3px;
-            }
-            .picker-list {
-                position: absolute;
-                bottom: -12px;
-                left: 4px;
-                height: 100%;
-                background: url(/juju-ui/assets/images/view_1px.png) repeat-x;
+.create-picker(@width, @top, @listtop, @listbottom) {
+    width: @width;
+    .picker-body {
+        width: @width - 15px;
+    }
+    .picker-expanded {
+        display: none;
+        top: @top;
+        &.active {
+            z-index: 100;
+            display: block;
+        }
+        .picker-edge {
+            display: block;
+            top: auto;
+            bottom: -10px;
+            margin-left: -3px;
+        }
+        .picker-edge-right {
+            left: auto;
+            right: 0;
+            margin-left: 0;
+            margin-right: -3px;
+        }
+        .picker-list {
+            position: absolute;
+            bottom: @listbottom;
+            top: @listtop;
+            left: 4px;
+            height: 100%;
+            background: url(/juju-ui/assets/images/view_1px.png) repeat-x;
+            list-style-type: none;
+            padding: 0;
+            margin: 0;
+            height: 54px;
+            width: @width - 8px;
+            cursor: pointer;
+
+            ul {
                 list-style-type: none;
                 padding: 0;
+                padding-top: 5px;
                 margin: 0;
-                height: 54px;
-                width: 108px;
-
-                ul {
-                    list-style-type: none;
-                    padding: 0;
-                    padding-top: 5px;
-                    margin: 0;
-
-                    li {
-                        padding: 1px;
-
-                        .activetick {
-                            width: 12px;
-                            height: 14px;
-                            float: right;
-                            background: url(/juju-ui/assets/images/selected_tick.png)
-                                no-repeat
-                                bottom left;
-                        }
+
+                li {
+                    padding: 1px;
+                    &.activetick {
+                        /* XXX bac: The positioning is off but I have been unable
+                         * to get it right.  Using background-position does not work.
+                         */
+                        background: url(/juju-ui/assets/images/selected_tick.png)
+                            no-repeat bottom right;
                     }
                 }
             }
         }
     }
-    &.zoom-picker {
-        width: 56px;
-        .picker-body {
-            width: 36px;
-        }
-        /*#zoom-btn-up {
-            position: absolute;
-            top: 8px;
-            right: 0px;
-            opacity: 0;
-            &.active {
-                opacity: 1;
-            }
-        }
-        #zoom-btn-down {
-            position: absolute;
-            bottom: -8px;
-            right: 0px;
-            opacity: 0;
-            &.active {
-                opacity: 1;
-            }
-        }*/
-    }
-
     .picker-edge {
         position: absolute;
-        top: 8px;
+        top: @top;
         left: 0;
     }
     .picker-body {
         position: absolute;
-        top: 8px;
+        top: @top;
         left: 4px;
         background: url(/juju-ui/assets/images/picker_1px.png) repeat-x;
         height: 20px;
@@ -748,7 +714,7 @@
     }
     .picker-button {
         position: absolute;
-        top: 8px;
+        top: @top;
         right: 0;
     }
     &.inactive {
@@ -762,6 +728,55 @@
             display: none;
         }
     }
+
+}
+.picker {
+    position: relative;
+    display: inline-block;
+    margin: 0;
+    padding: 0;
+    height: 24px;
+    &.graph-list-picker {
+        .create-picker(116px, 8px, initial, -12px);
+    }
+    &.charm-filter-picker {
+        left: 11px;
+        padding-bottom: 5px;
+        @top: 0px;
+        top: @top;
+        .create-picker(270px, @top, @top, initial);
+        .picker-expanded .picker-list {
+            @border-color: #DDD;
+            border: 4px outset @border-color;
+            background: white;
+            height: auto;
+        }
+    }
+    &.zoom-picker {
+        width: 56px;
+        .picker-body {
+            width: 36px;
+        }
+        /*#zoom-btn-up {
+            position: absolute;
+            top: 8px;
+            right: 0px;
+            opacity: 0;
+            &.active {
+                opacity: 1;
+            }
+        }
+        #zoom-btn-down {
+            position: absolute;
+            bottom: -8px;
+            right: 0px;
+            opacity: 0;
+            &.active {
+                opacity: 1;
+            }
+        }*/
+    }
+
 }
 
 #unit-status {

=== modified file 'test/test_charm_panel.js'
--- test/test_charm_panel.js	2012-11-12 14:52:08 +0000
+++ test/test_charm_panel.js	2012-11-29 21:07:22 +0000
@@ -287,3 +287,195 @@
   });
 
 });
+
+describe('charm panel filtering', function() {
+
+  var Y, models, views, juju, conn, env, container, db, app, charm,
+      charm_store_data, charm_store, charms, ENTER;
+
+  before(function(done) {
+    Y = YUI(GlobalConfig).use(
+        'juju-models',
+        'juju-views',
+        'juju-gui',
+        'juju-env',
+        'juju-tests-utils',
+        'node-event-simulate',
+        'node',
+        'datasource-local',
+        'json-stringify',
+        'juju-charm-store',
+
+        function(Y) {
+          models = Y.namespace('juju.models');
+          views = Y.namespace('juju.views');
+          juju = Y.namespace('juju');
+          ENTER = Y.Node.DOM_EVENTS.key.eventDef.KEY_MAP.enter;
+          done();
+        });
+
+  });
+
+  beforeEach(function() {
+    conn = new (Y.namespace('juju-tests.utils')).SocketStub(),
+    env = new (Y.namespace('juju')).Environment({conn: conn});
+    env.connect();
+    conn.open();
+    Y.one('#main').append(Y.Node.create(
+        '<div id="charm-search-test">' +
+        '  <input type="text" id="charm-search-field" />' +
+        '</div>'));
+    container = Y.Node.create('<div id="test-container"></div>');
+    Y.one('#main').append(container);
+    db = new models.Database();
+    charms = db.charms.add([
+      { id: 'cs:precise/mysql-7' },
+      { id: 'cs:precise/syslogd-1', is_subordinate: true}]);
+    charm_store_data = {responseText: '{}'};
+    charm_store = new juju.CharmStore(
+        {datasource: {
+          sendRequest: function(params) {
+            params.callback.success({
+              response: {
+                results: [{
+                  responseText: charm_store_data.responseText
+                }]
+              }
+            });
+          }
+        }
+        });
+    app = { db: db, env: env, charm_store: charm_store };
+  });
+
+  afterEach(function() {
+    Y.namespace('juju.views').CharmPanel.killInstance();
+    Y.one('#charm-search-test').remove(true);
+    container.remove(true);
+    db.destroy();
+    env.destroy();
+  });
+
+
+  it('should have `filters` default to `all`', function() {
+    var view = new views.CharmCollectionView({});
+    view.get('filter').should.equal('all');
+  });
+
+  it('should not filter entries when `all` is the filter', function() {
+    var entries = [
+      {
+        series: 'precise',
+        charms: [
+          new models.Charm({id: 'cs:precise/foo-1'}),
+          new models.Charm({id: 'cs:precise/logger-1',
+            is_subordinate: true})
+        ]
+      }
+    ];
+
+    var filtered = views.filterEntries(entries, 'all');
+    filtered.length.should.equal(1);
+    filtered[0].charms.length.should.equal(2);
+  });
+
+  it('should filter for `subordinate`', function() {
+    var entries = [
+      {
+        series: 'precise',
+        charms: [
+          new models.Charm({id: 'cs:precise/foo-1'}),
+          new models.Charm({id: 'cs:precise/logger-1',
+            is_subordinate: true}),
+          new models.Charm({id: 'cs:precise/sub-1',
+            is_subordinate: true}),
+          new models.Charm({id: 'cs:precise/nosub-3',
+            is_subordinate: false})
+        ]
+      },
+      {
+        series: 'oneiric',
+        charms: [
+          new models.Charm({id: 'cs:precise/foo-1',
+            is_subordinate: true})
+        ]
+      }
+    ];
+
+    var filtered = views.filterEntries(entries, 'subordinates');
+    filtered.length.should.equal(2);
+    filtered[0].charms.length.should.equal(2);
+    filtered[0].charms[0].get('id').should.equal('cs:precise/logger-1');
+    filtered[0].charms[0].get('is_subordinate').should.equal(true);
+    filtered[0].charms[1].get('id').should.equal('cs:precise/sub-1');
+    filtered[0].charms[1].get('is_subordinate').should.equal(true);
+  });
+
+  it('should filter out series with no remaining charms', function() {
+    var entries = [
+      {
+        series: 'precise',
+        charms: [
+          new models.Charm({id: 'cs:precise/foo-1'}),
+          new models.Charm({id: 'cs:precise/logger-1',
+            is_subordinate: true}),
+          new models.Charm({id: 'cs:precise/sub-1',
+            is_subordinate: true}),
+          new models.Charm({id: 'cs:precise/nosub-3',
+            is_subordinate: false})
+        ]
+      },
+      {
+        series: 'oneiric',
+        charms: [
+          new models.Charm({id: 'cs:oneiric/foo-1',
+            is_subordinate: false})
+        ]
+      }
+    ];
+
+    var filtered = views.filterEntries(entries, 'subordinates');
+    filtered.length.should.equal(1);
+  });
+
+
+  it('should filter for `deployed` charms', function() {
+    var entries = [
+      {
+        series: 'precise',
+        charms: [
+          new models.Charm({id: 'cs:precise/foo-1'}),
+          new models.Charm({id: 'cs:precise/logger-1',
+            is_subordinate: true}),
+          new models.Charm({id: 'cs:precise/sub-1',
+            is_subordinate: true}),
+          new models.Charm({id: 'cs:precise/nosub-3',
+            is_subordinate: false})
+        ]
+      },
+      {
+        series: 'oneiric',
+        charms: [
+          new models.Charm({id: 'cs:oneiric/foo-1',
+            is_subordinate: true})
+        ]
+      }
+    ];
+
+    db.services.add([
+      {id: 'wordpress', charm: 'cs:edgy/wordpress-9'},
+      {id: 'foo', charm: 'cs:oneiric/foo-2'},
+      {id: 'sub', charm: 'cs:precise/sub-1'}]);
+
+    var filtered = views.filterEntries(entries, 'deployed', db.services);
+
+    filtered.length.should.equal(2);
+    filtered[0].charms.length.should.equal(2);
+    filtered[0].charms[0].get('id').should.equal('cs:precise/foo-1');
+    filtered[0].charms[1].get('id').should.equal('cs:precise/sub-1');
+    filtered[1].charms.length.should.equal(1);
+    filtered[1].charms[0].get('id').should.equal('cs:oneiric/foo-1');
+  });
+
+
+});


Follow ups