Make settings page more touch friendly (font-size, checkbox, radio, adding language page).

Make settings page more touch friendly.
(font-size, checkbox, radio button, adding new language page)



1. Change font size to 16px. This font size proves to be more touch 
friendly while not affecting layout.
2. Rescale the checkbox and radio button. They were too tiny to touch.
3. Use drop down menu for adding new languages overlay page in touch ui builds.

BUG=100377
TEST=None


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

[bshe, 2011-10-28 15:01:22]

Hey Rick.

Could you please take a look at this CL? Thanks!

Biao

[Rick Byers, 2011-10-28 20:07:13]

Thanks Bioa, this looks good.  I've got a few suggestions to simplify it a
bit...

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/language_options.css (right):

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/language_options.css:140: </if>
Adding this rule and the one below doesn't feel like the right fix here.  You
want to behave just like the non-ChromeOS case for touchui.  Maybe you can
simply put all these html[os=chromos] rules inside of an <if> for not touchui?

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/options_page.css (right):

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:25: /**
Remove second * - not jsdoc

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:27: * options/preference pages
easier to touch.
Move this comment down since it applies just to the radio/checkbox rules

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:31: #search-field {
This seems wrong - you shouldn't have to hard-code a specific width here (this
is probably only right for a given navbar-container width).  Also you're really
forcing the input to be smaller than it thinks it's minimum size is (although
it's not being too smart about it - it's including the space at the end reserved
for the 'x' along with the placeholder text to determine it's minimal size).

Instead I'd suggest setting the font size back to a small value here (so the
search box still fits) but setting a height value to make it a little taller
than normal and so easier to touch.

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:37: div.checkbox > label >
span {
This seems like a hack.  If the webkit transform below is sufficient to increase
the touch target, then can we just remove this?

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:38: position: relative;
sort properties alphabetically

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:48: /**
Remove second *

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:49: * Overwrite the font-size
rule in shared_options.css file.
replace 'Overwrite' with 'Override'.

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:53: body{
add space before {

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:54: font-size:16px;
add space after :

[bshe, 2011-10-30 22:46:10]

Done. Thanks!

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/language_options.css (right):

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/language_options.css:140: </if>
On 2011/10/28 20:07:13, Rick Byers wrote:
> Adding this rule and the one below doesn't feel like the right fix here.  You
> want to behave just like the non-ChromeOS case for touchui.  Maybe you can
> simply put all these html[os=chromos] rules inside of an <if> for not touchui?


Done.

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/options_page.css (right):

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:25: /**
On 2011/10/28 20:07:13, Rick Byers wrote:
> Remove second * - not jsdoc

Done.

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:27: * options/preference pages
easier to touch.
On 2011/10/28 20:07:13, Rick Byers wrote:
> Move this comment down since it applies just to the radio/checkbox rules

Done.

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:31: #search-field {
I totally agree not to hard-code a specific width. The reason I use a specific
width is because the chrome navbar-container has a hard-code fixed size:
http://www.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/resources/sha...

Anyway, I have changed the code based on your suggestions. I used scaleY(1.2).
It is easier to touch and the text doesn't have much distortion due to scale.

On 2011/10/28 20:07:13, Rick Byers wrote:
> This seems wrong - you shouldn't have to hard-code a specific width here (this
> is probably only right for a given navbar-container width).  Also you're
really
> forcing the input to be smaller than it thinks it's minimum size is (although
> it's not being too smart about it - it's including the space at the end
reserved
> for the 'x' along with the placeholder text to determine it's minimal size).
> 
> Instead I'd suggest setting the font size back to a small value here (so the
> search box still fits) but setting a height value to make it a little taller
> than normal and so easier to touch.

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:37: div.checkbox > label >
span {
On 2011/10/28 20:07:13, Rick Byers wrote:
> This seems like a hack.  If the webkit transform below is sufficient to
increase
> the touch target, then can we just remove this?

Done.

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:38: position: relative;
No need to use padding hack. Only one property left.
On 2011/10/28 20:07:13, Rick Byers wrote:
> sort properties alphabetically

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:48: /**
On 2011/10/28 20:07:13, Rick Byers wrote:
> Remove second *

Done.

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:53: body{
On 2011/10/28 20:07:13, Rick Byers wrote:
> add space before {

Done.

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:54: font-size:16px;
On 2011/10/28 20:07:13, Rick Byers wrote:
> add space after :

Done.

[James Hawkins, 2011-10-31 04:36:20]

Drive-by nit, not adding myself to R.

http://codereview.chromium.org/8417029/diff/4001/chrome/browser/resources/opt...
File chrome/browser/resources/options/language_options.js (right):

http://codereview.chromium.org/8417029/diff/4001/chrome/browser/resources/opt...
chrome/browser/resources/options/language_options.js:102: if
(templateData.experimentalSpellCheckFeatures == 'true') {
nit: No braces for single-line blocks.

[bshe, 2011-10-31 14:18:21]

Done. Thanks!

http://codereview.chromium.org/8417029/diff/4001/chrome/browser/resources/opt...
File chrome/browser/resources/options/language_options.js (right):

http://codereview.chromium.org/8417029/diff/4001/chrome/browser/resources/opt...
chrome/browser/resources/options/language_options.js:102: if
(templateData.experimentalSpellCheckFeatures == 'true') {
On 2011/10/31 04:36:20, James Hawkins wrote:
> nit: No braces for single-line blocks.

Done.

[Rick Byers, 2011-10-31 15:18:18]

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/options_page.css (right):

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:31: #search-field {
On 2011/10/30 22:46:10, bshe wrote:
> I totally agree not to hard-code a specific width. The reason I use a specific
> width is because the chrome navbar-container has a hard-code fixed size:
>
http://www.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/resources/sha...
> 
> Anyway, I have changed the code based on your suggestions. I used scaleY(1.2).
> It is easier to touch and the text doesn't have much distortion due to scale.

Cool, thanks.  Stretching the text is sub-optimal still though.  Doesn't setting
height make the input box bigger without affecting the text (just adds padding
to the top/bottom)?  It seems to work for me on Linux Chrome, but I see now that
on Mac WebKit severely restricts how you can change search boxes (including
ignoring height and line-height).  I didn't try on ChromeOS though (don't have a
device at home with me right now).

If there's really no way on ChromeOS to make the box taller without stretching
the text, then I guess I prefer your original solution of a large font size with
a fixed width.  Just add a comment describing the size calculation (so people
know how to update it if the other constant changes).

http://codereview.chromium.org/8417029/diff/3003/chrome/browser/resources/opt...
File chrome/browser/resources/options/options_page.css (right):

http://codereview.chromium.org/8417029/diff/3003/chrome/browser/resources/opt...
chrome/browser/resources/options/options_page.css:26: /* Fix the width issue of
search box after increase font size. */
Update this comment

