feat: wrap contact method validate function inside a callback#21281
Conversation
|
@aimane-chnaif 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] |
Reviewer Checklist
Screenshots/VideosWebweb.movMobile Web - Chromemchrome.movMobile Web - Safarimsafari.movDesktopdesktop.moviOSios.movAndroidandroid.mov |
aimane-chnaif
left a comment
There was a problem hiding this comment.
@aldo-expensify LGTM 🎉
| // We don't need `props.loginList` because when submitting this form | ||
| // the props.loginList gets updated, causing this function to run again. |
There was a problem hiding this comment.
Why does the props.loginList gets updated? is there a way to prevent this?
There was a problem hiding this comment.
I see.. 😬
@allroundexperts can we move validateNumber, getPhoneLogin and addNewContactMethod out the component please, they are unnecessarily recreating on each render and the first two are used by validate
There was a problem hiding this comment.
@allroundexperts let's do that and then I think all good.
aldo-expensify
left a comment
There was a problem hiding this comment.
Works well for me, thanks for you work!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/aldo-expensify in version: 1.3.33-0 🚀
|
|
🚀 Deployed to production by https://github.com/thienlnam in version: 1.3.33-4 🚀
|
feat: wrap contact method validate function inside a callback

Details
This PR wraps the contact method validation function inside a
useCallbackhook such that it does not trigger the validation un-necessarily.Fixed Issues
$ #20610
PROPOSAL: #20610 (comment)
Tests
Offline tests
N/A
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Web
Screen.Recording.2023-06-22.at.4.03.07.AM.mov
Mobile Web - Chrome
Screen.Recording.2023-06-22.at.4.07.14.AM.mov
Mobile Web - Safari
Screen.Recording.2023-06-22.at.4.46.39.PM.mov
Desktop
Screen.Recording.2023-06-22.at.4.05.57.AM.mov
iOS
Screen.Recording.2023-06-22.at.4.45.10.PM.mov
Android
Screen.Recording.2023-06-22.at.4.12.20.AM.mov