Fix: Restore focus to trigger element on back navigation#79493
Fix: Restore focus to trigger element on back navigation#79493mavrickdeveloper wants to merge 6 commits intoExpensify:mainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
mavrickdeveloper
force-pushed
the
fix/76921-focus-restoration-back-navigation
branch
from
5fcfa13 to
5423a53
Compare
mavrickdeveloper
changed the title
This fixes Issue Expensify#76921 where focus was lost after navigating back, causing screen readers to announce incorrect elements. The root cause was PR Expensify#49240 which set `setReturnFocus: false` in FocusTrapForScreen to fix Issue Expensify#46109 (blue frame on new tab). This was an over-correction that broke focus restoration during in-app navigation. The fix implements context-aware focus restoration: - Initial page load: Don't restore focus (guards against Expensify#46109) - Navigation back: Restore focus to the previously focused element Changes: - Capture `previouslyFocusedElement` in `onActivate` callback - Track `wasActivatedViaNavigation` based on activeElement state - Implement intelligent `setReturnFocus` that distinguishes scenarios - Add `isElementFocusable` helper for robust element validation - Add comprehensive unit tests (18 test cases)
…y#76921) This implements a complete focus capture and restoration system for back navigation, fixing Issue Expensify#76921. Architecture: - NavigationFocusManager: Singleton that captures focused elements on pointerdown/keydown (capture phase) and stores them per route key - FocusTrapForScreen: Integrates with NavigationFocusManager to capture focus when screen loses focus, restore when it regains focus - initialFocus callback handles navigation-based restoration - setReturnFocus handles click-outside deactivation Key changes: - src/libs/NavigationFocusManager.ts: New focus capture/restore manager - src/App.tsx: Initialize NavigationFocusManager on app startup - FocusTrapForScreen: Integration with capture on blur, restore on focus - MenuItem: interactive={false} removes role="menuitem" for proper WCAG semantics and correct NavigationFocusManager element capture - ApprovalWorkflowSection: Uses interactive={false} pattern - ButtonWithDropdownMenu: Focus anchor after modal close for capture - MoneyRequestConfirmationList/NewTaskPage: Only blur inputs to preserve focus restoration for non-input elements - useSyncFocusImplementation: Don't steal focus from already-focused elements to preserve NavigationFocusManager restoration Tests: - NavigationFocusManagerTest.tsx: 20+ test cases covering all scenarios - FocusTrapForScreenTest.tsx: Updated with P0 test cases
The autoFocus prop has no cleanup mechanism, so it continues firing during back navigation as the component unmounts. This steals focus from NavigationFocusManager's restoration, causing focus to fall to document.body instead of the original menu item. useAutoFocusInput uses useFocusEffect which cancels pending focus operations when the screen loses focus, allowing proper restoration. This aligns with the established pattern used by 82 other pages.
…76921) - Add identifier-based element matching for screens that unmount/remount - Integrate beforeRemove listener to capture focus before screen removal - Fix MenuItem interactive={false} to allow event bubbling to parent - Fix ThreeDotsMenu nested keyboard activation (Enter/Space handling) - Prevent useAutoFocusInput from stealing focus on back navigation - Add comprehensive tests for NavigationFocusManager
mavrickdeveloper
force-pushed
the
fix/76921-focus-restoration-back-navigation
branch
from
78a5558 to
1063217
Compare
…ndex
After rebasing onto main, the new shouldBeAccessible and tabIndex props
(with defaults true/0) broke our fix for interactive={false} MenuItems.
Problem: MenuItems with interactive={false} were still accessible and
focusable because the new props ignored the interactive flag.
Solution: Make the JSX conditional on interactive:
- accessible={interactive && shouldBeAccessible}
- tabIndex={interactive ? tabIndex : -1}
This preserves main's flexibility (callers can override accessibility)
while ensuring interactive={false} always results in non-focusable,
non-accessible MenuItems - critical for ApprovalWorkflowSection and
90+ other places using display-only MenuItems.
mavrickdeveloper
force-pushed
the
fix/76921-focus-restoration-back-navigation
branch
from
acf0059 to
a387464
Compare
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
force-pushed
the
fix/76921-focus-restoration-back-navigation
branch
from
ccf1acc to
cfd1cb8
Compare
1 participant
Explanation of Change
This PR fixes the Web-only issue where focus was lost in
48 scenarios/pagesafter navigating back in the app, causing screen readers to announce incorrect elements.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 focus stealing
Solution: Implement a custom
NavigationFocusManagerthat:Notes to code reviewers: #79493 (comment)
Fixed Issues
$ #76921
PROPOSAL: #76921 (comment)
Prerequisite:
The user is logged in
Using Windows + Chrome, open the page https://staging.new.expensify.com/settings/about
Navigate using TAB to the 'App download links' button, press enter to activate
Navigate to the 'Back' button using TAB, press enter to activate
Observe the focus behavior
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