Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds menubar ARIA semantics to editor menu #3191

Merged
merged 6 commits into from
Jul 19, 2024

Conversation

tespin
Copy link
Contributor

@tespin tespin commented Jul 18, 2024

Related to #2933, makes progress on @lindapaiste's #2968 (thanks for starting some of this already!)

Changes:

  • moved role="menubar" from nav div to file menu
  • moved role="menuitem" from <li> to<button> or <a>, whichever element performs a function
  • added role="menuitem" to login and sign up links
  • added aria-expanded to dropdowns
  • updated snapshots
  • updated index.integration.test.jsx to look for role='menubar' instead of role='navigation'

This PR tries to add ARIA menubar semantics 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 both role=menubar and role=navigation on 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 gave role=navigation to 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 menubar pattern implies specific keyboard behaviors, it would help to implement those features as a next step.

References:

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

Copy link

welcome bot commented Jul 18, 2024

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already.

@tespin tespin changed the title Tespin fix/menu ally Adds menubar semantics to editor menu Jul 18, 2024
@tespin tespin changed the title Adds menubar semantics to editor menu Adds menubar ARIA semantics to editor menu Jul 18, 2024
Copy link
Collaborator

@raclim raclim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@raclim raclim merged commit fcaf3f5 into processing:develop Jul 19, 2024
1 check passed
@raclim raclim mentioned this pull request Jul 19, 2024
4 tasks
@tespin tespin mentioned this pull request Aug 15, 2024
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants