← Back to team overview

yellow team mailing list archive

Re: Charm config panel visual design (issue 6709062)

 

Thanks for the review and the trouble shooting.  Gary.  Your issues have
been addressed.


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">
On 2012/10/24 14:43:47, gary.poster wrote:
> 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

Done.

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;
When used in more places the actual color can be defined and these
variables can reference it or the variable can be made more generic.

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. */
On 2012/10/24 14:43:47, gary.poster wrote:
> I think this is good now.

Done.

https://codereview.appspot.com/6709062/diff/3001/lib/views/stylesheet.less#newcode1096
lib/views/stylesheet.less:1096: z-index:1;
On 2012/10/24 14:43:47, gary.poster wrote:
> Now that the cog is in a css background, the z-index is unnecessary

Done.

https://codereview.appspot.com/6709062/diff/3001/lib/views/stylesheet.less#newcode1102
lib/views/stylesheet.less:1102: z-index:1;
On 2012/10/24 14:43:47, gary.poster wrote:
> as above.

Done.

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