Adds menubar ARIA semantics to editor menu#3191
Merged
raclim merged 6 commits intoprocessing:developfrom Jul 19, 2024
Merged
Conversation
|
🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. |
raclim
approved these changes
Jul 19, 2024
Collaborator
raclim
left a comment
There was a problem hiding this comment.
Thanks so much for your work on this, this is really awesome!
I really appreciate how much thought and consideration went into this and the detailed insight into your process! I think a lot of these choices make sense to me, and would be a major improvement to what's there so far.
Since this seems good to me, I'm going to merge this in—please feel free to add any other amendments you'd like to make in a new PR!
4 tasks
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #2933, makes progress on @lindapaiste's #2968 (thanks for starting some of this already!)
Changes:
role="menubar"from nav div to file menurole="menuitem"from<li>to<button>or<a>, whichever element performs a functionrole="menuitem"to login and sign up linksaria-expandedto dropdownsindex.integration.test.jsxto look forrole='menubar'instead ofrole='navigation'This PR tries to add ARIA
menubarsemantics to the editor navigation menu, to further support accessibility features like the ones described in issue #2933. The main challenge was resolving how to maintain bothrole=menubarandrole=navigationon the page. ARIA guidelines differentiate on what makes a menubar (a widget offering a list of actions or functions) vs navigation (a collection of links used for navigating the web page or page content). Menubars are also more closely associated with the menus of desktop applications like Photoshop rather than typical examples of site navigation. Since the web editor's file menu functions more like an application than site navigation, I followed the W3C's menubar pattern where possible.The biggest change is probably substituting the header
<nav>element for a<div>; I gaverole=navigationto the right portion of the nav bar since that dealt more closely with links to other pages. It may also make sense to wrap those lists in a<nav>element but I have to look into that more.Since the
menubarpattern implies specific keyboard behaviors, it would help to implement those features as a next step.References:
I have verified that this pull request:
npm run lint)npm run test)developbranch.Fixes #123