← Back to team overview

yellow team mailing list archive

[Merge] lp:~gary/juju-gui/charmpanel into lp:juju-gui

 

Gary Poster has proposed merging lp:~gary/juju-gui/charmpanel into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)

For more details, see:
https://code.launchpad.net/~gary/juju-gui/charmpanel/+merge/131605

Convert charm store to full-side display

This makes relatively small changes to adjust the existing charm store panels to be displayed down the full right side, per design specs.  It does not touch the display of the pnales themselves, other than to adjust the scrolling.

Thanks

Gary

https://codereview.appspot.com/6775058/

-- 
https://code.launchpad.net/~gary/juju-gui/charmpanel/+merge/131605
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/charmpanel into lp:juju-gui.
=== removed file 'app/templates/charm-search-pop.handlebars'
--- app/templates/charm-search-pop.handlebars	2012-10-11 20:49:22 +0000
+++ app/templates/charm-search-pop.handlebars	1970-01-01 00:00:00 +0000
@@ -1,7 +0,0 @@
-<div id="juju-search-charm-panel" class="popover fade bottom in">
-  <div class="arrow"></div>
-  <div class="popover-inner">
-    <div class="popover-content">
-    </div>
-  </div>
-</div>

=== modified file 'app/views/charm-search.js'
--- app/views/charm-search.js	2012-10-26 12:07:08 +0000
+++ app/views/charm-search.js	2012-10-26 13:02:25 +0000
@@ -5,13 +5,14 @@
   var views = Y.namespace('juju.views'),
       utils = Y.namespace('juju.views.utils'),
       models = Y.namespace('juju.models'),
+      subscriptions = [],
       // Singleton
-      _instance = null;
+      _instance;
 
   var toggleSectionVisibility = function(ev) {
     var el = ev.currentTarget.ancestor('.charm-section')
-                .one('.collapsible'),
-        icon = ev.currentTarget.one('i');
+                    .one('.collapsible'),
+            icon = ev.currentTarget.one('i');
     icon = ev.currentTarget.one('i');
     if (el.getStyle('height') === '0px') {
       el.show('sizeIn', {duration: 0.25, width: null});
@@ -20,7 +21,18 @@
       el.hide('sizeOut', {duration: 0.25, width: null});
       icon.replaceClass('icon-chevron-down', 'icon-chevron-right');
     }
-  };
+  },
+      setScroll = function(container, height) {
+        var scrollContainer = container.one('.charm-panel');
+        if (scrollContainer && height) {
+          var diff = scrollContainer.getY() - container.getY(),
+              clientDiff = (
+              scrollContainer.getClientRect().height -
+              parseInt(scrollContainer.getComputedStyle('height'), 10)),
+              scrollHeight = height - diff - clientDiff;
+          scrollContainer.setStyle('height', scrollHeight + 'px');
+        }
+      };
 
   var CharmCollectionView = Y.Base.create('CharmCollectionView', Y.View, [], {
     template: views.Templates['charm-search-result'],
@@ -77,6 +89,7 @@
       this.after('resultEntriesChange', function() {
         this.render();
       });
+      this.after('heightChange', this._setScroll);
     },
     render: function() {
       var container = this.get('container'),
@@ -94,8 +107,17 @@
               }
           );
       container.setHTML(this.template({ charms: entries }));
+      this._setScroll();
       return this;
     },
+    _setScroll: function() {
+      var container = this.get('container'),
+          scrollContainer = container.one('.search-result-div'),
+          height = this.get('height');
+      if (scrollContainer && height) {
+        scrollContainer.setStyle('height', height + 'px');
+      }
+    },
     showDetails: function(ev) {
       ev.halt();
       this.fire(
@@ -135,6 +157,7 @@
         },
         initializer: function() {
           this.bindModelView(this.get('model'));
+          this.after('heightChange', this._setScroll);
         },
         render: function() {
           var container = this.get('container'),
@@ -149,8 +172,12 @@
             container.setHTML(
                 '<div class="alert">Waiting on charm data...</div>');
           }
+          this._setScroll();
           return this;
         },
+        _setScroll: function() {
+          setScroll(this.get('container'), this.get('height'));
+        },
         goBack: function(ev) {
           ev.halt();
           this.fire('changePanel', { name: 'charms' });
@@ -173,6 +200,7 @@
         configFileContent: null,
         initializer: function() {
           this.bindModelView(this.get('model'));
+          this.after('heightChange', this._setScroll);
         },
         render: function() {
           var container = this.get('container'),
@@ -194,8 +222,12 @@
             container.setHTML(
                 '<div class="alert">Waiting on charm data...</div>');
           }
+          this._setScroll();
           return this;
         },
