Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dan/add kbd navigation to categories #10

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

daelmaak
Copy link
Collaborator

@daelmaak daelmaak commented Jul 2, 2024

  • fix: add missing keyboard navigation for categories
  • fix: make emojis navigation more accessible in NVDA
  • fix: add missing keyboard navigation to search results
Screen.Recording.2024-07-02.at.11.18.07.mov

Previously, the category tabs were all tabbable. This is wrong according
to WAI-ARIA guidelines. I changed that to only allowing the current tab
to be tabbable, but broke the keyboard navigation by doing so since the
rest of the tabs wasn't tabbable anymore and the arrow keys weren't
working natively.

So I added a manual arrow key navigation. The tricky part was to handle
the fact that the full emoji list is always only partially rendered.
Practically, I can select an emoji in the 1st category, then select
let's say a 5th category in tablist, and now the 1st is not rendered
anymore. Which also means there would be no tabbable emoji for the user
to navigate to! For this reason, and for convenience as well, since user
should be able to tab to emojis of the just selected category, I always
make the first emoji in the currently selected category tabbable upon tab
selection.

Relates to PS-21279.
In the previous commits, I accepted the fact that NVDA user has to switch
from Browse mode to Focus mode to navigate through emojis. This was true
for our markup back then. But then I noticed, that user can use arrow
keys in the categories tablist just fine in the Browse mode. So I wondered
if I could make the emoji list more accessible by assigning the right
roles. And I found that yes, doing so allows the user to navigate through
emojis using arrow keys directly in the Browse mode!

I was torn between using `radiogroup` and `listbox`. The 2 are similar
in many ways, with the exception that only 1 radio can be selected at a
time whereas list box can have more. This is however not important
because only 1 emoji can be selected at a time. In the end, I went with
listbox because it sounds like a more natural choice for a list of items
but I don't have a strong opinion there.

Closes PS-21279.
@daelmaak daelmaak marked this pull request as ready for review July 2, 2024 09:22
Just to keep it consistent with the category based emoji list.

Relates to PS-21279.
Copy link
Member

@JanPodmajersky JanPodmajersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pure gold 🥇 , tahkns a lot 🦸 💪 .
With that much effort already in, we should promote our fork more 😛 .

@daelmaak
Copy link
Collaborator Author

daelmaak commented Jul 2, 2024

This is pure gold 🥇 , tahkns a lot 🦸 💪 . With that much effort already in, we should promote our fork more 😛 .

I think we should :). I still dream about pushing the changes over to the original repo though, that would be the safest bet.

@daelmaak daelmaak merged commit 6583648 into main Jul 2, 2024
2 checks passed
@daelmaak daelmaak deleted the dan/add-kbd-navigation-to-categories branch July 2, 2024 09:57
@JanPodmajersky
Copy link
Member

I think we should :). I still dream about pushing the changes over to the original repo though, that would be the safest bet.

Well yeah, it would be a dream but I've got a feeling it is slightly unrealistic, though 😢 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants