-
Notifications
You must be signed in to change notification settings - Fork 3.5k
DEV-79102: fixed navigation problem in reporting #79202
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
|
@MonilBhavsar 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ceb2d9fb50
ℹ️ 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".
| hideAutocompleteTimeoutRef.current = setTimeout(() => { | ||
| // Only hide if the input is truly blurred (not just temporarily unfocused during navigation) | ||
| if (!textInputRef.current?.isFocused() && !listRef.current?.getFocusedOption()) { | ||
| setIsAutocompleteListVisible(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hide autocomplete even when a stale focused option exists
Because getFocusedOption() just reflects the last focusedIndex (which isn’t cleared when the list loses focus), this new guard prevents hiding after the input actually blurs (e.g., arrow-key a suggestion, then click outside). In that flow textInputRef is not focused but getFocusedOption() still returns the last item, so setIsAutocompleteListVisible(false) never runs and the list stays open. Consider clearing the focused index on blur or basing the decision on actual list focus rather than a cached focused option.
Useful? React with 👍 / 👎.
|
@sohrabq Is this related to an issue you were hired for? If not, we won't be able to consider this PR at this time. |
|
Doesn't seem linked to an issue. So I'll close this. |
Explanation of Change
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Uploading Screen Recording 2026-01-09 at 17.42.11.mov…