Allow first and last name in send money additional details to be completely deleted and refactor to use Form

[joelbettner]

@Beamanator @Expensify/pullerbear will you please review?

Details

This PR allows the user to delete the entire default legal first name and last name in the additional details form for sending money. It also updates the AdditionalDetails form to use our Form functionality.

Fixed Issues

$ #12807
$ #12807 (comment)

Tests

• Verify that no errors appear in the JS console
  • There errors in the console that were unrelated, and when navigating to the form, filling it out and submitting it, there are no additional errors.

1. With an account that does not have a wallet set up yet, but does have a first and last name set up in the personal details, navigate to the Send Money flow:

2. Add an amount, select a recipient, and give a description about that the payment is for until you reach the "Additional Details" page:

3. Successfully delete all the letters from the first name input.

4. Leaving the first name blank do the same thing for the last name. Confirm that as you click into the last name input there is an error displayed for the first name input:

5. Add and delete something from all of the inputs and confirm an error message displays:

6. Click the "fix the errors" link above the "Save & continue" button and confirm it takes you to the first error in the form:
   

7. Start filling in every input and confirm the errors go away as you do, AND that only the last 4 of the SSN are requested:
   

8. Click the "x" in the upper right corner to close the form.

9. Navigate back to the form through the send money flow and confirm it is pre-filled out with the exception of the SSN:
   

10. Add 4 fake digits for the SSN and click "Save & continue":

11. Confirm you are prompted to set all 9 digits of the SSN:
    

12. Add 9 fake numbers for the SSN and confirm the error goes away:
    

13. Click "Save & continue" and confirm you see the following prompt:

14. Click "Save & continue" again and confirm you still get that prompt, but that it takes a little time (because it is calling the server again):

15. I repeated the above tests on iOS and Android platforms.

Offline tests

I confirmed that I could not submit the form when offline...

...but that while offline I could still save values to the Onyx object that will populate the form.

QA Steps

Repeat the steps from the "Tests" section above

