sanse icon indicating copy to clipboard operation
sanse copied to clipboard

Update aria-expanded on mobile nav <ul> and add aria-current="page"

Open mathalete opened this issue 7 years ago • 1 comments

Loving the theme.

Some feedback follows - this is more related to best practices to enhance what you've got.

On the mobile navigation, both the toggle button and the <ul> are given the aria-expanded state.

Note that only the button needs it. So I'd recommend updating the function in navigation.js as follows :

button.onclick = function() {
		if ( -1 !== container.className.indexOf( 'toggled' ) ) {
			container.className = container.className.replace( ' toggled', '' );
			button.setAttribute( 'aria-expanded', 'false' );
			// remove this menu.setAttribute( 'aria-expanded', 'false' );
		} else {
			container.className += ' toggled';
			button.setAttribute( 'aria-expanded', 'true' );
			// remove this menu.setAttribute( 'aria-expanded', 'true' );
		}
	};

An enhancement to add would be to include functionality to programmatically determine the current link in a <nav> element.

One way to do this would be to update your navigation.js by identifying any link in the DOM that has class current_page_item and add the aria-current="page" attribute.

Eg:

const ariacurrent = document.querySelector('li.current_page_item');
ariacurrent.setAttribute('aria-current','page');

For further info see : aria-current

mathalete avatar May 12 '18 19:05 mathalete

Thanks for feedback!

You're probably right, aria-expanded doesn't do anything on menu (<ul>). I'll look into it when updating the theme.

The aria-current should land in Core so every theme can have it, just like in pagination.

samikeijonen avatar May 13 '18 08:05 samikeijonen