WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#34607 closed defect (bug) (fixed)

Customizer: Wrapped labels should align to start of label, not checkbox

Reported by: melchoyce Owned by: westonruter
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-patch commit
Focuses: ui Cc:

Description

When you have a long label (for example, "Automatically add new top-level pages to this menu"), the label wraps onto a new line and aligns with the checkbox, rather than aligning with the start of the label. This breaks up the flow of the label and looks pretty bad. The second line should align with start of the label text.

Attachments (4)

customizer-label-wrapping.png (34.3 KB) - added by melchoyce 4 years ago.
34607.0.patch (820 bytes) - added by bordoni 4 years ago.
Possible solution to what was reported.
34607.1.patch (1.4 KB) - added by bordoni 4 years ago.
Fixes to responsive problems + radio
34607.2.diff (1.8 KB) - added by delawski 4 years ago.
https://github.com/xwp/wordpress-develop/commit/ff1866203198636bcde3447f431ce88212cb7be7

Download all attachments as: .zip

Change History (23)

#1 @bordoni
4 years ago

  • Keywords has-patch added

The following patch would solve the reported issue, but I'm not sure if the applying rules to .customize-control-nav_menu_auto_add is the best solution, instead of adding a class to the element to mark it as a checkbox.

@bordoni
4 years ago

Possible solution to what was reported.

#2 @westonruter
4 years ago

  • Keywords needs-screenshots added
  • Owner set to bordoni
  • Status changed from new to assigned

@bordoni I think we should scope these style changes to checkbox controls that appear under the .customize-control-nav_menu element. This will ensure the .customize-control-checkbox styles don't apply to any checkbox in the Customizer, which may introduce an unexpected regression.

Also, could you add a screenshot that shows how the style changes impact long labels that appear under Menu Locations as well as Menu Options?

#3 follow-up: @bordoni
4 years ago

Screenshot:

http://d.pr/i/1hoaA+

My only question is, shouldn't this behavior for the Checkbox be standard across the whole customizer? At least that was what I understood from the ticket.

Like on this one:

http://d.pr/i/19KRE+

#4 @bordoni
4 years ago

  • Keywords needs-screenshots removed

#5 in reply to: ↑ 3 @westonruter
4 years ago

Replying to bordoni:

My only question is, shouldn't this behavior for the Checkbox be standard across the whole customizer? At least that was what I understood from the ticket.

Oh! I didn't catch that. I incorrectly thought this was specifically regarding menus.

@melchoyce Thoughts on @bordoni's patch to apply generally for all checkboxes in the Customizer?

#6 @melchoyce
4 years ago

Haven't looked at the patch, but I agree it should apply to all checkboxes in the Customizer.

#7 @westonruter
4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 35608:

Customize: Prevent multi-line checkbox control labels from wrapping under checkbox.

Props bordoni.
Fixes #34607.

#8 @westonruter
4 years ago

  • Milestone changed from Awaiting Review to 4.4

#9 @afercia
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

On small screens, the checkboxes size is different (at least in Chrome, Firefox doesn't allow to fully restyle checkboxes) so the label text won't be perfectly aligned. It would be great to tweak a bit the styling in the proper media query.
Also, thinking this should probably apply to radio buttons too, see screenshot below:

https://cldup.com/2jgInOWqma.png

#10 follow-up: @bordoni
4 years ago

Great catch @afercia I will create a new patch to solve that mobile issue.

@melchoyce what do you think about applying the styles to Radio Input?

#11 in reply to: ↑ 10 @melchoyce
4 years ago

Replying to bordoni:

Great catch @afercia I will create a new patch to solve that mobile issue.

@melchoyce what do you think about applying the styles to Radio Input?

Yup, radios should get the styling too.

#12 @bordoni
4 years ago

On the Patch.1 I've applied the changes to the Radio buttons and fixed what @afercia noted in his comment.

Also, while I was testing the responsive, I also found that the widgets the checkboxes didn't have enough space.

So I've patched that too. ;)

http://d.pr/i/QaR7+

@bordoni
4 years ago

Fixes to responsive problems + radio

#13 @westonruter
4 years ago

@afercia @melchoyce please review.

#14 @delawski
4 years ago

In 34607.2.diff:

Extend Customizer checkbox patch on radios and fix minor media breakpoint glitch

  • I've extended the fix to checkboxes in Customizer by @bordoni so that it is applied to radios as well
  • I've unified the media query breakpoint value for small screens so that it is 782 px for all cases

#15 follow-up: @bordoni
4 years ago

Hi @delawski,

Why the change from 780 to 782? Really curious on that one =D

#16 in reply to: ↑ 15 @delawski
4 years ago

Replying to bordoni:

Hi @delawski,

Why the change from 780 to 782? Really curious on that one =D

Hi @bordoni,

In almost all media queries used in Customizer's stylesheets the value of 782px is used. Using 780px in that single location caused inconsistencies when the screen width was passing through one of the two breakpoints. The screencast shows the issue:

https://cloudup.com/cSCQdxdwmNg

#17 @westonruter
4 years ago

  • Keywords commit added
  • Owner changed from bordoni to westonruter
  • Status changed from reopened to accepted

I'll commit this later today unless there is any more feedback.

#18 @bordoni
4 years ago

Thanks for the answer @delawski.

#19 @westonruter
4 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 35643:

Customize: Ensure multi-line label alignment for checkbox/radio inputs on both desktop and mobile.

Follow-up to [35608].

Props bordoni, delawski.
Fixes #34607.

Note: See TracTickets for help on using tickets.