*** Submitted: CSS styling for buttons
* Apply CSS styling to buttons to get them closer to the visual design assets. * Add a top-level rule to specify font-family which is needed to override the bootstrap setting rule. * Drive-by: remove redundant 'font-family' rules. * Drive-by: fix lint-yuidoc. R=thiago, gary.poster CC= https://codereview.appspot.com/6811062 https://codereview.appspot.com/6811062/diff/5001/bin/lint-yuidoc File bin/lint-yuidoc (right): https://codereview.appspot.com/6811062/diff/5001/bin/lint-yuidoc#newcode90 bin/lint-yuidoc:90: dirs[:] = remove('assets', dirs) On 2012/11/01 15:06:30, gary.poster wrote: > I'd remove the function call and do it locally, once you remove the other use of > the function below. (I'd also switch to "try: dirs.remove('assets') except > ValueError: pass" but that's just me). > If you like it, do it. Done. https://codereview.appspot.com/6811062/diff/5001/bin/lint-yuidoc#newcode93 bin/lint-yuidoc:93: files = filter(lambda x: x.endswith('.js'), files) On 2012/11/01 15:06:30, gary.poster wrote: > Without much excitement, I suggest you remove the "remove" function and > consolidate this into one pass through the names. I'd lean towards the list > comprehension, so I'd replace lines 92 and 93 with > files = [i for i in files if i.endswith('.js') and i != 'templates.js'] > As you wish. Done. https://codereview.appspot.com/6811062/diff/5001/lib/views/stylesheet.less File lib/views/stylesheet.less (right): https://codereview.appspot.com/6811062/diff/5001/lib/views/stylesheet.less#newcode743 lib/views/stylesheet.less:743: .button-colors(@top-color, @bottom-color, @shadow-color, @v-pos) { On 2012/11/01 15:06:30, gary.poster wrote: > What do you think about defining this function at the top level, and thus making > it available generally? I think that's what we will want. Done. https://codereview.appspot.com/6811062/diff/5001/lib/views/stylesheet.less#newcode744 lib/views/stylesheet.less:744: //@shadow-color: blue; On 2012/11/01 12:03:35, thiago wrote: > This line is commented out. Probably you can remove it. Done. https://codereview.appspot.com/6811062/diff/5001/lib/views/stylesheet.less#newcode746 lib/views/stylesheet.less:746: @gradient-start: @top-color; On 2012/11/01 12:03:35, thiago wrote: > You could use the top-color and botton-color instead of the new pointers, right? > If you want less code changes, you can change the parameter names to > gradient-start and gradient-end. Done. https://codereview.appspot.com/6811062/diff/5001/lib/views/stylesheet.less#newcode757 lib/views/stylesheet.less:757: box-shadow: 0 (@v-pos + 3) @blur -@blur @shadow-color inset, 2px 0 3px -2px @shadow-color inset, -2px 0 3px -2px @shadow-color inset; I removed the browser-specific entries and regenerated the CSS. LESS did not put in those browser-specific rules so I have left them in stylesheet.less. https://codereview.appspot.com/6811062/diff/5001/lib/views/stylesheet.less#newcode789 lib/views/stylesheet.less:789: .button-colors(@charm-panel-cancel-button-color, @charm-panel-cancel-button-color, @charm-panel-cancel-button-shadow, 1px); On 2012/11/01 15:06:30, gary.poster wrote: > We generally are using four space indents in this file. Done. https://codereview.appspot.com/6811062/ -- https://code.launchpad.net/~bac/juju-gui/css-buttons/+merge/132365 Your team Juju GUI Hackers is requested to review the proposed merge of lp:~bac/juju-gui/css-buttons into lp:juju-gui. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
