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

disabled menu item w/ tooltip solution that follows best practices #1107

Closed
nicolethoen opened this issue Oct 28, 2021 · 4 comments
Closed

Comments

@nicolethoen
Copy link
Contributor

As we begin refactoring the existing dropdown-like components to use the new Menu component, we recognize that there has not been uniform consistency regarding how we handle attaching tooltips on disabled menu items.

There is currently no recommended or implemented solution for the doing this on the new Menu component.

Some existing dropdown-like components are allowing for browser focus on the disabled item, so that it can display the tooltip when a keyboard/screen reader user focuses on the item.

This solution does not follow best practices - tooltips should only be placed on interact-able elements - so a disabled menu item does not indicate to a user that there is a tooltip on it.

The recommendation is to consider placing a help text icon in a button on the disabled item which can receive browser focus and display the tooltip.

@nicolethoen nicolethoen changed the title disabled MENU item w/ tooltip solution that follows best practices disabled menu item w/ tooltip solution that follows best practices Oct 28, 2021
@mcarrano
Copy link
Member

@mcoker @jessiehuff Hadn't we had a whole discussion about the issue of placing tooltips on disabled elements almost 2 years ago. I remember it was a very controversial topic. I'm not in favor of adding popover help to these elements via a button. @nicolethoen is the issue for menu items any different than what we've done for buttons or tabs?

@nicolethoen
Copy link
Contributor Author

The issue is that Dropdown vs. Select vs. Menu all handle this differently.

But we also have open issues to make sure our docs/practices follow standard Tooltip recommendations, which is that tooltips are only applied to interact-able elements. Disabled elements are - by their nature - non interact-able. So rather than allowing tooltips to be placed on disabled menu items, and since menus will hopefully soon be used under the hood of Dropdown and Select, we can fix the different behaviors we currently experience by implementing this correctly in the Menu.

The updates made to Tabs, Buttons, and Dropdown do allow for navigation via keyboard to the disabled options. But this current solution is still not screen reader accessible.

That issue is documented here for dropdown:
patternfly/patternfly-react#6109
and here for Select:
patternfly/patternfly-react#5095

I do have access to much of the documented discussion around some of these concepts in this issue: patternfly/patternfly-react#1894

@mcarrano
Copy link
Member

Per 11/10 discussion, we propose to close and ensure that Menu has consistent behaviors with other components. OK with you, @jessiehuff

@jessiehuff
Copy link

That's ok with me. I think it makes the most sense to follow the pattern that we came up with for disabled button where it's in a "partially abled state" like in this PR and in the docs here. I do think it's a little weird that we have examples of static text with a tooltip where I don't know a scenario in which that would be interactive and expected to have a tooltip, but I think a disabled button (and here a menu item) could still be expected using aria-disabled. Elements with aria-disabled are interactive in the ways they need to be in order to host a tooltip, by default by allowing click and keypress events (only) and it's configurable further via the agreed upon prop name inoperableEvents (see Button component implementation). I do think tooltips deserve attention though as I've noticed that some of these tooltips on disabled elements in our components aren't working (for example, the tooltip in the disabled dropdown item does not announce in VO).

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

No branches or pull requests

3 participants