Code review comment for lp:~rharding/juju-gui/design-updates

Revision history for this message
Richard Harding (rharding) wrote :

Reviewers: mp+158363_code.launchpad.net,

Message:
Please take a look.

Description:
Adjust design per review, add truncate helper

- Per UX reviews adjust some of the design of the browser
- Add a truncate handlebars helper to aid in fitting content nicely

https://code.launchpad.net/~rharding/juju-gui/design-updates/+merge/158363

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   D app/assets/images/charm_commits_icon.jpg
   D app/assets/images/charm_downloads_icon.jpg
   M app/subapps/browser/templates/browser_charm.handlebars
   M app/views/utils.js
   M test/test_utils.js

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 address hidden>
+New revision: <email address hidden>

Index: test/test_utils.js
=== modified file 'test/test_utils.js'
--- test/test_utils.js 2013-04-10 21:17:18 +0000
+++ test/test_utils.js 2013-04-11 13:11:02 +0000
@@ -599,6 +599,38 @@
        assert.equal('fooi', html);
      });

+ it('truncates a string', function() {
+ var source = '{{ truncate text 30 }}',
+ template = Y.Handlebars.compile(source),
+ context = {text: 'Lorem ipsum dolor sit amet consectetur'},
+ html = template(context);
+ assert.equal('Lorem ipsum dolor sit amet con...', html);
+ });
+
+ it('truncates a string with a trailing space', function() {
+ var source = '{{ truncate text 30 }}',
+ template = Y.Handlebars.compile(source),
+ context = {text: 'Lorem ipsum dolor sit ametuco sectetur'},
+ html = template(context);
+ assert.equal('Lorem ipsum dolor sit ametuco...', html);
+ });
+
+ it('does not truncate a shorter string', function() {
+ var source = '{{ truncate text 30 }}',
+ template = Y.Handlebars.compile(source),
+ context = {text: 'Lorem ipsum dolor sit amet'},
+ html = template(context);
+ assert.equal('Lorem ipsum dolor sit amet', html);
+ });
+
+ it('truncate handles an undefined value', function() {
+ var source = '{{ truncate text 30 }}is empty',
+ template = Y.Handlebars.compile(source),
+ context = {text: undefined},
+ html = template(context);
+ assert.equal('is empty', html);
+ });
+
      describe('showStatus', function() {
        var html, obj, template;

Index: app/views/utils.js
=== modified file 'app/views/utils.js'
--- app/views/utils.js 2013-04-10 21:17:18 +0000
+++ app/views/utils.js 2013-04-11 12:47:10 +0000
@@ -1250,6 +1250,21 @@
          }
        });

+ /**
+ * Truncate helper to keep text sizes to a specified limit.
+ *
+ * {{truncate field 100}}
+ *
+ */
+ Y.Handlebars.registerHelper('truncate', function(string, length) {
+ if (string && string.length > length) {
+ return Y.Lang.trimRight(string.substring(0, length)) + '...';
+ }
+ else {
+ return string;
+ }
+ });
+
  }, '0.1.0', {
    requires: ['base-build',
               'handlebars',

Index: app/assets/images/charm_commits_icon.jpg
=== removed file 'app/assets/images/charm_commits_icon.jpg'
Binary files app/assets/images/charm_commits_icon.jpg 2013-04-03 06:23:33
+0000 and app/assets/images/charm_commits_icon.jpg 1970-01-01 00:00:00
+0000 differ

Index: app/assets/images/charm_downloads_icon.jpg
=== removed file 'app/assets/images/charm_downloads_icon.jpg'
Binary files app/assets/images/charm_downloads_icon.jpg 2013-04-03 06:23:33
+0000 and app/assets/images/charm_downloads_icon.jpg 1970-01-01 00:00:00
+0000 differ

Index: app/subapps/browser/templates/browser_charm.handlebars
=== modified file 'app/subapps/browser/templates/browser_charm.handlebars'
--- app/subapps/browser/templates/browser_charm.handlebars 2013-04-10
18:10:06 +0000
+++ app/subapps/browser/templates/browser_charm.handlebars 2013-04-11
05:11:21 +0000
@@ -29,12 +29,10 @@
                  <dl>
                      <dt>Recent activity</dt>
                      <dd>
- <img
src="/juju-ui/assets/images/charm_downloads_icon.jpg"
- alt="Minimize" />
                          {{ recent_download_count }}
- <img
src="/juju-ui/assets/images/charm_commits_icon.jpg"
- alt="Minimize" />
+ {{pluralize 'download' recent_download_count}},
                          {{ recent_commit_count }}
+ {{pluralize 'commit' recent_commit_count}}
                      </dd>
                      <dt>Ubuntu series</dt>
                      <dd>{{ distro_series }}</dd>
@@ -53,7 +51,11 @@
              <h2>Summary</h2>
              {{ summary }}
              <h2>Description</h2>
- {{ description }}
+ {{#if isFullscreen}}
+ {{ description }}
+ {{else}}
+ {{ truncate description 300 }}
+ {{/if}}
          </div>

          <div class="changelog">
@@ -101,12 +103,9 @@
                          other charms.
                      </p>
                      <div class="yui3-g">
- <div class="interfaces-heading yui3-u-1-3">
+ <div class="interfaces-heading yui3-u-1">
                              <h3>Requires</h3>
                          </div>
- <div class="interfaces-heading yui3-u-2-3">
- <h3>Related Charms</h3>
- </div>
                      </div>
                      {{#if requires}}
                          <ul class="interface-list">
@@ -116,9 +115,6 @@
                                          <h4>{{value.interface}}</h4>
                                          <div>{{key}}</div>
                                      </div>
- <div class="related yui3-u-2-3">
- &hellip;no related charms yet
- </div>
                                  </li>
                              {{/arrayObject}}
                          </ul>
@@ -126,12 +122,9 @@
                          <p>This charm does not require any interfaces.</p>
                      {{/if}}
                      <div class="yui3-g">
- <div class="interfaces-heading yui3-u-1-3">
+ <div class="interfaces-heading yui3-u-1">
                              <h3>Provides</h3>
                          </div>
- <div class="interfaces-heading yui3-u-2-3">
- <h3>Related Charms</h3>
- </div>
                      </div>
                      {{#if provides}}
                          <ul class="interface-list">
@@ -141,9 +134,6 @@
                                          <h4>{{value.interface}}</h4>
                                          <div>{{key}}</div>
                                      </div>
- <div class="related yui3-u-2-3">
- &hellip;no related charms yet
- </div>
                                  </li>
                              {{/arrayObject}}
                          </ul>

« Back to merge proposal