fix: [Web] Focus restoration mechanism on back navigation (#76921) #79834
Conversation
Implements NavigationFocusManager to capture and restore focus when navigating back to previous screens. Key changes: - Add NavigationFocusManager singleton for focus capture/restoration - Integrate with FocusTrapForScreen for seamless restoration - Add guard in useSyncFocus to prevent focus stealing - Fix MenuItem interactive prop to support display-only mode - Fix useAutoFocusInput to only focus on initial mount - Add selective blur for INPUT/TEXTAREA elements only Includes 97 focus-related tests covering all scenarios. Fixes Expensify#76921
Note for code-reviewersThis PR fixes Keyboard navigation focus loss during back navigation on Web. Three components work together:
The timing issue When a user clicks a button that triggers navigation, the events unfold in a specific order: at T+0ms the user clicks and activeElement is the button; at T+1ms the click handler calls Navigation.navigate(); sometime later React processes the state change and useNavigationState fires; and by then, the screen transition has begun and activeElement has already moved to body. By the time useNavigationState fires, focus has already moved. The hook tells you navigation happened, not what was focused before it happened. Reactive vs Proactive
My Solution Capture-phase DOM events (pointerdown, keydown with {capture: true}) run at T+0ms, before any click handlers execute, before navigation triggers, before focus moves. This is the only mechanism that fires early enough to capture the correct element. React hooks are designed to respond to state changes. But we need to capture state before the change occurs. This requires stepping outside React's reactive model entirely. Changes
|
mavrickdeveloper
changed the title
mavrickdeveloper
changed the title
…#76921) Replace time-based validation with route-based validation in NavigationFocusManager. Changes: - Remove timestamp from CapturedFocus and ElementIdentifier types - Tag captures with route key (forRoute) for deterministic validation - Add cleanupRemovedRoutes() called from NavigationRoot on state change - Add keyboard interaction tracking for modal focus handling - Integrate focus restoration in PopoverMenu, ConfirmModal, ThreeDotsMenu, ButtonWithDropdownMenu, and ComposerWithSuggestions This eliminates timing-dependent behavior on slow devices and ensures focus data is cleaned up when routes are removed from navigation state.
|
@ikevin127 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Use startsWith for robustness against minor content changes | ||
| const candidateText = (candidate.textContent ?? '').slice(0, 100).trim(); | ||
| if (identifier.textContentPreview && candidateText.startsWith(identifier.textContentPreview.slice(0, 20))) { | ||
| score += 30; |
There was a problem hiding this comment.
❌ CONSISTENCY-2 (docs)
The magic number 20 in slice(0, 20) is used in the element matching logic without clear documentation or being defined as a named constant. This reduces code maintainability.
Suggested fix:
Define a constant at the top of the file:
const TEXT_CONTENT_PREFIX_LENGTH = 20; // Chars to match for fuzzy text comparisonThen use it in the code:
if (identifier.textContentPreview && candidateText.startsWith(identifier.textContentPreview.slice(0, TEXT_CONTENT_PREFIX_LENGTH))) {
score += 30;
}
```\n\n---\n\nPlease 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: 50e2a4d6b7
ℹ️ 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".
| const computeInitialFocus = (() => { | ||
| const platform = getPlatform(); | ||
|
|
||
| // Skip for mouse/touch opens or non-web platforms | ||
| if (!wasOpenedViaKeyboardRef.current || platform !== CONST.PLATFORM.WEB) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
On the first render when isVisible flips to true, wasOpenedViaKeyboardRef.current is still undefined because it’s only set in the useLayoutEffect below. That means computeInitialFocus is computed as false and never re-evaluated (no state update), so a ConfirmModal opened via keyboard won’t auto-focus its first button on web. This is a regression for keyboard users because the focus trap never gets the intended initial target. Consider deferring the keyboard check to the initialFocus function itself or forcing a re-render after capturing the flag.
Useful? React with 👍 / 👎.
1 participant
Explanation of Change
This PR fixes the Web-only issue where keyboard navigation focus was lost across
48 scenarios/pagesafter navigating back in the app, causing screen readers to announce incorrect elements.adherence to WCAG 2.4.3 - Focus Order (Level A) , WCAG 3.2.3 Consistent Navigation (AA) , WCAG 2.1.2 No Keyboard Trap (A) & , WCAG 2.1.1 Keyboard (A)
Keyboard focus order , should not conflict with pre-existing custom focus baehavior
Notes to code reviewers: fix: [Web] Focus restoration mechanism on back navigation (#76921) #79834 (comment)
Root Cause:
PR Disable initialFocus and setReturnFocus in focusTrap for screen #49240 set
setReturnFocus: falseinFocusTrapForScreento fix Issue [$250] mWeb – Chat – Blue frame appears around + button when open conversation link URL in a new tab #46109 (blue frame appearing on new tab). This was an over-correction that broke focus restoration during in-app navigation.Fundamental race-condition issues causing cross-cutting focus stealing ( focus management touches many files by nature)
Solution: Implement a custom
NavigationFocusManagerthat:Fixed Issues
$ #76921
PROPOSAL: #76921 (comment)
Prerequisite:
The user is logged in
Using Windows + Chrome, open expensify dev
Navigate using TAB only in the scenarios below
Navigate to the 'Back' button using TAB, press enter to activate
Observe the focus behavior , the correct behavior is the back button (in RHP for eg) should restore the keyboard focus to the triggering element
Tests
Primary Test (Issue #76921):
Note : Tests should only be executed using keyboard navigation (TAB) , (See attached video below for demonstration)
Additional scope : Other pages to test on:
On Settings - About - Keyboard Shortcuts
On Settings - Save the world - I know a teacher
On Settings - Save the world - I am a teacher
On Settings - Save the world - Intro your school principal
On Settings - Preferences - Language
On Settings - Preferences - Priority mode
On Settings - Security
On Settings - Security - Validate your account
On Settings - Security - Close account
On Settings - Security - Two-factor authentication
On Settings - Profile - Display name
On Settings - Profile - Contact methods
On Settings - Profile - Pronouns
On Settings - Profile - Share Code
On Settings - Profile - Legal Name
On Settings - Profile - DOB
On Settings - Profile - Phone number
On Settings - Profile - Address
On Settings - Profile - Country
On Settings - Profile - Timezone
On Workspaces - Duplicate Workspaces
On Workspaces - Delete Workspace
On Workspaces - Overview - Workspace Name
On Workspaces - Overview - Expensify Policy
On Workspace - Categories - Add Category
On Workspace - Categories - Settings
On Workspace - Workflows - Edit Approval Workflow
On Workspace - Workflows - Expenses From
On Workspace - Workflows - Approver
On Workspace - Rules - Cash Expense Default
On Workspaces - Distance Rates - Rate Details
On Workspaces - Expensify Card - Add bank account
On Workspaces - Expensify Card - Bank info
On Workspaces - Expensify Card - Confirm currency and country
On Workspace - Company Card - Add Cards
On Workspace - Create Workspace - Confirm Workspace
On Workspace - Create Workspace - Invite new members
On Workspace - Create Workspace - Default Currency
On Create Report - Restricted
On Create Report - Add payment card
On Create Report - Change payment currency
On Track Distance
On Track Distance - Choose Recipient
On Send Invoice
On Wallet - Add bank account
On Create Expense flow
On Paid Expense details flow
On Reports flow
On Chat flow
Regression Test (Issue #46109):
Offline tests
N/A - Focus restoration is a client-side UI behavior that doesn't depend on network state.
QA Steps
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
2026-01-13.17-13-53.mp4