md settings: Change action link to button on lock screen.

md settings: Change action link to button on lock screen.

Removed the setup pin link and put a button inside the radio button instead to fire up the setup pin dialog.

https://screenshot.googleplex.com/Yvp9brRd2wS

Upstream paper-radio-button PR:
https://github.com/PolymerElements/paper-radio-button/pull/119

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2823713002
Cr-Commit-Position: refs/heads/master@{#466841}
Committed: https://chromium.googlesource.com/chromium/src/+/98593cf1c4918b063b60627f439d8f97cd6489ab

[sammiequon, 2017-04-14 22:15:34]

Description was changed from

==========
md settings: Padding for dialog buttons, subpage title.

Copied from comment 17 on the bug.

TEST=manual
BUG=703998
==========

to

==========
md settings: Padding for dialog buttons, subpage title.

Copied from comment 17 on the bug.

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[sammiequon, 2017-04-14 22:16:58]

Description was changed from

==========
md settings: Padding for dialog buttons, subpage title.

Copied from comment 17 on the bug.

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
md settings: Padding for dialog buttons, subpage title.

Copied from comment 17 on the bug.
1) Add padding between “Cancel” and “Continue”/”Confirm” buttons in PIN setup
dialog (see buttons spec [1])
2) On the subpage, the spacing between the title ("Screen lock") and the first
item ("Password only") should be reduced to 24px (see subpage spec [2])
3) The "Set up PIN" / "Change PIN" button should be the same row as "PIN or
password" (see how Sign Out appears in the row spec [3]; same pattern is used
for "SET UP" button with Smart lock)

Screenshots:
https://screenshot.googleplex.com/Yvp9brRd2wS
https://screenshot.googleplex.com/UA5zDR2VyiL

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[sammiequon, 2017-04-14 23:30:12]

Description was changed from

==========
md settings: Padding for dialog buttons, subpage title.

Copied from comment 17 on the bug.
1) Add padding between “Cancel” and “Continue”/”Confirm” buttons in PIN setup
dialog (see buttons spec [1])
2) On the subpage, the spacing between the title ("Screen lock") and the first
item ("Password only") should be reduced to 24px (see subpage spec [2])
3) The "Set up PIN" / "Change PIN" button should be the same row as "PIN or
password" (see how Sign Out appears in the row spec [3]; same pattern is used
for "SET UP" button with Smart lock)

Screenshots:
https://screenshot.googleplex.com/Yvp9brRd2wS
https://screenshot.googleplex.com/UA5zDR2VyiL

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
md settings: Padding for dialog buttons, subpage title.

Copied from comment 17 on the bug.
1) Add padding between “Cancel” and “Continue”/”Confirm” buttons in PIN setup
dialog (see buttons spec [1])
2) On the subpage, the spacing between the title ("Screen lock") and the first
item ("Password only") should be reduced to 24px (see subpage spec [2])
3) The "Set up PIN" / "Change PIN" button should be the same row as "PIN or
password" (see how Sign Out appears in the row spec [3]; same pattern is used
for "SET UP" button with Smart lock)

Screenshots:
https://screenshot.googleplex.com/Yvp9brRd2wS
https://screenshot.googleplex.com/EPt93iaGMKT

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[sammiequon, 2017-04-14 23:30:12]

sammiequon@chromium.org changed reviewers:
+ stevenjb@chromium.org

[sammiequon, 2017-04-14 23:40:24]

Patchset #1 (id:1) has been deleted

[sammiequon, 2017-04-14 23:43:52]

stevenjb@ - Please take a look. Thanks!

Tom has also requested the link become a button, so I am doing it here instead
since this one also contains the other changes he wants merged into M59.

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/lock_screen.js (right):

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/lock_screen.js:163: });
Steven, since this is similar to https://codereview.chromium.org/2822733002/, do
you know if there is a better approach? The other options I could think of were
to 1) not use paper-radio-group or 2) make edits to paper-radio-button.

[stevenjb, 2017-04-17 17:18:19]

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

[stevenjb, 2017-04-17 17:18:20]

+dbeam@, any suggestions?

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/lock_screen.js (right):

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/lock_screen.js:163: });
On 2017/04/14 23:43:52, sammiequon wrote:
> Steven, since this is similar to https://codereview.chromium.org/2822733002/,
do
> you know if there is a better approach? The other options I could think of
were
> to 1) not use paper-radio-group or 2) make edits to paper-radio-button.

You are correct in that I would really prefer to avoid this type of shadow root
modification if possible.

Did you try wrapping the content in a div and applying the above styling to
that? i.e.

           <paper-radio-button name="pin+password">
             <div id="pinPasswordContent">
               <div class="start">
                 ...
               </div>
               <paper-button id="setupPinButton" class="secondary-button"
                 ...

               </paper-button>
             </div>
           </paper-radio-button>


If that doesn't work, paper-radio-button is not especially complicated. We could
create a much simpler unlock-radio-button that has the same behaviors,
hostattributes, and properties.

Dan, any suggestions here?

[Dan Beam, 2017-04-17 21:58:36]

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/lock_screen.html (right):

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/lock_screen.html:47: <div
class="start">
can you just do

<style>

#pin-or-password-wrapper {
  display: flex;
}
/* NOTE: you might have to give the <paper-radio-button> a width: 100%; as well
*/

</style>

<paper-radio-button name="pin+password">
  <div id="pin-or-password-wrapper">
    <div class="start">...</div>
    <paper-button id="setupPinButton" ...></paper-button>
  </div>
</paper-radio-button>

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/lock_screen.js (right):

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/lock_screen.js:160: var radioLabel
= radioButton.shadowRoot.querySelector('#radioLabel');
nah, don't do this

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_page/settings_subpage.html
(right):

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_page/settings_subpage.html:23:
padding-bottom: 12px;
soooo, this would change all subpages, right?

[sammiequon, 2017-04-18 00:04:25]

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/lock_screen.html (right):

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/lock_screen.html:47: <div
class="start">
On 2017/04/17 21:58:36, Dan Beam wrote:
> can you just do
> 
> <style>
> 
> #pin-or-password-wrapper {
>   display: flex;
> }
> /* NOTE: you might have to give the <paper-radio-button> a width: 100%; as
well
> */
> 
> </style>
> 
> <paper-radio-button name="pin+password">
>   <div id="pin-or-password-wrapper">
>     <div class="start">...</div>
>     <paper-button id="setupPinButton" ...></paper-button>
>   </div>
> </paper-radio-button>

