yellow team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Reviewers: mp+134961_code.launchpad.net,

Message:
Please take a look.

Description:
Fix overflow on charm name.

The sections in the charm search results pane are fixed at 70px high.
Due to the way we construct the name, and the fact the charm authors
can set the name to be anything they want, some names exceed the
allotted one-line and wreak havoc on the layout.

Obvious lesson learned: any element with a fixed height and
user-supplied data must account for overflow.

The deploy button placement was also fixed.

Matt C. has seen screenshots of the truncation and offered tweaks on
the vertical spacing which are incorporated here.

https://code.launchpad.net/~bac/juju-gui/charm-spill/+merge/134961

(do not edit description out of merge proposal)


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

Affected files:
   A [revision details]
   M app/templates/charm-search-result.handlebars
   M app/views/charm-panel.js
   M lib/views/stylesheet.less


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: matthew.scott@xxxxxxxxxxxxx-20121116170946-hm7no35u3yqmim1a
+New revision: bac@xxxxxxxxxxxxx-20121119163118-pc2ym2fjkpnb53jx

Index: app/templates/charm-search-result.handlebars
=== modified file 'app/templates/charm-search-result.handlebars'
--- app/templates/charm-search-result.handlebars	2012-11-15 14:33:54 +0000
+++ app/templates/charm-search-result.handlebars	2012-11-19 16:29:35 +0000
@@ -9,8 +9,11 @@
            <div class="charm-div">
              <button class="btn btn-primary deploy"
               data-url="{{id}}">Deploy</button>
-            <a class="charm-detail" href="{{id}}">
-              {{#if owner}}{{owner}}/{{/if}}{{package_name}}</a>
+            {{! It is is important that the following 'href' line not be
+             broken, or the string processed by the ellipsis module will
+             include a lot of whitespace. }}
+            <a class="charm-detail"
+               href="{{id}}">{{#if  
owner}}{{owner}}/{{/if}}{{package_name}}</a>
              <div class="charm-summary">{{summary}}</div>
            </div>
          </li>


Index: app/views/charm-panel.js
=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2012-11-16 17:06:36 +0000
+++ app/views/charm-panel.js	2012-11-19 16:31:18 +0000
@@ -181,6 +181,7 @@
            raw_entries = searchText ? resultEntries : defaultEntries,
            entries = raw_entries && makeRenderableResults(raw_entries);
        container.setHTML(this.template({ charms: entries }));
+      container.all('.charm-detail').ellipsis();
        container.all('.charm-summary').ellipsis({'lines': 2});
        this._setScroll();
        return this;


Index: lib/views/stylesheet.less
=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2012-11-16 17:06:36 +0000
+++ lib/views/stylesheet.less	2012-11-19 16:29:35 +0000
@@ -1109,7 +1109,7 @@
                  border-bottom: 1px solid #BDBDBD;
                  height: 70px;
                  .charm-div {
-                    padding-top: 6px;
+                    padding-top: 11px;
                  }
                  &:last-child {
                      border-bottom: none;
@@ -1123,24 +1123,27 @@
                  &:hover {
                      background-color: white;
                  }
+                @charm-panel-text-width: 190px;
                  a.charm-detail {
-                    padding-top: 5px;
+                    display: block;
+                    width: @charm-panel-text-width;
                      font-weight: normal;
                      font-size: 16px;
                      color: @charm-panel-orange;
                      line-height: 18px;
                  }
                  .charm-summary {
+                    padding-top: 4px;
                      margin-bottom: 4px;
-                    width: 190px;
+                    width: @charm-panel-text-width;
                      font-size: 12px;
                      line-height: 14px;
-                    padding-top: 4px;
                  }
                  .btn {
                      float: right;
                      opacity: 0;
-                    margin-top: 20px;
+                    margin-top: 11px;
+                    margin-right: 8px;
                  }
              }
          }





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