Keep textinput focused if no amount is entered #9064

sobitneupane · 2022-05-18T12:10:11Z

Details

Solves the issue : Pressing the enter/return key on Request money page, when the button is disabled, takes focus away from the input field
Platform: Web and Desktop

Fixed Issues

$ #8362

Tests

Go to Request Money or Send Money or Split Bill Page.
Press enter without entering any amount. Focus should remain on input.
Click on the "Currency symbol". In the search bar, attempt to type something. User should be able to type.

Verify that no errors appear in the JS console

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Go to Request Money or Send Money or Split Bill Page.
Press enter without entering any amount. Focus should remain on input.
Click on the "Currency symbol". In the search bar, attempt to type something. User should be able to type.

Verify that no errors appear in the JS console

Screenshots

Web

screenrecording.mov

Mobile Web

Desktop

Screen.Recording.2022-05-18.at.16.54.11.mov

iOS

Android

rushatgabhane · 2022-05-19T10:25:27Z

@sobitneupane why changing the URL fixed the regression?

rushatgabhane · 2022-05-20T15:41:56Z

@sobitneupane before we can merge this PR, it's important you explain why the changes work.
This helps us avoid the same mistakes later on. Thanks for understanding!!

Beamanator · 2022-05-20T15:42:46Z

I agree with you @rushatgabhane 👍 👍

sobitneupane · 2022-05-24T11:07:19Z

@sobitneupane before we can merge this PR, it's important you explain why the changes work.
This helps us avoid the same mistakes later on. Thanks for understanding!!

Hello @rushatgabhane, you were absolutely right. While digging deeper, I found that this solution still has some issues. Even when we open Search Page with Cmd + K, the textinput remains blurred. So, I think we should dump this solution.I have come up with new solution.

We can use blurOnSubmit in the textinput. This is a simple change and works well.

<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}
+    blurOnSubmit={this.state.amount!==''}
/>

And one question, do we still want to keep the following change ?

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

I will update the PR once getting approval from you..

rushatgabhane · 2022-05-24T11:18:33Z

blurOnSubmit

@sobitneupane sounds good. Thanks for taking a look into this

And one question, do we still want to keep the following change ?

CurrencySelectionPage sets a value in Onyx, and it doesn't do anything differently depending on the link. So our choice doesn't affect the functionality.

But again, we're adding 2 lines of code to use the right link. Let's do it anyway.

rushatgabhane

Tests well!
@sobitneupane a minor code style comment and a question

rushatgabhane · 2022-05-29T20:22:22Z

src/pages/iou/steps/IOUAmountPage.js

