patternfly / patternfly-elements

PatternFly Elements. A set of community-created web components based on PatternFly design.
https://patternflyelements.org/
MIT License
375 stars 85 forks source link

`<pf-dropdown-item>`: `menutitem` role must be contained by parent with `menu` role #2705

Closed nikkimk closed 3 months ago

nikkimk commented 3 months ago

Description of the issue

When testing dropdown, axeDevTools has the following error:

Ensures elements with an ARIA role that require parent roles are contained by them

.overview > .example-preview > pf-dropdown > pf-dropdown-item:nth-child(1),#item
<div id="item" role="menuitem" tabindex="0">
          <slot name="icon"></slot>
          <slot></slot>
        </div>
To solve this problem, you need to fix the following:
Required ARIA parents role not present: menu, menubar, group

Impacted component(s)

Steps to reproduce

  1. Go to Patternfly Elements -> Dropdown
  2. Run axeDevTools
  3. See error message
  4. UPDATED: View pf-dropdown-menu's accessibility tree in the Element Inspector.
  5. UPDATED: role: menu is not being applied.

Expected behavior

The element marked as role="menuitem" needs to be a child of the element with role="menu". UPDATE: pf-dropdown-menu should have a menu role.

Related issues

bennypowers commented 3 months ago

This may arise when items are slotted into the default slot - in which case they'll be projected into a menu role parent in the shadow root. May we in that case allow ourselves to treat this as a false positive?

nikkimk commented 3 months ago

This may arise when items are slotted into the default slot - in which case they'll be projected into a menu role parent in the shadow root. May we in that case allow ourselves to treat this as a false positive?

After more investigation it looks like pf-dropdown-menu's role is not showing up as menu in the accessibility tree, which is an issue.

bennypowers commented 3 months ago

Are we failing to load the element internals polyfill?

bennypowers commented 3 months ago

This should be good now, following tests by @nikkimk and @hellogreg