← Back to team overview

yellow team mailing list archive

Re: Charm config panel visual design (issue 6709062)

 

Approved, with a small comment or two.  Thanks!

Gary


https://codereview.appspot.com/6709062/diff/3001/app/templates/charm-pre-configuration.handlebars
File app/templates/charm-pre-configuration.handlebars (right):

https://codereview.appspot.com/6709062/diff/3001/app/templates/charm-pre-configuration.handlebars#newcode11
app/templates/charm-pre-configuration.handlebars:11: class="btn
btn-primary deploy left">
As I mentioned, I have a small suggestion that it would be nice to not
introduce the new "left" class and instead only apply the "float: right"
to the buttons in the two other specific pages in the charm panel.  In
other words, I think we can do this pretty cleanly in the CSS without
introducing even more classes.  You said you did it already. :-) Thank
you

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

https://codereview.appspot.com/6709062/diff/3001/lib/views/stylesheet.less#newcode11
lib/views/stylesheet.less:11: @charm-panel-separator-color: #aeaeae;
Thank you, cool.

It worries me a bit for our design that these colors are so unique.  I
feel like they should apply to many things.  That's a question for the
designer though and doesn't affect this review...I'm just talking. :-)

https://codereview.appspot.com/6709062/diff/3001/lib/views/stylesheet.less#newcode1083
lib/views/stylesheet.less:1083: /* XXX: The positioning here feels like
a big hack.  Suggestions welcome. */
I think this is good now.

https://codereview.appspot.com/6709062/diff/3001/lib/views/stylesheet.less#newcode1096
lib/views/stylesheet.less:1096: z-index:1;
Now that the cog is in a css background, the z-index is unnecessary

https://codereview.appspot.com/6709062/diff/3001/lib/views/stylesheet.less#newcode1102
lib/views/stylesheet.less:1102: z-index:1;
as above.

https://codereview.appspot.com/6709062/

-- 
https://code.launchpad.net/~bac/juju-gui/charm-assets/+merge/130636
Your team Juju GUI Hackers is subscribed to branch lp:juju-gui.


References