Make WordPress Core

Opened 5 years ago

Last modified 5 weeks ago

#47051 assigned defect (bug)

Twenty Nineteen theme sub-menu returns error in WAVE accessibility tool

Reported by: johnfclifford's profile johnfclifford Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.0
Component: Bundled Theme Keywords: needs-patch
Focuses: accessibility Cc:

Description

I'm building a site in (a child theme of) Twenty Nineteen. In the top menu, each instance of a menu item that has a child item is returning one error in the WAVE accessibility tool. The reported error is "empty button" and below is the explanation:

What It Means
A button is empty or has no value text.
Why It Matters
When navigating to a button, descriptive text must be presented to screen reader users to indicate the function of the button.
How to Fix It
Place text content within the <button> element or give the <input> element a value attribute.
The Algorithm... in English
A <button> element is present that contains no text content (or alternative text), or an <input type="submit">, <input type="button">, or <input type="reset"> has an empty or missing value attribute.
Standards and Guidelines
1.1.1 Non-text Content (Level A)
2.4.4 Link Purpose (In Context) (Level A)

This is the HTML that is returning the alleged error:
<button class="menu-item-link-return" tabindex="-1"><svg class="svg-icon" width="24" height="24" aria-hidden="true" role="img" focusable="false" viewBox="0 0 24 24" version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><path d="M15.41 7.41L14 6l-6 6 6 6 1.41-1.41L10.83 12z"></path><path d="M0 0h24v24H0z" fill="none"></path></svg>About us</button>

Attachments (1)

button-accessibility-error.pdf (142.6 KB) - added by johnfclifford 5 years ago.

Download all attachments as: .zip

Change History (13)

#1 follow-up: @afercia
5 years ago

  • Component changed from Menus to Bundled Theme
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 5.2.1
  • Severity changed from minor to normal

@johnfclifford thanks for the ticket and welcome to Trac! Looks like the Twenty Nineteen menu has a few things that don't work well for accessibility.

I'm not sure the empty buttons are the ones in the snippet you provided (actually, there's an "About us" text there). Instead, seems to me the empty buttons are the ones with the CSS class submenu-expand.

On large screens, these buttons don't say anything and don't do anything. Even if they have a tabindex="-1" attribute and they're not focusable, assistive technology users can get to them by other means. For a screen reader user, for example, hearing just "button" and observe the button doesn't do anything is not a great experience:

http://cldup.com/UwlgRTNLbZ.png

On small screens, the whole menu transforms to adapt to touch:

http://cldup.com/A7mkjw4up5.png

These buttons do make the sub-menus slide-in, but they're actually still empty. Worth reminding that many screen reader users do use an external keyboard with their mobile devices.

If this different behavior on large and small screens is desired, then on large screens the buttons should be removed and there should be only the icons. On small screens, the buttons should be displayed and have some meaningful text.

Other things noticed that don't work well:

  • aria-expanded: seems to me on large screens it's never updated. On small screens it is updated, but seems to me the true/false values are randomly incorrect depending on the sub-menus state.
  • aria-haspopup should be used only for controls that require user interaction: on large screen no user interaction is required so this attribute should be removed. It should be added only on small screens where user interaction is required to expand the sub-menus.

@kjellr @allancole @laurelfulford when you have a chance, would you mind having a look at this please? Thank you.

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


5 years ago

#3 in reply to: ↑ 1 @johnfclifford
5 years ago

Thanks for your speedy response to my first-ever ticket. I'll await developments.
Replying to afercia:

@johnfclifford thanks for the ticket and welcome to Trac! Looks like the Twenty Nineteen menu has a few things that don't work well for accessibility.

I'm not sure the empty buttons are the ones in the snippet you provided (actually, there's an "About us" text there). Instead, seems to me the empty buttons are the ones with the CSS class submenu-expand.

On large screens, these buttons don't say anything and don't do anything. Even if they have a tabindex="-1" attribute and they're not focusable, assistive technology users can get to them by other means. For a screen reader user, for example, hearing just "button" and observe the button doesn't do anything is not a great experience:

http://cldup.com/UwlgRTNLbZ.png

On small screens, the whole menu transforms to adapt to touch:

http://cldup.com/A7mkjw4up5.png

These buttons do make the sub-menus slide-in, but they're actually still empty. Worth reminding that many screen reader users do use an external keyboard with their mobile devices.

If this different behavior on large and small screens is desired, then on large screens the buttons should be removed and there should be only the icons. On small screens, the buttons should be displayed and have some meaningful text.

Other things noticed that don't work well:

  • aria-expanded: seems to me on large screens it's never updated. On small screens it is updated, but seems to me the true/false values are randomly incorrect depending on the sub-menus state.
  • aria-haspopup should be used only for controls that require user interaction: on large screen no user interaction is required so this attribute should be removed. It should be added only on small screens where user interaction is required to expand the sub-menus.

@kjellr @allancole @laurelfulford when you have a chance, would you mind having a look at this please? Thank you.

#4 @ianbelanger
5 years ago

  • Milestone changed from 5.2.1 to 5.3
  • Version changed from 5.1.1 to 5.0

Changing milestone as Bundled Themes are only updated during major releases. Also changed to version 5.0 as this was introduced in that version.

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


5 years ago

#6 @audrasjb
5 years ago

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

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


5 years ago

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


5 years ago

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


5 years ago

#10 @afercia
5 years ago

  • Milestone changed from 5.3 to Future Release

Discussed during today's accessibility bug-scrub: giving WordPress 5.3 Beta 2 is very close, agreed to punt this ticket to Future Release. Just noting it's a bit unfortunate the Twenty Nineteen is still far from ideal.

#11 @nataliemac
5 years ago

Started digging into this at WordCamp US today. Just focusing on the main navigation, it needs a lot of work and we might want to divide this up into two or more separate tickets. Here are the individual issues that need to be addressed:

  • Mobile: Empty buttons need to have some kind of text alternative to indicate the purpose of the button
  • Desktop: Non-functional buttons should be replaced with just an svg icon. I believe no text alternative is needed for these icons
  • Desktop: Ensure aria-expanded attribute is correctly updated when submenus open/close
  • Mobile: Debug intermittent incorrect aria-expanded attributes for submenus
  • Desktop: Remove aria-haspopup attribute

Additionally, after we adopt WCAG 2.1, this menu will need some other updates and changes as well.

I've started work on the first two bullet point issues listed above as they're related the original reason for the ticket. The others perhaps could be moved to separate tickets.

#12 @karmatosed
5 weeks ago

  • Owner nataliemac deleted

@nataliemac I am not sure you are still working on this so for now going to remove you as the person assigned to take this ticket on it's journey forward. We can then see how it progresses from here. If you are that's great and thank you - we can always reassign to you.

Note: See TracTickets for help on using tickets.