fix(a11y): prevent refocus of textarea on keyboard navigation

[dancormier]

Closes STACKS-383

This PR ensures that the focused editor menu element retains focus when navigating via keyboard.

Previously, tabbing to a menu element then invoking it with enter would send focus to the "view" (aka the textarea). This would require those navigating with the keyboard to have to tab back to where they were to continue where they left off. This PR addresses this issue with the following changes:

1. Focus is no longer moved to the view unless editor menu item was clicked
2. The popover is only hidden if the popover menu item was clicked

TODO

• Add unit test
• Add ticket(s) for fully accessible menu (see fix(a11y): prevent refocus of textarea on keyboard navigation #254 (review)) https://stackoverflow.atlassian.net/browse/STACKS-402

[netlify]

✅ Deploy Preview for stacks-editor ready!

Name	Link
🔨 Latest commit	3260f5d
🔍 Latest deploy log	https://app.netlify.com/sites/stacks-editor/deploys/650cae6ac32a560008131455
😎 Deploy Preview	https://deploy-preview-254--stacks-editor.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 configuration.

[giamir]

Thanks @dancormier for this small fix (see comment in the code - there is a small bug).
We can add a unit test and we should be good to move on with this story.

Sidenote (More a11y issues)

While reviewing this fix I spent a good amount of time reading WAI authoring practices for menu and menubar. I think that what we have now is still not fully accessible.
Here are the issues I understood still affect our implementation:

• When a menu opens, keyboard focus should be placed on the first item (at the moment the focus stays on the menu button)
• Tab and Shift + Tab should NOT move focus among menuitems instead they should move to the next/previous focusable item which is not a menuitem and close the menu if it was open.
• Arrow Up and Arrow Down should be used instead to loop through the menuitems (focus should be trapped)
• When Enter (and optionally Space) is pressed and user is focused on a menuitem the item should activate and the menu should close (at the moment the menu remains open)

I think we could consider these a11y issues outside the scope of this fix (since they were not in the report). That said I would not be surprised if we will get a follow up report with these issues listed. The minimum we should do is to create a ticket for our backlog and in my opinion we should try to work on that ticket before the enterprise release too to be on the safe side.

What are your thoughts @dancormier?

I am also tagging @AvogadroSG1 for visibility/3rd opinion.

[dancormier]

Thanks @dancormier for this small fix (see comment in the code - there is a small bug). We can add a unit test and we should be good to move on with this story.

I'll add a unit test and merge it in. It's not perfect, but it accomplishes the stated goal and is an improvement over what we had.

Sidenote (More a11y issues)

While reviewing this fix I spent a good amount of time reading WAI authoring practices for menu and menubar. I think that what we have now is still not fully accessible. Here are the issues I understood still affect our implementation:

* When a menu opens, keyboard focus should be placed on the first item (at the moment the focus stays on the menu button)

* `Tab` and `Shift + Tab` should NOT move focus among `menuitem`s instead they should move to the next/previous focusable item which is not a `menuitem` and close the menu if it was open.

* `Arrow Up` and `Arrow Down` should be used instead to loop through the `menuitem`s (focus should be trapped)

* When `Enter` (and optionally `Space`) is pressed and user is focused on a `menuitem` the item should activate and the menu should close (at the moment the menu remains open)

Thank you for this research. I kinda took the ticket/report at face value and "fixed" the issue, but the approach you found in your research makes total sense. When I was working on this, I even had the thought about arrow navigation making sense (I thought it would be nice if it kinda worked like navigating a radio group), but just left it as-is.

Out of curiosity (and proximity), I tested out the ⋮ button on the top-right corner of these PR comments to see how they work. GitHub is not without its accessibility issues but I figured they'd meet MSFT's standards considering they're GitHub's parent company. What they do is close to what WCAG describes, with the exceptions that:

• Invoking the menu does not focus the first item
• Tab navigates the menuitems in addition to the arrows

For the record, I'm not suggesting we follow GitHub's lead here. I figured I'd mention it as a curiosity here.

I think we could consider these a11y issues outside the scope of this fix (since they were not in the report). That said I would not be surprised if we will get a follow up report with these issues listed. The minimum we should do is to create a ticket for our backlog and in my opinion we should try to work on that ticket before the enterprise release too to be on the safe side.

I agree. I think we can consider this as accomplishing the goal and resolving the stated issue. There's room for improvement, but this is moving in the right direction. Related, I've yet to see a concrete standard we're being held to by the third-party requesting the fix. The best I've heard is "we need to be able to show we're improving accessibility" which I think this accomplishes. We can prioritize other improvements that get us closer to something like WCAG AA adherence outside of this PR. For now, I'll write up some tickets.

Thanks @giamir for the review and all the information!