Visual polishing of the MD Clear Browsing Data dialog.

Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that it is a part of
   the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/67f25398fd95636137d6bf074955156cc797ab04
Cr-Commit-Position: refs/heads/master@{#404134}

[msramek, 2016-06-28 17:56:42]

Description was changed from

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Resize the "Google G" icon to the correct (24dp) size (was accidentaly
   shrinked by https://codereview.chromium.org/1959163002).
2. Add a divider line above the button container, ensure that it's the same
   color as the title bar border.
3. Paint the footer white (instead of the default grey) color.
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

BUG=595580
==========

to

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Resize the "Google G" icon to the correct (24dp) size (was accidentaly
   shrinked by https://codereview.chromium.org/1959163002).
2. Add a divider line above the button container, ensure that it's the same
   color as the title bar border.
3. Paint the footer white (instead of the default grey) color.
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[msramek, 2016-06-28 17:59:13]

Patchset #1 (id:1) has been deleted

[msramek, 2016-06-28 18:02:01]

Description was changed from

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Resize the "Google G" icon to the correct (24dp) size (was accidentaly
   shrinked by https://codereview.chromium.org/1959163002).
2. Add a divider line above the button container, ensure that it's the same
   color as the title bar border.
3. Paint the footer white (instead of the default grey) color.
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Resize the "Google G" icon to the correct (24dp) size (was accidentaly
   shrinked by https://codereview.chromium.org/1959163002).
2. Add a divider line above the button container, ensure that it's the same
   color as the title bar border.
3. Paint the footer white (instead of the default grey) color.
4. Align the time period selector to the left, so that only one space
   separates it from the sentence "Clear the following items from ".
5. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[msramek, 2016-06-28 18:03:07]

msramek@chromium.org changed reviewers:
+ dschuyler@chromium.org

[msramek, 2016-06-28 18:03:07]

Hi Dave,

Please have a look at this as well! This should be the last required change for
the CBD dialog...

Thanks,
Martin

[msramek, 2016-06-28 18:04:14]

msramek@chromium.org changed reviewers:
+ dbeam@chromium.org

[msramek, 2016-06-28 18:04:15]

And actually +Dan, please have a look as well, because of ui/webui/ and
especially your TODO there :)

[dschuyler, 2016-06-28 19:23:48]

It would be nice to see a screen shot of how this looks with this CL.

Oh, and if there is a mock or something that you're trying to match, a link
would be helpful to that too.

Also, consider breaking this up into separate CLs (maybe three or more?). It
looks like the icon change could be separate. The grey line css variable change
could also be separate. (This isn't required, I'm just making a suggestion).

[Dan Beam, 2016-06-28 20:39:44]

https://codereview.chromium.org/2103233002/diff/20001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html (right):

https://codereview.chromium.org/2103233002/diff/20001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html:26: border-bottom:
var(--settings-dialog-title-border);
this should not reference settings

[Dan Beam, 2016-06-28 20:40:25]

https://codereview.chromium.org/2103233002/diff/20001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html (right):

https://codereview.chromium.org/2103233002/diff/20001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html:25: /* TODO(dbeam):
should this be a --settings-separator-line? */
this TODO was written when this code lived in:
chrome/browser/resources/settings/settings_dialog.html

before it was moved here.

[msramek, 2016-06-29 10:42:35]

Patchset #2 (id:40001) has been deleted

[msramek, 2016-06-29 10:45:08]

Description was changed from

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Resize the "Google G" icon to the correct (24dp) size (was accidentaly
   shrinked by https://codereview.chromium.org/1959163002).
2. Add a divider line above the button container, ensure that it's the same
   color as the title bar border.
3. Paint the footer white (instead of the default grey) color.
4. Align the time period selector to the left, so that only one space
   separates it from the sentence "Clear the following items from ".
5. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container, ensure that it's the same
   color as the title bar border.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that only one space
   separates it from the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/VejnAGcQ26R.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[msramek, 2016-06-29 11:24:00]

Patchset #2 (id:60001) has been deleted

[msramek, 2016-06-29 11:28:45]

Description was changed from

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container, ensure that it's the same
   color as the title bar border.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that only one space
   separates it from the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/VejnAGcQ26R.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container, ensure that it's the same
   color as the title bar border.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that only one space
   separates it from the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[msramek, 2016-06-29 11:29:25]

Description was changed from

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container, ensure that it's the same
   color as the title bar border.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that only one space
   separates it from the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that only one space
   separates it from the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[msramek, 2016-06-29 11:29:54]

Description was changed from

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that only one space
   separates it from the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that it is a part of
   the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[msramek, 2016-06-29 11:32:22]

I extracted the icon change to https://codereview.chromium.org/2105083004/.

I made some additional changes in the CSS, uploaded two screenshots, and updated
the CL description. PTAL!

https://codereview.chromium.org/2103233002/diff/20001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html (right):

https://codereview.chromium.org/2103233002/diff/20001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html:25: /* TODO(dbeam):
should this be a --settings-separator-line? */
On 2016/06/28 20:40:25, Dan Beam wrote:
> this TODO was written when this code lived in:
> chrome/browser/resources/settings/settings_dialog.html
> 
> before it was moved here.

Acknowledged.

https://codereview.chromium.org/2103233002/diff/20001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/cr_dialog/cr_dialog.html:26: border-bottom:
var(--settings-dialog-title-border);
On 2016/06/28 20:39:44, Dan Beam wrote:
> this should not reference settings

Makes sense. I'll just use the exact color for now. Unless it's preferred to
define a global constant for cr_elements, but that seems to also affect
history/, so perhaps not in this CL.

[msramek, 2016-06-29 11:33:01]

msramek@chromium.org changed reviewers:
+ maxwalker@chromium.org

[msramek, 2016-06-29 11:33:01]

+Max to please comment on https://screenshot.googleplex.com/YjuuhcdEgCs.png

[maxwalker, 2016-06-29 13:48:34]

On 2016/06/29 11:33:01, msramek wrote:
> +Max to please comment on https://screenshot.googleplex.com/YjuuhcdEgCs.png

Yes, items should be clipped at the grey border:
https://screenshot.googleplex.com/4tqbcH1PaOj.png.

[msramek, 2016-06-29 13:56:49]

On 2016/06/29 13:48:34, maxwalker wrote:
> On 2016/06/29 11:33:01, msramek wrote:
> > +Max to please comment on https://screenshot.googleplex.com/YjuuhcdEgCs.png
> 
> Yes, items should be clipped at the grey border:
> https://screenshot.googleplex.com/4tqbcH1PaOj.png.

Ok, removed that margin (12px) in favor of the padding (10px - that's my visual
estimate from the mock).

[Dan Beam, 2016-06-30 00:36:51]

dbeam@chromium.org changed reviewers:
- dschuyler@chromium.org

[Dan Beam, 2016-06-30 00:37:06]

dbeam@chromium.org changed reviewers:
+ dschuyler@chromium.org
- dbeam@chromium.org, maxwalker@chromium.org

[Dan Beam, 2016-06-30 00:37:48]

you don't have any changes to ui/webui any more, so just dschuyler@'s review is
sufficient

[dschuyler, 2016-06-30 02:22:30]

dschuyler@chromium.org changed reviewers:
+ tbuckley@chromium.org

[dschuyler, 2016-06-30 02:22:30]

code-wise LGTM.

Tom, would you look at the two screen shot links
in the description and give this your LGTM if the
screenshots look ok to you.

[tbuckley, 2016-07-07 00:59:42]

On 2016/06/30 02:22:30, dschuyler wrote:
> code-wise LGTM.
> 
> Tom, would you look at the two screen shot links
> in the description and give this your LGTM if the
> screenshots look ok to you.

The margin called out in the second screenshot shouldn't be there, but other
than that it looks good to me!

[msramek, 2016-07-07 11:06:39]

Thanks Tom!

I already removed the margin in Patchset 3, as Max thought the same, so I'll
proceed to land the CL now.

[msramek, 2016-07-07 11:06:44]

The CQ bit was checked by msramek@chromium.org

[commit-bot: I haz the power, 2016-07-07 11:07:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-07 12:28:51]

Description was changed from

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that it is a part of
   the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that it is a part of
   the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-07-07 12:28:53]

Committed patchset #3 (id:100001)

[commit-bot: I haz the power, 2016-07-07 12:28:54]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-07 12:29:53]

Description was changed from

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that it is a part of
   the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Visual polishing of the MD Clear Browsing Data dialog.

1. Add a divider line above the button container.
2. Paint the footer white (instead of the default grey) color.
3. Align the time period selector to the left, so that it is a part of
   the sentence "Clear the following items from ".
4. When the screen is high enough, the whole dialog fits. When it is under
   a certain threshold, to prevent the case where the dialog body would be
   scrollable just a few pixels, we cap its max-size to make it smaller,
   so that the scrollbar has to be moved more to show all the checkboxes.
   In particular, we chose the cap to be 270px, which show 5 and a half
   checkboxes. It is necessary to only partially show one of the checkboxes
   to make it clear to the user that they need to scroll to see everything.

Screenshots:
https://screenshot.googleplex.com/A85ASa8P5Fq.png
https://screenshot.googleplex.com/YjuuhcdEgCs.png

BUG=595580
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/67f25398fd95636137d6bf074955156cc797ab04
Cr-Commit-Position: refs/heads/master@{#404134}
==========

[commit-bot: I haz the power, 2016-07-07 12:29:54]

Patchset 3 (id:??) landed as
https://crrev.com/67f25398fd95636137d6bf074955156cc797ab04
Cr-Commit-Position: refs/heads/master@{#404134}