[$250] iOS - Keyboard opens in front of the hold banner

[vincdargento]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.71-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Yes, reproducible on both
If this was caught during regression testing, add the test name, ID and link from TestRail: #49971
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Open hybrid app on a new account
2. Create a workspace
3. Submit expenses
4. Go into the transaction thread of one of the expenses
5. Tap on header > Hold

Expected Result:

The keyboard doesn't open while the banner is shown

Actual Result:

The keyboard opens up in front of the hold banner

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021866934797160828341
• Upwork Job ID: 1866934797160828341
• Last Price Increase: 2024-12-11
• Automatic offers:
• truph01 | Contributor | 105371091

Issue Owner

Current Issue Owner: @anmurali

[melvin-bot]

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[truph01]

Proposal

Please re-state the problem that we are trying to solve in this issue.

• The keyboard opens up in front of the hold banner

What is the root cause of that problem?

• When we "Hold expense", we display the popover:

App/src/components/MoneyReportHeader.tsx

Lines 512 to 513 in d7859ea

	{isSmallScreenWidth && shouldShowHoldMenu && (
	<ProcessMoneyRequestHoldMenu

App/src/components/MoneyRequestHeader.tsx

Lines 218 to 219 in d7859ea

	{isSmallScreenWidth && shouldShowHoldMenu && (
	<ProcessMoneyRequestHoldMenu

Meanwhile, the input will be focused automatically if we are focusing on it before, so the composer is focused and the keyboard is opening.

As a result, the keyboard opens in front of the hold banner.

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

• When we open the popover, we should hide the keyboard. And after the popover is closed, we should focus on the composer if it is focused before (Optional). So the Popover component:
  

  App/src/components/ProcessMoneyRequestHoldMenu.tsx

  Line 45 in d7859ea

  <Popover

  
  can be updated to:

        const composerStatusBeforeModalShowRef = useRef(false);
        <Popover
            ...
            onClose={() => {
                if (composerStatusBeforeModalShowRef.current) {
                    ReportActionComposeFocusManager.focus(true);
                    composerStatusBeforeModalShowRef.current = false;
                }
                onClose?.();
            }}
            onModalShow={() => {
                composerStatusBeforeModalShowRef.current = ReportActionComposeFocusManager.isFocused();
                Keyboard.dismiss();
            }}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

Alternative solution 1:

• We can add an useEffect to ReportScreen, which will auto close the keyboard if there is any modal will be opened:

    const [modal] = useOnyx(ONYXKEYS.MODAL);
    const prevModal = usePrevious(modal);
    useEffect(() => {
        if (modal?.willAlertModalBecomeVisible && !prevModal?.willAlertModalBecomeVisible) {
            Keyboard.dismiss();
        }
    }, [modal, prevModal]);

Alternative solution 2:

• We can call Keyboard.dismiss() in:

App/src/components/Modal/BaseModal.tsx

Line 106 in 5180ef4

Modal.willAlertModalBecomeVisible(true, type === CONST.MODAL.MODAL_TYPE.POPOVER || type === CONST.MODAL.MODAL_TYPE.BOTTOM_DOCKED);

• Should add a flag param such as shouldCloseKeyboardOnOpen and just call Keyboard.dismiss() if that param is true

Note: These two alternative solutions will work in all other modals, not only the hold modal. And, the feature "after the popover is closed, we should focus on the composer if it is focused before (Optional)" is handled like what I mentioned in the main solution.

[melvin-bot]

@anmurali Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021866934797160828341

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak (External)

[huult]

Edited by proposal-police: This proposal was edited at 2024-12-12 07:06:53 UTC.

Proposal

Please re-state the problem that we are trying to solve in this issue.

Keyboard opens in front of the hold banner

What is the root cause of that problem?

When we navigate back from the Hold page, the hold banner is triggered to show

App/src/components/MoneyRequestHeader.tsx

Line 129 in 5180ef4

setShouldShowHoldMenu(isOnHold && !dismissedHoldUseExplanation);

At this moment, the composer is also triggered by auto-focus, causing the keyboard to appear.

App/src/pages/home/ReportScreen.tsx

Line 859 in 7ba97ac

onComposerFocus={onComposerFocus}

So, the hold banner and keyboard are showing at the same time, causing this to happen

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

To resolve this issue, we should dismiss the keyboard if the hold banner is showing.

We add Keyboard.dimiss() inside here:

App/src/components/MoneyRequestHeader.tsx

Line 137 in 5180ef4

if (isSmallScreenWidth) {

        if (isSmallScreenWidth) {
            Keyboard.dismiss();
            if (Navigation.getActiveRoute().slice(1) === ROUTES.PROCESS_MONEY_REQUEST_HOLD.route) {
                Navigation.goBack();
            }
        } else {
            Navigation.navigate(ROUTES.PROCESS_MONEY_REQUEST_HOLD.getRoute(Navigation.getReportRHPActiveRoute()));
        }

https://github.com/Expensify/App/blob/5869428aa82aa7558408c5d286d3d6c3ca5d2df3/src/components/MoneyReportHeader.tsx

        if (isSmallScreenWidth) {
            Keyboard.dismiss();
            if (Navigation.getActiveRoute().slice(1) === ROUTES.PROCESS_MONEY_REQUEST_HOLD.route) {
                Navigation.goBack();
            }
        } else {
            Navigation.navigate(ROUTES.PROCESS_MONEY_REQUEST_HOLD.getRoute(Navigation.getReportRHPActiveRoute()));
        }

Note: After clicking ‘Got it’ on the hold banner, do we want the input to autofocus? We will discuss this in the pull request phase.

POC

Screen.Recording.2024-12-12.at.14.02.42.mov

### What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

This is a UI bug; no need for a test

What alternative solutions did you explore? (Optional)

Alternatively, we can expose a prop to trigger dismissing the keyboard from the ReportScreen

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[truph01]

Proposal updated

• Added alternative solutions.

[eVoloshchak]

@truph01's proposal looks good to me!

Alternative solution 2 looks like the most universal approach here. Keyboard should be hidden every time new popover is open by default (unless the popover contains a text input, but in that case, it should trigger automatically)

🎀👀🎀 C+ reviewed!

[melvin-bot]

Triggered auto assignment to @rlinoz, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 @truph01 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[truph01]

@eVoloshchak PR is ready

[rlinoz]

@eVoloshchak friendly bump on reviewing the PR

[truph01]

@eVoloshchak Could you review the PR once you have a chance? It is delayed for 3 weeks

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[huult]

I think we should handle specific cases like my proposal because this helps reduce the impact on other areas. Since this is the only case like this ticket, I think we should fix this specific case.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.84-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: dismiss keyboard once open modal #54276

If no regressions arise, payment will be issued on 2025-01-21. 🎊

For reference, here are some details about the assignees on this issue:

• @eVoloshchak requires payment through NewDot Manual Requests
• @truph01 requires payment automatic offer (Contributor)

[melvin-bot]

@eVoloshchak @anmurali @eVoloshchak The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[rlinoz]

Not awaiting payment, the solution was reverted

[truph01]

I am unable to reproduce this issue anymore:

Screen.Recording.2025-01-23.at.17.31.08.mov

[rlinoz]

Double checking #53572 (comment)

[rlinoz]

Can't repro anymore either

[anmurali]

Closing.

[truph01]

@anmurali Since both C+ and I spend efforts toward the PR, should we still be eligible for compensation here?

[truph01]

@rlinoz @anmurali Could you please help check my comment above once you have a chance? Thanks

[mallenexpensify]

@truph01 and @eVoloshchak are, very likely, due compensation because they hired/assigned.

The PR was created and reviewed so compensation is 100%

Contributor: @truph01 paid $250 via Upwork
Contributor+: @eVoloshchak due $250 via NewDot

Deets from this SO (also noticed there are two payment-related SOs, I'm going to combine those today
If a contributor has been hired for a job and we decide to close the job before it is successfully completed

• 50% payment to contributor and C+ if the contributor was 🎀👀🎀 by a C+ or hired/assigned.
• 100% due if contributor drafted a PR and requested a review.
• 100% due to C+ if they reviewed the PR.
• One caveat here might be if the hired contributor wasn't working on their PR with urgency.