Make the extended reporting opt-in checkbox into a flexbox.

Make the extended reporting opt-in checkbox into a flexbox.

This makes multi-line labels left align with themselves instead of wrap underneath the checkbox.

BUG=667389
Committed: https://crrev.com/a96c732633fe3292aed488f7bfcc3b31a05794c1
Cr-Commit-Position: refs/heads/master@{#438847}

[lpz, 2016-12-12 22:11:36]

lpz@chromium.org changed reviewers:
+ edwardjung@chromium.org

[lgarron, 2016-12-12 22:13:36]

lgarron@chromium.org changed reviewers:
+ lgarron@chromium.org

[lgarron, 2016-12-12 22:13:37]

https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
File
components/security_interstitials/core/browser/resources/interstitial_v2.css
(right):

https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
components/security_interstitials/core/browser/resources/interstitial_v2.css:173:
#opt-in-label {
This is a slightly brittle fix to compensate for the fact that the checkbox is
not taking up its "actual" width.

Can you add padding to the checkbox to the same effect?

[lpz, 2016-12-12 22:30:25]

https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
File
components/security_interstitials/core/browser/resources/interstitial_v2.css
(right):

https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
components/security_interstitials/core/browser/resources/interstitial_v2.css:173:
#opt-in-label {
On 2016/12/12 22:13:37, lgarron wrote:
> This is a slightly brittle fix to compensate for the fact that the checkbox is
> not taking up its "actual" width.
> 
> Can you add padding to the checkbox to the same effect?

Didn't seem to work. For reference, here's the HTML: 
<div id="extended-reporting-opt-in" class="hidden">
  <label>
    <input type="checkbox" id="opt-in-checkbox">
    <span class="checkbox" id="foo"></span> *I named this foo to test
    <span id="opt-in-label"></span>
  </label>
</div>

padding-right on "opt-in-checkbox" does nothing
padding-right on "foo" makes the white checkbox outline wider
padding-left on "opt-in-label" is equivalent to margin-left horizontally but the
text shifts down slightly (which is why I hacked it back up with margin-top
here).

I'm very open to other suggestions, since this change is outside of my
expertise.

[edwardjung, 2016-12-13 12:14:56]

https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
File
components/security_interstitials/core/browser/resources/interstitial_v2.css
(right):

https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
components/security_interstitials/core/browser/resources/interstitial_v2.css:173:
#opt-in-label {
Remove the margins here and add a wrapper to checkboxes, then apply the flex
attributes:

<div id="extended-reporting-opt-in" class="hidden">
  <label>
    <div class="checkboxes">
       <input type="checkbox" id="opt-in-checkbox">
       <span class="checkbox"></span>
    </div>
     <span id="opt-in-label"></span>
  </label>
</div>

.checkboxes {
  flex: 0 0 24px;
}

.checkbox {
  top: 3px;
}

[lpz, 2016-12-13 15:09:49]

https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
File
components/security_interstitials/core/browser/resources/interstitial_v2.css
(right):

https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
components/security_interstitials/core/browser/resources/interstitial_v2.css:173:
#opt-in-label {
On 2016/12/13 12:14:56, edwardjung wrote:
> Remove the margins here and add a wrapper to checkboxes, then apply the flex
> attributes:
> 
> <div id="extended-reporting-opt-in" class="hidden">
>   <label>
>     <div class="checkboxes">
>        <input type="checkbox" id="opt-in-checkbox">
>        <span class="checkbox"></span>
>     </div>
>      <span id="opt-in-label"></span>
>   </label>
> </div>
> 
> .checkboxes {
>   flex: 0 0 24px;
> }
> 
> .checkbox {
>   top: 3px;
> }
> 

Awesome thanks. That solves the horizontal alignment, but the text is still
shifted down a little bit: https://screenshot.googleplex.com/Hche711yHiq

Thoughts?

[edwardjung, 2016-12-13 15:59:52]

On 2016/12/13 15:09:49, lpz wrote:
>
https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
> File
> components/security_interstitials/core/browser/resources/interstitial_v2.css
> (right):
> 
>
https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
>
components/security_interstitials/core/browser/resources/interstitial_v2.css:173:
> #opt-in-label {
> On 2016/12/13 12:14:56, edwardjung wrote:
> > Remove the margins here and add a wrapper to checkboxes, then apply the flex
> > attributes:
> > 
> > <div id="extended-reporting-opt-in" class="hidden">
> >   <label>
> >     <div class="checkboxes">
> >        <input type="checkbox" id="opt-in-checkbox">
> >        <span class="checkbox"></span>
> >     </div>
> >      <span id="opt-in-label"></span>
> >   </label>
> > </div>
> > 
> > .checkboxes {
> >   flex: 0 0 24px;
> > }
> > 
> > .checkbox {
> >   top: 3px;
> > }
> > 
> 
> Awesome thanks. That solves the horizontal alignment, but the text is still
> shifted down a little bit: https://screenshot.googleplex.com/Hche711yHiq
> 
> Thoughts?

Try using an em size instead for the checkbox top property, the renderer differs
between platforms but 0.28em looks okay on Mac and Linux, I didn't test on
Windows.

[lpz, 2016-12-13 16:24:42]

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

[commit-bot: I haz the power, 2016-12-13 16:25:15]

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

[lpz, 2016-12-13 16:33:44]

On 2016/12/13 15:59:52, edwardjung wrote:
> On 2016/12/13 15:09:49, lpz wrote:
> >
>
https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
> > File
> > components/security_interstitials/core/browser/resources/interstitial_v2.css
> > (right):
> > 
> >
>
https://codereview.chromium.org/2566983005/diff/1/components/security_interst...
> >
>
components/security_interstitials/core/browser/resources/interstitial_v2.css:173:
> > #opt-in-label {
> > On 2016/12/13 12:14:56, edwardjung wrote:
> > > Remove the margins here and add a wrapper to checkboxes, then apply the
flex
> > > attributes:
> > > 
> > > <div id="extended-reporting-opt-in" class="hidden">
> > >   <label>
> > >     <div class="checkboxes">
> > >        <input type="checkbox" id="opt-in-checkbox">
> > >        <span class="checkbox"></span>
> > >     </div>
> > >      <span id="opt-in-label"></span>
> > >   </label>
> > > </div>
> > > 
> > > .checkboxes {
> > >   flex: 0 0 24px;
> > > }
> > > 
> > > .checkbox {
> > >   top: 3px;
> > > }
> > > 
> > 
> > Awesome thanks. That solves the horizontal alignment, but the text is still
> > shifted down a little bit: https://screenshot.googleplex.com/Hche711yHiq
> > 
> > Thoughts?
> 
> Try using an em size instead for the checkbox top property, the renderer
differs
> between platforms but 0.28em looks okay on Mac and Linux, I didn't test on
> Windows.

Ahh I see my error - there was another .checkbox down below that overwrote the
top to -1px. Using 3px down below looks good, and is consistent with
checkbox::before (not sure why they were different).

[lpz, 2016-12-13 16:34:29]

lpz@chromium.org changed reviewers:
+ estark@chromium.org

[commit-bot: I haz the power, 2016-12-13 16:35:36]

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

[commit-bot: I haz the power, 2016-12-13 16:35:36]

Dry run: This issue passed the CQ dry run.

[estark, 2016-12-15 00:37:07]

lgtm if edwardjung is happy

[edwardjung, 2016-12-15 15:27:25]

On 2016/12/15 00:37:07, estark wrote:
> lgtm if edwardjung is happy

lgtm although to be sure I would also try to check these values on Chrome for
Android, using dev tools to inspect the page on a device.

[lpz, 2016-12-15 16:27:28]

On 2016/12/15 15:27:25, edwardjung wrote:
> On 2016/12/15 00:37:07, estark wrote:
> > lgtm if edwardjung is happy
> 
> lgtm although to be sure I would also try to check these values on Chrome for
> Android, using dev tools to inspect the page on a device.

LGTM on android as well. Thanks!

[lpz, 2016-12-15 16:27:33]

The CQ bit was checked by lpz@chromium.org

[commit-bot: I haz the power, 2016-12-15 16:27:52]

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-12-15 16:35:56]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1481819253573340,
"parent_rev": "b5eb535c1c2dc45fe435ca9ea7f51df8e88a0d4b", "commit_rev":
"e1d904f25b25e55ca9973ba1c910d840a5e13485"}

[commit-bot: I haz the power, 2016-12-15 16:36:21]

Description was changed from

==========
Make the extended reporting opt-in checkbox into a flexbox.

This makes multi-line labels left align with themselves instead of wrap
underneath the checkbox.

BUG=667389
==========

to

==========
Make the extended reporting opt-in checkbox into a flexbox.

This makes multi-line labels left align with themselves instead of wrap
underneath the checkbox.

BUG=667389

Review-Url: https://codereview.chromium.org/2566983005
==========

[commit-bot: I haz the power, 2016-12-15 16:36:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-12-15 16:38:30]

Description was changed from

==========
Make the extended reporting opt-in checkbox into a flexbox.

This makes multi-line labels left align with themselves instead of wrap
underneath the checkbox.

BUG=667389

Review-Url: https://codereview.chromium.org/2566983005
==========

to

==========
Make the extended reporting opt-in checkbox into a flexbox.

This makes multi-line labels left align with themselves instead of wrap
underneath the checkbox.

BUG=667389

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

[commit-bot: I haz the power, 2016-12-15 16:38:32]

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