fix(overflow-menu): accessibility navigation issues

[francinelucca]

Closes #9471

• Fixes focus reset on tabbing out of open overflow menu and allows navigation of context menu items with up and down arrow keys.
• Rewrite aria labels for overflowMenu and button
• uses IconButton instead of button to automatically display tooltip

Changelog

Changed

• Cleanup arrow navigation logic (was not working): prevent undefined overflowItem by not destructuring next focusable element
• Focus menu element on tab out to preserve tab order
• Small closeMenuAndFocus refactor
• Rewrite ariaLabel and iconDescription props to "Options"
• Replace overflowMenu's button element with an IconButton to display tooltip
• Adds closeOnActivation optional prop to tooltip
• Updates IconButton to close tooltip on button activation
• Adds scss styles to maintain Breadcrumb's styling (accounting for new tooltip) when using overflowmenu
• Adds tests to cover new cases for tooltip
• Updates snapshots

Removed

• Unused handleKeyDown function

Testing / Reviewing

On local OverflowMenu SB:

• Test keyboard navigation: ESC, Tab and Up & Down arrows
• Overflow menu button should now have a tooltip "Options"
• Expect pressing "tab" on open overflow menu to close the menu and preserve current tab position, compare to behavior in https://react.carbondesignsystem.com/?path=/story/components-overflowmenu--default
• tooltip should close on activation (click/enter/space) of the button

Review/Take into consideration:

• I had to update the public API snapshot to include the new closeOnActivation prop for tooltip and got a warning about semver, is there something else I need to do or is this ok?
• I had to take the overflowmenu body outside of the button (was previously inside) and wrap the entire thing in a span to cover for the new tooltip behavior

---

TODO:

• Remove Test Data before merging

[netlify]

✅ Deploy Preview for carbon-components-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	5b7726a
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-components-react/deploys/63dc09ec57ea0a00092ba316
😎 Deploy Preview	https://deploy-preview-12977--carbon-components-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[netlify]

✅ Deploy Preview for carbon-elements ready!

Name	Link
🔨 Latest commit	5b7726a
🔍 Latest deploy log	https://app.netlify.com/sites/carbon-elements/deploys/63dc09eb7347160008ccaab3
😎 Deploy Preview	https://deploy-preview-12977--carbon-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[tay1orjones]

A few comments below.

I'm having a hard time figuring out how to validate this in storybook. Besides the tooltip (looks great btw!), the focus behavior seems to be the same to be between the deploy preview and prod.

What's the best way to test that the focus behavior is working correctly? Should we add another interactive element to the story to validate it moves correctly? Or is it enough to show that it moves correctly to the next focusable item outside of the storybook pane?

[mbgower]

