Bundled themes: Add ARIA attributes to menu toggle

[davidakennedy]

This patch enhances the accessibility of the mobile menu toggle by adding ARIA attributes to it, and changes them based on clicking the toggle. The aria-exapanded and aria-conrols communicate to screen readers and assistive technology what's happening on the screen.

The onResizeARIA function adds and removes the attributes based on the screen size since the toggle is only present on smaller screen sizes.

[davidakennedy]

Twenty Fifteen ARIA Menu Toggle

[lancewillett]

Thanks for the ticket and the patch. Should we also apply to older default themes?

[lancewillett]

Patch looks mostly good — only change I'd suggest is onResizeAria = function() { to function onResizeARIA() { for declaring the function.

[lancewillett]

Ignore wrong patch, sorry. ;)

.2 patch changes things a tiny bit:

1. Move event handlers to main ready clause and add twentyfifteen namespace.
2. Move onResizeARIA() function out of anonymous function so it can be called from elsewhere in the file.
3. Declare secondary and button variables earlier in the file so they can be used in any function.

[davidakennedy]

Replying to lancewillett:

Ignore wrong patch, sorry. ;)

.2 patch changes things a tiny bit:

1. Move event handlers to main ready clause and add twentyfifteen namespace.
2. Move onResizeARIA() function out of anonymous function so it can be called from elsewhere in the file.
3. Declare secondary and button variables earlier in the file so they can be used in any function.

Thanks for cleaning the patch up and making it better, @lancewillett! Everything works as expected. I'll work this weekend on getting this type of patch into other default themes.

[lancewillett]

Sounds great. I'll get Twenty Fifteen in now.

[lancewillett]

In 31644:

Twenty Fifteen: add ARIA attributes to menu toggle.

See #31527, props davidakennedy, lance.

[DrewAPicture]

@lance ... errr @lancewillett: What's left here? :-)

[lancewillett]

Hah. My bad. ;)

We're waiting on patches for Twenty Fourteen and Twenty Thirteen. However, they can wait for 4.3 if we need to move on — I can just open new tickets for them since Twenty Fifteen is done.

[lancewillett]

Add ARIA attribute toggle behavior to Twenty Fourteen

[lancewillett]

Add ARIA attribute toggle behavior to Twenty Thirteen

[lancewillett]

Patches attached for Twenty Fourteen and Thirteen -- testing and code review appreciated. :)

[lancewillett]

One note on Twenty Thirteen: using tertiary for the ARIA role of secondary since it's the sidebar; the widget area in the footer uses secondary ID, though.

[davidakennedy]

Replying to lancewillett:

Patches attached for Twenty Fourteen and Thirteen -- testing and code review appreciated. :)

Thanks for getting these in! They're close, but they need a bit more work.

In Twenty Thirteen:

• the aria-expanded attribute isn't changing to true and back when the menu is toggled.
• the aria-controls attribute should correspond with the id of the container being expanded, do either the div with a class of nav-menu or the ul inside of that div.

Same in Twenty Fourteen:

• the aria-expanded attribute isn't changing to true and back when the menu is toggled.
• the aria-controls attribute should correspond with the id of the container being expanded, do either the div with a class of nav-menu or the ul inside of that div.
• We could also add aria-expanded and aria-controls to the search toggle. The difference is that it would always be there since it's always a toggle.

[lancewillett]

We could also add aria-expanded and aria-controls to the search toggle. The difference is that it would always be there since it's always a toggle.

So that would be an HTML change? In the header template, probably.

[lancewillett]

Second pass at Twenty Fourteen

[lancewillett]

Second pass at Twenty Thirteen

[lancewillett]

Second pass at Twenty Thirteen and Twenty Fourteen based on davidakennedy feedback.

The search toggle changes to be done separately.

[davidakennedy]

Replying to lancewillett:

Second pass at Twenty Thirteen and Twenty Fourteen based on davidakennedy feedback.

The search toggle changes to be done separately.

@lancewillett I retested these both and they work as expected. Great job!

I found one wrinkle that I didn't think of before. If a custom menu is not assigned, the id won't be there to match the aria-controls. In that case, it relies on wp_page_menu instead of wp_nav_menu. I don't think wp_page_menu has a parameter to add an id, so we'd have to do something like:

