[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

@mountiny

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com 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:

Go to Workspace chat
Start Submit Expense flow
Add amount
Press Enter
Notice that the confirmation page has the back button focused
Click Merchant + write some
Press Enter to confirm the Merchant
Again on the confirmation page, the back button is focused
Click enter
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?

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

@greg-schroeder

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.

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

@alitoshmatov

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

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.

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:

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} 
        
               /> 
        
           );

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}>

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

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.

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:

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} 
        
           />

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?

@alitoshmatov

@alitoshmatov can you please review the proposals?

I think this is being handled here.

We can close this as dupe of #43662

Thanks

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

@alitoshmatov

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)

@alitoshmatov

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:

Navigation Menu

Repository navigation

[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 #44956

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Proposal

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

What is the root cause of that problem?

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

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore?

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	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}
	/>
	);

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

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

	return (
	<FocusTrapForScreens>

[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 #44956

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Jul 8, 2024

melvin-bot commented on Jul 8, 2024

melvin-bot commented on Jul 8, 2024

nkdengineer commented on Jul 8, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

bernhardoj commented on Jul 8, 2024

Proposal

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

What is the root cause of that problem?

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

devguest07 commented on Jul 8, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore?

mountiny commented on Jul 8, 2024

Krishna2323 commented on Jul 8, 2024

s77rt commented on Jul 8, 2024

mountiny commented on Jul 9, 2024

melvin-bot commented on Jul 18, 2024

melvin-bot commented on Jul 18, 2024

melvin-bot commented on Jul 18, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

[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 #44956