[EuiContextMenu] Tabbing regressions

[cjcenizal]

Original tab behavior

Originally, all of the items within a context menu behaved as if they were part of the tab order of the UI which triggers the menu. You could use the tab key to tab through them and beyond the menu throughout the rest of the surrounding UI.

Regressions

Two regressions have been introduced:

1. You now have to hit the tab key twice for the next item to receive focus. According to git bisect, this regression was introduced in Fix context menu focus jumping #1018, though from a cursory glance of the code I'm not sure what could be causing this.
2. If you tab beyond the context menu focus goes to the URL bar. I think this is because the popover element is now inside of a portal. This regression was introduced in Refactor EuiPopover to use new positioning service #948.

Solution

I think it's OK if we don't go back to the original behavior and instead do something different. I think intercepting the tab event and wrapping focus back to the beginning of the elements within the menu would be great UX, which is also how the popover behaves.

[cjcenizal]

If possible we should also add tests to guard against future regressions.

[snide]

So the good news at least is the up/down stuff still works as intended. I have no idea how that PR you linked would have changed this behavior, very odd.

My general read on this stuff (I've been dealing with something similar in SuperSelect, which uses menuitem) is that a lot of these lists within menus probably shouldn't be actual button tags, but utilize either role="listbox" or role="menu" and behave closer to select tags (where tab isn't tied so directly to the elements, but to the overall form). These kinds of systems are really tough to get right on screenreaders, and its very much compounded by the fact that our popover system portals these elements in (versus simply display: hide in the dom). A lot of the aria patterns just don't really account for that kind of stuff beyond aria live. For example, a listbox in a popover should be tied to the button that makes it appear. But if the listbox doesnt exist yet (because of a portal), then it isnt' read correctly.

I don't have great solutions here, know this isn't exactly related, just providing some background. Building accessibility in the world of a dynamic dom is challenging.

[chandlerprall]

I agree that the context menu should trap focus in some way, and Dave's observation about using up/down vs. tab sounds like a good intuition on these menus as well.

A more generic catch-all fallback that we may want to consider is having the popover watch for the focus leaving itself and the visually-surrounding elements (e.g. anchor), programmatically inserting itself in the tab order. I don't feel it's a big enough problem to put in that effort, but it is an option.

[myasonik]

Just leaving some notes here for investigation:

• This is might actually be a legitimate role=menu though I'd want to double check
  • If we go this route, we'd have to lock down the API a bit to limit random content being thrown into here (could help differentiate ContextMenus from generic Popovers)
• But role=menu is generally not recommended because of rampant misuse, difficultly to implement, and poor user expectations (role=menu came from native desktop experiences and doesn't translate to the web perfectly) so it might not be worth implementing even if it's technically correct
• Other than a proper menu, it might be appropriate to continue with the current implementation of a list of buttons
  • Need to do some investigative work on how to handle nested menus well
  • Seems like it requires two tab presses to move onto the next/previous item which is weird... (Firefox on Mac)
  • The popover should probably add ownFocus or potentially close itself when tabbing out of it and move focus to the next focusable item on the page

---

Related to #731