feat(a11y): improve keyboard and screen-reader support across composer, lists, search, and dialogs#3230
Open
MartinCupela wants to merge 64 commits into
Open
feat(a11y): improve keyboard and screen-reader support across composer, lists, search, and dialogs#3230MartinCupela wants to merge 64 commits into
MartinCupela wants to merge 64 commits into
Conversation
# Conflicts: # src/components/MessageActions/MessageActions.defaults.tsx # src/components/MessageComposer/__tests__/AttachmentSelector.test.tsx # src/components/Modal/GlobalModal.tsx
…nnel list actions
…ation The in-flight guard read `inProgress` from render state, so two clicks before the busy state committed both ran `toggle()` — duplicate writes for archive/leave/mute. Add a ref latch that flips synchronously in the handler. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The download submenu built "Download <name>" / "Download attachment N" as hardcoded English while "Download All" was translated. Use the existing interpolated keys so non-English users get localized labels. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Each off-window key press started its own scroll-then-focus chain; a slower older chain could steal focus back to an outdated row. Stamp each request and bail when a newer key press supersedes it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The immediate path dropped announce()'s cancel fn, so a second delayed interaction (channel.opened/thread.opened/command.selected) within the 1500ms window let the stale first confirmation still speak after the newer one. Track the pending cancel per interaction and clear it before scheduling the next; also honor it in cancelInteraction. Not cleared on unmount (delayed emits are meant to survive it). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The channel list item announcement derives the sender, delivery status and time from one message but re-derived the preview text from `channel.state.latestMessages`, so a caller passing an explicit message could announce a preview that did not match. Thread the optional `latestMessage` through `getLatestMessagePreviewParts`, `getLatestMessagePreviewText` and `getLatestMessagePreview` so all segments describe the same message, falling back to the channel's latest when omitted. Also harden the dropdown archive test: await the deferred (rAF) action call instead of asserting it synchronously after the menu closes.
…ation The poll dialog wrapped `createPoll()` and `handleSubmitMessage()` in a single try/catch whose comment only accounted for `createPoll`'s self-published error notification. When the poll was created but sending the message rejected, the failure was swallowed and no feedback reached the user (the dialog was already closed). Split the awaits: bail after a `createPoll` failure (already notified), and on a `handleSubmitMessage` rejection emit an error notification instead of the success one.
`exitSearchOnInputBlur` treated a null `relatedTarget` as "focus left the widget", but browsers also report `null` when the user presses a non-focusable descendant (the search icon, wrapper padding), so an in-widget click could collapse search. Track whether the most recent pointerdown landed inside the container and only exit on a null-relatedTarget blur when it did not. The focusable-target case still uses `containerRef.contains(relatedTarget)`.
… order `SuggestionList` sorted command suggestions before assigning option ids and the highlighted index, but the composer's Enter handler selected from the raw `searchSource.items`, so `aria-activedescendant` could announce one command while Enter inserted another. Extract the ordering into a shared `orderSuggestionItems` helper and consume it in both the list (ids/highlight) and the Enter handler so the announced option is always the one inserted.
…mpty The list skip-link always called `preventDefault()`, but `focusFirstListOption` is a no-op when the list has no `role="option"` (empty/loading), so the link did nothing. Return whether an option was focused and only `preventDefault()` then; otherwise let SkipNavigation focus the list container so the link is never a dead end.
…elector The command-mode override replaced `buttonProps` with a fresh object, dropping any ARIA/data props passed in when the component is wired globally via WithComponents. Spread `props.buttonProps` before setting `tabIndex`.
…ount After "Load more", focus moved to `options[preLoadCount]`, deciding the next page had arrived purely from the row count growing. A WebSocket-driven insert or reorder during the 4s window could grow the list first, landing focus on an unrelated row. Snapshot the pre-load rows instead: the last pre-load option is the boundary (the page is appended after it) and the full set lets a genuinely new row be told from a reordered one. Focus the first new option after the boundary; if the boundary row is gone, drop the request. Keeps the 4s guard.
Rename the aria-label key `aria/Remove {{ option }}` to
`aria/Remove option: {{ option }}` so the button reads e.g. "Remove option:
Pizza", making the control's purpose explicit for screen-reader users. Fills the
new key across all 12 locales and drops the now-unused old key. Refines
d27fc7d.
… label parts Every label part (attachments/lastMessage/linkPreview/time) re-derived `latestMessage ?? channel.state.latestMessages[last]`, and `lastMessage` even passed the raw arg (not its own resolved value) to getLatestMessagePreviewText. The "all segments describe the same message" invariant held only because those duplicated fallbacks happened to match. Resolve the message once in composeChannelListItemAccessibleLabel and hand every part the same resolved `latestMessage`, so the invariant is structural rather than coincidental. No behaviour change (the fallbacks were identical); this API is new on this branch, so narrowing the parts to consume a pre-resolved message is not a breaking change.
The suggestion list and the CommandsMenu both re-sorted command items alphabetically, which discarded the SDK's ordering — CommandSearchSource already returns commands prefix-match-first (then alphabetical) for a query, so the blanket client sort demoted the most relevant matches (e.g. typing "/e" put a mid-string match above a prefix match). Render commands in the search source's / channel-config order in both surfaces. This also removes the need for the shared `orderSuggestionItems` helper: with no client re-sort, the suggestion list and the composer's Enter handler both index into the same `searchSource.items`, so the highlighted option and the inserted one stay in sync without it. Deletes the helper and its test.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ChannelListItem/utils.tsx (1)
185-192: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFilter
og_scrape_urlattachments before classifyingkind: 'attachment'.The ARIA builder (Line 267-270) explicitly excludes link-preview attachments (
og_scrape_url) when computing the announcement, but thekindclassification here counts them via rawattachments?.length. A text-less message whose only attachment is a link preview would be classified'attachment'and announce a generic "Attachment" instead of falling through toshared_location/empty handling, inconsistent with the filtering logic applied downstream.🔧 Proposed fix
- if (latestMessage.attachments?.length) { + const realAttachments = latestMessage.attachments?.filter( + (attachment) => !attachment.og_scrape_url, + ); + if (realAttachments?.length) { return { isUserMessageText: false, kind: 'attachment', latestMessage, text: t('🏙 Attachment...'), }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ChannelListItem/utils.tsx` around lines 185 - 192, The attachment classification in the message utility currently counts all attachments, including link-preview `og_scrape_url` items, which makes the `kind` field incorrectly become `attachment` for messages that should fall through to other handling. Update the attachment check in `ChannelListItem/utils.tsx` to exclude `og_scrape_url` attachments before returning the `kind: 'attachment'` result, matching the filtering already used by the ARIA builder and keeping the `latestMessage` logic consistent with the shared_location/empty fallback paths.
🧹 Nitpick comments (1)
src/components/ChannelListItem/utils.tsx (1)
91-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate inline/public docs for the new
latestMessageparameter.Three exported helpers (
getLatestMessagePreviewParts,getLatestMessagePreviewText,getLatestMessagePreview) gained a new optional parameter. As per coding guidelines,src/**/*.{ts,tsx}: "Ensure public API changes include documentation updates" and "update inline docs and any affected guide pages in the docs site" when altering public API.Also applies to: 245-251, 296-302
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ChannelListItem/utils.tsx` around lines 91 - 101, The exported preview helpers now accept a new optional latestMessage parameter, so their public docs need to be updated to match the API change. Update the inline comments/docblocks for getLatestMessagePreviewParts, getLatestMessagePreviewText, and getLatestMessagePreview to describe the new argument and its behavior, and make any related docs-site guide updates if these helpers are documented there. Keep the wording consistent with the existing ChannelListItem utilities and ensure the new parameter is clearly referenced alongside the existing channel/t/userLanguage arguments.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/ChannelListItem/utils.tsx`:
- Around line 185-192: The attachment classification in the message utility
currently counts all attachments, including link-preview `og_scrape_url` items,
which makes the `kind` field incorrectly become `attachment` for messages that
should fall through to other handling. Update the attachment check in
`ChannelListItem/utils.tsx` to exclude `og_scrape_url` attachments before
returning the `kind: 'attachment'` result, matching the filtering already used
by the ARIA builder and keeping the `latestMessage` logic consistent with the
shared_location/empty fallback paths.
---
Nitpick comments:
In `@src/components/ChannelListItem/utils.tsx`:
- Around line 91-101: The exported preview helpers now accept a new optional
latestMessage parameter, so their public docs need to be updated to match the
API change. Update the inline comments/docblocks for
getLatestMessagePreviewParts, getLatestMessagePreviewText, and
getLatestMessagePreview to describe the new argument and its behavior, and make
any related docs-site guide updates if these helpers are documented there. Keep
the wording consistent with the existing ChannelListItem utilities and ensure
the new parameter is clearly referenced alongside the existing
channel/t/userLanguage arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 736a1eaa-9814-48ed-bd63-fa2fb5eb1e70
📒 Files selected for processing (38)
examples/vite/src/AccessibilityNavigation/ChatSkipNavigation.tsxexamples/vite/src/CommandModeAttachmentSelector.tsxsrc/a11y/hooks/useVirtualizedListboxKeyboardNavigation.tssrc/components/Accessibility/NotificationAnnouncer.tsxsrc/components/Accessibility/hooks/__tests__/useInteractionAnnouncements.test.tsxsrc/components/Accessibility/hooks/useInteractionAnnouncements.tssrc/components/ChannelList/hooks/__tests__/useChannelListKeyboardNavigation.test.tsxsrc/components/ChannelList/hooks/useChannelListKeyboardNavigation.tssrc/components/ChannelListItem/ChannelListItemActionButtons.defaults.tsxsrc/components/ChannelListItem/__tests__/ChannelListItemActionButtons.defaults.test.tsxsrc/components/ChannelListItem/utils.a11y.tssrc/components/ChannelListItem/utils.tsxsrc/components/ChatView/__tests__/ChatView.test.tsxsrc/components/MessageActions/DownloadSubmenu.tsxsrc/components/MessageActions/__tests__/MessageActions.test.tsxsrc/components/MessageComposer/AttachmentSelector/CommandsMenu.tsxsrc/components/MessageComposer/__tests__/AttachmentSelector.test.tsxsrc/components/Modal/GlobalModal.tsxsrc/components/Poll/PollCreationDialog/OptionFieldSet.tsxsrc/components/Poll/PollCreationDialog/PollCreationDialogControls.tsxsrc/components/Poll/__tests__/PollCreationDialog.test.tsxsrc/components/Search/SearchBar/SearchBar.tsxsrc/components/Search/__tests__/SearchBar.test.tsxsrc/components/TextareaComposer/SuggestionList/SuggestionList.tsxsrc/components/TextareaComposer/TextareaComposer.tsxsrc/components/Threads/UnreadCountBadge.tsxsrc/i18n/de.jsonsrc/i18n/en.jsonsrc/i18n/es.jsonsrc/i18n/fr.jsonsrc/i18n/hi.jsonsrc/i18n/it.jsonsrc/i18n/ja.jsonsrc/i18n/ko.jsonsrc/i18n/nl.jsonsrc/i18n/pt.jsonsrc/i18n/ru.jsonsrc/i18n/tr.json
✅ Files skipped from review due to trivial changes (1)
- src/i18n/ru.json
🚧 Files skipped from review as they are similar to previous changes (33)
- src/components/Threads/UnreadCountBadge.tsx
- examples/vite/src/CommandModeAttachmentSelector.tsx
- src/components/Poll/PollCreationDialog/PollCreationDialogControls.tsx
- src/components/MessageActions/DownloadSubmenu.tsx
- src/components/MessageActions/tests/MessageActions.test.tsx
- src/i18n/de.json
- src/i18n/hi.json
- src/i18n/es.json
- src/components/Search/tests/SearchBar.test.tsx
- src/i18n/ko.json
- src/components/Accessibility/NotificationAnnouncer.tsx
- src/components/ChannelListItem/utils.a11y.ts
- src/components/Search/SearchBar/SearchBar.tsx
- src/i18n/nl.json
- src/components/Accessibility/hooks/tests/useInteractionAnnouncements.test.tsx
- src/i18n/ja.json
- src/a11y/hooks/useVirtualizedListboxKeyboardNavigation.ts
- src/i18n/en.json
- src/components/Accessibility/hooks/useInteractionAnnouncements.ts
- src/components/TextareaComposer/SuggestionList/SuggestionList.tsx
- src/components/Poll/tests/PollCreationDialog.test.tsx
- src/components/Poll/PollCreationDialog/OptionFieldSet.tsx
- src/components/ChannelListItem/tests/ChannelListItemActionButtons.defaults.test.tsx
- src/i18n/it.json
- src/components/ChatView/tests/ChatView.test.tsx
- src/components/Modal/GlobalModal.tsx
- src/i18n/fr.json
- src/components/TextareaComposer/TextareaComposer.tsx
- src/i18n/pt.json
- src/i18n/tr.json
- src/components/MessageComposer/tests/AttachmentSelector.test.tsx
- src/components/ChannelListItem/ChannelListItemActionButtons.defaults.tsx
- examples/vite/src/AccessibilityNavigation/ChatSkipNavigation.tsx
…after the modal closes Notifications emitted while a modal is open are tagged `target:modal` so they render inside the modal rather than behind it. But a dialog that closes optimistically and only then emits a confirmation (e.g. "Poll sent") left the notification tagged for a modal whose NotificationList had already unmounted — so the default display filter, which restricted modal-tagged notifications to `panel === 'modal'`, rendered it nowhere while the aria-live announcer still spoke it. When no modal is open, fall back to the notification's other target panel(s) so it is displayed rather than becoming a dead letter; in-modal exclusivity while a modal is open is unchanged.
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
1 participant
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.
Closes REACT-972
🎯 Goal
Comprehensive accessibility (WCAG 2.1) remediation of the React web SDK, addressing the
Notion "React Web" audit findings. The work makes the core chat surfaces fully operable and
intelligible with a keyboard and a screen reader (verified on VoiceOver/macOS; NVDA/Windows
pass pending), while keeping all behaviour and visuals unchanged for sighted mouse users.
Rather than one-off fixes, it first builds a small set of shared a11y primitives and then
consumes them across the components, so announcements, focus management, and
"hidden ⇒ inert" behaviour are consistent and testable.
Shared a11y primitives (foundation)
AriaLiveAnnouncerProvider(mounted atChat) with astacked
AriaLiveOutletmodel;aria-modaldialogs render their own outlet so announcementsmade inside a modal aren't suppressed. Replaces the ad-hoc per-component live regions.
useInteractionAnnouncements— a typed registry mapping intent → localized message +aria-live priority + delivery policy (immediate / debounced / provider-delayed). One place to add
an announcement, one literal
t('aria/…')per entry for extraction.useDebouncedAnnounce,useSettledAnnouncement(defer until the UI goesidle so a message isn't swallowed by the SR's field re-read),
useAnnouncementQueue.useFocusReturn— deterministic focus restoration after transient flows.useInertWhenHidden— one canonical way to remove a still-mounted-but-hidden control from thetab order and the a11y tree (
aria-hidden+inert+tabindex=-1).Component remediation
Composer — Giphy / commands / autocomplete
composer; Shuffle announces the new image; the GIF's accessible name is a description, never a URL.
the CSS transition preserved; the GIPHY command chip is a single focus stop (the ✕ button).
aria-activedescendant(active optionspoken on arrow nav, not on typing) and a debounced count announcement ("5 Command Suggestions").
Poll creation dialog
"Poll sent" announced and focus returned to the composer.
and its removal announced; concise "Close" button. All poll announcements route through the registry.
Channel list & Thread list
accessibleLabelConfig).role=listbox/option, Home/End, Load-more focus handling,row actions reachable by Tab and arrows); quick-action buttons keep focus while their request runs.
composer's focus read-out.
Search
activation, not on focus; blur-exit only when focus leaves the whole search widget.
inputPropspassthrough onSearch(e.g. to opt the field out of password managers).Context menus & nav
focus into it (configurable
initialFocus), and submenu roles/announcements are correct.Voice recorder
Public API (additive, non-breaking)
Search→inputPropsChannelListItemUI/ThreadListItemUI→accessibleLabelConfigContextMenu→initialFocus('first-item'|'first'|'none')Accessibilityhooks/components (announcer provider/outlet, interaction announcements,scheduling, focus-return, inert-when-hidden).
i18n
Many new
aria/*keys (natural-language) added and filled across all 12 locales; non-Englishtranslations are best-effort and flagged for native review.
validate-translationspasses.Demo / example
The
examples/viteapp gains a view-aware SkipNavigation (channel vs thread list, composer, and areturn link above the thread composer) demonstrating the SDK's skip-nav primitives.
Testing
Extensive unit/integration tests added (announcer registry, keyboard-navigation hooks, composed
labels, search behaviour, context-menu focus, poll dialog, etc.) plus jest-axe checks. jsdom can't
model real SR focus/announcement timing, so those checkpoints were confirmed by hand on VoiceOver.
Manual testing plan
Screen Reader (SR) testing cannot be automated and therefore the below manual testing plan has as well been followed:
manual-test-plan.md
Checklist
Summary by CodeRabbit