Migrate to functional component#23272
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #16152 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
Reviewer Checklist
Screenshots/VideosWebweb.movMobile Web - Chromemweb.chrome.movMobile Web - Safarimweb.safari.movDesktopdesktop.moviOSios.movAndroidandroid.mov |
|
@techievivek PR is ready for review |
There was a problem hiding this comment.
let's use prop destructuring to follow guidelines: https://github.com/Expensify/App/blob/main/contributingGuides/STYLE.md#destructuring
techievivek
left a comment
There was a problem hiding this comment.
Looks good, thanks 🎉
|
@MaciejSWM Oops, just realized you will need to sign your commits. You can follow the instructions here https://github.com/Expensify/App/blob/main/contributingGuides/CONTRIBUTING.md#begin-coding-your-solution-in-a-pull-request |
|
@MaciejSWM Commits are still not verified, can you please look into it? It should appear like this.
|
30eb859 to
041e13c
Compare
|
@ArekChr @techievivek I force pushed a signed commit. Ready for reapproval |
|
@MaciejSWM It still doesn't show as verified, any idea why? |
|
@techievivek I'll be OOO until tuesday. You are right, still some problems with signing the commits. I didn't solve it yesterday so I'll postpone it until next week. |
aa9ef80 to
a30fa09
Compare
|
@techievivek I force pushed the same changes, but this time it is verified 👍 cc @MaciejSWM |
|
@techievivek looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
It's passing all the tests, not an emergency merge. |
|
✋ 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/techievivek in version: 1.3.50-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 1.3.50-3 🚀
|
|
🚀 Deployed to staging by https://github.com/techievivek in version: 1.3.51-0 🚀
|
|
🚀 Deployed to production by https://github.com/Julesssss in version: 1.3.51-2 🚀
|

Details
Fixed Issues
$ #16152
Tests
Settings->Profile->Display NameEnter, form is submittedOffline tests
QA Steps
Settings->Profile->Display NameEnter, form is submittedEnterkeyPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.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-07-20.at.10.39.21.mov
Mobile Web - Chrome
Screen.Recording.2023-07-20.at.11.03.22.mov
Mobile Web - Safari
Screen.Recording.2023-07-20.at.10.58.11.mov
Desktop
Screen.Recording.2023-07-20.at.10.53.47.mov
iOS
Screen.Recording.2023-07-21.at.11.19.17.mov
Android
Screen.Recording.2023-07-20.at.11.03.22.mov