WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#32726 closed defect (bug) (fixed)

Customizer: Accessibility: form controls need a label with text

Reported by: designsimply Owned by: celloexpressions
Milestone: 4.3 Priority: normal
Severity: normal Version: 4.3
Component: Customize Keywords: has-patch
Focuses: accessibility Cc:
PR Number:

Description

/hat tip @afercia for the original report at https://github.com/voldemortensen/menu-customizer/issues/97

Found the edit menu name in Customizer Menus has a label without any text, haven't checked all form controls though.

Currently:

<label>
<input class="menu-name-field live-update-section-title" value="Testing Menu Customizer" data-customize-setting-link="nav_menus[236][name]" type="text">
</label>

To my understanding, this would require something in in WP_Customize_Control to add CSS classes to the label text (the inner span, not the label itself) in order to use screen-reader-text and hide the label text.

Attachments (2)

32726.patch (2.9 KB) - added by afercia 4 years ago.
32726.2.diff (1.8 KB) - added by celloexpressions 4 years ago.
Add a label to the menu name field for screen readers.

Download all attachments as: .zip

Change History (19)

#1 @designsimply
4 years ago

Screenshot for reference:

https://cloud.githubusercontent.com/assets/1119271/8150752/e9204094-12c3-11e5-9fd3-5c42220a227f.png
Tested with WordPress 4.3-alpha-32775 and Menu Customizer plugin 0.6 (cef7fbc).

@afercia
4 years ago

#2 @afercia
4 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 4.3

First pass.

  • adds a new property $label_text_class to set CSS classes on the labels <span>
  • adds missing label text in WP_Customize_Nav_Menu_Name_Control::content_template()

#3 @afercia
4 years ago

  • Version set to trunk

#4 @celloexpressions
4 years ago

I'm not a fan of adding a label_text_class property to the main core control. But, I think it could be useful to add something along the lines of hidden_label, that allows devs to more easily include a label but not show it, including making it accessible for screen readers automatically in the process. Looking at the patch, but not the surrounding code, it looks like the change to the new menu control covers the issue for Menus, so we wouldn't need the change to the main control, but I haven't looked closely.

#5 @afercia
4 years ago

Would be happy to see any improvements to the patch, I'm not so familiar with the Customizer and implemented just a basic way to get the job done :) Please feel free to change it whatever way you think it's best, as long as form fields have a label.

#6 follow-up: @celloexpressions
4 years ago

  • Keywords needs-patch added; has-patch removed

I think the best approach here is to add the label via the API and then hide it manually with CSS in customize-nav-menus.css. Rather than changing the API.

#7 in reply to: ↑ 6 @obenland
4 years ago

Replying to celloexpressions:

I think the best approach here is to add the label via the API and then hide it manually with CSS in customize-nav-menus.css. Rather than changing the API.

Can you write a patch for that? It would be good to keep moving this ticket forward.

#8 @obenland
4 years ago

@celloexpressions, could you make this part of your weekend todo list?

#9 @celloexpressions
4 years ago

  • Owner set to celloexpressions
  • Status changed from new to assigned

I can do a patch for this in time for beta 3, should be pretty straightforward.

@celloexpressions
4 years ago

Add a label to the menu name field for screen readers.

#10 @celloexpressions
4 years ago

  • Keywords has-patch added; needs-patch removed

I really don't like that we have a custom control for a text input (or custom controls for checkboxes and selects), but that's apparently how it is right now, so 32726.2.diff should work. #30738 would've probably avoided the need for those custom controls with a bit of creativity in the implementation on the menus side.

#11 follow-up: @afercia
4 years ago

Tested the patch, works nicely. Noticed also the New menu name would need a label text, currently it has just a label without text. Is there already a ticket for that or maybe can be handled here?

#12 in reply to: ↑ 11 @obenland
4 years ago

Replying to afercia:

Tested the patch, works nicely. Noticed also the New menu name would need a label text, currently it has just a label without text. Is there already a ticket for that or maybe can be handled here?

Feel free to add to celloexpressions' patch.

This ticket was mentioned in Slack in #core-customize by afercia. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by obenland. View the logs.


4 years ago

#15 @afercia
4 years ago

Adding a label for the New menu name would be pretty easy but then the label text will be visible. Maybe we should consider to improve the default output adding an optional way to have hidden label text. A temporary fix would be some redundant CSS rule :)

#16 @obenland
4 years ago

It sounds like something that affects more than just the new menu input field. Let's get the existing patch in and create a new ticket for that for 4.4.

#17 @obenland
4 years ago

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

In 33346:

Customizer: Add label for menu names when editing a menu.

Props celloexpressions.
Fixes #32726.

Note: See TracTickets for help on using tickets.