-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TS migration] Migrate 'EnablePayments' pages to TypeScript #35438
[TS migration] Migrate 'EnablePayments' pages to TypeScript #35438
Conversation
…tion/EnablePayments
@ntdiary code has been migrated, and I am working on the testing the individual pages, I will update the Tests section and the attachments once I can run the app on my side, getting some weird issue |
@ntdiary I have added the additional details to enable-payments page and navigated to OnfidoStep, so not sure how to bypass the onfido verification, do you have idea how can we test all the pages under enable-payments page after adding the additional details page? enable-payment-flow.mov |
@jayeshmangwani, I've filled the c+ checklist, the next step is internal engineer review. 😂 App/src/components/FormScrollView.tsx Line 3 in 3d4504e
|
@ntdiary Fixed the lint issue, Internal engineer was not assigned automatically to PR, thats why I pinged you , I thought if anything was missing or 🙂 |
Yeah, but Melvin automatically assigned the issue to @Gonals, I think they'll review it when they have time. : ) |
@Gonals Whenever you get time please help with review |
On it. Not sure why I didn't get assigned to it |
🚀 Deployed to staging by https://github.com/Gonals in version: 1.4.55-0 🚀
|
🚀 Deployed to staging by https://github.com/Gonals in version: 1.4.55-0 🚀
|
@jayeshmangwani Which bank did you add? Screen_Recording_20240320_103245_Chrome.1.mp4 |
@kbecciv If I remember correctly , I tested with Regions Bank tested with below data
|
@jayeshmangwani The same result - no Ideology questions are presented if using above information Screen_Recording_20240320_154133_Chrome.mp4 |
I think it's pretty likely that this deploy blocker is related to this PR since it's the only one that touched the bank account flow in this deploy #38701 |
@jayeshmangwani, so far, I only know that the questions returned using "Bobbeth" are mock data, so it seems normal not to continue with the onfido flow. As for how to trigger questions and also be able to test onfido with a regular account, I don't know yet. 😞 |
Hi, @stitesExpensify, I think it's not a blocker, because it already exists in the production branch, and added video here. :) |
🚀 Deployed to production by https://github.com/AndrewGable in version: 1.4.55-3 🚀
|
Details
Fixed Issues
$ #25222
PROPOSAL: #25222 (comment)
Tests
Offline tests
Enable Payment Flow is disabled in offline
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android.mov
Android: mWeb Chrome
mWeb-chrome.mov
iOS: Native
iOS.mov
iOS: mWeb Safari
mWeb-safari_compressed.mp4
MacOS: Chrome / Safari
web_compressed.mp4
MacOS: Desktop
desktop_compressed.mp4