[MD settings] change button padding and placement

[MD settings] change button padding and placement

This CL changes the button padding to be 12px on each side and
causes the button padding to extend into the margins so that
the button text lines up with the .settings-box margins.

The button style for Sign Into Chrome has also changed from
an action style to a primary style.

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

Committed: https://crrev.com/b7fd553da5d05419301e2395118fa2e133b9e62f
Cr-Commit-Position: refs/heads/master@{#395427}

[dschuyler, 2016-05-17 19:00:58]

Description was changed from

==========
[MD settings] change button padding and placement

This CL changes the button padding to be 12px on each side and
causes the button padding to extend into the margins so that
the button text lines up with the .settings-box margins.

BUG=605338
==========

to

==========
[MD settings] change button padding and placement

This CL changes the button padding to be 12px on each side and
causes the button padding to extend into the margins so that
the button text lines up with the .settings-box margins.

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

[dschuyler, 2016-05-17 19:04:07]

Patchset #1 (id:1) has been deleted

[dschuyler, 2016-05-17 19:09:06]

Patchset #1 (id:20001) has been deleted

[dschuyler, 2016-05-18 00:11:50]

Patchset #1 (id:40001) has been deleted

[dschuyler, 2016-05-18 00:19:17]

Patchset #1 (id:60001) has been deleted

[dschuyler, 2016-05-18 00:19:33]

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

[Dan Beam, 2016-05-18 00:32:47]

can i have screenshots or a demo of this?

https://codereview.chromium.org/1981423002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/1981423002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:279: left: 12px;  /*
Equal to the paper-button padding. */
wont work in RTL

[dschuyler, 2016-05-18 18:21:48]

On 2016/05/18 00:32:47, Dan Beam wrote:
> can i have screenshots or a demo of this?
> 
>
https://codereview.chromium.org/1981423002/diff/80001/chrome/browser/resource...
> File chrome/browser/resources/settings/settings_shared_css.html (right):
> 
>
https://codereview.chromium.org/1981423002/diff/80001/chrome/browser/resource...
> chrome/browser/resources/settings/settings_shared_css.html:279: left: 12px; 
/*
> Equal to the paper-button padding. */
> wont work in RTL

Screen shot at imgur.com/PEeWpnr

[dschuyler, 2016-05-18 20:29:04]

RTL screen shot at http://imgur.com/w6EIEaa

https://codereview.chromium.org/1981423002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_shared_css.html (right):

https://codereview.chromium.org/1981423002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/settings_shared_css.html:279: left: 12px;  /*
Equal to the paper-button padding. */
On 2016/05/18 00:32:47, Dan Beam wrote:
> wont work in RTL

Done.

[Dan Beam, 2016-05-19 20:27:35]

i'm slightly confused as to how this is better than the previous impl (which
didn't need to specify between end and start).

can you show me a demo of this?  what happens there's a ton of buttons in either
a start or an end row?

[dschuyler, 2016-05-20 01:06:38]

Thanks for the tip.

https://codereview.chromium.org/1981423002/diff/120001/chrome/browser/resourc...
File chrome/browser/resources/settings/settings_shared_css.html (left):

https://codereview.chromium.org/1981423002/diff/120001/chrome/browser/resourc...
chrome/browser/resources/settings/settings_shared_css.html:139: .button-row {
This was not used at all. So this is just
some cleanup happening along with the real
changes.

[dschuyler, 2016-05-20 01:29:22]

On 2016/05/20 01:06:38, dschuyler wrote:
> Thanks for the tip.
> 
>
https://codereview.chromium.org/1981423002/diff/120001/chrome/browser/resourc...
> File chrome/browser/resources/settings/settings_shared_css.html (left):
> 
>
https://codereview.chromium.org/1981423002/diff/120001/chrome/browser/resourc...
> chrome/browser/resources/settings/settings_shared_css.html:139: .button-row {
> This was not used at all. So this is just
> some cleanup happening along with the real
> changes.

Alan got back to me about the sign in button. He requested that
the sign in button be changed to blue text on a white background
and then line it up by the label text in the button. That change
is included in Patch #4 of this CL.

[Dan Beam, 2016-05-23 19:24:22]

screenshots?

[dschuyler, 2016-05-23 19:33:59]

On 2016/05/23 19:24:22, Dan Beam wrote:
> screenshots?

Here are some shots with the blue and white reversed, in both rtl and ltl:
http://imgur.com/a/vN3zc

[Dan Beam, 2016-05-23 20:39:23]

lgtm

do you need to update the CL description?

[dschuyler, 2016-05-23 20:41:31]

Description was changed from

==========
[MD settings] change button padding and placement

This CL changes the button padding to be 12px on each side and
causes the button padding to extend into the margins so that
the button text lines up with the .settings-box margins.

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

to

==========
[MD settings] change button padding and placement

This CL changes the button padding to be 12px on each side and
causes the button padding to extend into the margins so that
the button text lines up with the .settings-box margins.

The button style for Sign Into Chrome has also changed from
an action style to a primary style.

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

[dschuyler, 2016-05-23 20:41:38]

The CQ bit was checked by dschuyler@chromium.org

[commit-bot: I haz the power, 2016-05-23 20:41:59]

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

[commit-bot: I haz the power, 2016-05-23 21:59:07]

Description was changed from

==========
[MD settings] change button padding and placement

This CL changes the button padding to be 12px on each side and
causes the button padding to extend into the margins so that
the button text lines up with the .settings-box margins.

The button style for Sign Into Chrome has also changed from
an action style to a primary style.

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

to

==========
[MD settings] change button padding and placement

This CL changes the button padding to be 12px on each side and
causes the button padding to extend into the margins so that
the button text lines up with the .settings-box margins.

The button style for Sign Into Chrome has also changed from
an action style to a primary style.

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

[commit-bot: I haz the power, 2016-05-23 21:59:08]

Committed patchset #4 (id:140001)

[commit-bot: I haz the power, 2016-05-23 22:04:00]

Description was changed from

==========
[MD settings] change button padding and placement

This CL changes the button padding to be 12px on each side and
causes the button padding to extend into the margins so that
the button text lines up with the .settings-box margins.

The button style for Sign Into Chrome has also changed from
an action style to a primary style.

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

to

==========
[MD settings] change button padding and placement

This CL changes the button padding to be 12px on each side and
causes the button padding to extend into the margins so that
the button text lines up with the .settings-box margins.

The button style for Sign Into Chrome has also changed from
an action style to a primary style.

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

Committed: https://crrev.com/b7fd553da5d05419301e2395118fa2e133b9e62f
Cr-Commit-Position: refs/heads/master@{#395427}
==========

[commit-bot: I haz the power, 2016-05-23 22:04:02]

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