[HOLD for payment 2024-08-09] [$250] Expensify Card - Hitting Enter on limit amount page does not proceed to the next page

[lanitochka17]

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: 9.0.6-1
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to workspace settings > Expensify Card (enable in More features)
3. Click Issue new card
4. Proceed to Step 4 - Set a limit
5. Enter an amount, then hit Enter (keyboard)

Expected Result:

Hitting Enter on limit amount page will proceed to the next page

Actual Result:

Hitting Enter on limit amount page does not proceed to the next page

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

Add any screenshot/video evidence

Bug6538919_1720701047945.20240711_202859.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0131bf3ff1bba82181
• Upwork Job ID: 1812597871244092170
• Last Price Increase: 2024-07-21
• Automatic offers:
• rayane-djouah | Reviewer | 103253113
• nyomanjyotisa | Contributor | 103253115

Issue Owner

Current Issue Owner: @adelekennedy

[melvin-bot]

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

[github-actions]

👋 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.

[lanitochka17]

@srikarparsi FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[lanitochka17]

We think that this bug might be related to #wave-collect - Release 2

[nyomanjyotisa]

Proposal

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

Hitting Enter on limit amount page does not proceed to the next page

What is the root cause of that problem?

Default disablePressOnEnter here is true

App/src/components/Form/FormWrapper.tsx

Line 66 in 105dce5

disablePressOnEnter = true,

We don't set disablePressOnEnter on FormProvider here

App/src/pages/workspace/card/issueNew/LimitStep.tsx

Lines 80 to 90 in 105dce5

	<FormProvider
	formID={ONYXKEYS.FORMS.ISSUE_NEW_EXPENSIFY_CARD_FORM}
	// TODO: change the submitButtonText to 'common.confirm' when editing and navigate to ConfirmationStep
	submitButtonText={translate('common.next')}
	onSubmit={submit}
	style={[styles.flex1]}
	submitButtonStyles={[styles.mh5, styles.mt0]}
	submitFlexEnabled={false}
	enabledWhenOffline
	validate={validate}
	>

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

Add disablePressOnEnter={false} on <FormProvider> here

What alternative solutions did you explore? (Optional)

[nkdengineer]

Proposal

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

Hitting Enter on limit amount page does not proceed to the next page

What is the root cause of that problem?

We don't pass disablePressOnEnter={false} into FormProvider as in other pages like RatePage, WorkspaceNewRoomPage, ...

App/src/pages/workspace/card/issueNew/LimitStep.tsx

Line 80 in 105dce5

<FormProvider

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

Pass disablePressOnEnter={false} into FormProvider here

App/src/pages/workspace/card/issueNew/LimitStep.tsx

Line 80 in 105dce5

<FormProvider

What alternative solutions did you explore? (Optional)

[nyomanjyotisa]

Proposal updated

[Julesssss]

Not a blocker, this is a minor problem.

[melvin-bot]

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

[srikarparsi]

Hi @adelekennedy! Do you know if it's expected that pressing enter takes you to the next screen? I'm not sure if this is something we want to fix. I can also check in product.

[bernhardoj]

Proposal

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

Pressing Enter doesn't proceed the user to the next page in the limit amount page.

What is the root cause of that problem?

The limit amount page is wrapped with a FormProvider. In FormProvider, if we press enter on an input, it will trigger onSubmitEditing and submit the form, but we will only handle it if shouldSubmitForm is true.

App/src/components/Form/FormProvider.tsx

Lines 268 to 275 in 58e2575

	...(shouldSubmitForm && {
	onSubmitEditing: (event: NativeSyntheticEvent<TextInputSubmitEditingEventData>) => {
	submit();
	inputProps.onSubmitEditing?.(event);
	},
	returnKeyType: 'go',
	}),

shouldSubmitForm is calculated inside InputWrapper.

App/src/components/Form/InputWrapper.tsx

Lines 29 to 59 in 58e2575

	if (textInputBasedComponents.includes(InputComponent)) {
	const isEffectivelyMultiline = !!multiline || !!autoGrowHeight;
	// If the user can use the hardware keyboard, they have access to an alternative way of inserting a new line
	// (like a Shift+Enter keyboard shortcut). For simplicity, we assume that when there's no touch screen, it's a
	// desktop setup with a keyboard.
	const canUseHardwareKeyboard = !canUseTouchScreen();
	// We want to avoid a situation when the user can't insert a new line. For single-line inputs, it's not a problem and we
	// force-enable form submission. For multi-line inputs, ensure that it was requested to enable form submission for this specific
	// input and that alternative ways exist to add a new line.
	const shouldReallySubmitForm = isEffectivelyMultiline ? !!shouldSubmitForm && canUseHardwareKeyboard : true;
	return {
	// There are inputs that don't have onBlur methods, to simulate the behavior of onBlur in e.g. checkbox, we had to
	// use different methods like onPress. This introduced a problem that inputs that have the onBlur method were
	// calling some methods too early or twice, so we had to add this check to prevent that side effect.
	// For now this side effect happened only in `TextInput` components.
	shouldSetTouchedOnBlurOnly: true,
	blurOnSubmit: (isEffectivelyMultiline && shouldReallySubmitForm) || blurOnSubmit,
	shouldSubmitForm: shouldReallySubmitForm,
	};
	}
	return {
	shouldSetTouchedOnBlurOnly: false,
	// Forward the originally provided value
	blurOnSubmit,
	shouldSubmitForm: !!shouldSubmitForm,
	};
	}