+                        if (this.props.hasMultipleParticipants) {
+                            return Navigation.navigate(ROUTES.getIouBillCurrencyRoute(this.props.reportID));
+                        }
+                        return Navigation.navigate(this.props.iouType === CONST.IOU.IOU_TYPE.SEND


NAB (Not a Blocker): I'd somewhat prefer -

if (x) return abc if (y) return def return ghi

rushatgabhane · 2022-05-29T20:24:22Z

src/pages/iou/steps/IOUAmountPage.js

@@ -236,6 +241,7 @@ class IOUAmountPage extends React.Component {
                        value={formattedAmount}
                        placeholder={this.props.numberFormat(0)}
                        keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
+                        blurOnSubmit={this.state.amount !== ''}


@sobitneupane can we keep blurOnSubmit always as true?

We are trying to make blurOnSubmit false when there is no amount to proceed to next step.
If You mean to keep it always false, I am not sure. I avoid it thinking it might cause regression.

Let's not add extra logic if it isn't needed.

The keyboard isn't open on this page, so it should be fine

Tested and pushed the change. Working fine in my side. Please test it in your side as well.

Yes, thanks for testing it

rushatgabhane · 2022-06-01T15:41:17Z

src/pages/iou/steps/IOUAmountPage.js

-                            ? ROUTES.getIouSendCurrencyRoute(this.props.reportID)
-                            : ROUTES.getIouRequestCurrencyRoute(this.props.reportID));
+                        if (this.props.hasMultipleParticipants) { return Navigation.navigate(ROUTES.getIouBillCurrencyRoute(this.props.reportID)); }
+                        if (this.props.iouType === CONST.IOU.IOU_TYPE.SEND) { return Navigation.navigate(ROUTES.getIouSendCurrencyRoute(this.props.reportID)); }


@sobitneupane please bring the return statements to a new line.

rushatgabhane

@Beamanator LGTM 🎉

@sobitneupane Thanks for the quick changes!

#### PR Reviewer Checklist

I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- iOS / native
- Android / native
- iOS / Safari
- Android / Chrome
- MacOS / Chrome
- MacOS / Desktop
I verified there are no console errors (if there’s a console error not related to the PR, report it or open an issue for it to be fixed)
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained “why” the code was doing something instead of only explaining “what” the code was doing.
- I verified any copy / text shown in the product was added in all src/languages/* files
- I verified any copy / text that was added to the app is correct English and approved by marketing by tagging the marketing team on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named “index.js”. All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- Any functional components have the displayName property
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose and it is broken down into smaller components in order to separate concerns and functions
If a new CSS style is added I verified that:
- A similar style doesn’t already exist
- The style can’t be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.

Beamanator · 2022-06-02T17:57:49Z

Tested and it's working well! 👍

@sobitneupane can you please update your test steps to also include the steps followed in this issue (#8926) which is a regression your old PR caused?

It looks like the test steps in this PR were copy / pasted from your old one, even though your videos show extra test steps

OSBotify · 2022-06-02T18:07:56Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2022-06-06T18:29:50Z

🚀 Deployed to staging by @Beamanator in version: 1.1.72-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

kavimuru · 2022-06-07T17:37:14Z

@sobitneupane #8362 is still able to reproduce. This PR is failing.

172446657-9462f9a9-96da-4dc4-bb87-a2a4d0a35c4f.mp4

rushatgabhane · 2022-06-07T18:05:17Z

This is weird.

Issue is reproducible on staging (and on pulling main to @sobitneupane's branch )
But the issue isn't reproducible on @sobitneupane's branch . This is the one me and @Beamanator tested on.

rushatgabhane · 2022-06-07T18:15:45Z

PR #8299 is the cause.
It removed blurOnSubmit={false} while refactoring the code which basically reverted this PR

I've commented on the PR to sort it out.
@Beamanator @sobitneupane no further action required by you.

mountiny · 2022-06-08T10:40:22Z

The fix is merged, I haven't applied the CP Staging label as this felt like not a deploy blocker. But if we would prefer to CP it, the PR is here and it can be done manually. Thanks for pointing to the missed prop 🙌

rushatgabhane · 2022-06-08T10:42:14Z

Thanks for fixing this!! @mountiny yeah no rush to deploy this

OSBotify · 2022-06-08T16:30:48Z

🚀 Deployed to production by @yuwenmemon in version: 1.1.73-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

Keep textinput focused if no amount is entered

1a552e2

sobitneupane marked this pull request as ready for review May 18, 2022 12:15

sobitneupane requested a review from a team as a code owner May 18, 2022 12:15

melvin-bot bot requested review from Beamanator and rushatgabhane and removed request for a team May 18, 2022 12:15

trjExpensify mentioned this pull request May 23, 2022

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

Closed

Make use of blurOnSubmit

3b17c39

rushatgabhane suggested changes May 29, 2022

View reviewed changes

sobitneupane added 2 commits May 30, 2022 09:12

Change code styling IOUAmountPage

3e5c260

Set blurOnSubmit false

74322f1

rushatgabhane reviewed Jun 1, 2022

View reviewed changes

Syntactic Change

8a8c6c4

rushatgabhane approved these changes Jun 1, 2022

View reviewed changes

Beamanator approved these changes Jun 2, 2022

View reviewed changes

Beamanator merged commit e1dc845 into Expensify:main Jun 2, 2022

mdneyazahmad mentioned this pull request Jun 2, 2022

[Fix] Localize currency symbol #8299

Merged

89 tasks

yuwenmemon mentioned this pull request Jun 6, 2022

Deploy Checklist: New Expensify 2022-06-06 #9326

Closed

47 tasks

SumitDiyora mentioned this pull request Jun 8, 2022

[Medium]: Color Contrast: Insufficient color contrast for control. #8395

Closed

SumitDiyora mentioned this pull request Jun 8, 2022

[med] Chrome + Jaws: Success message after uploading image is not announced. #8624

Closed

mountiny mentioned this pull request Jun 8, 2022

Revert a forgotten blurOnsubmit prop #9351

Merged

89 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep textinput focused if no amount is entered #9064

Keep textinput focused if no amount is entered #9064

sobitneupane commented May 18, 2022 •

edited

Loading

rushatgabhane commented May 19, 2022

rushatgabhane commented May 20, 2022

Beamanator commented May 20, 2022

sobitneupane commented May 24, 2022 •

edited

Loading

rushatgabhane commented May 24, 2022 •

edited

Loading

rushatgabhane left a comment

rushatgabhane May 29, 2022

sobitneupane May 30, 2022

rushatgabhane May 29, 2022 •

edited

Loading

sobitneupane May 30, 2022

rushatgabhane May 30, 2022 •

edited

Loading

sobitneupane May 30, 2022

rushatgabhane May 30, 2022 •

edited

Loading

rushatgabhane Jun 1, 2022

sobitneupane Jun 1, 2022

rushatgabhane left a comment •

edited

Loading

Beamanator commented Jun 2, 2022

OSBotify commented Jun 2, 2022

OSBotify commented Jun 6, 2022

kavimuru commented Jun 7, 2022

rushatgabhane commented Jun 7, 2022 •

edited

Loading

rushatgabhane commented Jun 7, 2022 •

edited

Loading

mountiny commented Jun 8, 2022

rushatgabhane commented Jun 8, 2022 •

edited

Loading

OSBotify commented Jun 8, 2022

Keep textinput focused if no amount is entered #9064

Keep textinput focused if no amount is entered #9064

Conversation

sobitneupane commented May 18, 2022 • edited Loading

Details

Fixed Issues

Tests

PR Review Checklist

Contributor (PR Author) Checklist

PR Reviewer Checklist

QA Steps

Screenshots

Web

Mobile Web

Desktop

iOS

Android

rushatgabhane commented May 19, 2022

rushatgabhane commented May 20, 2022

Beamanator commented May 20, 2022

sobitneupane commented May 24, 2022 • edited Loading

rushatgabhane commented May 24, 2022 • edited Loading

rushatgabhane left a comment

Choose a reason for hiding this comment

rushatgabhane May 29, 2022

Choose a reason for hiding this comment

sobitneupane May 30, 2022

Choose a reason for hiding this comment

rushatgabhane May 29, 2022 • edited Loading

Choose a reason for hiding this comment

sobitneupane May 30, 2022

Choose a reason for hiding this comment

rushatgabhane May 30, 2022 • edited Loading

Choose a reason for hiding this comment

sobitneupane May 30, 2022

Choose a reason for hiding this comment

rushatgabhane May 30, 2022 • edited Loading

Choose a reason for hiding this comment

rushatgabhane Jun 1, 2022

Choose a reason for hiding this comment

sobitneupane Jun 1, 2022

Choose a reason for hiding this comment

rushatgabhane left a comment • edited Loading

Choose a reason for hiding this comment

#### PR Reviewer Checklist

Beamanator commented Jun 2, 2022

OSBotify commented Jun 2, 2022

OSBotify commented Jun 6, 2022

kavimuru commented Jun 7, 2022

rushatgabhane commented Jun 7, 2022 • edited Loading

rushatgabhane commented Jun 7, 2022 • edited Loading

mountiny commented Jun 8, 2022

rushatgabhane commented Jun 8, 2022 • edited Loading

OSBotify commented Jun 8, 2022

sobitneupane commented May 18, 2022 •

edited

Loading

sobitneupane commented May 24, 2022 •

edited

Loading

rushatgabhane commented May 24, 2022 •

edited

Loading

rushatgabhane May 29, 2022 •

edited

Loading

rushatgabhane May 30, 2022 •

edited

Loading

rushatgabhane May 30, 2022 •

edited

Loading

rushatgabhane left a comment •

edited

Loading

rushatgabhane commented Jun 7, 2022 •

edited

Loading

rushatgabhane commented Jun 7, 2022 •

edited

Loading

rushatgabhane commented Jun 8, 2022 •

edited

Loading