yellow team mailing list archive

[Francesco Banconi <francesco.banconi@xxxxxxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/juju-gui/bug-1075672-icons into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1075672 in juju-ui: "Charm panel icons are not per visual spec"
  https://bugs.launchpad.net/juju-gui/+bug/1075672

For more details, see:
https://code.launchpad.net/~frankban/juju-gui/bug-1075672-icons/+merge/133944

Replaced charm panel icons with our assets.

In the charm panel, replaced bootstrap icons with
the ones from our sprite. Also added the back triangle
to the sprite.
This branch also fixes the charm menu chevrons: now they
are correctly replaced on panel open/close.
Added "docstrings" for relevant methods.

https://codereview.appspot.com/6819131/

-- 
https://code.launchpad.net/~frankban/juju-gui/bug-1075672-icons/+merge/133944
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~frankban/juju-gui/bug-1075672-icons into lp:juju-gui.

=== added file 'app/assets/images/back_triangle.png'
Binary files app/assets/images/back_triangle.png	1970-01-01 00:00:00 +0000 and app/assets/images/back_triangle.png	2012-11-12 15:24:20 +0000 differ
=== modified file 'app/index.html'
--- app/index.html	2012-11-07 21:52:54 +0000
+++ app/index.html	2012-11-12 15:24:20 +0000
@@ -51,7 +51,7 @@
                     <span id="charm-search-trigger">
                       <i id="charm-search-icon" class="sprite charm_icon"></i>
                       Charms
-                      <i class="icon-chevron-down"></i>
+                      <i id="charm-search-chevron" class="sprite chevron_down"></i>
                     </span>
                     <input type="text" id="charm-search-field"
                      required="required" placeholder="Search for a charm" />

=== modified file 'app/templates/charm-description.handlebars'
--- app/templates/charm-description.handlebars	2012-11-07 18:25:14 +0000
+++ app/templates/charm-description.handlebars	2012-11-12 15:24:20 +0000
@@ -1,5 +1,5 @@
 <div>
-  <div class="charm-nav-back"><i class="icon-chevron-left"></i> Back</div>
+  <div class="charm-nav-back"><i class="sprite back_triangle"></i> Back</div>
 
   <div class="charm-description charm-panel">
     <div id="charm-panel-head">
@@ -12,7 +12,7 @@
     </div>
 
     <div class="charm-section">
-      <h4 class="first"><i class="icon-chevron-down"></i> Description</h4>
+      <h4 class="first"><i class="sprite chevron_up"></i> Description</h4>
       <div class="collapsible">
         <h5>Summary</h5>
         <p>{{summary}}</p>
@@ -33,7 +33,7 @@
 
     {{#any requires provides}}
     <div class="charm-section">
-      <h4><i class="icon-chevron-up"></i> Interfaces</h4>
+      <h4><i class="sprite chevron_down"></i> Interfaces</h4>
       <div class="collapsible">
         {{#if provides}}
         <h5>Provides</h5>
@@ -57,7 +57,7 @@
 
     {{#if last_change}}
     <div class="charm-section">
-      <h4><i class="icon-chevron-up"></i> Change Log</h4>
+      <h4><i class="sprite chevron_down"></i> Change Log</h4>
       <div class="collapsible">
         {{#with last_change}}
         <h5>Last Change</h5>
@@ -71,7 +71,7 @@
 
     {{#any requires provides}}
     <div class="charm-section">
-      <h4><i class="icon-chevron-up"></i> Related Charms</h4>
+      <h4><i class="sprite chevron_down"></i> Related Charms</h4>
       <div class="collapsible" id="related-charms">
         Loading...
       </div>

=== modified file 'app/templates/charm-pre-configuration.handlebars'
--- app/templates/charm-pre-configuration.handlebars	2012-11-09 11:45:00 +0000
+++ app/templates/charm-pre-configuration.handlebars	2012-11-12 15:24:20 +0000
@@ -43,7 +43,7 @@
       {{#if settings}}
       <!-- Service configuration form -->
       <div class="charm-section">
-        <h4>Service Settings<i class="icon-chevron-down"></i></h4>
+        <h4>Service Settings<i class="sprite chevron_up"></i></h4>
         <div class="collapsible">
           <div class="config-file-upload control-group">
             <input class="config-file-upload-widget" type="file">

=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2012-11-09 20:51:13 +0000
+++ app/views/charm-panel.js	2012-11-12 15:24:20 +0000
@@ -39,10 +39,10 @@
         // clientHeight and offsetHeight are not as reliable in tests.
         if (parseInt(el.getStyle('height'), 10) === 0) {
           el.show('sizeIn', {duration: 0.25, width: null});
-          icon.replaceClass('icon-chevron-up', 'icon-chevron-down');
+          icon.replaceClass('chevron_down', 'chevron_up');
         } else {
           el.hide('sizeOut', {duration: 0.25, width: null});
-          icon.replaceClass('icon-chevron-down', 'icon-chevron-up');
+          icon.replaceClass('chevron_up', 'chevron_down');
         }
       },
       /**
@@ -337,7 +337,7 @@
               charm = this.get('model');
           if (Y.Lang.isValue(charm)) {
             container.setHTML(this.template(charm.getAttrs()));
-            container.all('i.icon-chevron-up').each(function(el) {
+            container.all('i.chevron_down').each(function(el) {
               el.ancestor('.charm-section').one('div')
                 .setStyle('height', '0px');
             });
@@ -881,6 +881,13 @@
       }
     });
 
+    /**
+     * Hide the charm panel.
+     * Set isPanelVisible to false.
+     *
+     * @method hide
+     * @return {undefined} Mutates only.
+     */
     function hide() {
       if (isPanelVisible) {
         var headerBox = Y.one('#charm-search-trigger-container'),
@@ -893,8 +900,8 @@
         }
         container.hide(!testing, {duration: 0.25});
         if (Y.Lang.isValue(trigger)) {
-          trigger.one('i').replaceClass(
-              'icon-chevron-up', 'icon-chevron-down');
+          trigger.one('i#charm-search-chevron').replaceClass(
+              'chevron_up', 'chevron_down');
         }
         isPanelVisible = false;
       }
@@ -912,6 +919,13 @@
       }
     }));
 
+    /**
+     * Show the charm panel.
+     * Set isPanelVisible to true.
+     *
+     * @method show
+     * @return {undefined} Mutates only.
+     */
     function show() {
       if (!isPanelVisible) {
         var headerBox = Y.one('#charm-search-trigger-container'),
@@ -927,11 +941,19 @@
         isPanelVisible = true;
         updatePanelPosition();
         if (Y.Lang.isValue(trigger)) {
-          trigger.one('i').replaceClass(
-              'icon-chevron-down', 'icon-chevron-up');
+          trigger.one('i#charm-search-chevron').replaceClass(
+              'chevron_down', 'chevron_up');
         }
       }
     }
+
+    /**
+     * Show the charm panel if it is hidden, hide it otherwise.
+     *
+     * @method toggle
+     * @param {Object} ev An event object (with a "halt" method).
+     * @return {undefined} Dispatches only.
+     */
     function toggle(ev) {
       if (Y.Lang.isValue(ev)) {
         // This is important to not have the clickoutside handler immediately

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2012-11-09 14:30:56 +0000
+++ lib/views/stylesheet.less	2012-11-12 15:24:20 +0000
@@ -852,7 +852,7 @@
         color: @charm-panel-deploy-button-color;
         cursor: pointer;
         font-size: 14px;
-        padding: 7px 4px;
+        padding: 7px @charm-panel-padding-left;
     }
     .charm-panel {
         background-color: @charm-panel-background-color;
@@ -875,6 +875,7 @@
             padding: 8px @charm-panel-padding-left;
             i {
                 float: right;
+                margin-top: 8px;
             }
             &.first {
                 border-top: 1px solid @charm-paneel-border-top-color;

=== modified file 'test/test_charm_panel.js'
--- test/test_charm_panel.js	2012-11-07 18:36:31 +0000
+++ test/test_charm_panel.js	2012-11-12 15:24:20 +0000
@@ -266,12 +266,12 @@
         // We use the last change div.
         section_container = html.one('div.charm-section:last-child');
     section_container.one('div').getStyle('height').should.equal('0px');
-    assert(section_container.one('h4 i').hasClass('icon-chevron-up'));
+    assert(section_container.one('h4 i').hasClass('chevron_down'));
     section_container.one('h4').simulate('click');
-    assert(section_container.one('h4 i').hasClass('icon-chevron-down'));
+    assert(section_container.one('h4 i').hasClass('chevron_up'));
     section_container.one('div').getStyle('height').should.not.equal('0px');
     section_container.one('h4').simulate('click');
-    assert(section_container.one('h4 i').hasClass('icon-chevron-up'));
+    assert(section_container.one('h4 i').hasClass('chevron_down'));
     // The transition is still running, so we can't check display.
   });
 

=== modified file 'undocumented'
--- undocumented	2012-11-09 11:45:00 +0000
+++ undocumented	2012-11-12 15:24:20 +0000
@@ -68,7 +68,6 @@
 app/views/charm-panel.js calculatePanelPosition
 app/views/charm-panel.js createInstance
 app/views/charm-panel.js getInstance
-app/views/charm-panel.js hide
 app/views/charm-panel.js hideDescription
 app/views/charm-panel.js initializer
 app/views/charm-panel.js killInstance
@@ -79,10 +78,8 @@
 app/views/charm-panel.js setDefaultSeries
 app/views/charm-panel.js setPanel
 app/views/charm-panel.js setupOverlay
-app/views/charm-panel.js show
 app/views/charm-panel.js showConfiguration
 app/views/charm-panel.js showDescription
-app/views/charm-panel.js toggle
 app/views/charm-panel.js updatePanelPosition
 app/views/charm.js _deployCallback
 app/views/charm.js initializer