Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#50512 closed defect (bug) (fixed)

Improve accessibility of the Plugin & Themes Auto-Updates feature

Reported by: ryokuhi's profile ryokuhi Owned by:
Milestone: 5.5 Priority: high
Severity: normal Version: 5.5
Component: Upgrade/Install Keywords: has-patch
Focuses: ui, accessibility, administration Cc:

Description

The Accessibility Team made a review of the Plugin & Themes auto-updates feature, recently introduced into core through #50052. A few accessibility issues where pointed out.

Currently, Enable / Disable controls are links. According to the accessibility coding standards (see the third row in the table):

  • the control should be a link when JavaScript is not available and
  • a button the rest of the time.

According to the current implementation, instead:

  • when JavaScript is off, the links work because they have a working href attribute;
  • when JavaScript is on, the links behave like buttons, but they’re still perceived as links.

A partial solution would be to add the CSS class aria-button-if-js, that adds a role=button for these cases; this doesn't make the link-as-button work through spacebar as expected, though.
To replicate the standard button behaviour, there's the need to:

  • add a keydown event and prevent the default action (page scrolling);
  • add a keyup event to trigger a click on the link, as native buttons are activated when the spacebar is released.

A better solution would be to change the link element to a button element, but the class aria-button-if-js should be changed with caution because of the risk to break existing behaviour (in case keydown and keyup events are already attached to the link).

Another solution might be to create a new CSS class aria-button-behavior-if-js, that can be assigned instead of aria-button-if-js whenever it is safe to do that.

Icons don't use aria-hidden=true

This problem was already addressed and solved during WordCamp Europe Contributor Day (see #50293)

Button text change

Currently, the "enabling / disabling" button text changes depending on the state, but it's a pattern that shouldn't be encouraged in general.

Two possible solutions were proposed.

  1. Create two separate notifications for enabling and disabling, insert them after the button and then transition the text of the button after the event (for example, from “Enable auto-updates” to “Disable auto-updates”).
  2. Have two buttons, one for enabling auto-updates and the other for disabling them.

The first solution might cause problems to some users. For example:

  • Users learn there’s a button that says “Enable auto-updates”.
  • Users do their job and enable (or disable) updates for various plugins.
  • They search again for a button named “Enable auto-updates”, because the UI taught them there’s such a button.
  • The button isn’t there any longer, at least apparently, because it now has a different name.

The second solution means there are always two buttons and one of them is always disabled, since it can't be used.

It's possible that there won't be a good solution for this specific issue and that it would be better to leave the interface as is.

Inline notifications

There's some code for inline notifications in case of errors: it's an empty paragraph that probably gets filled with some error message. It should be tested on how the message is announced and exposed to assistive technologies.

Attachments (1)

50512.diff (4.8 KB) - added by afercia 4 years ago.

Download all attachments as: .zip

Change History (12)

#1 @audrasjb
4 years ago

Related: #50516

#2 @afercia
4 years ago

  • Component changed from Security to Upgrade/Install

#3 @afercia
4 years ago

Note: the first part (button / links) has a patch on #50516.

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


4 years ago

#5 @desrosj
4 years ago

  • Priority changed from normal to high

I'm marking this as a high priority because these a11y issues should be fixed prior to release.

I read through the chat log discussion, and it seems only items 3 & 4 remain.

#6 @afercia
4 years ago

Correct thanks @desrosj! Only 3) Button text change, and 4) Inline notifications remain.
Not sure 3) can be addressed for 5.5 as it would need some design decision. I'll test 4).

#7 @afercia
4 years ago

Double checking 4) Inline notifications:

I see the pattern where a notice with an empty paragraph is printed out and hidden until it receives some text is already used in a few places, for example:

  • the posts/terms quick edit/bulk edit
  • wp_comment_reply()

We should check all those instances. For what concerns the specific instances used for the auto-updates, I see that whenever there's an error, the notice gets revealed to display an error message and the same message is sent to the polite ARIA live region via wp.a11y.speak() 👍

The only thing I'd change is the politeness level of the ARIA live messages. Since these notices are errors, I'd use assertive so that the audible messages are equivalent to alerts and assistive technologies will immediately notify the user interrupting the current announcement.

Quick patch incoming.

Re: 3) Button text change:

I think this should be postponed to the next release cycle. Will wait for the next accessibility team meeting on Friday to make a final decision. To summarize what was discussed so far:

  • the auto-updates links are more an on/off toggle
  • the more appropriate native element for that would be a checkbox
  • concerns were expressed for the presence of too many checkboxes in the plugins and (network) themes tables
  • we'd need to evaluate some other kind of "toggle" control
  • for example a toggle button with normal and "pressed" state, or a switch toggle
  • the goal for this change is, as usual:
    • a control's text should not change dynamically to represent its state
    • the state should be conveyed separately
    • the control should only communicate the underlying action and maybe have a visual hint for its state (like the checkmark for native checkboxes) but it should never change its text

@afercia
4 years ago

#8 @afercia
4 years ago

  • Keywords has-patch commit added; needs-patch removed

50512.diff does two things:

  • changes the politeness level of the two calls to wp.a11y.speak() for the error messages introduced in [47835] to assertive
  • removes the parameter polite from all the other usages: unnecessary because polite is the default level

#9 @afercia
4 years ago

In 48479:

Accessibility: Security: Improves the accessible audible messages for Plugins & Themes Auto-Updates.

  • changes the politeness level of the two error messages introduced in [47835] to assertive
  • remove unnecessary polite parameters as that's the default value

See #50512, #50052.

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


4 years ago

#11 @desrosj
4 years ago

  • Keywords commit removed
  • Resolution set to fixed
  • Status changed from new to closed

Per the last linked a11y chat by Slackbot, the remaining issues will be tackled in 5.6 through a new ticket. Closing this one out.

Can someone please just make sure to open new tickets for any other remaining issues?

Note: See TracTickets for help on using tickets.