I tried this this morning, but it doesn't seem to work. The paper-radio-button
itself is 100% of the parents width, but the div (#radioLabel) which contains
<content> in the paper-radio-button is only as wide as the text gets it seems,
which squashes the button to be under the text.

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_page/settings_subpage.html
(right):

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/settings_page/settings_subpage.html:23:
padding-bottom: 12px;
On 2017/04/17 21:58:36, Dan Beam wrote:
> soooo, this would change all subpages, right?

Yes, tbuckley@ suggested this (based on the specs from UX) in the bug.

[stevenjb, 2017-04-19 20:28:42]

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/settings/people_page/lock_screen.html (right):

https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
chrome/browser/resources/settings/people_page/lock_screen.html:47: <div
class="start">
On 2017/04/18 00:04:24, sammiequon wrote:
> On 2017/04/17 21:58:36, Dan Beam wrote:
> > can you just do
> > 
> > <style>
> > 
> > #pin-or-password-wrapper {
> >   display: flex;
> > }
> > /* NOTE: you might have to give the <paper-radio-button> a width: 100%; as
> well
> > */
> > 
> > </style>
> > 
> > <paper-radio-button name="pin+password">
> >   <div id="pin-or-password-wrapper">
> >     <div class="start">...</div>
> >     <paper-button id="setupPinButton" ...></paper-button>
> >   </div>
> > </paper-radio-button>
> 
> I tried this this morning, but it doesn't seem to work. The paper-radio-button
> itself is 100% of the parents width, but the div (#radioLabel) which contains
> <content> in the paper-radio-button is only as wide as the text gets it seems,
> which squashes the button to be under the text.

I poked around with this and the problem is that in paper-radio-button,
#radioLabel is display: inline-block and has no @apply var.

One fix would be to modify paper-radio-button to include
@apply(--paper-radio-button-radio-label) for #radioLabel and use that to make it
display: flex and flex: 1.

Another fix would just be to create a custom button element using
Polymer.PaperCheckedElementBehavior.

Dan, thoughts?

[Dan Beam, 2017-04-20 18:42:21]

On 2017/04/19 20:28:42, stevenjb wrote:
>
https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
> File chrome/browser/resources/settings/people_page/lock_screen.html (right):
> 
>
https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
> chrome/browser/resources/settings/people_page/lock_screen.html:47: <div
> class="start">
> On 2017/04/18 00:04:24, sammiequon wrote:
> > On 2017/04/17 21:58:36, Dan Beam wrote:
> > > can you just do
> > > 
> > > <style>
> > > 
> > > #pin-or-password-wrapper {
> > >   display: flex;
> > > }
> > > /* NOTE: you might have to give the <paper-radio-button> a width: 100%; as
> > well
> > > */
> > > 
> > > </style>
> > > 
> > > <paper-radio-button name="pin+password">
> > >   <div id="pin-or-password-wrapper">
> > >     <div class="start">...</div>
> > >     <paper-button id="setupPinButton" ...></paper-button>
> > >   </div>
> > > </paper-radio-button>
> > 
> > I tried this this morning, but it doesn't seem to work. The
paper-radio-button
> > itself is 100% of the parents width, but the div (#radioLabel) which
contains
> > <content> in the paper-radio-button is only as wide as the text gets it
seems,
> > which squashes the button to be under the text.
> 
> I poked around with this and the problem is that in paper-radio-button,
> #radioLabel is display: inline-block and has no @apply var.
> 
> One fix would be to modify paper-radio-button to include
> @apply(--paper-radio-button-radio-label) for #radioLabel and use that to make
it
> display: flex and flex: 1.
> 
> Another fix would just be to create a custom button element using
> Polymer.PaperCheckedElementBehavior.
> 
> Dan, thoughts?

i think there's a CSS-only solution.  it might require a new --var or
@apply(--mixin) in the polymer element.

[stevenjb, 2017-04-20 19:28:47]

On 2017/04/20 18:42:21, Dan Beam wrote:
> On 2017/04/19 20:28:42, stevenjb wrote:
> >
>
https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
> > File chrome/browser/resources/settings/people_page/lock_screen.html (right):
> > 
> >
>
https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
> > chrome/browser/resources/settings/people_page/lock_screen.html:47: <div
> > class="start">
> > On 2017/04/18 00:04:24, sammiequon wrote:
> > > On 2017/04/17 21:58:36, Dan Beam wrote:
> > > > can you just do
> > > > 
> > > > <style>
> > > > 
> > > > #pin-or-password-wrapper {
> > > >   display: flex;
> > > > }
> > > > /* NOTE: you might have to give the <paper-radio-button> a width: 100%;
as
> > > well
> > > > */
> > > > 
> > > > </style>
> > > > 
> > > > <paper-radio-button name="pin+password">
> > > >   <div id="pin-or-password-wrapper">
> > > >     <div class="start">...</div>
> > > >     <paper-button id="setupPinButton" ...></paper-button>
> > > >   </div>
> > > > </paper-radio-button>
> > > 
> > > I tried this this morning, but it doesn't seem to work. The
> paper-radio-button
> > > itself is 100% of the parents width, but the div (#radioLabel) which
> contains
> > > <content> in the paper-radio-button is only as wide as the text gets it
> seems,
> > > which squashes the button to be under the text.
> > 
> > I poked around with this and the problem is that in paper-radio-button,
> > #radioLabel is display: inline-block and has no @apply var.
> > 
> > One fix would be to modify paper-radio-button to include
> > @apply(--paper-radio-button-radio-label) for #radioLabel and use that to
make
> it
> > display: flex and flex: 1.
> > 
> > Another fix would just be to create a custom button element using
> > Polymer.PaperCheckedElementBehavior.
> > 
> > Dan, thoughts?
> 
> i think there's a CSS-only solution.  it might require a new --var or
> @apply(--mixin) in the polymer element.

You mean something like @apply(--paper-radio-button-radio-label)? :P

Do you recommend creating a patch for paper-radio-button on github and tying to
get that merged in, or just adding a patch to polymer/v1_0/chromium.patch?

[sammiequon, 2017-04-21 00:31:52]

On 2017/04/20 19:28:47, stevenjb wrote:
> On 2017/04/20 18:42:21, Dan Beam wrote:
> > On 2017/04/19 20:28:42, stevenjb wrote:
> > >
> >
>
https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
> > > File chrome/browser/resources/settings/people_page/lock_screen.html
(right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
> > > chrome/browser/resources/settings/people_page/lock_screen.html:47: <div
> > > class="start">
> > > On 2017/04/18 00:04:24, sammiequon wrote:
> > > > On 2017/04/17 21:58:36, Dan Beam wrote:
> > > > > can you just do
> > > > > 
> > > > > <style>
> > > > > 
> > > > > #pin-or-password-wrapper {
> > > > >   display: flex;
> > > > > }
> > > > > /* NOTE: you might have to give the <paper-radio-button> a width:
100%;
> as
> > > > well
> > > > > */
> > > > > 
> > > > > </style>
> > > > > 
> > > > > <paper-radio-button name="pin+password">
> > > > >   <div id="pin-or-password-wrapper">
> > > > >     <div class="start">...</div>
> > > > >     <paper-button id="setupPinButton" ...></paper-button>
> > > > >   </div>
> > > > > </paper-radio-button>
> > > > 
> > > > I tried this this morning, but it doesn't seem to work. The
> > paper-radio-button
> > > > itself is 100% of the parents width, but the div (#radioLabel) which
> > contains
> > > > <content> in the paper-radio-button is only as wide as the text gets it
> > seems,
> > > > which squashes the button to be under the text.
> > > 
> > > I poked around with this and the problem is that in paper-radio-button,
> > > #radioLabel is display: inline-block and has no @apply var.
> > > 
> > > One fix would be to modify paper-radio-button to include
> > > @apply(--paper-radio-button-radio-label) for #radioLabel and use that to
> make
> > it
> > > display: flex and flex: 1.
> > > 
> > > Another fix would just be to create a custom button element using
> > > Polymer.PaperCheckedElementBehavior.
> > > 
> > > Dan, thoughts?
> > 
> > i think there's a CSS-only solution.  it might require a new --var or
> > @apply(--mixin) in the polymer element.
> 
> You mean something like @apply(--paper-radio-button-radio-label)? :P
> 
> Do you recommend creating a patch for paper-radio-button on github and tying
to
> get that merged in, or just adding a patch to polymer/v1_0/chromium.patch?

I added a mixin directly to paper-radio-button.html for now. If that's OK i'll
proceed with either patching it in github or to chromium.patch.

[Dan Beam, 2017-04-21 00:56:32]

On 2017/04/21 00:31:52, sammiequon wrote:
> On 2017/04/20 19:28:47, stevenjb wrote:
> > On 2017/04/20 18:42:21, Dan Beam wrote:
> > > On 2017/04/19 20:28:42, stevenjb wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
> > > > File chrome/browser/resources/settings/people_page/lock_screen.html
> (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/2823713002/diff/20001/chrome/browser/resource...
> > > > chrome/browser/resources/settings/people_page/lock_screen.html:47: <div
> > > > class="start">
> > > > On 2017/04/18 00:04:24, sammiequon wrote:
> > > > > On 2017/04/17 21:58:36, Dan Beam wrote:
> > > > > > can you just do
> > > > > > 
> > > > > > <style>
> > > > > > 
> > > > > > #pin-or-password-wrapper {
> > > > > >   display: flex;
> > > > > > }
> > > > > > /* NOTE: you might have to give the <paper-radio-button> a width:
> 100%;
> > as
> > > > > well
> > > > > > */
> > > > > > 
> > > > > > </style>
> > > > > > 
> > > > > > <paper-radio-button name="pin+password">
> > > > > >   <div id="pin-or-password-wrapper">
> > > > > >     <div class="start">...</div>
> > > > > >     <paper-button id="setupPinButton" ...></paper-button>
> > > > > >   </div>
> > > > > > </paper-radio-button>
> > > > > 
> > > > > I tried this this morning, but it doesn't seem to work. The
> > > paper-radio-button
> > > > > itself is 100% of the parents width, but the div (#radioLabel) which
> > > contains
> > > > > <content> in the paper-radio-button is only as wide as the text gets
it
> > > seems,
> > > > > which squashes the button to be under the text.
> > > > 
> > > > I poked around with this and the problem is that in paper-radio-button,
> > > > #radioLabel is display: inline-block and has no @apply var.
> > > > 
> > > > One fix would be to modify paper-radio-button to include
> > > > @apply(--paper-radio-button-radio-label) for #radioLabel and use that to
> > make
> > > it
> > > > display: flex and flex: 1.
> > > > 
> > > > Another fix would just be to create a custom button element using
> > > > Polymer.PaperCheckedElementBehavior.
> > > > 
> > > > Dan, thoughts?
> > > 
> > > i think there's a CSS-only solution.  it might require a new --var or
> > > @apply(--mixin) in the polymer element.
> > 
> > You mean something like @apply(--paper-radio-button-radio-label)? :P
> > 
> > Do you recommend creating a patch for paper-radio-button on github and tying
> to
> > get that merged in, or just adding a patch to polymer/v1_0/chromium.patch?
> 
> I added a mixin directly to paper-radio-button.html for now. If that's OK i'll
> proceed with either patching it in github or to chromium.patch.

i would create a polymer PR as well as add it chromium.patch for now

[Dan Beam, 2017-04-21 00:59:43]

otherwise this looks good

[Dan Beam, 2017-04-21 01:17:25]

https://codereview.chromium.org/2823713002/diff/40001/third_party/polymer/v1_...
File
third_party/polymer/v1_0/components-chromium/paper-radio-button/paper-radio-button.html
(right):

https://codereview.chromium.org/2823713002/diff/40001/third_party/polymer/v1_...
third_party/polymer/v1_0/components-chromium/paper-radio-button/paper-radio-button.html:45:
`--paper-radio-button-radio-label` | A mixin applied to the internal radio
container | `{}`
fwiw: I would name this simply --paper-radio-button-label

[sammiequon, 2017-04-21 17:47:11]

Patchset #3 (id:60001) has been deleted

[sammiequon, 2017-04-21 17:52:41]

Steven, Dan, please take another look. Thanks! Are you guys fine with the
proposed settings-wide padding changes?

https://codereview.chromium.org/2823713002/diff/40001/third_party/polymer/v1_...
File
third_party/polymer/v1_0/components-chromium/paper-radio-button/paper-radio-button.html
(right):

https://codereview.chromium.org/2823713002/diff/40001/third_party/polymer/v1_...
third_party/polymer/v1_0/components-chromium/paper-radio-button/paper-radio-button.html:45:
`--paper-radio-button-radio-label` | A mixin applied to the internal radio
container | `{}`
On 2017/04/21 01:17:25, Dan Beam wrote:
> fwiw: I would name this simply --paper-radio-button-label

Done.

[stevenjb, 2017-04-21 18:09:12]

lgtm. If you separate out the global changes you are less likely to also have
this reverted if there are problems with those changes, but I don't feel
strongly.

https://codereview.chromium.org/2823713002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_page/settings_subpage.html
(right):

https://codereview.chromium.org/2823713002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/settings_page/settings_subpage.html:23:
padding-bottom: 12px;
I might suggest separating this change, in case it affects some other page or
platform badly somehow.

https://codereview.chromium.org/2823713002/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/shared_style_css.html (right):

https://codereview.chromium.org/2823713002/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/shared_style_css.html:39: }
Maybe move these changes also?

[sammiequon, 2017-04-21 18:36:22]

Description was changed from

==========
md settings: Padding for dialog buttons, subpage title.

Copied from comment 17 on the bug.
1) Add padding between “Cancel” and “Continue”/”Confirm” buttons in PIN setup
dialog (see buttons spec [1])
2) On the subpage, the spacing between the title ("Screen lock") and the first
item ("Password only") should be reduced to 24px (see subpage spec [2])
3) The "Set up PIN" / "Change PIN" button should be the same row as "PIN or
password" (see how Sign Out appears in the row spec [3]; same pattern is used
for "SET UP" button with Smart lock)