+        _setScroll: function() {
+          setScroll(this.get('container'), this.get('height'));
+        },
         events: {
           '.btn.cancel': {click: 'goBack'},
           '.btn.deploy': {click: 'onCharmDeployClicked'},
@@ -404,10 +436,8 @@
         charms = new models.CharmList(),
         app = config.app,
         testing = !!config.testing,
-        container = Y.Node.create(views.Templates['charm-search-pop']({
-          title: 'All Charms'
-        })),
-        contentNode = container.one('.popover-content'),
+        container = Y.Node.create('<div></div>').setAttribute(
+        'id', 'juju-search-charm-panel'),
         charmsSearchPanelNode = Y.Node.create(),
         charmsSearchPanel = new CharmCollectionView(
               { container: charmsSearchPanelNode,
@@ -442,8 +472,9 @@
           throw 'Developer error: Unknown panel name ' + config.name;
         }
         activePanelName = config.name;
-        contentNode.get('children').remove();
-        contentNode.append(panels[config.name].get('container'));
+        container.get('children').remove();
+        container.append(panels[config.name].get('container'));
+        newPanel.set('height', calculatePanelPosition().height);
         if (config.charmId) {
           newPanel.set('model', null); // Clear out the old.
           var charm = charms.getById(config.charmId);
@@ -466,13 +497,12 @@
     }
 
     Y.Object.each(panels, function(panel) {
-      panel.on('changePanel', setPanel);
+      subscriptions.push(panel.on('changePanel', setPanel));
     });
     // The panel starts with the "charmsSearchPanel" visible.
     setPanel({name: 'charms'});
 
     // Update position if we resize the window.
-    // It tries to keep the popup arrow under the charms search icon.
     Y.on('windowresize', function(e) {
       if (isPopupVisible) {
         updatePopupPosition();
@@ -481,6 +511,14 @@
 
     function hide() {
       if (isPopupVisible) {
+        var headerBox = Y.one('#charm-search-trigger-container'),
+            headerSpan = headerBox && headerBox.one('span');
+        if (headerBox) {
+          headerBox.setStyle('borderLeftColor', 'transparent');
+          if (headerSpan) {
+            headerSpan.setStyle('borderLeftColor', 'lightgray');
+          }
+        }
         container.hide(!testing, {duration: 0.25});
         if (Y.Lang.isValue(trigger)) {
           trigger.one('i').replaceClass(
@@ -489,21 +527,39 @@
         isPopupVisible = false;
       }
     }
-    container.on('clickoutside', hide);
+    subscriptions.push(container.on('clickoutside', hide));
+    subscriptions.push(Y.on('beforePageSizeRecalculation', function() {
+      container.setStyle('display', 'none');
+    }));
+    subscriptions.push(Y.on('afterPageSizeRecalculation', function() {
+      if (isPopupVisible) {
+        // We need to do this both in windowresize and here because
+        // windowresize can only be fired with "on," and so we can't know
+        // which handler will be fired first.
+        updatePopupPosition();
+      }
+    }));
 
     function show() {
       if (!isPopupVisible) {
+        var headerBox = Y.one('#charm-search-trigger-container'),
+            headerSpan = headerBox && headerBox.one('span');
+        if (headerBox) {
+          headerBox.setStyle('borderLeftColor', 'lightgray');
+          if (headerSpan) {
+            headerSpan.setStyle('borderLeftColor', 'transparent');
+          }
+        }
         container.setStyles({opacity: 0, display: 'block'});
-        updatePopupPosition();
         container.show(!testing, {duration: 0.25});
+        isPopupVisible = true;
+        updatePopupPosition();
         if (Y.Lang.isValue(trigger)) {
           trigger.one('i').replaceClass(
               'icon-chevron-down', 'icon-chevron-up');
         }
-        isPopupVisible = true;
       }
     }
-
     function toggle(ev) {
       if (Y.Lang.isValue(ev)) {
         // This is important to not have the clickoutside handler immediately
@@ -518,27 +574,31 @@
     }
 
     function updatePopupPosition() {
+      // This should only be called when the popup is supposed to be visible.
+      // We need to hide the popup before we calculate positions so that it
+      // does not cause scrollbars to appear while we are calculating
+      // positions.  The temporary scrollbars can cause the calculations to be
+      // incorrect.
+      container.setStyle('display', 'none');
       var pos = calculatePanelPosition();
-      container.setXY([pos.x, pos.y]);
-      container.one('.arrow').setX(pos.arrowX);
+      container.setStyle('display', 'block');
+      container.setX(pos.x);
+      if (pos.height) {
+        container.setStyle('height', pos.height + 'px');
+        panels[activePanelName].set('height', pos.height);
+      }
     }
 
     function calculatePanelPosition() {
-      var icon = Y.one('#charm-search-icon'),
-          pos = icon.getXY(),
-          content = Y.one('#content'),
-          contentWidth = parseInt(content.getComputedStyle('width'), 10),
-          containerWidth = parseInt(container.getComputedStyle('width'), 10),
-          iconWidth = parseInt(icon.getComputedStyle('width'), 10);
-      return {
-        x: content.getX() + contentWidth - containerWidth,
-        y: pos[1] + 30,
-        arrowX: icon.getX() + (iconWidth / 2)
-      };
+      var headerBox = Y.one('#charm-search-trigger-container'),
+          svg = Y.one('svg');
+      return {x: headerBox && Math.round(headerBox.getX()),
+        height:
+            svg && parseInt(Y.one('svg').getAttribute('height'), 10) + 17};
     }
 
     if (Y.Lang.isValue(trigger)) {
-      trigger.on('click', toggle);
+      subscriptions.push(trigger.on('click', toggle));
     }
 
     var handleKeyDown = function(ev) {
@@ -564,9 +624,9 @@
     };
 
     if (searchField) {
-      searchField.on('keydown', handleKeyDown);
-      searchField.on('blur', handleBlur);
-      searchField.on('focus', handleFocus);
+      subscriptions.push(searchField.on('keydown', handleKeyDown));
+      subscriptions.push(searchField.on('blur', handleBlur));
+      subscriptions.push(searchField.on('focus', handleFocus));
     }
 
     // The public methods
@@ -590,6 +650,10 @@
       return _instance;
     },
     killInstance: function() {
+      while (subscriptions.length) {
+        var sub = subscriptions.pop();
+        // if (sub) { sub.detach(); }
+      }
       if (_instance) {
         _instance.node.remove(true);
         _instance = null;
@@ -611,6 +675,7 @@
     'overlay',
     'svg-layouts',
     'dom-core',
-    'juju-models'
+    'juju-models',
+    'event-resize'
   ]
 });

=== modified file 'app/views/environment.js'
--- app/views/environment.js	2012-10-26 06:35:16 +0000
+++ app/views/environment.js	2012-10-26 13:02:25 +0000
@@ -1239,6 +1239,7 @@
          * Set the visualization size based on the viewport
          */
         setSizesFromViewport: function() {
+          Y.fire('beforePageSizeRecalculation');
           // start with some reasonable defaults
           var vis = this.vis,
               container = this.get('container'),
@@ -1291,6 +1292,7 @@
 
           this.width = width;
           this.height = height;
+          Y.fire('afterPageSizeRecalculation');
         },
 
         /*

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2012-10-26 06:33:29 +0000
+++ lib/views/stylesheet.less	2012-10-26 13:02:25 +0000
@@ -70,6 +70,7 @@
             &#charm-search-trigger-container {
                 width: 292px;
                 white-space: nowrap;
+                border-left: 1px solid transparent;
             }
             .nav-section {
                 border-left: 1px solid lightgray;
@@ -721,10 +722,14 @@
 
 #juju-search-charm-panel {
     display: block;
-    padding-left: 0px;
-    padding-right: 0px;
     z-index: 999;
-    float: none;
+    padding: 0;
+    top: 70px;
+    height: 100px;
+    position: absolute;
+    width: 292px;
+    background-color: white;
+    border-left: lightgray solid 1px;
 
     .charm-panel-configure .btn {
         float: right;
@@ -779,11 +784,6 @@
         width: 100%;
         height: 500px;
     }
-    @popover-height: 607px;
-    .popover-content {
-        padding: 0px;
-        height: @popover-height;
-    }
     .charm-nav-back {
         background-color: black;
         text-transform: uppercase;
@@ -822,8 +822,7 @@
             padding: 1ex 1em;
             .charm-section {
                 .commitmessage {
-                    padding-top: 0.8ex;
-                    padding-left: 1em;
+                    padding: 0.8ex 0 1ex 1em;
                     font-style: italic;
                     white-space: pre-wrap;
                 }
@@ -866,7 +865,6 @@
         }
     }
     .search-result-div {
-        height: @popover-height;
         overflow-y:auto;
         overflow-x:hidden;
         .series-charms {


Follow ups