Closed Bug 1330121 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Preferences, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 55
Tracking Status
firefox55 --- fixed
firefox57 --- verified

People

(Reporter: mconley, Assigned: timdream)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-preference])

Attachments

(1 file)

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.
Whiteboard: [photon-preference]
Priority: -- → P1
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 55
Flags: qe-verify+
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 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 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 on attachment 8862317 [details]
Bug 1330121 - Allow <checkbox> in preferences to expand to full width.

Will resubmit
Attachment #8862317 - Flags: review?(jaws)
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 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+
QA Contact: hani.yacoub
Actually, I was waiting for bug 1357348 to land.
Depends on: 1357348
No longer depends on: 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
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
https://hg.mozilla.org/mozilla-central/rev/866dd3f20949
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
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
You need to log in before you can comment on or make changes to this bug.