• Verify that no errors appear in the JS console
  • There errors in the console that were unrelated, and when navigating to the form, filling it out and submitting it, there are no additional errors.

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover 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 tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed 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 followed proper code patterns (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 adding the Waiting for Copy label for a copy review 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 followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• 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
• I verified that if a function's arguments changed that all usages have also been updated correctly
• 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 */
  • 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.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

PR Reviewer Checklist

The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed

• I have verified the author checklist is complete (all boxes are checked off).
• 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 expected offline behavior are in the Offline steps 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 tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I checked that screenshots or videos are included for tests on all platforms
• I included screenshots or videos 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
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• 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 adding the Waiting for Copy label for a copy review 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 other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• 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
• I verified that if a function's arguments changed that all usages have also been updated correctly
• 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 */
  • 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.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

See all screenshots above

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

High Traffic Account on Prod

[melvin-bot]

@NikkiWines Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

[joelbettner]

@Beamanator fixed it and tested to make sure empty values in the Address Street box show an error:

[Beamanator]

Looking great!

[Beamanator]

Reviewer Checklist

• I have verified the author checklist is complete (all boxes are checked off).
• 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 included screenshots or videos for tests on all platforms
• I verified tests pass on all platforms & I tested again on:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop
• If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
• 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 is localized by adding it to src/languages/* files and using the translation method
  • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
  • I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review 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 other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
• 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 */
  • 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 any new file was added I verified that:
  • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
• 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.
• If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
• I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

Screen.Recording.2023-01-06.at.9.55.26.AM.mov

Mobile Web - Chrome

Screen.Recording.2023-01-06.at.10.06.04.AM.mov

Mobile Web - Safari

Screen.Recording.2023-01-06.at.10.20.55.AM.mov

Desktop

Screen.Recording.2023-01-06.at.10.08.46.AM.mov

iOS

Screen.Recording.2023-01-06.at.10.15.29.AM.mov

Android

Screen.Recording.2023-01-06.at.10.03.12.AM.mov

[Beamanator]

Checklist posted ^

Something weird: In Desktop I see this console error any time I submit the form:

Also, when the state picker is the "first" field with an error and I click "Fix errors" link, the picker doesn't open automatically... I thinkkkkk that's fine? Just making sure we're aware :D

[joelbettner]

@NikkiWines @roryabraham let me know if you're able to take a look at this. It would be great to get this one wrapped up.

[NikkiWines]

Looks good, once question about the error message for the 9-digit SSN entry. Should we see the error on the SSN field as soon as 9-digits are required? Or only after they click on-and-away from the field? Currently it's the latter. If that's the expected behavior lemme know and I'll approve

Screen.Recording.2023-01-09.at.14.44.48.mov

[joelbettner]

@NikkiWines I believe what you are seeing is the desired behavior. The error message first comes up over the submit button letting the user know to add a 9 digit SSN, and the SSN field changes to say "Full 9 digits of SSN". Then, after the user clicks in and out of the SSN box without adding a 9 digit SSN we show the error on the SSN box.

[roryabraham]

I haven't tested this so I'll leave it to @NikkiWines to formally approve, but I looked through this code and didn't notice anything that looked wrong. Overall it looks like a great simplification.

[roryabraham]

Actually I guess I have to approve so it doesn't appear as Requested Changes

[NikkiWines]

👍

[OSBotify]

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

[github-actions]

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	660.690 ms → 680.260 ms (+19.570 ms, +3.0%)
App start nativeLaunch	9.484 ms → 18.839 ms (+9.355 ms, +98.6%) 🟡
App start runJsBundle	178.469 ms → 185.688 ms (+7.219 ms, +4.0%)
Open Search Page TTI	602.632 ms → 608.699 ms (+6.068 ms, +1.0%)
App start regularAppStart	0.014 ms → 0.014 ms (-0.000 ms, -1.2%)

Show details

Name	Duration
App start TTI	Baseline Mean: 660.690 ms Stdev: 26.396 ms (4.0%) Runs: 621.9284009998664 623.0781749999151 630.574777000118 631.5921289999969 636.3318130001426 636.9628530000336 637.9003909998573 638.7927069999278 645.0369739998132 645.152910000179 645.9458209997974 646.0760889998637 646.0980159998871 646.6699769999832 647.6763280001469 649.2944789999165 650.156665999908 655.1843779999763 658.6681559998542 663.3523309999146 664.6756509998813 666.9708839999512 672.863212000113 679.0793590000831 691.2027759999037 692.7285170000978 694.5732149998657 696.8679510001093 697.8706950000487 702.0664570000954 706.4568090001121 720.2488770000637 Current Mean: 680.260 ms Stdev: 27.079 ms (4.0%) Runs: 629.6007300000638 629.7151990002021 635.6988369999453 639.9147250000387 643.7184750000015 649.1123390002176 657.771541999653 662.0319530004635 664.4647540003061 668.1370489997789 673.5348619995639 675.4880449995399 676.7424579998478 677.8437320003286 679.6728469999507 679.8046500002965 683.915853000246 684.7931589996442 685.5947660002857 686.0835159998387 686.2552690003067 688.2598029999062 692.6802719999105 697.2558739995584 697.885479000397 707.7072369996458 708.436781000346 714.0335060004145 714.3365650000051 720.7677130000666 727.2673000004143 729.794614999555
App start nativeLaunch	Baseline Mean: 9.484 ms Stdev: 1.563 ms (16.5%) Runs: 7 7 7 8 8 8 8 8 8 9 9 9 9 9 9 9 9 10 10 10 10 10 10 11 11 11 11 12 12 12 13 Current Mean: 18.839 ms Stdev: 1.526 ms (8.1%) Runs: 17 17 17 17 17 18 18 18 18 18 18 18 18 18 18 19 19 19 19 19 19 19 19 19 20 20 20 21 21 23 23
App start runJsBundle	Baseline Mean: 178.469 ms Stdev: 20.471 ms (11.5%) Runs: 151 153 156 156 160 160 161 163 164 164 164 166 167 170 171 172 172 174 175 177 180 182 185 186 198 201 208 211 212 215 216 221 Current Mean: 185.688 ms Stdev: 15.903 ms (8.6%) Runs: 161 162 167 167 167 170 173 173 175 176 177 177 179 179 179 181 182 182 186 187 192 194 195 197 199 201 203 205 207 209 219 221
Open Search Page TTI	Baseline Mean: 602.632 ms Stdev: 20.173 ms (3.3%) Runs: 556.7526850001886 560.4169519999996 572.0438230000436 574.0484629999846 575.7570799998939 588.6184080000967 589.2915449999273 590.3854580000043 590.8795580002479 595.1749269999564 595.6154789999127 597.6864020000212 598.2258310001343 598.5938319996931 598.5949299996719 599.0898029999807 603.2762870001607 604.0810149996541 604.6890470003709 606.2991129998118 606.3205970004201 611.3575450000353 612.6108399997465 615.134440000169 616.2711590002291 619.9520670003258 622.1845700000413 625.279826999642 626.641643000301 626.8262129998766 629.4470629999414 629.5804449999705 645.7195640001446 Current Mean: 608.699 ms Stdev: 16.967 ms (2.8%) Runs: 576.0045980000868 576.7990319998935 578.3145350003615 585.0198160000145 589.222942000255 590.741333999671 595.3519699992612 596.559407999739 597.353964000009 600.1581219993532 600.4817310003564 600.5059409998357 601.8613689998165 601.9018559996039 605.463134999387 607.1635739998892 611.918619999662 613.7930509997532 615.9037680001929 615.9340010005981 616.749715000391 616.8310549994931 618.0342210000381 618.3374029994011 618.6054279999807 620.4050699993968 624.7113039996475 625.9386809999123 626.1367190005258 630.9554040003568 632.391072999686 633.7086999993771 643.8204749999568
App start regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.002 ms (14.1%) Runs: 0.012329000048339367 0.012450999580323696 0.012492000125348568 0.012817999813705683 0.012858000118285418 0.012898999731987715 0.013142000418156385 0.013183000031858683 0.013306000269949436 0.013346000108867884 0.013346000108867884 0.01334700034931302 0.013427999801933765 0.013590000104159117 0.013712999876588583 0.01383400009945035 0.014078999869525433 0.014160000253468752 0.014200999867171049 0.014485999941825867 0.014852000400424004 0.015055000316351652 0.015502000227570534 0.016153999604284763 0.01643799990415573 0.01688600005581975 0.018188000191003084 0.018350999802350998 0.02127999998629093 Current Mean: 0.014 ms Stdev: 0.001 ms (5.6%) Runs: 0.013224000111222267 0.0133050000295043 0.013306000269949436 0.013345999643206596 0.013427999801933765 0.01358999963849783 0.013671999797224998 0.01375299971550703 0.013753000646829605 0.013753999955952168 0.013834000565111637 0.013916000723838806 0.013997000642120838 0.014119000174105167 0.014240999706089497 0.014241000171750784 0.014241999946534634 0.014405000023543835 0.014444999396800995 0.01444500032812357 0.014526000246405602 0.014648000709712505 0.014688999392092228 0.0147299999371171 0.014730000868439674 0.014973999932408333 0.015421999618411064 0.015666000545024872 0.015949999913573265 0.016479999758303165

[OSBotify]

🚀 Deployed to staging by @NikkiWines in version: 1.2.53-0 🚀

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

[mvtglobally]

@NikkiWines @joelbettner @Beamanator
The flow in the steps is by Send money but PR details is about the additional info page. Can we test directly going to this page in web at least https://staging.new.expensify.com/enable-payments?

Also, We are not able to navigate to these screens on IOS as https://staging.new.expensify.com/enable-payments leads to mWeb only. It works on Android

[joelbettner]

@mvtglobally I'm a bit confused...

The flow in the steps is by Send money but PR details is about the additional info page. Can we test directly going to this page in web at least https://staging.new.expensify.com/enable-payments?

Are you able to go through the "Send Money" flow? Really all you need to do is go through the Send Money flow until you hit the page asking for the additional details (i.e. first name, last name, address, etc.).

[mvtglobally]

There is a really old issue on Native apps, we get the connectivity issue. so we were trying to use the URL directly to bypass it. It works on Android, but not on IOS
https://user-images.githubusercontent.com/43995119/212179850-2c10d1ae-2827-429c-825b-ebf2346b8637.mp4

[mvtglobally]

@joelbettner and additionally on Web when trying with card, ew were getting an error like this and blocked from proceeding to Additional details page. Team is doublechecking with Bank account too

Recording.1269.mp4

bank flow

Recording.1271.mp4

[joelbettner]

From the videos above it looks like the bank account option for Send Money flow is getting you to the correct screen.

[joelbettner]

Ok...just a heads up...I was able to test on desktop and it passed:

[joelbettner]

Can we test directly going to this page in web at least https://staging.new.expensify.com/enable-payments?

@mvtglobally to answer that question^, yes you can.

[mvtglobally]

@joelbettner Yes, we did it and here is the issue found
#14251 - repro on WEB, mWeb & Android

[joelbettner]

@mvtglobally That linked issue shows the desired behavior for the form (see the screenshot in step 10). I went ahead and closed it.

[joelbettner]

Just finished testing it on iOS. I was able to send the https://staging.new.expensify.com/enable-payments link as a comment in a chat, and then click on that link which took me to the correct page to test.

Here are some screenshots from the testing:

[OSBotify]

🚀 Deployed to production by @Julesssss in version: 1.2.53-0 🚀

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