yellow team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Brad Crittenden has proposed merging lp:~bac/juju-gui/charm-search-style-2 into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)

For more details, see:
https://code.launchpad.net/~bac/juju-gui/charm-search-style-2/+merge/134355

Charm search results upated styling

After the last changes Matt made some styling suggestions.  His
requirements are in the 'store_front_review.pdf' on the Google drive.

One requirement was that the charm sections be a fixed height of 70px
regardless of how long the charm summary is.  This required we find a
solution for truncating the text cleanly.  The YUI gallery has
'gallery-ellipsis', which I've chosen to use.  In order for it to
work, the width of the element must be used, instead of using
padding-right to cause wrapping.  Once that was done, and the width of
the float:right Deploy button was taken into account it all worked
well.

Matt asked for the filter drop down box but since it is not well
spec'ed yet and not Ready To Code I decided to defer adding it at this
time.

https://codereview.appspot.com/6846053/

-- 
https://code.launchpad.net/~bac/juju-gui/charm-search-style-2/+merge/134355
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~bac/juju-gui/charm-search-style-2 into lp:juju-gui.

=== modified file 'Makefile'
--- Makefile	2012-11-09 15:37:08 +0000
+++ Makefile	2012-11-14 18:29:21 +0000
@@ -66,7 +66,7 @@
 
 spritegen: $(SPRITE_GENERATED_FILES)
 
-$(COMPRESSED_FILES): node_modules/yui node_modules/d3/d3.v2.min.js $(JSFILES)
+$(COMPRESSED_FILES): node_modules/yui node_modules/d3/d3.v2.min.js $(JSFILES) ./bin/merge-files
 	@rm -Rf app/assets/javascripts/generated/
 	@mkdir app/assets/javascripts/generated/
 	@./bin/merge-files

=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2012-11-13 15:27:51 +0000
+++ app/views/charm-panel.js	2012-11-14 18:29:21 +0000
@@ -107,6 +107,7 @@
               Y.Object.values(data),
               function(val) { return val['interface']; });
         }
+        return undefined;
       };
 
   /**
@@ -180,6 +181,7 @@
           raw_entries = searchText ? resultEntries : defaultEntries,
           entries = raw_entries && makeRenderableResults(raw_entries);
       container.setHTML(this.template({ charms: entries }));
+      container.all('.charm-summary').ellipsis({'lines': 2});
       this._setScroll();
       return this;
     },
@@ -492,7 +494,7 @@
   views.CharmDescriptionView = CharmDescriptionView;
 
   /**
-   * Display a unit.
+   * Display a charms configuration panel.
    *
    * @class CharmConfigurationView
    * @namespace views
@@ -1053,6 +1055,7 @@
     'overlay',
     'dom-core',
     'juju-models',
-    'event-resize'
+    'event-resize',
+    'gallery-ellipsis'
   ]
 });

=== modified file 'bin/merge-files'
--- bin/merge-files	2012-11-13 13:55:16 +0000
+++ bin/merge-files	2012-11-14 18:29:21 +0000
@@ -10,7 +10,7 @@
  *
  * The final product will provide three js files: one for the YUI dependencies,
  * one for our custom js code and one for third party js like d3.
- * 
+ *
  * Known issues:
  * (1) If we set "bootstrap=false" in the GlobalConfig object, yui disables the
  *     loader object. It means it wont even try to download modules. We cannot
@@ -48,6 +48,7 @@
   reqs.push('parallel');
   reqs.push('app-transitions-native');
   reqs.push('gallery-markdown');
+  reqs.push('gallery-ellipsis');
   reqs.push('loader-base');
 
   // Get all of the YUI files and their dependencies, and combine them.

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2012-11-13 21:51:29 +0000
+++ lib/views/stylesheet.less	2012-11-14 18:29:21 +0000
@@ -1032,6 +1032,7 @@
         background-color: #EEEEEE;
         width: @charm-panel-width - 1px;
         float: right;
+        border-top: 1px solid #989898;
         .series-charms {
             h3 {
                 font-weight: normal;
@@ -1039,7 +1040,8 @@
                 text-transform: capitalize;
                 padding-left: @charm-panel-padding-left;
                 background-color: #CBCBCB;
-                border-top: 1px solid #989898;
+                height: 20px;
+                line-height: 16px;
             }
             ul {
                 margin-bottom: 0;
@@ -1048,6 +1050,7 @@
                 padding: 11px @charm-panel-padding-left;
                 border-top: 1px solid #FFFFFF;
                 border-bottom: 1px solid #BDBDBD;
+                height: 70px;
                 &:first-child {
                     padding-top: 11px;
                 }
@@ -1071,6 +1074,7 @@
                 }
                 .charm-summary {
                     margin-bottom: 4px;
+                    width: 190px;
                     font-size: 12px;
                     line-height: 14px;
                     padding-top: 6px;
@@ -1078,6 +1082,7 @@
                 .btn {
                     float: right;
                     opacity: 0;
+                    margin-top: 20px;
                 }
             }
         }

=== modified file 'test/test_app.js'
--- test/test_app.js	2012-10-23 19:54:44 +0000
+++ test/test_app.js	2012-11-14 18:29:21 +0000
@@ -42,7 +42,7 @@
 
   beforeEach(function(done) {
     //  XXX Apparently removing a DOM node is asynchronous (on Chrome at least)
-    //  and we occasionally loose the race if this code is in the afterEach
+    //  and we occasionally lose the race if this code is in the afterEach
     //  function, so instead we do it here, but only if one has previously been
     //  created.
     if (container) {