Make WordPress Core

Opened 7 weeks ago

Closed 3 weeks ago

Last modified 3 weeks ago

#62892 closed defect (bug) (fixed)

Twenty Twelve: Add aria-expanded to menu button (mobile)

Reported by: bschneidewind's profile bschneidewind Owned by: joedolson's profile joedolson
Milestone: 6.8 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch commit
Focuses: accessibility Cc:

Description

The aria-expanded and aria-controls attributes are missing on the mobile nav for the Twenty Twelve theme: https://wp-themes.com/twentytwelve/

Change History (13)

This ticket was mentioned in PR #8235 on WordPress/wordpress-develop by @bschneidewind.


7 weeks ago
#1

  • Keywords has-patch added

This adds the aria-expanded and aria-controls attributes to the mobile nav of the Twenty Twelve theme - they currently are not present, ex: https://wp-themes.com/twentytwelve/

Trac ticket: https://core.trac.wordpress.org/ticket/62892

umeshie commented on PR #8235:


7 weeks ago
#2

@bschneidewind Please use const and let for block-scoped variables (no var).

umeshie commented on PR #8235:


7 weeks ago
#3

@bschneidewind Please use const and let for block-scoped variables (no var).

@umeshsinghin commented on PR #8235:


7 weeks ago
#4

@bschneidewind Please use const and let for block-scoped variables (no var).

#5 @sabernhardt
7 weeks ago

  • Component changed from Themes to Bundled Theme

#6 @joedolson
7 weeks ago

  • Milestone changed from Awaiting Review to 6.8
  • Owner set to joedolson
  • Status changed from new to accepted

@joedolson commented on PR #8235:


7 weeks ago
#7

@umeshsinghin While that's generally good advice, it's generally WordPress policy to keep things stylistically consistent within the context they're written. Since most of this file uses var to declare variables, it isn't a requirement.

Note the JavaScript coding standards, which explicitly indicate that const and let should be used in code written using ES2015 or newer; which is definitely not the case with Twenty Twelve.

It's not a problem to use more modern notations in older files, but not necessary, either.

@sabernhardt commented on PR #8235:


7 weeks ago
#8

I think this can be done without declaring new variables by editing the main function (and reusing its button and menu variables). I also recommend setting aria-expanded="false" in the script file in case a child theme overrides the header template.

	// Assign an ID for the default page list if no menu is set as Primary.
	if ( ! menu.id ) {
		menu.setAttribute( 'id', 'twentytwelve-page-list-menu' );
	}

	button.setAttribute( 'aria-controls', menu.id );
	button.setAttribute( 'aria-expanded', 'false' );

	button.onclick = function() {
		if ( -1 === menu.className.indexOf( 'nav-menu' ) ) {
			menu.className = 'nav-menu';
		}

		if ( -1 !== button.className.indexOf( 'toggled-on' ) ) {
			button.className = button.className.replace( ' toggled-on', '' );
			menu.className = menu.className.replace( ' toggled-on', '' );
			button.setAttribute( 'aria-expanded', 'false' );
		} else {
			button.className += ' toggled-on';
			menu.className += ' toggled-on';
			button.setAttribute( 'aria-expanded', 'true' );
		}
	};

@bschneidewind commented on PR #8235:


7 weeks ago
#9

Thanks @sabernhardt ~ just adjusted the approach!

#10 @joedolson
3 weeks ago

  • Keywords commit added

Tested and reviewed; lgtm.

#11 @joedolson
3 weeks ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 59911:

Bundled Themes: Twenty Twelve: Add ARIA attributes on menu toggle.

Add aria-expanded and aria-controls attributes to the Twenty Twelve mobile menu toggle.

Props bschneidewind, joedolson, sabernhardt, umeshsinghin.
Fixes #62892.

#12 @joedolson
3 weeks ago

In 59912:

Bundled Themes: Twenty Twelve: Move skip link to top of body.

Move the skip link to the top of the body content, so it is the first focusable item on the page.

Props joedolson, sabernhardt, abcd95, vgnavada, shailu25.
Fixes #62892.

#13 @joedolson
3 weeks ago

I accidentally cited this in two commit messages; the first one is correct.

Note: See TracTickets for help on using tickets.