If my suggestion is leading to challenges, happy to discuss them.
On the question of positioning, one thing I was wondering...
Do we want the tooltip to disappear on Enter and space (i.e., activation)?
At the moment, one can dismiss a tooltip by pressing Esc. That makes a lot of sense. When I'm on a button, to me it seems logical that if I activate it, the tooltip should also disappear since I've made a decision to act on the button (I obviously don't need to know what it was for anymore).
If that were put in place, you'd never have a conflict between the tip and the menu shown from a menu-button (i.e., overflow menu). The tip would disappear even as the menu showed up.

Does that give us anything?

[francinelucca]

If my suggestion is leading to challenges, happy to discuss them. On the question of positioning, one thing I was wondering... Do we want the tooltip to disappear on Enter and space (i.e., activation)? At the moment, one can dismiss a tooltip by pressing Esc. That makes a lot of sense. When I'm on a button, to me it seems logical that if I activate it, the tooltip should also disappear since I've made a decision to act on the button (I obviously don't need to know what it was for anymore). If that were put in place, you'd never have a conflict between the tip and the menu shown from a menu-button (i.e., overflow menu). The tip would disappear even as the menu showed up.

Does that give us anything?

@mbgower would that behavior be overflowMenu only or does it also apply to iconButton?

[francinelucca]

I'm having a hard time figuring out how to validate this in storybook. Besides the tooltip (looks great btw!), the focus behavior seems to be the same to be between the deploy preview and prod.

What's the best way to test that the focus behavior is working correctly? Should we add another interactive element to the story to validate it moves correctly? Or is it enough to show that it moves correctly to the next focusable item outside of the storybook pane?

@tay1orjones added a button element after the overflow to make it easier to test, basically on prod tabbing out of the open menu will reset the tab order (you'll have to tab twice to get to the menu button again), in the preview you should stay at the current position

[mbgower]

would that behavior be overflowMenu only or does it also apply to iconButton?

I think it would apply to both. I can't see any reason why it wouldn't. Not only does me acting on a button with a tooltip suggest I no longer need the tooltip displayed, but what I am effectively doing by pressing that button is moving focus, right? And that should make the tooltip disappear at any rate.
There are some odd interactions in Carbon at the moment where the focus remains on a button after being activated -- it's effectively a togglebutton. But even then, I think it makes sense for it to go away.

Screen.Recording.2023-01-25.at.2.40.16.PM.mov

BTW, i thought I'd just QA this by looking at Google Sheets where I know they do a lot of these tooltips on buttons (and even on other controls, such as unlabelled inputs). The action seems to make sense across all the ones where it exists. The tooltip goes away on user interaction, where opening or just acting on it.

Screen.Recording.2023-01-25.at.2.43.24.PM.mov

[francinelucca]

@mbgower does this look about right in terms of keyboard behavior and the tooltip? Should have the tooltip on IconButton as well 🙏🏻
https://deploy-preview-12977--carbon-components-react.netlify.app/?path=/story/components-overflowmenu--default

[mbgower]

does this look about right in terms of keyboard behavior and the tooltip?

Yes, the default overflow button's tooltip behaviour works well. It disappears on Esc, activation (Enter/Space) or Tab (loss of focus). Nice :)

Note that in the playground, there's a focus issue when the kebab is opened (menu items not getting focus). Not relevant to the tooltip, but thought I'd point it out.

[francinelucca]

Note that in the playground, there's a focus issue when the kebab is opened (menu items not getting focus). Not relevant to the tooltip, but thought I'd point it out.

@mbgower my bad, should be working now!

[tay1orjones]

This looks solid to me, thanks for taking the time to iterate on this so much!

[tw15egan]

Looks good! Arrow keys can navigate the menu, and focus is where I expect it to be.

There is one issue that may be unrelated to this, but the ul still seems to get focus. Is there any we can avoid this?

[francinelucca]

There is one issue that may be unrelated to this, but the ul still seems to get focus. Is there any we can avoid this?

@tw15egan so that happens when focusTrap is on (which is the default), if you try it in the playground (starts at false in playground) you'll see if won't happen; so I understand it's a focusTrap thing but I'm actually not sure what is the intent for that prop's behavior 😅

[tw15egan]

@francinelucca hmm, that's weird; not sure why we have the focusTrap on there since it isn't navigated by tabs anyways 🤔 Thanks for filling me in.

@tay1orjones, any ideas? Anyways, it seems unrelated to this specific fix, so we can create a separate issue if we want to take action on it!

[mbgower]

I'll mention that the focus around the whole menu is not necessarily a BAD thing; it really depends on how the focus handling happens on the menu items.
FOr instance, I've seen some implementations where highlighting is used to show which menu item has focus and there is also a focus indicator around the whole list which helps define the border (sorta analogous to how there is a cursor flashing inside this text area while there is a focus indicator around the whole thing).
I'm certainly not saying that's necessary here! Just observing that it could be part of a good, intentional design when tackled holistically.

[francinelucca]

@tw15egan @mbgower I'll take these notes and create an exploration/discussion issue to look into the focusTrap behavior on OverflowMenu 👍

[carbon-bot]

Hey there! v11.22.0 was just released that references this issue/PR.