πŸ”₯ HOT: Ticket/ - Uncensored 2025

Opened 11 months ago

Closed 10 months ago

Last modified 10 months 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.


11 months 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:


11 months ago
#2

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

​umeshie commented on ​PR #8235:


11 months ago
#3

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

​@umeshsinghin commented on ​PR #8235:


11 months ago
#4

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

#5 @sabernhardt
11 months ago

  • Component changed from Themes to Bundled Theme

#6 @joedolson
11 months ago

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

​@joedolson commented on ​PR #8235:


11 months 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:


11 months 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:


11 months ago
#9

Thanks @sabernhardt ~ just adjusted the approach!

#10 @joedolson
10 months ago

  • Keywords commit added

Tested and reviewed; lgtm.

#11 @joedolson
10 months 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
10 months ago

In 59912:

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