Display category icon vs default if none provided

Display category icon vs default if none provided

- Add pngs of the icons at the sizes we require for display.
- Update the charm-token to determine if it needs to display the category icon
and if so, generate the correct sprite class name.
- Add an if_eq handlebars helper so that we can directly check the size of the
token and generate the right class for the sprite.
- Add support to the charm details page to display the category icon
- Add the categories and the mainCategory to the model
- Add tests around the handlebars helper and generation of the classnames.

Notes:
Does not currently handle the issue of multiple categories on a charm though
we do support that in the data. It's not currently used and requires a lot
more logic and thinking through to determine which category to display in
different cases. Potential follow-up if it becomes an issue.

https://code.launchpad.net/~rharding/juju-gui/category-icon-fallback/+merge/167340

(do not edit description out of merge proposal)

[rharding, 2013-06-04 16:49:23]

Please take a look.

[rharding, 2013-06-04 16:52:59]

Please take a look.

[jeff.pihach, 2013-06-04 17:07:48]

LGTM with a few improvements in the tests.

I do not think we should have the kind of logic in the templates you have with
if_eq. I would propose moving the conditional inside the helper and having the
helper return with the proper value.

https://codereview.appspot.com/9975047/diff/1031/app/templates/charm-token.ha...
File app/templates/charm-token.handlebars (right):

https://codereview.appspot.com/9975047/diff/1031/app/templates/charm-token.ha...
app/templates/charm-token.handlebars:13: <div class="category-icon sprite
charm-{{mainCategory}}-{{#if_eq size 'small'}}64{{else}}96{{/if_eq}}">
I don't think that this logic belongs in the template

https://codereview.appspot.com/9975047/diff/1031/test/test_browser_charm_deta...
File test/test_browser_charm_details.js (right):

https://codereview.appspot.com/9975047/diff/1031/test/test_browser_charm_deta...
test/test_browser_charm_details.js:681: Y.io('data/browsercharm.json', {sync:
true}).responseText);
there is a method called loadFixture in utils which we could implement caching
in this should probably be changed over there to support the upgrade.

https://codereview.appspot.com/9975047/diff/1031/test/test_browser_charm_deta...
test/test_browser_charm_details.js:686: view = new CharmView({
see makeContainer in utils to avoid rendering this into the DOM and causing a
flash

https://codereview.appspot.com/9975047/diff/1031/test/test_browser_charm_deta...
test/test_browser_charm_details.js:693:
iconNode.hasClass('charm-app-servers-160').should.eql(true);
I believe we had a discussion that we are trying to get away from these types of
tests in favour of 

assert.equal(iconNode.hasClass('charm-app-servers-160'), true);

https://codereview.appspot.com/9975047/diff/1031/test/test_charm_token.js
File test/test_charm_token.js (right):

https://codereview.appspot.com/9975047/diff/1031/test/test_charm_token.js#new...
test/test_charm_token.js:100:
iconNode.hasClass('charm-app-servers-64').should.eql(true);
See the note about assertion lib in previous file

https://codereview.appspot.com/9975047/diff/1031/test/test_charm_token.js#new...
test/test_charm_token.js:118:
iconNode.hasClass('charm-app-servers-96').should.eql(true);
See the note about assertion lib in previous file

[curtis, 2013-06-04 17:47:35]

LGTM.

I have a grammatical fix request inline and implementation suggestion to add to
Jeff's (and your's) ick reaction to the logic in the templates.

https://codereview.appspot.com/9975047/diff/1031/app/models/charm.js
File app/models/charm.js (right):

https://codereview.appspot.com/9975047/diff/1031/app/models/charm.js#newcode509
app/models/charm.js:509: @return {String|Null} If a category is found, it's
value else null.
s/it's/its/

https://codereview.appspot.com/9975047/diff/1031/app/templates/charm-token.ha...
File app/templates/charm-token.handlebars (right):

https://codereview.appspot.com/9975047/diff/1031/app/templates/charm-token.ha...
app/templates/charm-token.handlebars:13: <div class="category-icon sprite
charm-{{mainCategory}}-{{#if_eq size 'small'}}64{{else}}96{{/if_eq}}">
If the size value was a number <64|96>, or the css class was a word
<small|large> this case would be to simply put the size value in the class. This
case is awkward because we need to translate two different terminologies. This
mapping could be another attribute such as cssSize.

[rharding, 2013-06-05 12:35:01]

*** Submitted:

Display category icon vs default if none provided

- Add pngs of the icons at the sizes we require for display.
- Update the charm-token to determine if it needs to display the category icon
and if so, generate the correct sprite class name.
- Add an if_eq handlebars helper so that we can directly check the size of the
token and generate the right class for the sprite.
- Add support to the charm details page to display the category icon
- Add the categories and the mainCategory to the model
- Add tests around the handlebars helper and generation of the classnames.

Notes:
Does not currently handle the issue of multiple categories on a charm though
we do support that in the data. It's not currently used and requires a lot
more logic and thinking through to determine which category to display in
different cases. Potential follow-up if it becomes an issue.

R=jeff.pihach, curtis
CC=
https://codereview.appspot.com/9975047