Fix workspace keyboard focus not returning to 3-dot trigger after Back - #97171
Fix workspace keyboard focus not returning to 3-dot trigger after Back#97171Krishna2323 wants to merge 15 commits into
Conversation
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@MelvinBot review |
ReviewOverall this is a solid, well-reasoned change — the LauncherStack registration + 1. Blast radius on shared components (main concern).
The registration looks self-cleaning ( 2. 3. Coverage on the new Minor / nits
Nothing here is a blocker on the approach — the design is sound. The asks are really about confirming the shared-component changes don't ripple into other flows, plus a test on the new hide-restore branch. |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@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] |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b25fd3c0a2
ℹ️ 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".
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@codex review |
|
@MelvinBot review |
ReviewSolid, well-scoped accessibility fix with strong test coverage (4 test files, pure 1.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cff41306b1
ℹ️ 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".
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
@MelvinBot review |
ReviewRe-reviewed at 1. One load-bearing assumption is not covered by the unit tests. The whole fix depends on the Modal firing 2. Shared-component blast radius still needs a manual sanity pass (carried over from the earlier reviews, still the main gate). Two behaviors now change for every consumer, not just the 4 workspace flows in the Tests section:
The registration looks self-cleaning ( Things I checked that look correct
Next Steps: Reply with |
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
| defaultValue={policy?.name} | ||
| spellCheck={false} | ||
| autoFocus | ||
| ref={inputCallbackRef} |
There was a problem hiding this comment.
This doesn't fix the root cause.
I think we should hold this PR for #97306 as immediate auto focused pages like this will be fixed there.
…'t steal the focus-return trigger Signed-off-by: krishna2323 <belivethatkg@gmail.com>
|
Please fix conflict. Holding PR was merged |
|
@mkhutornyi conflict resolved |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efaa3d641c
ℹ️ 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".
| headerText={menuHeaderText} | ||
| shouldUseScrollView={shouldPopoverUseScrollView} | ||
| containerStyles={containerStyles} | ||
| shouldEnableNewFocusManagement |
There was a problem hiding this comment.
Restore focus when dismissing dropdowns without selection
With this prop set, PopoverMenu passes shouldReturnFocus={false} to its focus trap (src/components/PopoverMenu/index.tsx:733-735), and ButtonWithDropdownMenu only calls restoreFocusWithModality when shouldRestoreAnchorOnHideRef was armed by a deferred selection. If a keyboard user opens any of these dropdowns and dismisses it with Escape/backdrop without selecting an item, ComposerFocusManager has no text input saved for the button trigger, so focus is left on <body> instead of returning to the dropdown button. Restore the anchor for ordinary dismissals or keep the focus-trap return path for non-navigation closes.
Useful? React with 👍 / 👎.
| onSubItemSelected?.(selectedSubitem, index, event); | ||
| // Match PopoverMenu: Safari runs shouldCallAfterModalHide immediately (no defer), | ||
| // so do not arm post-hide restore — that would refocus the anchor behind the destination. | ||
| const willDeferSelection = !!selectedSubitem.shouldCallAfterModalHide && !isSafari(); |
There was a problem hiding this comment.
Do not arm restore for kept-open dropdown items
PopoverMenu does not defer onSelected for items that keep the menu open because the shouldCloseModalOnSelect === false branch runs before the shouldCallAfterModalHide branch (src/components/PopoverMenu/index.tsx:428-433). Since this component marks every dropdown item shouldCallAfterModalHide: true, a kept-open action such as duplicate expense/report is treated as deferred here; if that action then closes the menu via the exposed ref and navigates (for example to the restricted-action page), onModalHide refocuses the old More button behind the destination. Include the keep-open/close-false checks in willDeferSelection or clear the restore flag for immediate actions.
Useful? React with 👍 / 👎.
Regression hunt (live web run at
|
|
Let's hold this for #97678 as it fixes most "trigger after back from modal" case Example: Screen.Recording.2026-08-13.at.5.48.56.PM.mov |
|
Please merge main, fix conflict, retest and revert unnecessary changes as some are fixed already in another PR |











Explanation of Change
Fixed Issues
$ #96971
PROPOSAL:
Tests
Offline tests
QA Steps
Same as tests
Verify that no errors appear in the JS console
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
web_chrome_mac.mp4
web_chrome_windows.mp4