WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 3 weeks ago

#39662 assigned defect (bug)

Color picker accessibility improvements

Reported by: afercia Owned by: afercia
Milestone: 4.9 Priority: normal
Severity: normal Version:
Component: Customize Keywords: has-screenshots has-patch needs-testing
Focuses: accessibility, javascript Cc:

Description

While investigating #39096, I've noticed the color picker control could be greatly improved and simplified. The main accessibility issue is about the HTML output, which turns out to be confusing for screen readers.

Ideally, a <label> element should contain just the label text (if it's explicitly associated) or the label text and the associated form control (if it's implicitly associated). That is what screen readers (and any other software) expect to find.

In this case instead, the <label> element contains other controls (a link, a button, the notification container, and the whole iris color picker). Not surprisingly, this can make screen readers not able to understand "what" the color picker is and how it is supposed to work.

Testing with Safari 10 + VoiceOver, the confusion is so severe that VoiceOver starts announcing "Safari busy" and then just hangs on a "busy" state:

https://cldup.com/zvk6tk3Bk8.png

Note: I think the message "Safari busy" is unrelated to the real issue. I guess what's happening here is VoiceOver just assumes it has to wait for Safari to correctly expose the information it needs, but that never happens.

Attachments (2)

39662.diff (9.2 KB) - added by afercia 7 months ago.
39662.2.diff (12.3 KB) - added by afercia 7 months ago.

Download all attachments as: .zip

Change History (15)

@afercia
7 months ago

#1 @afercia
7 months ago

  • Keywords has-patch added; needs-patch removed

39662.diff is just a first pass. There's still the need to check a few things, for example the notification container and the hue control. What the proposed patch does so far:

  • the toggle button is now a real button
  • thus, it needs just a click event: no need to handle the Spacebar and Enter keys separately
  • the label element now contains just the label text and the input field
  • refactored the HTML to have a better overall structure
  • simplified the way elements visibility gets toggled
  • added an aria-expanded attribute on the toggle button
  • added aria-label attributes on the "Default/Clear" button
  • CSS clean-up
Last edited 5 months ago by afercia (previous) (diff)

#2 @afercia
7 months ago

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

#3 @afercia
7 months ago

Forgot to mention it also needs to check what happens when the color picker is used outside of the Customizer.

Last edited 5 months ago by afercia (previous) (diff)

#4 @celloexpressions
7 months ago

  • Milestone changed from Awaiting Review to 4.8

This looks like a good start. It should be safe to make all of these changes but let's double-check touch, mouse, keyboard, and screen reader with the final patch to be sure.

@afercia
7 months ago

#5 @afercia
7 months ago

  • Keywords needs-testing added

Yep, 39662.diff works in the Customizer but doesn't in the standalone version. The new patch takes a different approach and uses a default label to wrap the input field. This way, either in the standalone version or in the Customizer one, there's always a label that wraps the input field and it can be used as "hook" in the DOM to inject all the other elements.

The color picker could probably benefit from some kind of JS templating, and that would be a nice improvement for the future. For back-compat reasons, I guess the safer option for now is to stick to jQuery methods to build all the HTML.

39662.2.diff improves a bit also the responsive view. To test the standalone version, it's possible to directly access the old wp-admin/themes.php?page=custom-header and wp-admin/themes.php?page=custom-background pages, if the active theme supports them (try with Twenty Sixteen).

Note: the patch includes some labels for the Iris color picker meant for future improvements. Iris could be greatly improved for accessibility, however some of the required changes should be implemented upstream. Will open a separate ticket.

Responsive view, no default color:

https://cldup.com/Euj7jm3pvW.png

Responsive view, with default color:

https://cldup.com/Xxrbg6EMgE.png

In other languages, the translated strings may be a bit longer and there's not enough space. The input field and the Clear/Default button drop below the toggle button but that doesn't look too bad:

https://cldup.com/0EETPm2zHC.png

Last edited 7 months ago by afercia (previous) (diff)

#6 @afercia
7 months ago

Couple additional things to mention:

  • the patch removes moving the focus on the input field: for better accessibility, focus should stay on the toggle button to make 'aria-expanded` work properly
  • the preset palette colors on the bottom have now a focus style

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


7 months ago

#8 @afercia
7 months ago

Forgot to mention the patch removes:

  • the title attribute: because it just repeats the initial button text
  • the switching of the button text to "Current Color" since it doesn't add a great value and it's potentially confusing for assistive technologies users

https://cldup.com/2W-XlIlZtq.png

#9 @afercia
3 months ago

  • Milestone changed from 4.8 to Future Release

Punting as we're running out of time for the 4.8 release.

#10 @afercia
3 months ago

  • Milestone changed from Future Release to 4.8.1

#11 @Cheffheid
3 months ago

I'm testing in Firefox, with NVDA, on Windows. Some of these issues are not directly related to your changes, so I'm sorry if this is not the place for them. :)

When I toggle a colour picker field (in the Customizer) it doesn't announce the change in the aria-expanded state. I can tell it's changing, so I'm not sure what's going on there (will investigate further).

Additionally, while we're on the subject of improvements, when you enter an invalid hex code (like, #0000) the field turns red. But no feedback is given by NVDA that the value is invalid. I can still hit the "Save & Publish" (assuming it is active, it doesn't always enable when the value is invalid), but my value is never saved. I only find this out after refreshing.

#12 @westonruter
2 months ago

  • Milestone changed from 4.8.1 to 4.9

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


3 weeks ago

Note: See TracTickets for help on using tickets.