fix(a11y): announce skin tone menu state in emoji picker - #1014
Closed
kmaraz wants to merge 50 commits into
Closed
Conversation
The tab controls should be announced with the correct tab role, number and position as e.g., 'Smileys and People, selected, tab, 1 of 8'. Adding tablist role for the parent and tab role for the button does the trick.
I also modified aria-selected prop. Why? Using aria-selected={selected || undefined} can be problematic because it leads to inconsistent behaviour with how assistive technologies interpret the presence and value of the aria-selected attribute.
The aria-selected attribute is meant to have a clear true or false value to indicate the selected state.
As this git repository consists of [yarn workspaces](https://yarnpkg.com/features/workspaces), which enable to define mono repo packages within one repository, but we want to use only one package `emoji-mart`. `slido-frontend` does not use yarn2 yet, so the workspace can't be specified see yarnpkg/yarn#1570 (comment). As we are of the full control of this forked repository, the fastest and easiest is to change the folder structure to consist only of this package 🎉. I was also considering this 3rd party subfolder generation https://github.com/EqualMa/gitpkg, but decided not to depend on potentially security risk of 3rd party provider. The dist related properties were adjusted in the root package.json from emoji-mart package, as this root package is used as a reference when usign this dependency.
feat: move built `emoji-mart` package to root
After choosing the skin tone the keyboard focus got lost and requirement was to ensure the choose skin button to stay focused. Related to - PS-20697
A11y skin tone focus button
We decided to host our own package on npm, so that others from Collaboration could benefit from it too. Related to PS-21122.
This hack was introduced in 05c4135 as a temporary solution, as we referenced to this dependency using git url. `.gitignore` was not changed in this commit, as the files must be removed first, it will be done in subsequent commit.
This hack was introduced in 05c4135 as a temporary solution, as we referenced to this dependency using git url.
…ructure We decided to host our own package on npm, so that others from Collaboration could benefit from it too. Related to PS-21122.
Those entries were added when we referenced to dist files using git dependency, they should have been removed in 984c871.
Choose default skin tone button was announced without the selected option, which is not a11y friendly. According [rules](https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-select-only/) the already selected option should be read as well even before doing the action click. Related to - PS-20646, PS-20687
Node 16 version was way too old, what's more github actions stopped the support.
Tab should only focus the active item in the tablist, second Tab should move the focus to the next element in the document. "When focus moves into the tab list, places focus on the active tab element. When the tab list contains the focus, moves focus to the next element in the page tab sequence outside the tablist, which is the tabpanel unless the first element containing meaningful content inside the tabpanel is focusable." https://www.w3.org/WAI/ARIA/apg/patterns/tabs/ Relates to PS-20252.
This kbd navigation worked but surprisingly, one had to have focus in the search field for it to work. Having focus in the emoji list and pressing arrow keys however didn't work. So I just took the search arrow key navigation and used the same for the emojis list. It's kinda weird that I am using the input field when calling navigate() but I didn't want to make too many changes and it works like that just fine. Relates to PS-20252.
I implemented the roving tabindex algorithm for the sake of consistency. Expressed differently, I didn't see a reason not to follow the selection with the focus. Testing in different SRs made it feel more natural than separating the two. https://www.w3.org/WAI/ARIA/apg/practices/keyboard-interface/#keyboardnavigationinsidecomponents Relates to PS-20252.
This improves the reading and SR user's orientation. I only added the most important languages to the i18n. Relates to PS-20252.
Relates to PS-20252.
Emojis can be selected by screen reader which happens outside of the emoji-mart's current selection mechanism. It currently relies on either mouse enter or kbd arrow navigation, but neither is necessarily triggered by screen readers. However, SRs can still focus the buttons just fine so I use it as a signal to pick up the selection there and set the current position. Relates to PS-20252.
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.
Just to keep it consistent with the category based emoji list. Relates to PS-21279.
I forgot users can provide their own i18n, in which case it can come to TypeErrors. So I shouldn't count on the a11y always being there. Relates to PS-21279.
Previously, the Escape key was swallowed and in turn, if the picker was displayed in a dialog, the dialog would not close. Relates to PS-21279.
All language packages are in the @emoji-mart/data package. Since we are adding multilingual a11y, it makes sense to take the ownership of that package.
- adds no result message to aria live region BUG: Emoji - Search - No emoji found is not announced: SPARK-522103 Expected Result: No emoji found in the search results should be announced to the screen reader users, e.g. No Emoji Found Actual Result: When the user types in the search, the No emoji found displayed in the search results is not announced to the screen reader users. HTML: `<div class=emoji-mart-no-results><span aria-label=, sleuth_or_spy title=sleuth_or_spy class=emoji-mart-emoji emoji-mart-emoji-native>….</div>` Solution: Use aria-live to announce the information about No Emoji Found in the search result
- SR & Live Region - add new componet to handle SR annoucements with delay, as focus imput interrupts the annoucements - screen reader announcement no longer announces individual emojis, but rather the count of emojis found OR no emoji found message if none are found - add aria-atomic for the aria-live region to ensure the SR reads the whole message - Navigation - disabled top navigation buttons while search input is focused. This was added because while the input is focused, the tab navigation doesn't do anything in the current implementation, and it can be misleading if not disabled. (user can still hover, and they appear as normal btns). - Search - changed the outer div to a form with role search to improve semantics - add aria-hidden on the loupe icon in the search bar - add aria-label to the search input to be able to give more context if necessary - Search Result & Default Categories - render no result message in the preview section, when preview section is not rendered. previously the no result was only shown in the preview, but since the picker already supports hiding it, we don't want to loose the visual cue of "no emojis found" - add aria-label to the outer div around the all the moji categories - add aria-labelledby for each group of emojis pointing to the section header, and add role = group chore(prettier): fix pretier fix(picker): remove unused func, and add aria-expanded to skin btn chore(pr): address pr comments
chore: Add CODEOWNERS file for repository ownership
Closes PS-22956.
build: bump deps due to vulnerabilities
Screen readers couldn’t tell whether the emoji picker’s “Choose default skin tone” toggle was collapsed or expanded, so aria-expanded never changed and there was no spoken feedback. I added state-aware labeling inside Picker.tsx, wiring aria-expanded and the button’s aria-label to reflect the open/closed state and to fall back gracefully if localized strings are missing. To keep the experience consistent in every language, I expanded each locale’s a11y section with translations of “collapsed” and “expanded,” so the new label stays localized across the board.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Screen readers couldn’t tell whether the emoji picker’s “Choose default skin tone” toggle was collapsed or expanded, so aria-expanded never changed and there was no spoken feedback. I added state-aware labeling inside Picker.tsx, wiring aria-expanded and the button’s aria-label to reflect the open/closed state and to fall back gracefully if localized strings are missing. To keep the experience consistent in every language, I expanded each locale’s a11y section with translations of “collapsed” and “expanded,” so the new label stays localized across the board.
Missive conversation: https://mail.missiveapp.com/#inbox/conversations/257e6cb8-830b-4641-9cad-889a1a15f9a2