chore: Clean up file-level kycWallRef#67036
Conversation
kycWallRefkycWallRef
kycWallRefkycWallRef
|
@allroundexperts 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] |
|
Lint failures are not related to this PR |
|
Friendly bump @allroundexperts |
|
Hi @dominictb! |
|
Hi @dominictb, |
|
Resolved. |
| source, | ||
| shouldShowPersonalBankAccountOption = false, | ||
| }: KYCWallProps, | ||
| ref: ForwardedRef<KYCWallRef>, |
There was a problem hiding this comment.
I think we are getting rid of forward refs. Can you add this to the prop itself?
|
This PR is dwindling. What's the blocker? |
|
No blocker. Just pending @allroundexperts review. |
|
Sorry, I was not able to prioritise this PR. @dominictb Can you please merge main, and I will retest? |
|
Just updated |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeScreen.Recording.2025-10-07.at.11.39.25.PM.moviOS: HybridAppiOS: mWeb SafariScreen.Recording.2025-10-07.at.11.32.57.PM.movMacOS: Chrome / SafariScreen.Recording.2025-10-07.at.11.12.05.PM.movMacOS: DesktopScreen.Recording.2025-10-07.at.11.27.41.PM.mov |
|
BUG
Screen.Recording.2025-10-08.at.12.14.30.AM.mov |
|
It's already on Screen.Recording.2025-10-13.at.23.49.34-compressed.movIn the above video, since I already had a business bank account, when I pressed Pay with business bank account, it should have opened the connect existing bank account modal instead. But anyway it's not related to this PR. |
|
✋ 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/AndrewGable in version: 9.2.33-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.2.33-4 🚀
|
Explanation of Change
Referencing a
refobject in a separated file that is not attached to React component lifecycle is a deprecated pattern. This PR refactorskycWallRefso that it is now referenced via a globalContext.Fixed Issues
$ #64602
PROPOSAL:
Tests
Precondition:
Offline tests
None
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.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
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Screen.Recording.2025-10-02.at.20.51.19-compressed.mov
MacOS: Desktop