Kdropdown keyboard navigation

[marcellamaki]

Description

This PR adds tab navigability for KDropdownModal. Currently, a user cannot use keyboard navigation to enter into the dropdown.

Issue addressed

Fixes #178

Before/after screenshots

BEFORE

AFTER

Steps to test

1. Use the tab key to navigate to a dropdown menu
2. Use the enter key to open the dropdown
3. The focus should be directed to the first item in the menu
4. Using the tab key should direct you down each item in the menu, and then a double-tab at the end should bring you back to the first element
5. Using escape key when you are inside the menu should close the menu, and bring the focus back to the dropdown menu button

Comments

Any feedback for general code improvement (and if I'm using some of the existing KDS patterns correctly) or about how this affects other components that I might not be thinking of would be very helpful.

Also, @jtamiace / @khangmach, I am not sure if there is already a design convention for the focus on menu items. I did review the existing docs but know they're a WIP and it's possible you have some work on this that I'm just unaware of. If you have design feedback or recommendations of how I can improve this, please let me know! So far, I just tried to adapt existing patterns and styles, but I'm sure there is room for improvement.

[nucleogenesis]

The code looks good aside from the console.log

@radinamatic might have some more insights here on the a11y.

When I got focused into the first element of the dropdown, I tried using the arrow keys to navigate the menu items which scrolled the whole page and I realized you can only tab through the children.

I did some digging and found this: https://www.w3.org/TR/wai-aria-practices/examples/listbox/listbox-collapsible.html - if you scroll down you see some specs for Keyboard Navigation.

Perhaps the use case there is slightly different than ours? In any case - I'll let @radinamatic weigh in on that aspect.

[marcellamaki]

@nucleogenesis - thanks for your notes! You are right about the arrow keys and not the tab. I had read some resources saying either was fine, but I think it's a better idea to go with the w3 standard. I'll make those changes today or tomorrow and hopefully get this wrapped up before we move into all-product sprints 😊

[jtamiace]

Approving from the design side - the behaviors you implemented make a lot of sense, and so does the highlight itself

[marcellamaki]

@nucleogenesis - I made some changes to the code since you last reviewed. One making sure the arrow keys worked and two adding a $focus-outline variable. Previously I had been using some javascript and active styles, but it was getting unnecessarily complicated to be creating so many computed properties just to add some basic css. That said, I'm not sure if there is a reason why there wasn't this variable already, and why the value was mostly being accessed in different ways. I'd be happy to switch back to using javascript if either you or Devon could give me a few hints on a simpler approach. Thanks for your feedback!

[nucleogenesis]

With @indirectlylit's note about the CSS and the fiddly a11y tab/keyboard navigation stuff I noted both resolved this should be good to go, thanks @marcellamaki !

[nucleogenesis]

When we're on the lastChild and hit tab (keycode 9) then the focus outline is lost altogether and I'm not sure where it is. The menu stays open as well.

On that W3 link I shared in my previous comment, if you keyboard navigate to the dropdown (you'll have to eyeball it because it doesn't show a focus indicator on that site) - but if you hit space to open it, arrows to navigate within it, and then hit tab - the tab key moves the focus to the next element in the DOM.

So the listed items in the drop-down should not be a part of the tab navigation.

[marcellamaki]

I've made some changes in this PR that were not changes when I started it, but now are reverting other changes that have been made in the interim around focus and keyboard navigation. Would especially appreciate review from @sairina and @MisRob in regards to these changes, as it essentially undid some recent changes.

For context and the thought process:

• Most of the elements that are accessed through keyboard navigation are through tab navigation
• However, a handful of elements such as dropdown menus, radio buttons, and scolling, use the arrow keys (as @nucleogenesis mentioned above) Further details
• The original logic applied a focus outline to the default elements but did not check for whether or not there was a keyboard event, which caused the issue with the focus outline appearing around textbox elements on the sign in page, even when the user was not navigating with keyboard
• The other consideration is when the focus is added through javascript, and is not tied to a keyboard event, but we want to set the focus outline (i.e. for the select dropdown menu), then we need to manage this as well
• The original logic also did not include focus outline for KSwitch which was addressed

General approach to changes:

• Some elements should always default to having a focus outline, and this should be associated only with keyboard events
• Focus outline should not appear around "focusable elements" when navigated with the keyboard
• There may be use cases where we want to add focus outline and keyboard navigation ability, especially around arrow keys, so we should have an option to add this explicitly
• Goal: to reduce the number of places in future development that we need to manage an "activeClass" that requires an outline as a one off within Kolibri, especially for non-tabbable elements.

Outcomes of implementation:

Element and interaction	Functionality
Navigating checkboxes with keyboard
Download resource button with keyboard
Dropdown menu with keyboard (arrow keys)
Dropdown with mouse click (no focus)
Logging in using mouse (no focus outline)
Logging in when starting to use the tab key to navigate
KSwitch (a variation of input=checkbox) with keyboard

NOT SOLVED BY THIS

The user profile dropdown menu in the top right corner does not use this same style of menu. It is currently navigable with tab keys, which might be confusing to users... However it is being removed in 0.16. Worth discussing whether there should be a follow up issue to change this... I think it might be a low lift and the consistency might be beneficial?

[radinamatic]

Progress looks very exciting, thank you @marcellamaki!
I see nothing concerning in the screenshots and the description, very much looking forward to have an actual asset to test 😍

[MisRob]

Thank you @marcellamaki, this is great work. I've only checked the code so far. To be honest, I don't recall exactly all updates to the tracking input modality script I did and reasons for them but I think that your updates with all background information and comments in this PR make sense overall. Thank you for sharing the WebAIM resource on keyboard testing, that's very helpful. I learned a lot from the discussion and work in this PR.

I left some minor comments, neither of them is blocking.

The only thing that needs to be resolved is kolibri-tools upgrade. I saw that you posted a message on Slack about it so I responded there with some issues I experienced.

[indirectlylit]

See learningequality/kolibri#4822 and learningequality/kolibri#4847 for some historical context on how the code got into its current state.

[rtibbles]

One thing still missing for the body modality attribute.

Comments addressed.