WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#28293 closed defect (bug) (fixed)

Twenty Thirteen: Menu not accessible in "mobile" size

Reported by: rclations Owned by: lancewillett
Milestone: 4.0 Priority: high
Severity: normal Version: 3.6
Component: Bundled Theme Keywords: has-patch
Focuses: accessibility Cc:

Description

This is the same issue reported by @artychan in Twenty Twelve (#28224) - making a new ticket for Twenty Thirteen

"When the screen gets narrow enough that the menu is just a button-looking "Menu", the keyboard and screen readers cannot access the links inside the menu."

Similar issues:
#27147 on Twenty Fourteen
#28224 on Twenty Twelve

Attachments (4)

28293.diff (1.3 KB) - added by rclations 5 years ago.
28293.2.diff (1.5 KB) - added by rclations 5 years ago.
28293.3.diff (1.5 KB) - added by rclations 5 years ago.
28293.4.diff (1.8 KB) - added by obenland 5 years ago.

Download all attachments as: .zip

Change History (23)

@rclations
5 years ago

#1 @rclations
5 years ago

  • Keywords has-patch added

Implemented the same type of patch used in #27147 and #28224

Not sure we need any :focus styles here since we already have a carrot next to the menu, but I'm happy to add it in if we should.

#2 @lancewillett
5 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from Awaiting Review to 4.0

#3 @lancewillett
5 years ago

Thanks for the report. I'd love a second opinion on the focus styles.

#4 @lancewillett
5 years ago

  • Keywords needs-testing added

#5 follow-up: @DavidTheMachine
5 years ago

Google Chrome still doesn't have a focus-caret feature, so an outline on :focus would be a good addition.

#6 in reply to: ↑ 5 ; follow-up: @DrewAPicture
5 years ago

  • Keywords needs-patch added; has-patch 2nd-opinion removed

With 28293.diff I'm seeing a jarring jump on clicking the mobile menu in both FF and Chrome. Seems like a bordered focus style might be a better option, though I'm sure the accessibility folks would have a better-formed idea of what's called for here.

This ticket was mentioned in IRC in #wordpress-themes by obenland. View the logs.


5 years ago

#8 @obenland
5 years ago

  • Keywords needs-testing removed
  • Priority changed from normal to high
  • Version set to 3.6

Needs a new patch, addressing the buttons styles and missing :focus styles in the mobile menu.

@rclations
5 years ago

#9 @rclations
5 years ago

Thanks for the feedback - this more what we're looking for?

#10 in reply to: ↑ 6 @davidakennedy
5 years ago

Replying to DrewAPicture:

With 28293.diff I'm seeing a jarring jump on clicking the mobile menu in both FF and Chrome. Seems like a bordered focus style might be a better option, though I'm sure the accessibility folks would have a better-formed idea of what's called for here.

I started a patch for this and then found this ticket. :)

I've tested this and it's solid from an accessibility standpoint. Good job, rclations!

I also wanted to mention the source order of the toggle and skip link, since it came up in #28224. Here's what I said there:

Ideally, from an accessibility standpoint, we'd do something like to Underscores: https://github.com/Automattic/_s/blob/master/header.php#L23-L34

The issue here is similar to what obenland described. If you're on a touch device and moving around, listening for feedback, it's pretty easy for that Skip to Content link to get in the way of getting to the menu items because of source order.

However, I would hate to break themes for people. That's not cool. The skip link/toggle position isn't perfect, but not bad either. The skip link does come before the actual menu items. I'd say leave it, but make the effort to improve change things in default themes from here on out.

A big bug was fixed here and that's the important part.

This ticket was mentioned in IRC in #wordpress-ui by davidakennedy. View the logs.


5 years ago

#12 @obenland
5 years ago

rclations, patch is almost there, nice!

A couple of things to add:
Color should be #141412 rather than #000. We should probably pull the padding: 12px 0 12px 20px; from the .menu-toggle selector to the next one, so it applies to all states. We used outline: thin dotted; for focused links, let's do that for the menu toggle too. And we need focus styles for child elements, along the lines of:

.toggled-on .nav-menu > li a:focus,
.toggled-on .nav-menu > ul a:focus {
        background-color: #220e10;
        color: #fff;
}

Could you test it with both, assigned menus and the fallback?

#13 @rclations
5 years ago

Definitely - thanks for the feedback!

#14 @lancewillett
5 years ago

Is this still pending? Needs a new patch, or?

#15 @rclations
5 years ago

Yeah, sorry about that - I've been delinquent about adding another patch. I'll get that wrapped up today

@rclations
5 years ago

#16 @rclations
5 years ago

Rerolled the patch with the easy stuff mixed in

---

@obenland which selector were you thinking for the padding? It's on the main .menu-toggle already, I only added to :active to override

button:active,
input[type="submit"]:active,
input[type="button"]:active,
input[type="reset"]:active {
	padding: 10px 24px 11px;
}

I can add the child element styles you recommend, but I think they're already covered by

.nav-menu li:hover > a,
.nav-menu li a:hover,
.nav-menu li:focus > a,
.nav-menu li a:focus {
	background-color: #220e10;
	color: #fff;
}

let me know if I'm missing anything or what I can do to get this one wrapped up

@obenland
5 years ago

#17 @obenland
5 years ago

  • Keywords has-patch added; needs-patch removed

@rclations: 28293.4.diff was what I had in mind. Would you mind double-checking it?

#18 @rclations
5 years ago

I see what you mean - looks good to me. Tested with assigned & fallback menus

#19 @lancewillett
5 years ago

  • Owner set to lancewillett
  • Resolution set to fixed
  • Status changed from new to closed

In 29181:

Twenty Thirteen: make small-screen menu accessible to keyboard commands and voice-driven software by using a focusable button element rather than h3 for the toggle element. Fixes #28293, props rclations and obenland.

Note: See TracTickets for help on using tickets.