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

[context-menu] Update role attribute on checkable items to use menuitemcheckbox #687

Open
anezthes opened this issue Apr 28, 2020 · 2 comments
Labels
a11y Accessibility issue refactor Internal improvement Severity: Minor vaadin-context-menu

Comments

@anezthes
Copy link
Contributor

The current implementation uses <vaadin-list-box role="list"> which is semantically incorrect if we wish to build a menu. The role should be set to "menu" and the items should have role="menuitem".

(Ideally aria-haspopup, aria-controls and aria-expanded should also exist on the element that triggers the popup.)

The semantics change a bit depending on the menu type. Some examples below...

Example 1: Application Menus

<ul role="menu">
  <li role="menuitem">New File</li>
  <li role="menuitem">Open File...</li>
  <li role="menuitem">Print...</li>
  ...
</ul>

It's important to note that elements with role="menuitem" should not contain interactive content.


Example 2: Text Style

<ul role="menu">
  <li role="menuitemcheckbox" aria-checked="false">Bold</li>
  <li role="menuitemcheckbox" aria-checked="true">Italics</li>
  <li role="menuitemcheckbox" aria-checked="false">Underline</li>
  ...
</ul>

Allows multiple items to be selected.


Example 3: Font Selection

<ul role="menu">
  <li role="menuitemradio" aria-checked="false">Arial</li>
  <li role="menuitemradio" aria-checked="false">Courier New</li>
  <li role="menuitemradio" aria-checked="true">Helvetica Neue</li>
  ...
</ul>

Should only allow one selection.


Example 4: Navigation

<ul role="menu">
  <li role="none">
    <a role="menuitem" href="www.vaadin.com">Home</a>
  </li>
  ...
</ul>

While this technically works VoiceOver will announce "Home, menu item" instead of "Home, link". The latter is more accessible IMO. We should be careful in recommending this approach over the established usage of <nav>, <ul>, <li> and <a>.


Prototyping
Tested a few approaches and found that putting the attributes (role and aria-checked) on <vaadin-context-menu> and <vaadin-context-menu-item> produces a result that's fairly close to the native HTML approach.

<vaadin-context-menu role="menu">
  <vaadin-context-menu-item role="menuitem">Item 1</vaadin-context-menu-item>
  <vaadin-context-menu-item role="menuitem">Item 2</vaadin-context-menu-item>
  ...
</vaadin-context-menu>
<vaadin-context-menu role="menu">
  <vaadin-context-menu-item role="menuitemcheckbox/menuitemradio" aria-checked="true">Item 1</vaadin-context-menu-item>
  <vaadin-context-menu-item role="menuitemcheckbox/menuitemradio" aria-checked="false">Item 2</vaadin-context-menu-item>
  ...
</vaadin-context-menu>

Tested in Chrome and Safari with VoiceOver.

@anezthes anezthes changed the title Accessibility issues a11y semantics Apr 28, 2020
@web-padawan
Copy link
Member

This can be probably workarounded in vaadin-context-menu-list-box.

Proper implementation would be to split lists (select) and menus (context-menu).
Related improvement idea: vaadin/vaadin-list-box#67

@vaadin-bot vaadin-bot transferred this issue from vaadin/vaadin-context-menu May 19, 2021
@web-padawan web-padawan changed the title a11y semantics [context-menu] a11y semantics May 20, 2021
@web-padawan web-padawan added refactor Internal improvement a11y phase 2 Candidates for a second batch of a11y fixes labels Sep 27, 2021
@yuriy-fix yuriy-fix removed the a11y phase 2 Candidates for a second batch of a11y fixes label Oct 18, 2021
@web-padawan web-padawan changed the title [context-menu] a11y semantics [context-menu] Update role attribute on checkable items to use menuitemcheckbox Jan 30, 2023
@web-padawan
Copy link
Member

Most of problems described here were addressed in Vaadin 24, see issue and PR below:

However, we still need to add menuitemcheckbox role support. Renamed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility issue refactor Internal improvement Severity: Minor vaadin-context-menu
Projects
None yet
Development

No branches or pull requests

4 participants