Screenshots:
https://screenshot.googleplex.com/Yvp9brRd2wS
https://screenshot.googleplex.com/EPt93iaGMKT

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
md settings: Change action link to button on lock screen.

Removed the setup pin link and put a button inside the radio button instead to
fire up the setup pin dialog.

https://screenshot.googleplex.com/Yvp9brRd2wS

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[sammiequon, 2017-04-21 18:36:57]

Thanks! I removed those changes as per suggestion.

https://codereview.chromium.org/2823713002/diff/80001/chrome/browser/resource...
File chrome/browser/resources/settings/settings_page/settings_subpage.html
(right):

https://codereview.chromium.org/2823713002/diff/80001/chrome/browser/resource...
chrome/browser/resources/settings/settings_page/settings_subpage.html:23:
padding-bottom: 12px;
On 2017/04/21 18:09:12, stevenjb wrote:
> I might suggest separating this change, in case it affects some other page or
> platform badly somehow.

Done.

https://codereview.chromium.org/2823713002/diff/80001/ui/webui/resources/cr_e...
File ui/webui/resources/cr_elements/shared_style_css.html (right):

https://codereview.chromium.org/2823713002/diff/80001/ui/webui/resources/cr_e...
ui/webui/resources/cr_elements/shared_style_css.html:39: }
On 2017/04/21 18:09:12, stevenjb wrote:
> Maybe move these changes also?

Done.

[stevenjb, 2017-04-21 18:40:27]

lgtm++

[Dan Beam, 2017-04-21 23:27:11]

Description was changed from

==========
md settings: Change action link to button on lock screen.

Removed the setup pin link and put a button inside the radio button instead to
fire up the setup pin dialog.

https://screenshot.googleplex.com/Yvp9brRd2wS

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
md settings: Change action link to button on lock screen.

Removed the setup pin link and put a button inside the radio button instead to
fire up the setup pin dialog.

https://screenshot.googleplex.com/Yvp9brRd2wS

Upstream paper-radio-button PR:
https://github.com/PolymerElements/paper-radio-button/pull/119

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-04-21 23:27:31]

found your Polymer issue and PR, thanks for sending!

lgtm

[sammiequon, 2017-04-22 03:59:56]

On 2017/04/21 23:27:31, Dan Beam wrote:
> found your Polymer issue and PR, thanks for sending!
> 
> lgtm

Thanks!

[sammiequon, 2017-04-22 04:00:06]

The CQ bit was checked by sammiequon@chromium.org

[commit-bot: I haz the power, 2017-04-22 04:00:12]

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, 2017-04-22 04:03:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-22 04:03:37]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sammiequon, 2017-04-22 23:20:31]

The CQ bit was checked by sammiequon@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-22 23:20:44]

