css hack n' slash, continued: New styles for buttons and selects.

css hack n' slash, continued: New styles for buttons and selects.

I am moving all widget styles into button.css. I will later rename it widget.css. This way, the rules that are common can be shared.

The next step will be to update/merge checkbox.css into button.css.

BUG=103688, 115404
TEST=visual


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124845

[Evan Stade, 2012-03-02 00:55:23]

dpapad: print preview
csilv/dbeam: everything else (shared and options2)

[csilv, 2012-03-02 02:04:05]

woot!  lgtm

[dpapad, 2012-03-02 02:49:01]

On 2012/03/02 00:55:23, Evan Stade wrote:
> dpapad: print preview
> csilv/dbeam: everything else (shared and options2)

Noticed 3 regression with this change. The arrow in the dropdown list in print
preview disappears when the list is focused. Also the print/cancel buttons are
closet together and the "Print" text is not bold.

[Evan Stade, 2012-03-02 03:36:33]

all 3 fixed.

[dpapad, 2012-03-02 16:34:09]

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.css (right):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:249: width: 2em;
Could we make this 2.75em, so that it can fit the maximum allowed number of
digits (which is 3)?

[Dan Beam, 2012-03-02 17:58:43]

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/login/login.html (left):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/login/login.html:8: <link rel="stylesheet"
href="chrome://resources/css/select.css">
I'd move button.css to the old position of select.css here and throughout, IMO,
as otherwise you might have cascade issues.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/proxy_settings.html (left):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/proxy_settings.html:8: <link rel="stylesheet"
href="chrome://resources/css/chrome_shared.css">
why isn't chrome_shared.css first?

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.css (right):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:249: width: 2em;
On 2012/03/02 16:34:10, dpapad wrote:
> Could we make this 2.75em, so that it can fit the maximum allowed number of
> digits (which is 3)? 

