#39662 closed defect (bug) (fixed)
Color picker accessibility improvements
Reported by: | afercia | Owned by: | afercia |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Administration | Keywords: | has-screenshots has-patch commit |
Focuses: | ui, 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:
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 (5)
Change History (31)
#3
@
8 years ago
Forgot to mention it also needs to check what happens when the color picker is used outside of the Customizer.
#4
@
8 years 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.
#5
@
8 years 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:
Responsive view, with default color:
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:
#6
@
8 years 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.
8 years ago
#9
@
8 years ago
- Milestone changed from 4.8 to Future Release
Punting as we're running out of time for the 4.8 release.
#11
@
8 years 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.
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
7 years ago
This ticket was mentioned in Slack in #core-editor by afercia. View the logs.
7 years ago
This ticket was mentioned in Slack in #core-customize by afercia. View the logs.
7 years ago
#17
@
7 years ago
@Cheffheid I've finally found some time to check this. I can confirm Firefox+NVDA don't announce aria-expanded. Crazily enough, it works with IE 11 + NVDA, and even with Chrome + NVDA (which is known to not be the best combination). It works with IE 11 and JAWS and Firefox and JAWS.
After some hours of debugging, I've found that removing the CSS class button
(or simply renaming it, say to xxxbutton
) fixes it. No idea why, seems not related to CSS to me. Can you please try this, ignoring the temporarily broken style? The CSS class is set in the wp-admin/js/color-picker.js
file:
_before = '<button type="button" class="button wp-color-result" aria-expanded ...
#18
@
7 years ago
No idea why, the CSS rule triggering this bug is the transform applied to the buttons when they're in an :active
state, i.e. while they're "clicked". See transform: translateY(1px);
in buttons.css
.
(just a wild guess: Firefox repaints the button and in combination with "something else" this makes NVDA think it's landed on a new element).
The refreshed patch:
- resets the CSS transform for this button
- cleans up a CSS rule that as using a no more used selector
- removes translatable strings that were introduced to test future improvements and not actually used in this patch
aria-expanded
is now announced correctly by al the major screen readers.
#20
@
7 years ago
39662.5.diff slightly improves the responsive view. Worth noting is basically impossible to achieve a pixel-perfect layout across browsers and platforms. Since the introduction of system fonts (see #36753), some form elements that base their height on the line-height value get rendered differently depending on the platform font metrics. This is particularly noticeable when buttons use line-height: normal
. To improve consistency across platforms and different system fonts, great part of the WordPress CSS should be refactored and this is out of the scope of this ticket.
Worth also noting buttons in the Customizer use an additional media query at 640px, so there's the need of some additional adjustments.
Screenshots to recap:
Closed and open, before and after:
Responsive view in macOS Chrome, before and after
#21
@
7 years ago
- Component changed from Customize to Administration
- Focuses ui added
Changing component to "Administration" since the color picker is not specific to the Customizer.
This ticket was mentioned in Slack in #core-customize by aristath. View the logs.
7 years ago
#25
@
7 years ago
@afercia Just a note on this one:
A lot of plugins and themes use 3rd-party scripts to modify the default behaviour of colorpickers and add an alpha channel to them. The most widely used script to accomplish that is https://github.com/kallookoo/wp-color-picker-alpha
The changes in this ticket are great, the only downside is that most 3rd-party scripts (including the wp-color-picker-alpha mentioned above) will now fail because of the html changes in the colorpickers. Ideally rgba would be possible without using any hacky scripts (see https://core.trac.wordpress.org/ticket/39681) but for the time being Plugin & Theme authors should be very careful and release updates before WP 4.9 is released.
Some support tickets will be inevitable though until everyone is up to speed and everything gets updated.
#26
@
7 years ago
@aristath thanks. Seems to me they're aware of this change, see https://github.com/kallookoo/wp-color-picker-alpha/issues/10
Worth noting the markup to build controls around the Iris color picker is not part of any standardized UI. Plugins or themes that extend this part are actually hacking around it. No objections to that, but they should be aware that this code is not guaranteed to be immutable and they should follow closely core development to update timely.
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:aria-expanded
attribute on the toggle buttonaria-label
attributes on the "Default/Clear" button