yellow team mailing list archive

[Brad Crittenden <bac@xxxxxxxxxxxxx>]

Nice changes Francesco.

I think the config file button still needs some attention.  I find it
odd that it isn't part of the collapsible div and remains visible when
Service Settings is collapsed. Did you discuss that UX with Matt or
Jovan?  Of course I defer to them but think it currently looks funny.

Also, when a file is chosen, Service Settings collapses but the chevron
still points down.  Further you can click on it and the chevron changes
but nothing else happens.  I guess it'd be nice to collapse, set the
chevron to up, and lock it in place.


https://codereview.appspot.com/6827066/diff/1/app/views/charm-panel.js
File app/views/charm-panel.js (right):

https://codereview.appspot.com/6827066/diff/1/app/views/charm-panel.js#newcode601
app/views/charm-panel.js:601: */
Nice, thanks.

https://codereview.appspot.com/6827066/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/6827066/diff/1/lib/views/stylesheet.less#newcode43
lib/views/stylesheet.less:43:
Thanks!  Maybe 'set-' instead of 'create-' for both?

https://codereview.appspot.com/6827066/diff/1/lib/views/stylesheet.less#newcode987
lib/views/stylesheet.less:987: padding-left: 5px;
The vis design shows this lined up with the other elements and the charm
icon up top.  I think the padding-left should be 11px.

Good to see you and Matt discussing it.

And as I mentioned in IRC we should decide on one value and apply it to
all charm panels, abstracting it into a LESS variable.

https://codereview.appspot.com/6827066/diff/1/undocumented
File undocumented (left):

https://codereview.appspot.com/6827066/diff/1/undocumented#oldcode82
undocumented:82: app/views/charm-panel.js render
yay

https://codereview.appspot.com/6827066/

-- 
https://code.launchpad.net/~frankban/juju-gui/bug-1075679-charm-config-panel/+merge/133657
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~frankban/juju-gui/bug-1075679-charm-config-panel into lp:juju-gui.