Customizer: controls description inside labels are not real labels nor descriptions

[afercia]

Splitting this out from #32493 (which changed its scope)

I'd propose to investigate about the Customizer controls descriptions. Currently, they're inside the label element. The result is they're not a proper form label, since labels should be short. Longer descriptions should be associated with form controls using aria-describedby.

As consequence for accessibility: as soon as a form control is focused, its label gets read out. In the following example from the new "Site Icon" functionality in 4.3, all the text highlighted in the blue box will be read out but the actual button text "Select File" won't. ​Buttons can have labels, that's correct HTML, but of course the label text will override the button text.

Not to mention all the label text is clickable and will activate the button when clicked.

Same for input fields, the control description used as label will be read out as... a label.

Related: #31540

[celloexpressions]

For reference, this pattern is used by pretty much all of the core Customizer controls currently. Probably shouldn't have been done that way in #27981, but it was.

[valendesigns]

@afercia Could you please test out 33085.diff​ and see if the approach works for screen readers. I would still need to update custom controls, this patch only updates the controls in WP_Customize_Control::render_content. However, I want to verify this is the correct direction before tackling the custom controls. Which are not as easy and straight forward as these default ones were to change, mainly because most of them are JS templates.

[afercia]

@valendesigns Coulnd't find any of the "default" controls from WP_Customize_Control::render_content with a description so I've manually added a couple of them, see in the screenshot below how they're announced using Firefox + NVDA: works out the way it's supposed to :)

Having the labels associated with the for attribute (i.e. no more "wrapping" the control) is a nice improvement too especially thinking at a way to provide the ability to hide labels, see #33064.

[valendesigns]

@afercia Awesome! I'll roll the changes out to the other controls and create a new patch. Thanks for verifying.

[wonderboymusic]

soon please

[westonruter]

@valendesigns Will you be able to update the patch today? This needs to be committed today ideally so we don't cut it too close to RC1.

[valendesigns]

Sure, I'll take another crack at it when I wake up. Right now I'm going to sleep.

[valendesigns]

@westonruter As promised, the patch has been updated.

@afercia Could you please test the markup changes if you have time?

[afercia]

@valendesigns tried to do my best, please consider I'm not so familiar with the Customizer. For example, about the Media control I've checked just the Site Icon not sure if all the other controls that inherit from Media control use the same template. I guess they should. The controls I've seen look OK to me, the descriptions are moved out from the labels and the aria-describedby attributes just work.

The color picker is a bit weird since the description gets read out only in a second moment, when the control is expanded and the real input is focused. I guess there's not much we can do here for now.

From a UI perspective, I'm not sure the descriptions should be printed out before their controls, maybe they should be moved after, probably something to consider for the next release :)

All in all I think this is a very nice improvement.

[westonruter]

Punting due to the size of the patch in relation to the start of RC. Let's commit at the beginning of 4.5.

[westonruter]

This has also been raised at https://core.trac.wordpress.org/ticket/30738#comment:16

[westonruter]

The label needs to go _inside_ the customize-control-title not wrapping around it. This is because notifications container gets injected after the title and we don't want notifications to appear inside the label.

[afercia]

@westonruter assigning to you, since you set the milestone and I can't be of any help here :)

[celloexpressions]

This is fixed by #30738, so we'll wait until we can address both in the same release.

[afercia]

Needs a decision for the 4.8 release or punt? /cc @westonruter

[westonruter]

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

[westonruter]

#42079 was marked as a duplicate.

[westonruter]

In 41740:

Customize: Improve accessibility of markup for base WP_Customize_Control and WP_Customize_Nav_Menu_Control with proper use of label elements and inclusion of aria-describedby.

See #33085.
Props valendesigns, afercia, westonruter.

[westonruter]

I'll leave the other controls to your capable hands.

[westonruter]

In 41817:

Customize: Fix vertical alignment of radio and checkbox inputs after [41740].

Props Shital Patel, subrataemfluence, sayedwp.
Amends [41740].
See #33085.
Fixes #42157.

[westonruter]

In 41822:

Customize: Prevent outputting value attribute if already supplied among input_attrs.

This allows for input[type=button] controls to be added without producing illegal HTML.

Amends [41740].
See #30738, #33085.

[afercia]

I'm sorry but realistically I don't have time to handle the other controls in the next week, nor I've ever asked to own this ticket. This doesn't need just a review, it needs actual work to complete the unfinished job. On a side note, I think assigning to someone a ticket that still needs some work without asking before if he/she has time to dedicate to the ticket is not a good approach.

[westonruter]

Punting.

[westonruter]

In 42031:

Customize: Improve Media control accessibility and compatibility for settings passed as arrays or as solitary setting.

• Eliminate Media control template from having dependency on params.settings.default for element ID, to fix compat with params.settings array or single params.setting. See #36167.
• Move description out of label and add aria-describedby to Media control's Select button. See #30738, #33085.
• Obtain notification container whenever content is (re-)rendered (such as for Media control). See #38794.
• Re-render notifications after control content is re-rendered, if control is in expanded section. See #38794.

Amends [41390].
See #36167, #38794, #33085, #30738.

[joedolson]

Hi, @westonruter - do you have a clear idea what's left to be done on this ticket? Lots of commits, but I'm not clear what still needs doing. We can re-test if necessary, but if you have a good idea, that would definitely help!

[westonruter]

I think re-testing is warranted. I'm not clear on what's left either. If I recall correctly, the required changes were applied to a few controls but there were others that need the changes applied as well.

[alh0319]

I'll retest for this ticket on Friday.