Fix: Screen reader does not announce or move focus after emoji category selection#83143
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
melvin-bot
bot
requested review from
suneox and
trjExpensify
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| emojiListRef, | ||
| } = useEmojiPickerMenu(); | ||
| const StyleUtils = useStyleUtils(); | ||
| const headerRefs = useRef<Record<number, React.RefObject<View | null>>>({}); |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
The entire block of accessibility focus management logic (headerRefs, pendingHeaderFocusIndexRef, selectedHeaderIndex, getHeaderRef, focusHeaderAtIndex, scheduleHeaderFocus, handleHeaderLayout) is duplicated nearly identically between index.native.tsx and index.tsx. This increases maintenance overhead and bug risk -- any fix must be applied in both places.
Extract the shared logic into a custom hook (e.g. useHeaderFocusManagement) that both platform files can import. This hook would encapsulate the refs, state, and callbacks for managing header focus.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| return; | ||
| } | ||
|
|
||
| if (attempt >= 2) { |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
The magic number 2 represents the maximum number of retry attempts for focusing the header, but this is not self-documenting. Extract it into a named constant for clarity.
const MAX_HEADER_FOCUS_ATTEMPTS = 2;
// ...
if (attempt >= MAX_HEADER_FOCUS_ATTEMPTS) {
return;
}Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| return; | ||
| } | ||
|
|
||
| setTimeout(() => schedule(headerIndex, attempt + 1), CONST.ANIMATED_TRANSITION); |
There was a problem hiding this comment.
❌ PERF-12 (docs)
The setTimeout inside scheduleHeaderFocus is never cleaned up. If the component unmounts while a retry is pending, the timeout will fire and attempt to call focus() on a potentially stale ref, which can cause warnings or errors. The same issue applies to the onMomentumScrollEnd callback which also calls scheduleHeaderFocus.
Store the timeout ID in a ref and clear it on unmount via a useEffect cleanup function.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| pendingHeaderFocusIndexRef.current = headerIndex; | ||
| emojiListRef.current?.scrollToOffset({offset: calculatedOffset, animated: true}); | ||
| setFocusedIndex(headerIndex); | ||
| setTimeout(() => { |
There was a problem hiding this comment.
❌ PERF-12 (docs)
This setTimeout (and the one inside scheduleHeaderFocus) is never cleaned up. If the component unmounts while the timeout is pending, it will fire and attempt to call scheduleHeaderFocus on a potentially unmounted component. Store the timeout ID in a ref and clear it on unmount via a useEffect cleanup function.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
| return; | ||
| } | ||
|
|
||
| if (attempt >= 2) { |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
Same magic number 2 for max retry attempts as in index.native.tsx. Extract it into a named constant (e.g. MAX_HEADER_FOCUS_ATTEMPTS) for clarity, ideally shared with the native file.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b4498778e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function schedule(headerIndex: number, attempt = 0) { | ||
| if (focusHeaderAtIndex(headerIndex)) { | ||
| return; |
There was a problem hiding this comment.
If a user selects category shortcuts in quick succession, older schedule() callbacks can still execute and call focusHeaderAtIndex for an outdated header because this path never checks whether that header is still the pending target. In that case the stale callback clears pendingHeaderFocusIndexRef, focuses the wrong section, and may prevent the latest selection from being announced correctly by a screen reader; add a guard that pendingHeaderFocusIndexRef.current === headerIndex (or cancel previous timers) before focusing.
Useful? React with 👍 / 👎.
1 participant
Explanation of Change
Users can now see which emoji category shortcut is currently selected, and assistive technologies also get that selected state. When someone chooses a category, the picker not only scrolls to the right section but also moves focus to the corresponding category header, making orientation clearer for keyboard and screen-reader users.
Fixed Issues
$ #77490
PROPOSAL: #77490 (comment)
Tests
Offline tests
Same as Tests.
QA Steps
Same as Tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Screen.Recording.2026-02-22.at.11.11.39.AM.mov
Android: mWeb Chrome
Screen.Recording.2026-02-22.at.11.55.59.AM.mov
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2026-02-22.at.10.33.43.AM.mov