a11y improvements on primary navigation menu toggle

[philiparthurmoore]

See #438. Also see https://core.trac.wordpress.org/ticket/27147.

Minor issue: before the change, there was no styling on the menu toggle. After the change, the menu toggle takes on the styles for buttons defined around https://github.com/Automattic/_s/blob/master/style.css#L256.

This is a matter of taste that I have no strong opinion about—removing all styles on the menu toggle or leaving the button styles intact. I also added the word "Primary" into the menu. This is also a matter of preference, but if it's accepted then we'll need to update the POT file for the project.

[davidakennedy]

Just wanted to say that from an accessibility perspective, this is solid and improves things quite a bit. Thanks @philiparthurmoore for the PR.

[obenland]

After the change, the menu toggle takes on the styles for buttons defined around

I don't think that is a big deal, after all it is a button. We even named the JavaScript variable button when it was still an h1 element. But it (possibly) adds to the list of things that authors have to tear down before building up. Even if its just adding a :not(.menu-toggle) to the CSS button selector.

I don't feel strongly about going with "Primary Menu" over "Menu". It is more consistent with the theme location and less arbitrary though.

Nice PR, I expected that more changes would have been necessary. Pleasant surprise.

[philiparthurmoore]

I overlooked something.

Right now, .menu-toggle has cursor: pointer; on it, but if this PR is accepted then that rule will become redundant, as cursor: pointer; is already defined for all button elements and input elements of type button. I think that I should gut cursor: pointer; from .menu-toggle.

With regard to tearing down before building up, _s itself uses a fairly heavy-handed reset for all elements, so it's mostly a decision between a) resetting .menu-toggle explicitly or b) letting theme authors code in their own overrides for .menu-toggle against the button styles for _s. I'm okay with the second approach-all this resetting and resetting feels quite aggressive. But I'm also fine with the first; can't be bothered to fight either way.

What do you think?

[obenland]

I think that I should gut cursor: pointer; from .menu-toggle.

You can continue to commit to your menu-a11y branch. All subsequent commits will show up in this PR.

I'm okay with the second approach[…]

Me too.

[philiparthurmoore]

Okay. I think we're good to go.