http://codereview.chromium.org/8417029/diff/3003/chrome/browser/resources/opt...
chrome/browser/resources/options/options_page.css:38: padding-left: 5px;
use -webkit-padding-start so this still works for RTL layout

[bshe, 2011-10-31 21:41:18]

Done. Thanks!

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/options_page.css (right):

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:31: #search-field {
The height works for chrome os. Didn't test it on Mac, but I guess we dont have
to worry about it right now since it only applies to TouchUI. Directly add
padding also works and I think its a better solution here. The drawback of
height is the extra top and bottom blank space, which makes the position of x
button awkward (1 px on the right of x button, however 5px on both bottom and
top).

Also, I rescaled the webkit X button in search box to make it better for touch.
It looks good too.

On 2011/10/31 15:18:18, Rick Byers wrote:
> On 2011/10/30 22:46:10, bshe wrote:
> > I totally agree not to hard-code a specific width. The reason I use a
specific
> > width is because the chrome navbar-container has a hard-code fixed size:
> >
>
http://www.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/resources/sha...
> > 
> > Anyway, I have changed the code based on your suggestions. I used
scaleY(1.2).
> > It is easier to touch and the text doesn't have much distortion due to
scale.
> 
> Cool, thanks.  Stretching the text is sub-optimal still though.  Doesn't
setting
> height make the input box bigger without affecting the text (just adds padding
> to the top/bottom)?  It seems to work for me on Linux Chrome, but I see now
that
> on Mac WebKit severely restricts how you can change search boxes (including
> ignoring height and line-height).  I didn't try on ChromeOS though (don't have
a
> device at home with me right now).
> 
> If there's really no way on ChromeOS to make the box taller without stretching
> the text, then I guess I prefer your original solution of a large font size
with
> a fixed width.  Just add a comment describing the size calculation (so people
> know how to update it if the other constant changes).

http://codereview.chromium.org/8417029/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/options_page.css:49: * Overwrite the font-size
rule in shared_options.css file.
On 2011/10/28 20:07:13, Rick Byers wrote:
> replace 'Overwrite' with 'Override'.

Done.

http://codereview.chromium.org/8417029/diff/3003/chrome/browser/resources/opt...
File chrome/browser/resources/options/options_page.css (right):

http://codereview.chromium.org/8417029/diff/3003/chrome/browser/resources/opt...
chrome/browser/resources/options/options_page.css:26: /* Fix the width issue of
search box after increase font size. */
On 2011/10/31 15:18:18, Rick Byers wrote:
> Update this comment

Done.

http://codereview.chromium.org/8417029/diff/3003/chrome/browser/resources/opt...
chrome/browser/resources/options/options_page.css:38: padding-left: 5px;
On 2011/10/31 15:18:18, Rick Byers wrote:
> use -webkit-padding-start so this still works for RTL layout

Done.

[Rick Byers, 2011-11-01 17:06:10]

This is great, thanks Biao!

LGTM with minor nit

http://codereview.chromium.org/8417029/diff/11008/chrome/browser/resources/op...
File chrome/browser/resources/options/options_page.css (right):

http://codereview.chromium.org/8417029/diff/11008/chrome/browser/resources/op...
chrome/browser/resources/options/options_page.css:47: label >
input[type="checkbox"],
style nit: remove quotes
(looks like we're not consistent with this - in this file we have single quotes,
double quotes and no quotes, let's just go with no quotes for new code since
it's unnecessary extra characters).

[commit-bot: I haz the power, 2011-11-01 19:21:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/8417029/19001

[bshe, 2011-11-01 19:23:46]

Done and committed it to tree.

http://codereview.chromium.org/8417029/diff/11008/chrome/browser/resources/op...
File chrome/browser/resources/options/options_page.css (right):

http://codereview.chromium.org/8417029/diff/11008/chrome/browser/resources/op...
chrome/browser/resources/options/options_page.css:47: label >
input[type="checkbox"],
On 2011/11/01 17:06:10, Rick Byers wrote:
> style nit: remove quotes
> (looks like we're not consistent with this - in this file we have single
quotes,
> double quotes and no quotes, let's just go with no quotes for new code since
> it's unnecessary extra characters).

Done.

[commit-bot: I haz the power, 2011-11-01 20:33:43]

Change committed as 108161