[EuiContextMenu] Fix tab keypress requiring two presses to work correctly

[cee-chen]

Summary

closes #1101
closes #5239

What was happening was that document.activeElement was stale/looking at the previously focused element (before the tab event) and focus was moving forward (on tab) and then immediately backwards to the previously focused element when this.setState({ focusedItemIndex }) was called, hence the 2 key presses bug.

Wrapping the entire thing in a requestAnimationFrame removes the stale factor and everything works on all latest evergreen browsers.

Checklist

• Checked in Chrome, Safari, Edge, and Firefox
• Added or updated jest and cypress tests
• Checked for accessibility including keyboard-only and screenreader modes
• A changelog entry exists and is marked appropriately

[cee-chen]

This is mostly just indentation changes due to the setTimeout wrapper, so I recommend viewing the diff with whitespace changes hidden. I moved comments around a little bit as well but otherwise that was it

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5719/

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5719/

[cchaos]

Thank you!!!!

[chandlerprall]

I know (now...) that FF doesn't tab to links, but I think we'd need to 'correct' that behaviour when the menu item is a link. That likely means not relying on the browser to find the next element but interrupting the tab behaviour with a call to tabbable to get the next element and manually .focus() it.

That's a bigger ask than the original issue itself called for, and the WAI guidelines for menus/menubars says

Tab: Moves focus to the next element in the tab sequence, and if the item that had focus is not in a menubar, closes its menu and all open parent menu containers.

and their examples (navigation, editor) show that pressing tab advances out of the menu bar altogether.

So I was curious about other menus/menu bars.

Mac app menu: tab advances to the next menubar item
Windows app menu: tab does nothing
FF/Chrome/Safari in an open select options menu: tab does nothing

Seems like some wiggle room or our own interpretation, but doesn't feel like that includes tabbing between menu items.

My hope was that we could have a quick fix to the double-tab bug before re-evaluating the context menu's a11y conformance, but FF's link+tab handling might be a wrench in that plan.

[1Copenut]

I wonder if this is a Mac-specific...feature?... I found an MDN thread that Firefox can follow the MacOS system prefs for focus handling if not set in the browser itself. I generally change my system prefs immediately on a new box, so I've never noticed Firefox doesn't focus links by default.

Found this article on the Web Archive from MDN that might alleviate the issue Chandler is seeing: http://web.archive.org/web/20201112003144/https://developer.mozilla.org/en-US/docs/Mozilla/Preferences/Preference_reference/accessibility.tabfocus

[cee-chen]

I mediumly disagree with that stance. We shouldn't be attempting to hijack native Tab behavior for keyboard users and give them an experience they don't expect.

1. This only affects Mac OS and FF/Safari users. Windows doesn't even have this system level setting.
2. Keyboard users who care about navigating to links will have that system level setting checked:
   
3. We're already providing arrow key navigation which works perfectly instead if users don't want to use native tab behavior. The only thing we're doing with tabs is syncing our arrow key navigation state with tab focus.

[1Copenut]

I agree with you @constancecchen we shouldn't be hijacking native tab focus behavior. I'm just looking for reasons why links might not be receiving focus as expected.

[cee-chen]

It's because FF and Safari are respecting the checkbox in the Mac OS system setting that I highlighted above. Once you check it FF and Safari will tab to links.

[chandlerprall]

I mediumly disagree with that stance. We shouldn't be attempting to hijack native Tab behavior for keyboard users and give them an experience they don't expect.

100% agree we shouldn't change a user's expected behaviour. Links in context menus don't visually look like links, and I think it would be confusing why tab would skip an item seemingly "at random". That, together with the WAI recommendation of an altogether different tab behaviour, is has me hesitant on merging this as-is or putting more effort into an alternate solution.

@1Copenut do you have thoughts on the WAI conformance here? Is it worth patching the tab behaviour this component used to have:

That's a bigger ask than the original issue itself called for, and the WAI guidelines for menus/menubars says

Tab: Moves focus to the next element in the tab sequence, and if the item that had focus is not in a menubar, closes its menu and all open parent menu containers.

and their examples (navigation, editor) show that pressing tab advances out of the menu bar altogether.

[cee-chen]

has me hesitant on merging this as-is or putting more effort into an alternate solution.

It doesn't have to be an either-or, we can merge this in and get a nice quality of life fix in for Windows/Chrome keyboard users in the interim while we create a follow-up issue for larger accessibility/WAI-ARIA fixes.

[kibanamachine]

Preview documentation changes for this PR: https://eui.elastic.co/pr_5719/

[chandlerprall]

Ah! Constance pointed out over zoom that MacOS FF+Safari users are already affected by the skip-a-link 'functionality' in the context menu, and I definitely missed that!

This is a QOL improvement and we'll revist the WAI issues as a separate thing.

[1Copenut]

I'll give Chandler's comment a look tomorrow and open an issue if needed.

[1Copenut]

After taking a longer look at our EuiContextMenu and the WAI guidelines for menus and menubars, I have some better guidance. True menus and menubars behave like OS menus, which is a subtle but important distinction:

A menu is a widget that offers a list of choices to the user, such as a set of actions or functions. Menu widgets behave like native operating system menus, such as the menus that pull down from the menubars commonly found at the top of many desktop application windows.

This is where it's open to interpretation, but reading the spec a second and third time, it sounds like this pattern expects a menubar that has several links/buttons to expose menu elements. The EuiContextMenu doesn't appear to rise to this level, but does seem to be a good example of a WAI navigation menu button pattern. I think we could improve the handling of our component by doing a few things:

1. Assume the menu "trigger" is always a button and add aria-haspopup and aria-controls attributes
2. Adjust the menu of elements to not announce as a dialog, and add proper menu and menuitem ARIA attributes
3. Wrap each menuitem in a span or something and do not allow them to take keyboard focus. We would use roving tabindex like radio buttons do with arrow keys).
4. Pressing TAB closes the menu and sets focus on the next focusable element on the page

This pattern holds up as a composable child for the larger menubar pattern, so I think it's a good improvement and works to @constancecchen point about not overriding expected user behavior.