[HOLD for payment 2024-07-25] [$250] When pressing Enter in the amount page in submit expense flow the focus is on back button instead of on Submit button

[m-natarajan]

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: v9.0.5-3
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @mountiny
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1720438943674149

Action Performed:

1. Go to Workspace chat
2. Start Submit Expense flow
3. Add amount
4. Press Enter
5. Notice that the confirmation page has the back button focused
6. Click Merchant + write some
7. Press Enter to confirm the Merchant
8. Again on the confirmation page, the back button is focused
9. Click enter
10. You are back on the Amount page

Expected Result:

On the confirmation page, the Submit button should be highlighted/ focused

Actual Result:

On the confirmation page, the back button is highlighted/focused

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Screen.Recording.2024-07-08.at.12.35.54.mp4

Recording.299.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01637c01d053be30ae
• Upwork Job ID: 1810300384848703191
• Last Price Increase: 2024-07-08

Issue Owner

Current Issue Owner: @greg-schroeder

[melvin-bot]

Triggered auto assignment to @greg-schroeder (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.

[melvin-bot]

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

[melvin-bot]

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

[nkdengineer]

Proposal

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

On the confirmation page, the back button is highlighted/focused

What is the root cause of that problem?

There was no previous logic to focus on the Submit button by default when entering a screen

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

Add SCREENS.MONEY_REQUEST.STEP_CONFIRMATION to the list SCREENS_WITH_AUTOFOCUS

App/src/components/FocusTrap/SCREENS_WITH_AUTOFOCUS.ts

Line 5 in b214e77

const SCREENS_WITH_AUTOFOCUS: string[] = [

Because the IOU confirmation screen should auto focus on the confirm button by default. By doing this we make sure focus-trap-react doesn't attempt to give focus to the first element it sees.

Same can be done for any screens with a Submit button that we want to get focus by default.

What alternative solutions did you explore? (Optional)

In the Button component, add a prop shouldFocusOnEnteringScreen.

Add a useFocusEffect in Button, if shouldFocusOnEnteringScreen is true, trigger focus on the button by its inner ref.

When we use the Submit button in submit expense flow, pass shouldFocusOnEnteringScreen={true}. We can do this for other cases of Submit button too.

An improvement on this is to trigger the focus after CONST.ANIMATED_TRANSITION time has passed, just like useAutoFocusInput here, implementation can be similar, maybe we can even put it to a useAutoFocusButton, or we can refactor useAutoFocusInput to work for both inputs and buttons.

[bernhardoj]

Proposal

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

The back button is highlighted when going to the confirmation page using the keyboard.

What is the root cause of that problem?

We have a focus trap and it will focus on the first focusable element, which is the back button.

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

We can make use of the focus trap initial focus. If we want to focus the submit button, here is the step:

1. Assign an ID to the button (we need to pass down the ID props down to the Button component)
   

   App/src/components/MoneyRequestConfirmationList.tsx

   Lines 737 to 768 in e0ccba5

   	const button = shouldShowSettlementButton ? (
   	<SettlementButton
   	pressOnEnter
   	isDisabled={shouldDisableButton}
   	onPress={confirm}
   	enablePaymentsRoute={ROUTES.IOU_SEND_ENABLE_PAYMENTS}
   	addBankAccountRoute={bankAccountRoute}
   	shouldShowPersonalBankAccountOption
   	currency={iouCurrencyCode}
   	policyID={policyID}
   	buttonSize={CONST.DROPDOWN_BUTTON_SIZE.LARGE}
   	kycWallAnchorAlignment={{
   	horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT,
   	vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
   	}}
   	paymentMethodDropdownAnchorAlignment={{
   	horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.RIGHT,
   	vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM,
   	}}
   	enterKeyEventListenerPriority={1}
   	/>
   	) : (
   	<ButtonWithDropdownMenu
   	success
   	pressOnEnter
   	isDisabled={shouldDisableButton}
   	onPress={(event, value) => confirm(value as PaymentMethodType)}
   	options={splitOrRequestOptions}
   	buttonSize={CONST.DROPDOWN_BUTTON_SIZE.LARGE}
   	enterKeyEventListenerPriority={1}
   	/>
   	);

2. Pass initialFocus prop to ScreenWrapper which will be passed down further to FocusTrapForScreens
   

   App/src/components/ScreenWrapper.tsx

   Lines 244 to 245 in e0ccba5

   	return (
   	<FocusTrapForScreens>

   
   <FocusTrapForScreens initialFocus={initialFocus}>

3. Returns the initalFocus prop instead of undefined.
   

   App/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx

   Lines 55 to 60 in e0ccba5

   	initialFocus: () => {
   	if (screensWithAutofocus.includes(activeRouteName)) {
   	return false;
   	}
   	return undefined;
   	},

   
   return initialFocus

4. Pass the ID of the button as the selector to the confirmation page ScreenWrapper.
   

   App/src/pages/iou/request/step/IOURequestStepConfirmation.tsx

   Lines 543 to 548 in e0ccba5

   	return (
   	<ScreenWrapper
   	includeSafeAreaPaddingBottom={false}
   	shouldEnableMaxHeight={DeviceCapabilities.canUseTouchScreen()}
   	testID={IOURequestStepConfirmation.displayName}
   	>

   
   initialFocus="#confirm"

However, this won't work yet because the focus trap can't find the element with the assigned ID because MoneyRequestConfirmationList is wrapped with withOnyx and the render is delayed. We can take this chance to migrate the withOnyx to useOnyx of the MoneyRequestConfirmationList.

[devguest07]

Proposal

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

The confirmation button is not focused on the money request confirmation page.

What is the root cause of that problem?

Screens like the money request confirmation page use Focus-trap, which by default focuses on the main focusable element if we don't set a value for initialFocus.

App/src/components/FocusTrap/FocusTrapForScreen/index.web.tsx

Line 55 in e0ccba5

initialFocus: () => {

We have logic to remove focus from some screens like screensWithAutofocus, but this is for screens with input autofocus. We don't have autofocus for submit buttons.

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

As described in the Focus-Trap documentation, we can set an initialFocus value. In this case, we can set initialFocus to the confirmation button element.

To locate the confirmation button, we can use document.querySelector(). One solution is to set an ID to the submit button:

1. In MoneyRequestConfirmationList, add an ID to the button component inside ButtonWithDropdownMenu:

   buttonId="confirm-button"

   App/src/components/MoneyRequestConfirmationList.tsx

   Lines 759 to 767 in e0ccba5

   	<ButtonWithDropdownMenu
   	success
   	pressOnEnter
   	isDisabled={shouldDisableButton}
   	onPress={(event, value) => confirm(value as PaymentMethodType)}
   	options={splitOrRequestOptions}
   	buttonSize={CONST.DROPDOWN_BUTTON_SIZE.LARGE}
   	enterKeyEventListenerPriority={1}
   	/>

2. Update the initialFocus logic in FocusTrap:

   initialFocus: () => {
     if (screensWithAutofocus.includes(activeRouteName)) {
       return false;
     }
     const confirmButton = document.querySelector('#confirm-button');
     return confirmButton ?? undefined;
   },

This change will make the FocusTrap search the DOM for the new element with the ID. Once found, it will set the focus on the confirmation button.

What alternative solutions did you explore?

[mountiny]

@alitoshmatov can you please review the proposals?

[Krishna2323]

I think this is being handled here.

[s77rt]

We can close this as dupe of #43662

[mountiny]

Thanks

[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.8-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• prevent initial focus trap while has a focused element. #44932

If no regressions arise, payment will be issued on 2024-07-25. 🎊

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

• @alitoshmatov requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@alitoshmatov] The PR that introduced the bug has been identified. Link to the PR:
• [@alitoshmatov] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@alitoshmatov] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@alitoshmatov] Determine if we should create a regression test for this bug.
• [@alitoshmatov] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: