Service Icon + Misc Fix

Service Icon + Misc Fix

Include service icon on inspector pane.
Fix status bar label ordering in DOM so they always appear.

https://code.launchpad.net/~bcsaller/juju-gui/service-icon/+merge/174276

(do not edit description out of merge proposal)

[bcsaller, 2013-07-11 19:21:59]

Please take a look.

[gary.poster, 2013-07-11 19:34:05]

Code LGTM.

https://codereview.appspot.com/11181043/diff/1/app/views/service.js
File app/views/service.js (right):

https://codereview.appspot.com/11181043/diff/1/app/views/service.js#newcode1440
app/views/service.js:1440: icon = Y.one(node).append('<img>');
If we could get the category icon that would be preferable.  Is that an easy
option?  If not, maybe file a bug?

[benjamin.saller, 2013-07-11 19:56:39]

On 2013/07/11 19:34:05, gary.poster wrote:
> Code LGTM.
> 
> https://codereview.appspot.com/11181043/diff/1/app/views/service.js
> File app/views/service.js (right):
> 
>
https://codereview.appspot.com/11181043/diff/1/app/views/service.js#newcode1440
> app/views/service.js:1440: icon = Y.one(node).append('<img>');
> If we could get the category icon that would be preferable.  Is that an easy
> option?  If not, maybe file a bug?

Actually it looks like there might be a few problems with the icon handling
though they don't come from this branch. Ideally asking for the icon for the
service should return the best match. This could work if service.get('icon')
operated as a read through cache of the best match from the charm browser.
However it looks like icon is statically assigned only when ghosted through the
GUI. This means CLI deployed environment won't have icons bound to them. I'd
suggest that we leave this in place for now and that the longer term interaction
of GUI models with the charm store API be a short round table meeting at the
sprint. Is that ok with you?

[gary.poster, 2013-07-11 20:41:54]

On 2013/07/11 19:56:39, benjamin.saller wrote:
> On 2013/07/11 19:34:05, gary.poster wrote:
> > Code LGTM.
> > 
> > https://codereview.appspot.com/11181043/diff/1/app/views/service.js
> > File app/views/service.js (right):
> > 
> >
>
https://codereview.appspot.com/11181043/diff/1/app/views/service.js#newcode1440
> > app/views/service.js:1440: icon = Y.one(node).append('<img>');
> > If we could get the category icon that would be preferable.  Is that an easy
> > option?  If not, maybe file a bug?
> 
> Actually it looks like there might be a few problems with the icon handling
> though they don't come from this branch. Ideally asking for the icon for the
> service should return the best match. This could work if service.get('icon')
> operated as a read through cache of the best match from the charm browser.
> However it looks like icon is statically assigned only when ghosted through
the
> GUI. This means CLI deployed environment won't have icons bound to them. I'd
> suggest that we leave this in place for now and that the longer term
interaction
> of GUI models with the charm store API be a short round table meeting at the
> sprint. Is that ok with you?

Yeah, I was planning that.  Could you add this as a note to
https://bugs.launchpad.net/juju-gui/+bug/1200008 and mark this locally as an
XXX?

Thanks

[rharding, 2013-07-12 00:24:45]

LGTM, this works for reviewed charms with an icon. As noted, we can work out a
better way to pull the icon logic into a better shared location apart from this.

[bcsaller, 2013-07-12 01:37:52]

*** Submitted:

Service Icon + Misc Fix

Include service icon on inspector pane.
Fix status bar label ordering in DOM so they always appear.

R=gary.poster, benjamin.saller, rharding
CC=
https://codereview.appspot.com/11181043