If the InputComponent is included in textInputBasedComponents, then we always return shouldSubmitForm as true for a single line. Otherwise, the shouldSubmitForm value will rely on the props that we pass, and there is no default value for it, which means it will be false.

For the limit amount page, the input that we use is AmountForm, which isn't included in textInputBasedComponents,

App/src/components/Form/InputWrapper.tsx

Line 14 in 58e2575

const textInputBasedComponents: TextInputBasedComponents = [TextInput, RoomNameInput];

and we don't set the shouldSubmitForm prop to the AmountForm,

App/src/pages/workspace/card/issueNew/LimitStep.tsx

Lines 91 to 97 in 58e2575

	<InputWrapper
	InputComponent={AmountForm}
	defaultValue={CurrencyUtils.convertToFrontendAmountAsString(issueNewCard?.data?.limit, CONST.CURRENCY.USD, false)}
	isCurrencyPressable={false}
	inputID={INPUT_IDS.LIMIT}
	ref={inputCallbackRef}
	/>

that's why pressing Enter does nothing compared to other pages, such as new task.

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

We have 2 options. Add AmountForm to textInputBasedComponents (preferrable),

App/src/components/Form/InputWrapper.tsx

Line 14 in 58e2575

const textInputBasedComponents: TextInputBasedComponents = [TextInput, RoomNameInput];

or pass shouldSubmitForm as true for AmountForm input wrapper.

App/src/pages/workspace/card/issueNew/LimitStep.tsx

Lines 91 to 97 in 58e2575

	<InputWrapper
	InputComponent={AmountForm}
	defaultValue={CurrencyUtils.convertToFrontendAmountAsString(issueNewCard?.data?.limit, CONST.CURRENCY.USD, false)}
	isCurrencyPressable={false}
	inputID={INPUT_IDS.LIMIT}
	ref={inputCallbackRef}
	/>

[srikarparsi]

It does looking like pressing enter to go to the next page works everywhere else so let's get this fixed. Making this external to pick a proposal.

[melvin-bot]

@srikarparsi, @adelekennedy, @rayane-djouah Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[rayane-djouah]

friendly bump @srikarparsi ^^

[srikarparsi]

This sounds good to me

[melvin-bot]

📣 @rayane-djouah 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @nyomanjyotisa 🎉 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 📖

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

• allow press on enter on limit amount page #46177

If no regressions arise, payment will be issued on 2024-08-09. 🎊

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

• @nyomanjyotisa requires payment automatic offer (Contributor)
• @rayane-djouah requires payment automatic offer (Reviewer)

[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:

• [@rayane-djouah] The PR that introduced the bug has been identified. Link to the PR:
• [@rayane-djouah] 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:
• [@rayane-djouah] 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:
• [@rayane-djouah] Determine if we should create a regression test for this bug.
• [@rayane-djouah] 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.
• [@adelekennedy] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[adelekennedy]

Payments made! @rayane-djouah just the checklist here!

[rayane-djouah]

• The PR that introduced the bug has been identified. Link to the PR: [No QA] Card flow screens #44376
• 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: https://github.com/Expensify/App/pull/44376/files#r1712720155
• 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: N/A
• Determine if we should create a regression test for this bug. Yes
• 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.

Regression Test Proposal

• General regression test steps for forms:

1. Open the app.
2. Navigate to any page that contains a form.
3. Focus on a non-multiline input field and enter text.
4. Press the Enter key on the keyboard.
5. Verify that pressing the enter key submits the form.

• Regression test steps specific to this bug (The form on limit amount page):

1. Go to workspace settings > Expensify Card (enable in More features).
2. Click Issue new card.
3. Proceed to Step 4 - Set a limit.
4. Enter an amount, then press the Enter key on the keyboard.
5. Verify that hitting Enter on limit amount page will proceed to the next page.

Do we agree 👍 or 👎

[adelekennedy]

makes sense to me! @srikarparsi any additional thoughts on the steps above?

[srikarparsi]

Nope, the steps look good to me as well!