Active area for checkbox + label pairs in a group should equal width of longest checkbox + label pair

VERIFIED FIXED in Firefox 55

Status

()

Firefox
Preferences
P1
normal
VERIFIED FIXED
8 months ago
12 days ago

People

(Reporter: mconley, Assigned: timdream)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 55
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox55 fixed, firefox57 verified)

Details

(Whiteboard: [photon-preference])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Reporter)

Description

8 months ago
From Slide 16 of https://docs.google.com/presentation/d/1EA-qGEjhkebW9IbUZ_fv_XYflu5dlutc60Nw57nT5sQ/edit#slide=id.g18289739c9_3_7

Suppose we have a series of checkboxes and labels, like this:


[ ] This is option 1
[ ] And this, this is a longer string, and this is option 2

The "active" area, AKA the area that one can click to toggle the checkbox, is currently just the width of the checkbox plus the width of the label.

What we want is that, for a set of checkboxes in a section like this, the active area width should match the widest pair in a particular group. So the whitespace to the right of "This is option 1" would be clickable in the above example.

Updated

4 months ago
Whiteboard: [photon-preference]

Updated

4 months ago
Priority: -- → P1
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Target Milestone: --- → Firefox 55

Updated

4 months ago
Flags: qe-verify+

Comment 2

4 months ago
mozreview-review
Comment on attachment 8862317 [details]
Bug 1330121 - Allow <checkbox> in preferences to expand to full width.

https://reviewboard.mozilla.org/r/134254/#review137424

This looks really good. I think one more look and it will be r+

I noticed that we still need to fix up the checkbox for notificationsDoNotDisturb and offlineNotify.

::: browser/components/preferences/in-content/advanced.xul:72
(Diff revision 1)
>        <checkbox id="useService"
>                  label="&useService.label;"
>                  accesskey="&useService.accesskey;"
>                  preference="app.update.service.enabled"/>

Any reason why this checkbox wasn't moved out of the <hbox> too?

::: browser/components/preferences/in-content/main.xul:323
(Diff revision 1)
>    <menulist id="defaultEngine">
>      <menupopup/>
>    </menulist>

Please indent the contents of the <box>

::: browser/components/preferences/in-content/privacy.xul:248
(Diff revision 1)
>                        preference="network.cookie.cookieBehavior"
>                        accesskey="&acceptCookies.accesskey;"
>                        onsyncfrompreference="return gPrivacyPane.readAcceptCookies();"
> -                      onsynctopreference="return gPrivacyPane.writeAcceptCookies();"/>
> -            <spacer flex="1" />
> +                      onsynctopreference="return gPrivacyPane.writeAcceptCookies();"
> +                      flex="1" />
> +            <spacer />

This spacer and the other spacers without flex can be removed.
Attachment #8862317 - Flags: review?(jaws) → review-
Comment hidden (mozreview-request)

Comment 4

4 months ago
mozreview-review
Comment on attachment 8862317 [details]
Bug 1330121 - Allow <checkbox> in preferences to expand to full width.

https://reviewboard.mozilla.org/r/134254/#review137534

Looks good, thanks for the cleanup too!
Attachment #8862317 - Flags: review?(jaws) → review+

Comment 5

4 months ago
mozreview-review
Comment on attachment 8862317 [details]
Bug 1330121 - Allow <checkbox> in preferences to expand to full width.

https://reviewboard.mozilla.org/r/134254/#review137536

Actually, I think the useService checkbox should be moved next to the enableSearchUpdate checkbox still. It looks like my previous question was unanswered.
Attachment #8862317 - Flags: review+ → review-
Comment hidden (mozreview-request)
Comment on attachment 8862317 [details]
Bug 1330121 - Allow <checkbox> in preferences to expand to full width.

Will resubmit
Attachment #8862317 - Flags: review?(jaws)
(Assignee)

Comment 8

4 months ago
mozreview-review-reply
Comment on attachment 8862317 [details]
Bug 1330121 - Allow <checkbox> in preferences to expand to full width.

https://reviewboard.mozilla.org/r/134254/#review137536

Rev 2 was incomplete. Sorry about that. All has since fixed.
Comment on attachment 8862317 [details]
Bug 1330121 - Allow <checkbox> in preferences to expand to full width.

(I wish mozreview do things atomic & don't keep separate states..)
Attachment #8862317 - Flags: review?(jaws)
Comment hidden (mozreview-request)

Comment 11

4 months ago
mozreview-review
Comment on attachment 8862317 [details]
Bug 1330121 - Allow <checkbox> in preferences to expand to full width.

https://reviewboard.mozilla.org/r/134254/#review137554

Thanks for the quick fixes.
Attachment #8862317 - Flags: review?(jaws) → review+

Updated

4 months ago
QA Contact: hani.yacoub
Comment hidden (obsolete)
Actually, I was waiting for bug 1357348 to land.
Depends on: 1357348
No longer depends on: 1356507
Blocks: 1356507
There wasn't any conflict apply the patch after bug 1357348 and the checkboxes there require no additional fixup. This is good.
No longer depends on: 1357348
Keywords: checkin-needed

Comment 15

4 months ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/866dd3f20949
Allow <checkbox> in preferences to expand to full width. r=jaws
Keywords: checkin-needed

Comment 16

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/866dd3f20949
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED

Comment 17

12 days ago
Build ID: 20170810100255
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0

Verified as fixed on Firefox Nightly 57.0a1 on Windows 10 x 64, Mac OS X 10.12 and Ubuntu 16.04 x64.
Status: RESOLVED → VERIFIED
status-firefox57: --- → verified
You need to log in before you can comment on or make changes to this bug.