Repository navigation
fix: keyboard focus lost after Back on Enter-activated rows - #97306
Conversation
|
@mkhutornyi Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
…d-Navigation-Many-Pages-The-focus-is-lost-and-does-not-return-to-triggering-element
…rgets, and background nav
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5c2ad2215
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
@TaduJR I am not sure about your RCA. Why does this issue happen only on Personal details page? Screen.Recording.2026-07-29.at.3.08.24.PM.movcc: @MelvinBot |
|
The distinguishing factor isn't whether the destination has an autofocused input — both pages do — it's when that autofocus fires relative to the trigger capture.
So "also has an auto focused input" is true, but the Pronouns focus is pushed 300ms past the capture window, while the personal-details focus fires right inside it. Latching the trigger at keydown fixes both cases uniformly regardless of when (or whether) the destination steals focus. Note on the RCA wordingOne precision point: the Note also that the standalone |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Code reviewVerdict: Looks solid — I couldn't construct a confirmed correctness bug. The keydown-latch mechanism is unusually well-guarded: text-input vs button semantics, IME/Safari-Enter, auto-repeat, Cmd+Enter, Escape/Tab, Space-on-checkbox, and the synthetic-click-from-Enter path are all explicitly handled, and the new Observations (all low-severity / non-blocking)1. Stale latch could hijack a purely-programmatic forward nav (theoretical). 2. One detached node can be pinned until the next interaction. 3. Minor skip-path inconsistency. Not defects: the keydown/keyup |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-07-29.at.3.40.12.PM.movAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-07-29.at.3.40.51.PM.mov |
|
🤖 Regression sweep — no confirmed regressions found. I ran an adversarial review across the three risk areas of this diff (capture lifecycle changes, the new global keyboard listeners, and platform/test coverage). Everything material checks out. A few low-severity notes are below, none blocking. What I verified is safe
Low-severity notes (not regressions — worth a glance)Details
Confidence: high — all conclusions are from static analysis of the diff and surrounding code, which is sufficient here. The one thing static analysis can't fully close is note #2's runtime edge; a quick manual pass of Tab→Enter→Back on the pages in the test steps (plus one RHP form-submit-save flow) would confirm it in practice. |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #96970 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
…d-Navigation-Many-Pages-The-focus-is-lost-and-does-not-return-to-triggering-element
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8a5d51486
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…d-Navigation-Many-Pages-The-focus-is-lost-and-does-not-return-to-triggering-element
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Code review — commit
|
🤖 Regression sweep — no confirmed regressions found (commit
|
| const launcher = pickLauncher(); | ||
| let inner: HTMLElement | null; | ||
| if (getHadTabNavigation()) { | ||
| // Re-check focusability. Form-submit spinners can add aria-disabled between keydown and here. We skip ancestors so the outgoing RHP pane (transiently aria-hidden) doesn't false-reject rows inside it. |
There was a problem hiding this comment.
This still isn't very clear. Can we rewrite this in a way that someone who doesn't have much context can understand? Minimally, can we explain
- The precondition that leads to the problem
- The problem
- What the code below does to fix it
For example a comment a written something like this (doesn't have to be exactly the same)
When the user {precondition}, then {problem} occurs which breaks our logic because {reason}. Here we do {solution} to fix that
I'm asking for this because I think this code is pretty confusing for most people. A good comment can help others contribute and avoid breaking your code. Also the use of a delay to fix a problem is usually a code smell, so it's important to explain why we're taking this approach.
There was a problem hiding this comment.
Sure thing rewritten it.
…d-Navigation-Many-Pages-The-focus-is-lost-and-does-not-return-to-triggering-element
…d-Navigation-Many-Pages-The-focus-is-lost-and-does-not-return-to-triggering-element
|
🚧 arosiclair has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/arosiclair in version: 9.4.52-0 🚀
|
|
🤖 I reviewed the changes in this PR and no help site changes are required. Why: This PR is an internal accessibility/keyboard-navigation bug fix. It changes how focus is tracked and restored (latching the activation target at Enter/Space keydown so that pressing Back returns focus to the row the user activated). The changes are confined to internal focus-tracking utilities and their tests:
There is no change to any user-facing feature, workflow, settings label, tab, or button — it simply restores the expected keyboard-focus behavior that was already the intended experience. A search of No draft help site PR was created because there is no documentation impact. |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.4.52-11 🚀
Bundle Size Analysis (Sentry): |






Explanation of Change
Keyboard users on pages like Settings → Profile (Legal name, Phone, Address, and similar rows) lost focus to
<body>after pressing Back instead of returning to the row they activated.Root cause: the tracker recorded the "trigger" element when React Navigation fired its state event, but by then the destination screen's text input had already synchronously autofocused. So it captured the wrong element, and on Back that element was detached and focus fell through.
Fix: latch the activation target at Enter/Space keydown — before any destination can steal focus. On Back, focus returns to the row the user actually activated.
Fixed Issues
$ #96970
PROPOSAL:
Tests
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Mac-Chrome.mp4