/**
 *  Add ID attributes to the first <ul> occurence in wp_page_menu.
 *  Helps ARIA attributes match proper elements.
 */
function twentyfourteen_add_menuid( $ulid ) {
	return preg_replace( '/<ul>/', '<ul id="primary-menu">', $ulid, 1 );
}
add_filter( 'wp_page_menu','twentyfourteen_add_menuid' );

Not the cleanest solution, and maybe there's a better way I'm not thinking of right now. Sorry I didn't think of this before.

Thoughts?

[lancewillett]

Thanks, DK.

Not the cleanest solution, and maybe there's a better way I'm not thinking of right now.

Hmm. I hadn't thought of that case, either.

For a slightly cleaner solution, what about filtering link_before and link_after arguments for wp_page_menu() and adding in a wrapper element with the ID we require. div maybe. Could do it using the wp_page_menu_args hook.

It's really a bummer that classes don't work, but I can see why -- the control wouldn't be unique on the page otherwise.

[lancewillett]

However we decide to solve it, should be applied to both Twenty Thirteen and Fourteen as they both add the menu ID.

We also need to address the search toggle in a new patch.

[lancewillett]

In 31784:

Twenty Fourteen: add ARIA attributes to menu toggle. See #31527.

[lancewillett]

In 31785:

Twenty Thirteen: add ARIA attributes to menu toggle. See #31527.

[lancewillett]

Toggle ARIA properties on Twenty Fourteen's search behavior

[lancewillett]

For the ID on wp_page_menu() we could also suggest a core patch to add menu_id to the argument list. Both to pass a filterable argument and to also inherit from wp_nav_menu maybe.

[DrewAPicture]

I think we're going to need a DocBlock on the new onResizeARIA() function. See the relevant section in the ​JS documentation standards.

Edit: That goes for both Twenty Thirteen and Twenty Fourteen.

[lancewillett]

Replying to DrewAPicture:

I think we're going to need a DocBlock on the new onResizeARIA() function.

Good point. Anyone want to take a stab at a patch? There are other functions in Twenty Fifteen's functions.js that have just a one-line comment, also.

[lancewillett]

Replying to lancewillett:

For the ID on wp_page_menu() we could also suggest a core patch to add menu_id to the argument list. Both to pass a filterable argument and to also inherit from wp_nav_menu maybe.

Added a ticket and patch to add this in with core, avoiding a filter in the theme: #31656

[davidakennedy]

@lancewillett:

I tested the search toggle ARIA addition. Let's simplify it. We only really need to add the ARIA attributes to the a href="#search-container"> because that's the control a keyboard user or screen reader user would toggle it with. And that's what will give them the feedback if they're focused on it. The other elements are not focusable, so they would not hear the ARIA feedback. Also, currently, the ARIA attributes there do not change, it stays: aria-expanded="false".

[lancewillett]

Second pass at search toggle behavior for Twenty Fourteen

[davidakennedy]

@lancewillett:

31527.twentyfourteen.searchtoggle.2.patch for the ARIA on the search toggle looks good. Works as expected. Great job!

[lancewillett]

In 31794:

Twenty Fourteen: add ARIA attributes to search toggle. See #31527.

[lancewillett]

Two things to close this ticket:

1) DocBlocks
2) Decision & commit on #31656 for the page menu fallback ID for Twenty Fourteen and Thirteen

[DrewAPicture]

For those following along at home, my suggestion to @lancewillett in Slack was:

My suggestion is to just add the docs even if sparse everywhere else. Sends kind of a bad message if we don’t apply docs standards everywhere going forward.

[davidakennedy]

Twenty Fifteen onResizeARIA Doc Block

[davidakennedy]

Twenty Fourteen onResizeARIA Doc Block

[davidakennedy]

Twenty Thirteen onResizeARIA Doc Block

[davidakennedy]

Replying to lancewillett:

Two things to close this ticket:

1) DocBlocks
2) Decision & commit on #31656 for the page menu fallback ID for Twenty Fourteen and Thirteen

Added the DocBlock patches.

[lancewillett]

Possible to have decision & commit on #31656 also? It's a minor change.

[lancewillett]

In 31814:

Bundled themes: add documentation for new onResizeARIA function.

See #31527. Props davidakennedy, lancewillett.