Dry run: 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, 2017-04-23 00:44:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-23 00:44:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sammiequon, 2017-04-24 16:28:52]

The CQ bit was checked by sammiequon@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-24 16:29:47]

Dry run: 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, 2017-04-24 17:50:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-24 17:50:03]

Dry run: This issue passed the CQ dry run.

[sammiequon, 2017-04-24 23:19:35]

Patchset #7 (id:160001) has been deleted

[sammiequon, 2017-04-24 23:21:26]

stevenjb@, dbeam@ - Modified test to match the change. Please take another look.
Thanks!

[stevenjb, 2017-04-24 23:23:29]

still lgtm

[sammiequon, 2017-04-24 23:35:58]

On 2017/04/24 23:23:29, stevenjb wrote:
> still lgtm

Thanks!

[sammiequon, 2017-04-24 23:36:07]

The CQ bit was checked by sammiequon@chromium.org

[sammiequon, 2017-04-24 23:36:07]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/2823713002/#ps180001
(title: "Use offset width ")

[commit-bot: I haz the power, 2017-04-24 23:36:32]

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, 2017-04-25 00:40:18]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1493076967098280,
"parent_rev": "425afe03ad0dca72dce8d1334298bce08e3f353e", "commit_rev":
"98593cf1c4918b063b60627f439d8f97cd6489ab"}

[commit-bot: I haz the power, 2017-04-25 00:41:10]

Description was changed from

==========
md settings: Change action link to button on lock screen.

Removed the setup pin link and put a button inside the radio button instead to
fire up the setup pin dialog.

https://screenshot.googleplex.com/Yvp9brRd2wS

Upstream paper-radio-button PR:
https://github.com/PolymerElements/paper-radio-button/pull/119

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
md settings: Change action link to button on lock screen.

Removed the setup pin link and put a button inside the radio button instead to
fire up the setup pin dialog.

https://screenshot.googleplex.com/Yvp9brRd2wS

Upstream paper-radio-button PR:
https://github.com/PolymerElements/paper-radio-button/pull/119

TEST=manual
BUG=703998
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2823713002
Cr-Commit-Position: refs/heads/master@{#466841}
Committed:
https://chromium.googlesource.com/chromium/src/+/98593cf1c4918b063b60627f439d...
==========

[commit-bot: I haz the power, 2017-04-25 00:41:11]

Committed patchset #7 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/98593cf1c4918b063b60627f439d...