MD Settings: Bring <settings-dialog> closer to dialog spec.

MD Settings: Bring <settings-dialog> closer to dialog spec.

BUG=595804
Committed: https://crrev.com/0c5cee3f2dc56e81cef3a02e673114db58131370
Cr-Commit-Position: refs/heads/master@{#387998}

[dpapad, 2016-04-15 23:56:58]

Description was changed from

==========
MD Settings: Bring <settings-dialog> closer to dialog spec, WIP.

BUG=595804
==========

to

==========
MD Settings: Bring <settings-dialog> closer to dialog spec.

BUG=595804
==========

[dpapad, 2016-04-16 00:03:16]

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

[dpapad, 2016-04-16 00:03:17]

Before after screenshots at http://imgur.com/a/JxeVz.

Values used come from the dialog spec. Things that have been tweaked:
 - top row height, font size and bottom border.
 - 'x' close button size, margin/padding
 - margins around button-container.
 - min height of the dialog
 - margin between cancel and action buttons.
 - paper-button shared style, such that the button has the default padding.

[dschuyler, 2016-04-16 00:31:16]

https://codereview.chromium.org/1893663003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_dialog.html (right):

https://codereview.chromium.org/1893663003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_dialog.html:34: font-size: 16px;
Please use a percentage of current font
size instead of a fixed font size. (with
the percentage bringing the font size to
16px).

https://codereview.chromium.org/1893663003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_dialog.html:35: height: 52px;
will min-height work here? I'm wondering
if a 'very large' font would clip with this
setting.

[dpapad, 2016-04-16 01:09:51]

https://codereview.chromium.org/1893663003/diff/40001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_dialog.html (right):

https://codereview.chromium.org/1893663003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_dialog.html:34: font-size: 16px;
On 2016/04/16 at 00:31:16, dschuyler wrote:
> Please use a percentage of current font
> size instead of a fixed font size. (with
> the percentage bringing the font size to
> 16px).

Done. (Also moved the font-size style further down within the ":host ::content
.title" selector).

https://codereview.chromium.org/1893663003/diff/40001/chrome/browser/resource...
chrome/browser/resources/settings/settings_dialog.html:35: height: 52px;
On 2016/04/16 at 00:31:16, dschuyler wrote:
> will min-height work here? I'm wondering
> if a 'very large' font would clip with this
> setting.

Done, min-height seems to work just as fine.

[dpapad, 2016-04-18 17:24:26]

https://codereview.chromium.org/1893663003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_dialog.html (right):

https://codereview.chromium.org/1893663003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_dialog.html:85: font-size: 114.28%;
/* (16px / 14px) * 100 */
Per our last week's conversation, I figured out why 100% => 14px instead of 13px
here. Because <settings-dialog> is using the shared CSS style module
"paper-dialog-shared-styles" (see [1]), which in turn is using
@apply(--paper-font-body1), see [2], which is defined at [3].

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/polyme...
[3]
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/polyme...

So basically using "paper-dialog-shared-styles" causes the 13px setting to be
overriden.

[dschuyler, 2016-04-18 18:52:52]

LGTM with suggestion.

https://codereview.chromium.org/1893663003/diff/60001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_dialog.html (right):

https://codereview.chromium.org/1893663003/diff/60001/chrome/browser/resource...
chrome/browser/resources/settings/settings_dialog.html:85: font-size: 114.28%;
/* (16px / 14px) * 100 */
On 2016/04/18 17:24:26, dpapad wrote:
> Per our last week's conversation, I figured out why 100% => 14px instead of
13px
> here. Because <settings-dialog> is using the shared CSS style module
> "paper-dialog-shared-styles" (see [1]), which in turn is using
> @apply(--paper-font-body1), see [2], which is defined at [3].
> 
> [1]
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> [2]
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/polyme...
> [3]
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/polyme...
> 
> So basically using "paper-dialog-shared-styles" causes the 13px setting to be
> overriden.

That sounds right :) So there are a couple options.
We could set the font size on the body text or we
can use the --paper-dialog mixin to inherit the body
font size.  I'd vote for the --paper-dialog mixin.

This can be done in this CL or another CL.

[dpapad, 2016-04-18 19:24:15]

On 2016/04/18 at 18:52:52, dschuyler wrote:
> LGTM with suggestion.
> 
>
https://codereview.chromium.org/1893663003/diff/60001/chrome/browser/resource...
> File chrome/browser/resources/settings/settings_dialog.html (right):
> 
>
https://codereview.chromium.org/1893663003/diff/60001/chrome/browser/resource...
> chrome/browser/resources/settings/settings_dialog.html:85: font-size: 114.28%;
/* (16px / 14px) * 100 */
> On 2016/04/18 17:24:26, dpapad wrote:
> > Per our last week's conversation, I figured out why 100% => 14px instead of
13px
> > here. Because <settings-dialog> is using the shared CSS style module
> > "paper-dialog-shared-styles" (see [1]), which in turn is using
> > @apply(--paper-font-body1), see [2], which is defined at [3].
> > 
> > [1]
> >
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> > [2]
> >
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/polyme...
> > [3]
> >
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/polyme...
> > 
> > So basically using "paper-dialog-shared-styles" causes the 13px setting to
be
> > overriden.
> 
> That sounds right :) So there are a couple options.
> We could set the font size on the body text or we
> can use the --paper-dialog mixin to inherit the body
> font size.  I'd vote for the --paper-dialog mixin.
> 
> This can be done in this CL or another CL.

Thanks for the suggestions, will try the mixin approach first, in a separate CL.

[dpapad, 2016-04-18 19:25:02]

The CQ bit was checked by dpapad@chromium.org

[commit-bot: I haz the power, 2016-04-18 19:25:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1893663003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1893663003/60001

[commit-bot: I haz the power, 2016-04-18 20:17:13]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-18 20:18:46]

Description was changed from

==========
MD Settings: Bring <settings-dialog> closer to dialog spec.

BUG=595804
==========

to

==========
MD Settings: Bring <settings-dialog> closer to dialog spec.

BUG=595804

Committed: https://crrev.com/0c5cee3f2dc56e81cef3a02e673114db58131370
Cr-Commit-Position: refs/heads/master@{#387998}
==========

[commit-bot: I haz the power, 2016-04-18 20:18:47]

Patchset 4 (id:??) landed as
https://crrev.com/0c5cee3f2dc56e81cef3a02e673114db58131370
Cr-Commit-Position: refs/heads/master@{#387998}