tabbed options - new clear browsing data overlay

tabbed options - new clear browsing data overlay

I tried to make this look as nice as possible (and close to the mocks), but Alex will have to fiddle with the CSS to finalize it. I didn't implement the button changing to "clearing..." because it changes the width of the button and thus the whole overlay. The throbber + disablement should hopefully be enough of a visual cue.

BUG=63843
TEST=manual

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

[Evan Stade, 2011-01-14 21:12:55]

+smorgan

[stuartmorgan, 2011-01-14 22:25:31]

LGTM

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.css (right):

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.css:50:
-webkit-box-orient: horizontal;
I think we've been putting these first rather than last when alphabetizing CSS.

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.html (right):

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.html:48:
i18n-content="clearBrowserDataCommit">
Indent should be 4, not 8.

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

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/options_page.css:319: overlay >
div:only-of-type label {
Make sure to revert this change per discussion.

[csilv, 2011-01-18 18:46:32]

LGTM

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.html (right):

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.html:1: <div
class="page hidden" id="clearBrowserDataOverlay">
nit: lines 1 & 2, order 'id' parameter first for consistency w/ rest of file.

[arv (Not doing code reviews), 2011-01-18 20:16:37]

LGTM

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.css (right):

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.css:15: padding:
0px;
s/0px/0/

[Evan Stade, 2011-01-18 21:56:44]

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.css (right):

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.css:15: padding:
0px;
On 2011/01/18 20:16:38, arv wrote:
> s/0px/0/

Done.

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.css:50:
-webkit-box-orient: horizontal;
On 2011/01/14 22:25:32, stuartmorgan wrote:
> I think we've been putting these first rather than last when alphabetizing
CSS.

Done.

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
File chrome/browser/resources/options/clear_browser_data_overlay.html (right):

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.html:1: <div
class="page hidden" id="clearBrowserDataOverlay">
On 2011/01/18 18:46:32, csilv wrote:
> nit: lines 1 & 2, order 'id' parameter first for consistency w/ rest of file.

Done.

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/clear_browser_data_overlay.html:48:
i18n-content="clearBrowserDataCommit">
On 2011/01/14 22:25:32, stuartmorgan wrote:
> Indent should be 4, not 8.

Done.

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

http://codereview.chromium.org/6340002/diff/2001/chrome/browser/resources/opt...
chrome/browser/resources/options/options_page.css:319: overlay >
div:only-of-type label {
On 2011/01/14 22:25:32, stuartmorgan wrote:
> Make sure to revert this change per discussion.

Done.

[James Hawkins, 2011-01-18 22:23:31]

You could set the minimum width of the button that fits both strings, though
it's likely there will be issues with localized strings being longer than this
minimum width.

[Evan Stade, 2011-01-18 23:04:48]

On 2011/01/18 22:23:31, James Hawkins wrote:
> You could set the minimum width of the button that fits both strings, though
> it's likely there will be issues with localized strings being longer than this
> minimum width.

yea, that's a rabbit hole I'm not interested in going down.

[stuartmorgan, 2011-01-18 23:09:30]

On 2011/01/18 23:04:48, Evan Stade wrote:
> On 2011/01/18 22:23:31, James Hawkins wrote:
> > You could set the minimum width of the button that fits both strings, though
> > it's likely there will be issues with localized strings being longer than
this
> > minimum width.
> 
> yea, that's a rabbit hole I'm not interested in going down.

Another possibility might be to stack both buttons in a div, and set whichever
one is not showing to something like height:0 margin:0 padding:0 visibility
hidden; I think that would still force the wider width.

[arv (Not doing code reviews), 2011-01-18 23:49:02]

On Tue, Jan 18, 2011 at 15:09,  <stuartmorgan@chromium.org> wrote:
> On 2011/01/18 23:04:48, Evan Stade wrote:
>>
>> On 2011/01/18 22:23:31, James Hawkins wrote:
>> > You could set the minimum width of the button that fits both strings,
>> > though
>> > it's likely there will be issues with localized strings being longer
>> > than
>
> this
>>
>> > minimum width.
>
>> yea, that's a rabbit hole I'm not interested in going down.
>
> Another possibility might be to stack both buttons in a div, and set
> whichever
> one is not showing to something like height:0 margin:0 padding:0 visibility
> hidden; I think that would still force the wider width.
>
> http://codereview.chromium.org/6340002/
>

Another option is to use an hbox and let it grow as needed?