[CP Staging] Revert "fix: [Web] Focus restoration mechanism on back navigation (#76921)"#85887
Conversation
|
🚧 @marcaaron has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
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.
|
|
@cristipaval 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] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6baa39b1c4
ℹ️ 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".
| } | ||
| inputRef.current?.focus(); | ||
| }); | ||
| isWindowReadyToFocus().then(() => inputRef.current?.focus()); |
There was a problem hiding this comment.
Avoid stealing focus in deferred auto-focus
The hook now always executes inputRef.current?.focus() after isWindowReadyToFocus() resolves, even if focus has already moved to another interactive element in the meantime. On web, this causes back-navigation/restored-focus flows (and any user-initiated focus change during the delay) to be overridden by the input, producing unexpected focus jumps across the many screens that use useAutoFocusInput.
Useful? React with 👍 / 👎.
| return; | ||
| } | ||
|
|
||
| ref.current?.focus({preventScroll: true}); |
There was a problem hiding this comment.
Prevent useSyncFocus from overriding existing DOM focus
useSyncFocusImplementation now calls ref.current?.focus(...) unconditionally whenever isFocused is true, without checking whether another element is already focused. In keyboard-navigation and modal-close scenarios, this can immediately steal focus from the element that was intentionally restored, breaking expected tab order and causing visible focus flicker.
Useful? React with 👍 / 👎.
|
@MonilBhavsar looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
Straight revert |
|
🚧 @MonilBhavsar 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. |
|
🚀 Cherry-picked to staging by https://github.com/cristipaval in version: 9.3.41-2 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.41-4 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/cristipaval in version: 9.3.42-0 🚀
Bundle Size Analysis (Sentry): |
Reverts #79834
$ #85866
$ #85874
$ #85878
$ #85886
$ #85896
$ #85895