Fixes #1173105 - Added borders between charms

Fixes #1173105 - Added borders between charms



https://code.launchpad.net/~huwshimi/juju-gui/charm-token-dividers-1173105/+merge/162295

(do not edit description out of merge proposal)

[huwshimi, 2013-05-03 06:54:09]

Please take a look.

[curtis, 2013-05-03 14:08:50]

Thanks Huw. LGTM. I have a few stylistic remarks that you can choose to address
before landing.

https://codereview.appspot.com/9159044/diff/1/app/widgets/charm-container.js
File app/widgets/charm-container.js (right):

https://codereview.appspot.com/9159044/diff/1/app/widgets/charm-container.js#...
app/widgets/charm-container.js:73: _selectLast: function() {
The indention in this function is inconsistent. Is there ever more than one node
with with the 'last' class in the search? The last line of this function implies
no. if no, the three lines with 'existing' can become this single line:

contentBox.all('.yui3-charmtoken.last').removeCalss('last')

[gary.poster, 2013-05-03 14:14:53]

LGTM assuming sinzui has qa'd it.  If not lemme know and I will.

https://codereview.appspot.com/9159044/diff/1/app/widgets/charm-container.js
File app/widgets/charm-container.js (right):

https://codereview.appspot.com/9159044/diff/1/app/widgets/charm-container.js#...
app/widgets/charm-container.js:73: _selectLast: function() {
On 2013/05/03 14:08:51, curtis wrote:
> The indention in this function is inconsistent. 

It's as consistent as it gets, I think: you can't propose code without getting
the google closure linter to bless the indentation et al.  This looks about
right to me, FBOW.

[curtis, 2013-05-03 15:31:27]

On 2013/05/03 14:14:53, gary.poster wrote:
> LGTM assuming sinzui has qa'd it.  If not lemme know and I will.

Yes I did QA it, run it to see it.

[gary.poster, 2013-05-03 15:35:28]

On 2013/05/03 15:31:27, curtis wrote:
> On 2013/05/03 14:14:53, gary.poster wrote:
> > LGTM assuming sinzui has qa'd it.  If not lemme know and I will.
> 
> Yes I did QA it

Cool, sounds good.  Thanks.

[huwshimi, 2013-05-07 19:28:09]

On 2013/05/03 14:08:50, curtis wrote:
> Thanks Huw. LGTM. I have a few stylistic remarks that you can choose to
address
> before landing.
> 
> https://codereview.appspot.com/9159044/diff/1/app/widgets/charm-container.js
> File app/widgets/charm-container.js (right):
> 
>
https://codereview.appspot.com/9159044/diff/1/app/widgets/charm-container.js#...
> app/widgets/charm-container.js:73: _selectLast: function() {
> The indention in this function is inconsistent. Is there ever more than one
node
> with with the 'last' class in the search? The last line of this function
implies
> no. if no, the three lines with 'existing' can become this single line:
> 
> contentBox.all('.yui3-charmtoken.last').removeCalss('last')

You are correct there will only ever be one (or none) so I've cleaned this up.

[huwshimi, 2013-05-07 20:34:51]

*** Submitted:

Fixes #1173105 - Added borders between charms

R=curtis, gary.poster
CC=
https://codereview.appspot.com/9159044