[HOLD for payment 2022-06-15] [$250] Pressing the enter/return key on Request money page, when the button is disabled, takes focus away from the input field - reported by @adeel0202

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Click on + and then Request money option
2. Press enter key without entering any value in the input field
3. Now enter some value in the input field
4. Observe that nothing is being entered in the input field as the focus is no longer on the input field

Expected Result:

Pressing the enter key when the button is disabled should keep the focus on the input field

Actual Result:

Pressing the enter key when the button is disabled takes focus away from the input field

Workaround:

unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.1.43-2
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2022-03-20.at.6.06.02.PM.mov

Upwork URL: https://www.upwork.com/jobs/~01c7ab5a8744a037c1
Issue reported by: @adeel0202
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1647782854047619

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @conorpendergrast (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[conorpendergrast]

I reproduced on Production too, thanks!

[melvin-bot]

Triggered auto assignment to @Beamanator (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[melvin-bot]

Triggered auto assignment to @trjExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[Beamanator]

Looks like a good external issue 👍

[melvin-bot]

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

[melvin-bot]

Current assignee @Beamanator is eligible for the Exported assigner, not assigning anyone new.

[trjExpensify]

Job is on Upwork here: https://www.upwork.com/jobs/~01c7ab5a8744a037c1

CC: @adeel0202 if you were interested in this one!

[sobitneupane]

Proposal:
We can achieve the required output by passing prop alwaysFocusIfEmpty in the following component.

App/src/pages/iou/steps/IOUAmountPage.js

Lines 228 to 239 in 564ec88

	<TextInput
	disableKeyboard
	autoGrow
	hideFocusedState
	inputStyle={[styles.iouAmountTextInput, styles.p0, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
	textInputContainerStyles={[styles.borderNone, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
	onChangeText={this.updateAmount}
	ref={el => this.textInput = el}
	value={formattedAmount}
	placeholder={this.props.numberFormat(0)}
	keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
	/>

And using the prop in BaseTextInput to focus the input.

App/src/components/TextInput/BaseTextInput.js

Lines 64 to 69 in 564ec88

	componentDidUpdate() {
	// Activate or deactivate the label when value is changed programmatically from outside
	// Only update when value prop is provided
	if (_.isUndefined(this.props.value) || this.state.value === this.props.value) {
	return;
	}

    componentDidUpdate() {
        // Activate or deactivate the label when value is changed programmatically from outside
        // Only update when value prop is provided
+       if (this.props.alwaysFocusIfEmpty && this.props.value === '') {
+           this.input.focus();
+       }

[rushatgabhane]

@sobitneupane wouldn't it be better to focus after onBlur in IOUAmount page?
And is it really necessary to put this in TextInput? Where else do you anticipate this prop being used?

[sobitneupane]

Yes. We can use onBlur function in IOUAmountPage.
Updated Proposal:

    onBlur() {
        if (this.state.amount === '') {
            this.textInput.focus();
        }

App/src/pages/iou/steps/IOUAmountPage.js

Lines 228 to 239 in 564ec88

	<TextInput
	disableKeyboard
	autoGrow
	hideFocusedState
	inputStyle={[styles.iouAmountTextInput, styles.p0, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
	textInputContainerStyles={[styles.borderNone, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
	onChangeText={this.updateAmount}
	ref={el => this.textInput = el}
	value={formattedAmount}
	placeholder={this.props.numberFormat(0)}
	keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
	/>

<TextInput
    disableKeyboard
    autoGrow
    hideFocusedState
    inputStyle={[styles.iouAmountTextInput, styles.p0, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
    textInputContainerStyles={[styles.borderNone, styles.noLeftBorderRadius, styles.noRightBorderRadius]}
    onChangeText={this.updateAmount}
    ref={el => this.textInput = el}
    value={formattedAmount}
    placeholder={this.props.numberFormat(0)}
    keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
+   onBlur = {this.onBlur}
/>

[rushatgabhane]

And is it really necessary to put this in TextInput
Where else do you anticipate this prop being used

@sobitneupane I didn't mean to say that we should use onBlur, I just wanted your thoughts on where all this prop could be used.

If we can find more than one input where we want this, then we should add a prop to TextInput.

What do you think?

[sobitneupane]

I don't find any other specific use case of the prop. I prefer not to tamper TextInput for now. If we find other places where we want similar behavior than we can pass prop. Rather than onBlur I think we should use EventHandler in IOUAmountPage. If Enter is pressed and there is no amount, textInput remains focused.

setupEventHandlers() {
    if (!document) {
        return;
    }

    this.keyUpHandler = (keyBoardEvent) => {
        // Select the currently highlighted emoji if enter is pressed
        if (keyBoardEvent.key === 'Enter') {
            if (this.state.amount === '') {
                console.log('debug', this.state.amount);
                this.textInput.focus();
            }
            return;
        }
    };

    document.addEventListener('keyup', this.keyUpHandler, true);

}

[rushatgabhane]

@sobitneupane I don't like the idea of using event handlers because it isn't really aplicable for native (eg: iPad with a keyboard attached to it).

Why is it better than onBlur?

[sobitneupane]

@sobitneupane I don't like the idea of using event handlers because it isn't really aplicable for native (eg: iPad with a keyboard attached to it).

Why is it better than onBlur?

Oh! In that case, we can go with onBlur. I suggested the event handler because 'Enter' event makes the TextInput blur. So, tried to catch the main cause. In this case, onBlur works fine as well.

[Beamanator]

@sobitneupane can you please check the bug that your PR caused and come up with a new proposal?

[sobitneupane]

@sobitneupane can you please check the bug that your PR caused and come up with a new proposal?

Strangely, the bug was occurring only for Send Money. The change made by the PR effects other two IOU options Request Money and Split Bill as well. But the other two IOU options were working fine. So, taking a deeper look I found that on pressing the currency, we were redirected to wrong url in case of Send Money. So, following change solves the issue:-

App/src/pages/iou/steps/IOUAmountPage.js

Lines 220 to 223 in 564ec88

	<TouchableOpacity onPress={() => Navigation.navigate(this.props.hasMultipleParticipants
	? ROUTES.getIouBillCurrencyRoute(this.props.reportID)
	: ROUTES.getIouRequestCurrencyRoute(this.props.reportID))}
	>

<TouchableOpacity onPress={() => Navigation.navigate(this.props.hasMultipleParticipants
   ? ROUTES.getIouBillCurrencyRoute(this.props.reportID)
   : this.props.iouType === CONST.IOU.IOU_TYPE.SEND
       ? ROUTES.getIouSendCurrencyRoute(this.props.reportID)
       : ROUTES.getIouRequestCurrencyRoute(this.props.reportID))}
>

After above change:

Screen.Recording.2022-05-12.at.18.52.04.mov

[rushatgabhane]

@sobitneupane interesting...
I have no idea why that fixes the issue. Maybe you can help me with that :)

[rushatgabhane]

@Beamanator I don't know why it works, but the fix suggested by @sobitneupane is simple #8362 (comment)

There should be no regressions because CurrencySelectionPage just sets a value in Onyx.

[Beamanator]

Good find @sobitneupane and thanks for reviewing @rushatgabhane - lets get that PR up and make sure we understand why it fixes things before we merge again 😅

[trjExpensify]

I don't think this is ready to settle up, right? We're waiting on this PR to merge, is that correct @Beamanator @rushatgabhane? #9064 (comment)

[rushatgabhane]

@trjExpensify yep, that's right!

[trjExpensify]

Cool, updated the title and removed the label.

[Beamanator]

Next PR still under review

[trjExpensify]

PR is waiting to be deployed to staging, Melv. 👍

[melvin-bot]

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

• Keep textinput focused if no amount is entered #9064

If no regressions arise, payment will be issued on 2022-06-15. 🎊

[trjExpensify]

Settled up with @sobitneupane, @rushatgabhane & @adeel0202.