Reviewers: mp+133944_code.launchpad.net,

Message:
Please take a look.

Description:
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://code.launchpad.net/~frankban/juju-gui/bug-1075672-icons/+merge/133944

(do not edit description out of merge proposal)


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

Affected files:
   A [revision details]
   A app/assets/images/back_triangle.png
   M app/index.html
   M app/templates/charm-description.handlebars
   M app/templates/charm-pre-configuration.handlebars
   M app/views/charm-panel.js
   M lib/views/stylesheet.less
   M test/test_charm_panel.js
   M undocumented


Index: [revision details]
=== added file '[revision details]'
--- [revision details]  2012-01-01 00:00:00 +0000
+++ [revision details]  2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: [email protected]
+New revision:  
[email protected]

Index: app/index.html
=== modified file 'app/index.html'
--- app/index.html      2012-11-07 21:52:54 +0000
+++ app/index.html      2012-11-12 14:46: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"  
/>


Index: app/templates/charm-description.handlebars
=== 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 14:46: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>


Index: app/assets/images/back_triangle.png
=== 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 14:46:20 +0000 differ

Index: app/templates/charm-pre-configuration.handlebars
=== 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 14:46: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">


Index: app/views/charm-panel.js
=== 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:19:43 +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


Index: lib/views/stylesheet.less
=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less   2012-11-09 14:30:56 +0000
+++ lib/views/stylesheet.less   2012-11-12 14:46: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;


Index: test/test_charm_panel.js
=== 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 14:52:08 +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.
    });



Index: undocumented
=== modified file 'undocumented'
--- undocumented        2012-11-09 11:45:00 +0000
+++ undocumented        2012-11-12 15:19:43 +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





-- 
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.

-- 
Mailing list: https://launchpad.net/~yellow
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~yellow
More help   : https://help.launchpad.net/ListHelp

Reply via email to