Skip to content

Create expense - Keyboard navigation is broken when user add a currency #96614

Open

Description

@izarutskaya

If you haven’t already, check out our contributing guidelines for onboarding. To join our Slack channel, fill out this form.


Version Number: 9.4.41-0
Reproducible in staging?: Yes
Reproducible in production?: No
If this was caught during regression testing, add the test name, ID and link from BrowserStack:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @iwiznia
Slack conversation (hyperlinked to channel name): #expensify_quality

Action Performed:

  1. Go to https://staging.new.expensify.com/
  2. Open the manual create expense dialog
  3. Type in an amount and press tab
  4. Press enter and type in a currency

Expected Result:

User is back and being in the currency field.

Actual Result:

User is back to the form, but the keyboard navigation got reset, instead of being in the currency field, it's on the first element of the form.

Workaround:

Unknown

Platforms:

Select the officially supported platforms where the issue was reproduced:

  • Android: App
  • Android: mWeb Chrome
  • iOS: App
  • iOS: mWeb Safari
  • iOS: mWeb Chrome
  • Windows: Chrome
  • MacOS: Chrome / Safari

Screenshots/Videos

Recording.3873.mp4

View all open jobs on GitHub

Issue OwnerCurrent Issue Owner: @ikevin127

Activity

  1. added
    DeployBlockerCashThis issue or pull request should block deployment
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jul 21, 2026
  2. melvin-bot commented on Jul 21, 2026

    @melvin-bot

    Triggered auto assignment to @lakchote (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

  3. melvin-bot commented on Jul 21, 2026

    @melvin-bot

    💬 A slack conversation has been started in #expensify-open-source

  4. github-actions commented on Jul 21, 2026

    @github-actions
    Contributor

    👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

    1. Identify the pull request that introduced this issue and revert it.
    2. Find someone who can quickly fix the issue.
    3. Fix the issue yourself.
  5. izarutskaya commented on Jul 21, 2026

    @izarutskaya
    Author
  6. added and removed on Jul 21, 2026
  7. lakchote commented on Jul 21, 2026

    @lakchote
    Contributor

    Coming from #96228 with the new manual expense flow

    @thelullabyy please handle it.

  8. 93 remaining items

  9. ikevin127 commented on Oct 6, 2026

    @ikevin127
    Contributor

    It will be in the next part, I tried to fix it in part 5 but no luck

  10. thelullabyy commented on Oct 7, 2026

    @thelullabyy
    Contributor

    Comeback this issue today

  11. thelullabyy commented on Oct 7, 2026

    @thelullabyy
    Contributor

    @MelvinBot Could you please investigate and drop proposal + testing branch here?

  12. MelvinBot commented on Oct 7, 2026

    @MelvinBot
    Contributor

    ⏱️ Ran out of time before I could finish. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in #melvinbot-ai.

    Run details

  13. joekaufmanexpensify commented on Oct 12, 2026

    @joekaufmanexpensify
    Contributor

    How's this coming?

  14. ikevin127 commented on Oct 12, 2026

    @ikevin127
  15. MelvinBot commented on Oct 12, 2026

    @MelvinBot
    Contributor

    Proposal

    What is the root cause of that problem?

    When the currency picker closes, nothing returns focus to the currency button. Focus drops to <body>, so the next Tab starts again at the top of the page. Three things combine to cause this:

    1. The opener is lost before the picker records it. The picker wraps its content in a ScreenWrapper, and that wrapper has its own screen focus trap (ScreenWrapper/index.tsx:290). React mounts children first, so this inner trap activates before the modal's trap. On activation it blurs any focused element that isn't an input (FocusTrapForScreen/index.web.tsx:50-56). The currency button is a div with a button role, so it gets blurred. When the modal trap then records its opener (FocusTrapForModal/index.web.tsx:38), focus is already on <body>, and no launcherRef is passed to fall back on.
    2. The trap's focus return is off. The picker sets shouldEnableNewFocusManagement (IOURequestStepCurrencyModal.tsx:46). That turns off the trap's focus return (ReanimatedModal/index.tsx:262).
    3. The replacement only restores text inputs. With that flag on, focus restore is left to ComposerFocusManager. It only remembers text inputs (ComposerFocusManager.ts:41-42), so it has nothing to restore for a button.

    That explains why adding only shouldReturnFocus (the earlier proposal) didn't work in batch 5. It fixes the second cause, but the trap still has no opener to return focus to.

    What changes do you think we should make in order to solve the problem?

    Give the picker the currency button as its anchor, the same way PopoverMenu passes anchorRef as launcherRef:

    1. In NumberWithSymbolForm, add an optional currencyButtonRef prop and attach it to the currency Button.
    2. In IOURequestStepCurrencyModal, add an optional launcherRef prop. Pass it to Modal along with shouldReturnFocus={!!launcherRef}.
    3. In AmountField, create a ref and pass it to both components.

    The modal trap then falls back to the anchor when focus is on <body>, and returns focus to it on close. That path is already covered by the launcherRef fallback tests in tests/unit/FocusTrapForModalTest.tsx. The change is opt-in, so the other two callers (IOURequestStepAmount and SearchEditMultipleAmountPage) don't change. Both already refocus the amount input on purpose after the picker closes. Mouse users also see no change, because the button's onMouseDown keeps the amount input focused.

    Testing branch: claude-currencyPickerReturnFocus (1 commit, 3 files, +22/−2)

    What alternative solutions did you explore? (Optional)

    Alternatives, ruled-out causes, and verification status

    Alternative: Pass focusTrapSettings={{active: false}} to the picker's ScreenWrapper, so its trap never blurs the button. The modal trap already keeps Tab inside the picker. I didn't choose it because it changes all three call sites, and anything else that blurs the button would break the fix again. launcherRef exists for exactly this case.

    Ruled out:

    • The button or form remounting when the currency changes. No key depends on the currency, and the update only merges the transaction draft.
    • AmountField's autofocus effect. Its dependencies don't include the picker's visibility (AmountField.tsx:149-162).
    • Enter submitting the form. The footer's Enter shortcut is skipped while an element with a role has focus.

    Verification:

    • Type check passes.
    • ESLint reports nothing new on the changed lines.
    • I couldn't run it in the browser. The web test session came up blank (0 DOM nodes, no network traffic), the same infra failure as the last run.
    • Jest isn't available in this environment, so I didn't run FocusTrapForModalTest either.

    Please check the branch by hand with the issue's steps. After the picker closes, document.activeElement should be the currency button, and the next Tab should move forward from it.


    Next Steps for Contributor+ team:
    To accept: @MelvinBot implement [this](https://github.com/Expensify/App/compare/main...claude-currencyPickerReturnFocus) to create a draft PR.
    To refine: @MelvinBot <your feedback>
    To reject: Explain why you are rejecting Melvin's proposal.


    view run · no recording available

  16. ikevin127 commented on Oct 12, 2026

    @ikevin127
  17. melvin-bot commented on Oct 15, 2026

    @melvin-bot

    @ikevin127 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

  18. ikevin127 commented on Oct 16, 2026

    @ikevin127
    Contributor

    @thelullabyy I asked again, check out Melvin's proposal so we can move towards fixing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

BugSomething is broken. Auto assigns a BugZero manager.DailyKSv2EngineeringExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions