Add keyboard behaviors to menubar
Progress on #2933
Tasks:
- [ ] Add controls for navigating the menubar via Up, Down, Home, Escape keys
- [ ] Implement a way to move focus to a menuitem that matches a typed character (focus does not move if there is no match)
- [ ] Mouse pointer events for nested submenus
- [ ] Add unit tests
- [ ] Focus styles should align with currently focused menu item
- [ ] Test with mobile view
Changes:
- added roving tab index for focus management of menubar items
- moved language dropdown to left menu
Hi all! I'm using this PR to work on implementing keyboard behaviors for the editor's menubar. This will support the accessibility features implied by the recently-added ARIA attributes in #3191. Currently I've added a roving tab focus and changing the focused item with the left and right keys. The language drop down was also moved to the left side of the menu.
I'm referencing the implementation details for the menubar in a sandbox linked below. Would love any feedback if anyone has some!
References:
I have verified that this pull request:
- [x] has no linting errors (
npm run lint) - [x] has no test errors (
npm run test) - [x] is from a uniquely-named feature branch and is up to date with the
developbranch. - [x] is descriptively named and links to an issue number, i.e.
Fixes #123
thanks! yes i didn't realize arrow keys should be used either until reviewing the wai aria guide. i learned that you can tab to get into the menubar but once that component is focused, the expectation for keyboard users is that arrow keys would be used to get to the other items. would love to get others' thoughts. as a reference, the initial commits in this pr allow navigation with the left and right arrow keys:
i also just pushed some recent commits that begin to add functionality for up and down arrows. there's some work to be done; for example, when first opening a submenu, the 1st item doesn't get a focus outline for some reason; and the keyboard behaviors are not all there (pressing the left and right keys while in a submenu should close the current submenu and focus the next one, expanding if it is another submenu; additionally, the down and up keys should both open the submenu and focus the first or last element respectively). there are also lots of code changes so i'm sure a refactor would be helpful down the line.
i'll keep poking at it and drop an update here.
Great work @tespin! And thanks for tagging me @raclim -- I'll do some research and testing this weekend and will share any suggestions next week.
@tespin my apologies for the delay! Your upgrades work well overall. As you pointed out, the Left and Right Arrow keys behave differently in a submenu than suggested by WAI-ARIA:
When focus is in a submenu of an item in a menubar, performs the following 3 actions:
- Closes the submenu.
- Moves focus to the previous item in the menubar.
- If focus is now on a menuitem with a submenu, either: (Recommended) opens the submenu of that menuitem without moving focus into the submenu, or opens the submenu of that menuitem and places focus on the first item in the submenu.
Actions 1 and 3 seem particularly helpful. Do you have a sense of what changes are needed to implement them?
@nickmcintyre no worries, thanks for getting back to me!
i spent some time on it today. my idea was to add an onKeyDown to createDropdownHandlers in NavBar.jsx. i tested use with the down arrow (which WAI-ARIA says, if focus is on a menuitem with a submenu, the down arrow should open the submenu). the current submenu opens, but i'm having trouble when it comes to the left and right keys -- they expand the previously active menu rather than the current one.
i'm learning that it has to do with react's asynchronous updates but i'm not sure if i'm accessing the current state correctly.
should i not do a functional state update with prevState?
@nickmcintyre no worries, thanks for getting back to me!
i spent some time on it today. my idea was to add an
onKeyDowntocreateDropdownHandlersinNavBar.jsx. i tested use with the down arrow (which WAI-ARIA says, if focus is on a menuitem with a submenu, the down arrow should open the submenu). the current submenu opens, but i'm having trouble when it comes to the left and right keys -- they expand the previously active menu rather than the current one.i'm learning that it has to do with react's asynchronous updates but i'm not sure if i'm accessing the current state correctly.
should i not do a functional state update with `prevState`?
hmmm. A functional state update with prevState is correct. I think the problem is which menu item is triggering the event handler, and consequently what is the value of dropdown. Like if "Sketch" is focused and you press the right arrow key, it's the "Sketch" menu that handles the event. But we don't want to do anything with "Sketch", we want to shift the focus (and possibly open the menu) for the next item in the list, which is "Help", because we are navigating to the right. The way that I coded the nav bar does not make that easy because there is no awareness of what other items are in the menu. This might take some refactoring to implement.
hmmm. A functional state update with
prevStateis correct. I think the problem is which menu item is triggering the event handler, and consequently what is the value ofdropdown. Like if "Sketch" is focused and you press the right arrow key, it's the "Sketch" menu that handles the event. But we don't want to do anything with "Sketch", we want to shift the focus (and possibly open the menu) for the next item in the list, which is "Help", because we are navigating to the right. The way that I coded the nav bar does not make that easy because there is no awareness of what other items are in the menu. This might take some refactoring to implement.
ah i see. i think i did have some issues implementing certain changes with the current component, so a refactor would make sense. if there isn't one already, i'll start putting together a separate issue to start a conversation on what a refactor might look like.
thanks for your input!
closing, continued in #3320
should i not do a functional state update with `prevState`?