Tooltip examples are not accessible on dropdown items

[bartoval]

What: Closes #6109

it seems to work also adding the aria-labelledby prop, but the screen reader announces the tooltip before the item.

I tested the voice using the screen reader chrome plugin

[patternfly-build]

Preview: https://patternfly-react-pr-7524.surge.sh

A11y report: https://patternfly-react-pr-7524-a11y.surge.sh

[nicolethoen]

My expectation is that the tooltip would display on VO focus just like it would on keyboard focus or mouse hover.

Here is a gif of what happens when I use a mouse to hover over an enabled dropdown item with a tooltip and a disabled item with a tooltip:

In this second recording, you'll see that the VO does not trigger the tooltip in either case until I try to click on it. When I click on the disabled dropdown item, it does finally reveal the tooltip and read it (so that's an improvement!) but I cannot trigger the tooltip on the enabled dropdown item because that also selects the item and closes the dropdown

[thatblindgeye]

My expectation is that the tooltip would display on VO focus just like it would on keyboard focus or mouse hover.

@nicolethoen I'm noticing this behavior as well. It looks like VO is placing focus on the li element rather than the button within that list item. If you go into the list item with VO + Shift + down arrow, focus should go onto that button and trigger the tooltip. The Tab component had a similar issue back when I had to resolve an issue similar to this. It looks like the InternalDropdownItem component has logic setup similar to my fix for Tab, but one difference is the li element within the Tab component has role="presentation" whereas the dropdown items have `role="menuitem".

My previous comment about making this change here most likely wouldn't work since we'd lose the semantics of the items being menu items.

[bartoval]

I updated the code. Now everything should work as expected.
However, this solution seems more of a 'workaround'.

Another solution could be

• set <li> with the role="presentation"
• pass to the button or link a prop role='menuitem'

This solution should work well and it is clean.
However, the screen reader will no longer announce the world 'group'.
So instead:
Link, menu item, group, 1 of 7
the screen reader will announce
Link, menu item,1 of 7

Honestly, for our menu items, I don't see the utility of announcing group

Another small difference:
For item disabled the screen reader announces a new word: dimmed:
Disable menu item, dimmed, menu item, 4 of 7)

WDYT
@thatblindgeye @nicolethoen

[thatblindgeye]

Going off of Valerio's comment about the utility of having "group" announced on these menu items, it does seem odd that we end up indicating to a user that they are on a menu item group that contains a single element.

Some links that lean more towards Valerio's other solution of moving the role="menuitem" to the button/link inside the li (which would then be given role="presentation"):

• Editor menu bar example from ARIA authoring practices guide (APG)
• Navigation menubar example also from ARIA APG.
• StackOverflow article for role="menuitem" placement

Based on these links I think I might be leaning more towards the alternative solution mentioned.

[nicolethoen]

I agree with @thatblindgeye, I think.

However, I'm noticing that the Menu component uses <li role="none"> instead of presentation.
It also adds a menu role to the <ul>.

So I think it's more in line with other solutions to do the following (if it all works with tooltips)

• set <ul> with the role="menu"
• set <li> with the role="none"
• pass to the button or link a prop role='menuitem'

Other notes:

• the word dimmed is standard for disabled elements, so that's good news
• We need to be sure the tooltip can be triggered with the screen reader on disabled AND enabled dropdown items

[bartoval]

role="menu" was already set in <ul>, so no need to add it.

[thatblindgeye]

Looks good! The tooltip triggers via mouse, keyboard, and with VO navigation for me, and the tooltip gets announced when triggered as well. Had a nitpick/question below, but otherwise awesome job on this 💪🏼

[thatblindgeye]

Nitpick: could this also be written as ...(this.props.role !== 'separator' && { role, ref: this.componentRef })?

[thatblindgeye]

Looks and sounds as I'd expect! Awesome work on this

[kmcfaul]

Looks great!

[jessiehuff]

This seems like an improvement! I hear everything announced as I would expected in VO. Out of curiosity, I tested in NVDA and JAWS as well, and I noticed that it didn't work in either of those screen readers. Since this still moves things forward, I'm wondering if it makes sense to approve this but open another issue for JAWS and NVDA? Those are pretty big screen readers that I wouldn't want to completely neglect.

[patternfly-build]

Your changes have been released in:

• eslint-plugin-patternfly-react@4.61.5
• @patternfly/react-catalog-view-extension@4.73.5
• @patternfly/react-charts@6.75.5
• @patternfly/react-code-editor@4.63.5
• @patternfly/react-console@4.73.5
• @patternfly/react-core@4.222.5
• @patternfly/react-docs@5.83.5
• @patternfly/react-icons@4.73.5
• @patternfly/react-inline-edit-extension@4.67.5
• demo-app-ts@4.182.5
• @patternfly/react-integration@4.184.5
• @patternfly/react-log-viewer@4.67.5
• @patternfly/react-styles@4.72.5
• @patternfly/react-table@4.91.5
• @patternfly/react-tokens@4.74.5
• @patternfly/react-topology@4.69.5
• @patternfly/react-virtualized-extension@4.69.5
• transformer-cjs-imports@4.60.5

Thanks for your contribution! 🎉