2.75em - why not 3em? confident that no number digit will ever take up as much
space as an m in any font? (even ... monospace! dun dun dun!)

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:337: display: none
!important;
this is in chrome_shared2.css now, isn' it?

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:386:
-webkit-animation-name: dancing-dots-jump;
you could probably collapse this to the shorthand -webkit-animation: eh?

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:404: .button-strip {
shouldn't you be namespacing these two rules as well? (here and below) or do we
just always count on these styles not conflicting with anything else or being
included after?

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
File chrome/browser/resources/shared/css/button.css (right):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:1: /* Copyright (c) 2012 The
Chromium Authors. All rights reserved.
can we name this like form_elements.css or something now that it affects
buttons, selects, input[type='button'], input[type='submit'],
input[type='search'] (and maybe throw checkboxes and radios in)?

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:11: select {
there's no .custom-appearance for selects?

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:13: border-radius: 2px;
so this intentionally changes the border-radius from 3px -> 2px?

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:18: font: inherit;
this also inherits line-height, font-weight, font-variant, and font-style, btw.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:19: height: 2em;
where did this come from?

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:41: background-image:
url("../images/select.png"),
single quotes

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:70: background-image:
url("../images/select.png"),
single quotes

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:99: background-image:
url("../images/select.png"),
here too

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:125: background-image:
url("../images/disabled_select.png"),
here too

[Evan Stade, 2012-03-02 19:46:20]

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/login/login.html (left):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/login/login.html:8: <link rel="stylesheet"
href="chrome://resources/css/select.css">
On 2012/03/02 17:58:43, Dan Beam wrote:
> I'd move button.css to the old position of select.css here and throughout,
IMO,
> as otherwise you might have cascade issues.

depending on order is inherently a bug imo (except maybe if it's two rules in
the same file --- even that is iffy though). I'd rather keep them alphabetical,
and if this uncovers a dormant ordering bug, we'll find it and squash it (also,
while it exists, it's not likely to be very high impact).

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/proxy_settings.html (left):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/proxy_settings.html:8: <link rel="stylesheet"
href="chrome://resources/css/chrome_shared.css">
On 2012/03/02 17:58:43, Dan Beam wrote:
> why isn't chrome_shared.css first?

I don't know. I didn't write this file. But I can fix it.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.css (right):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:249: width: 2em;
On 2012/03/02 17:58:43, Dan Beam wrote:
> On 2012/03/02 16:34:10, dpapad wrote:
> > Could we make this 2.75em, so that it can fit the maximum allowed number of
> > digits (which is 3)? 
> 
> 2.75em - why not 3em? confident that no number digit will ever take up as much
> space as an m in any font? (even ... monospace! dun dun dun!)

no, actually, in CJK fonts most characters are wider than an em. However they
still use arabic numerals AFAIK, and I don't think it's possible to change this
font to monospace (well anything is possible but I don't think it's easy). I
think 2.75em is a safe bet, and if you keep going wider it eventually just looks
bad; if 2.75 is not enough the consequences are non-disastrous.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:337: display: none
!important;
On 2012/03/02 17:58:43, Dan Beam wrote:
> this is in chrome_shared2.css now, isn' it?

yes. However print preview does not yet use chrome_shared2.css. That is a
project for another day.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:386:
-webkit-animation-name: dancing-dots-jump;
On 2012/03/02 17:58:43, Dan Beam wrote:
> you could probably collapse this to the shorthand -webkit-animation: eh?

I suppose. Can we just leave out the properties we don't want to change?

-webkit-animation: dancing-dots-jump 1800ms infinite;

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:404: .button-strip {
On 2012/03/02 17:58:43, Dan Beam wrote:
> shouldn't you be namespacing these two rules as well? (here and below) or do
we
> just always count on these styles not conflicting with anything else or being
> included after?

I think we're depending on not conflicting. If we had .button-strip in a shared
location we'd want to remove or at least change these rules here.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
File chrome/browser/resources/shared/css/button.css (right):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:11: select {
On 2012/03/02 17:58:43, Dan Beam wrote:
> there's no .custom-appearance for selects?

no. I suppose there could be but we haven't had a need yet.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:13: border-radius: 2px;
On 2012/03/02 17:58:43, Dan Beam wrote:
> so this intentionally changes the border-radius from 3px -> 2px?

yes

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:18: font: inherit;
On 2012/03/02 17:58:43, Dan Beam wrote:
> this also inherits line-height, font-weight, font-variant, and font-style,
btw.

yea. I don't think any of those will be a problem. Easy enough to revisit if we
do find an issue. (I haven't tested/perfected this all for multiple fonts and
font sizes, although I gave it a quick once over for a larger font. On Chromeos
we have issues, but they exist before this patch)

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:19: height: 2em;
On 2012/03/02 17:58:43, Dan Beam wrote:
> where did this come from?

in the mocks it was height: 24px. I changed it to 2em.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:41: background-image:
url("../images/select.png"),
On 2012/03/02 17:58:43, Dan Beam wrote:
> single quotes

Done.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:70: background-image:
url("../images/select.png"),
On 2012/03/02 17:58:43, Dan Beam wrote:
> single quotes

Done.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:99: background-image:
url("../images/select.png"),
On 2012/03/02 17:58:43, Dan Beam wrote:
> here too

Done.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:125: background-image:
url("../images/disabled_select.png"),
On 2012/03/02 17:58:43, Dan Beam wrote:
> here too

Done.

[Dan Beam, 2012-03-02 20:24:42]

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.css (right):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:249: width: 2em;
On 2012/03/02 19:46:20, Evan Stade wrote:
> On 2012/03/02 17:58:43, Dan Beam wrote:
> > On 2012/03/02 16:34:10, dpapad wrote:
> > > Could we make this 2.75em, so that it can fit the maximum allowed number
of
> > > digits (which is 3)? 
> > 
> > 2.75em - why not 3em? confident that no number digit will ever take up as
much
> > space as an m in any font? (even ... monospace! dun dun dun!)
> 
> no, actually, in CJK fonts most characters are wider than an em. However they
> still use arabic numerals AFAIK, and I don't think it's possible to change
this
> font to monospace (well anything is possible but I don't think it's easy). I
> think 2.75em is a safe bet, and if you keep going wider it eventually just
looks
> bad; if 2.75 is not enough the consequences are non-disastrous.

it will be possible when users are allowed to change the font of all WebUI
pages, but I agree we should probably optimize for the common case (of 2.75em
being the best looking).

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:337: display: none
!important;
On 2012/03/02 19:46:20, Evan Stade wrote:
> On 2012/03/02 17:58:43, Dan Beam wrote:
> > this is in chrome_shared2.css now, isn' it?
> 
> yes. However print preview does not yet use chrome_shared2.css. That is a
> project for another day.

ah, OK, that does sounds like a project, lol

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:386:
-webkit-animation-name: dancing-dots-jump;
On 2012/03/02 19:46:20, Evan Stade wrote:
> On 2012/03/02 17:58:43, Dan Beam wrote:
> > you could probably collapse this to the shorthand -webkit-animation: eh?
> 
> I suppose. Can we just leave out the properties we don't want to change?
> 
> -webkit-animation: dancing-dots-jump 1800ms infinite;

I think this would work fine (subsequent changes to -webkit-animation-* should
work the same way).

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:414: #print-preview
.button-strip button {
if we're depending on not conflicting with other .button-strip styles why is
this necessary? specificity? there should be nothing you need to override, eh?

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
File chrome/browser/resources/shared/css/button.css (right):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:19: height: 2em;
On 2012/03/02 19:46:20, Evan Stade wrote:
> On 2012/03/02 17:58:43, Dan Beam wrote:
> > where did this come from?
> 
> in the mocks it was height: 24px. I changed it to 2em.

ok, and I take it that's the same computed value, right? (16px * 75% * 2em =
24px)

[dpapad, 2012-03-02 21:19:11]

print preview: LGTM

[Evan Stade, 2012-03-02 21:47:02]

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
File chrome/browser/resources/print_preview/print_preview.css (right):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:386:
-webkit-animation-name: dancing-dots-jump;
On 2012/03/02 20:24:42, Dan Beam wrote:
> On 2012/03/02 19:46:20, Evan Stade wrote:
> > On 2012/03/02 17:58:43, Dan Beam wrote:
> > > you could probably collapse this to the shorthand -webkit-animation: eh?
> > 
> > I suppose. Can we just leave out the properties we don't want to change?
> > 
> > -webkit-animation: dancing-dots-jump 1800ms infinite;
> 
> I think this would work fine (subsequent changes to -webkit-animation-* should
> work the same way).

Done.

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/pri...
chrome/browser/resources/print_preview/print_preview.css:414: #print-preview
.button-strip button {
On 2012/03/02 20:24:42, Dan Beam wrote:
> if we're depending on not conflicting with other .button-strip styles why is
> this necessary? specificity? there should be nothing you need to override, eh?

yep, specificity

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
File chrome/browser/resources/shared/css/button.css (right):

http://codereview.chromium.org/9579001/diff/4001/chrome/browser/resources/sha...
chrome/browser/resources/shared/css/button.css:19: height: 2em;
On 2012/03/02 20:24:42, Dan Beam wrote:
> On 2012/03/02 19:46:20, Evan Stade wrote:
> > On 2012/03/02 17:58:43, Dan Beam wrote:
> > > where did this come from?
> > 
> > in the mocks it was height: 24px. I changed it to 2em.
> 
> ok, and I take it that's the same computed value, right? (16px * 75% * 2em =
> 24px)

bingo

[Dan Beam, 2012-03-03 02:03:51]

lgtm

[commit-bot: I haz the power, 2012-03-03 02:26:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/9579001/9003

[commit-bot: I haz the power, 2012-03-03 08:51:32]

Change committed as 124845