-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Card Settings - Rename PaymentsPage to WalletPage #24632
Conversation
@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] |
Alright new PR is up if you can prioritize this one today to prevent merge conflicts, that would be great @allroundexperts ! If not let me know so I can have an internal engineer do the review |
Sounds good! I'll review this today. |
Also heads up, I noticed that for the add bank account page, http://localhost:8082/settings/wallet/add-bank-account the plaid modal is loading very slowly on main too |
Reviewer Checklist
Screenshots/VideosWebScreen.Recording.2023-08-17.at.2.41.39.AM.movMobile Web - ChromeScreen.Recording.2023-08-17.at.3.23.20.AM.movMobile Web - SafariScreen.Recording.2023-08-17.at.2.54.09.AM.movDesktopScreen.Recording.2023-08-17.at.2.49.53.AM.moviOSscreen-recording-2023-08-17-at-25655-am_BdHrDJPo.mp4AndroidScreen.Recording.2023-08-17.at.3.27.34.AM.mov |
@allroundexperts any updates on review? |
Shouldn't it be |
@grgia Just waiting on the above. |
@grgia Any update on above? Let's try to merge this today! |
@allroundexperts I apologize, I was working on an urgent issue this morning. Looking now |
The transfer page works fine for me, I think you need a specific beta for wallet transfers? I'll update that copy now |
pushed copy changes @allroundexperts |
Thanks. Just doing a sanity check now. Give me like 20 minutes. |
@grgia Conflicts again 🤯 |
what in the world keeps getting updated in these files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go 🟢
@AndrewGable 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] |
✋ 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: 1.3.56-0 🚀
|
🚀 Deployed to production by https://github.com/roryabraham in version: 1.3.56-24 🚀
|
@grgia @allroundexperts |
Details
Part of Card Settings Project (Wave 8) we are renaming the
Payments
pages toWallet
Fixed Issues
$ #22870
Tests
Verify that no errors appear in the JS console
Open the settings page
/settings/
/settings/wallet
/settings/wallet
is "Wallet"Open the wallet page
/settings/wallet
Add Payment Method
buttonBank Account
, ensure that route issettings/wallet/add-bank-account
Debit Card
, ensure that route issettings/wallet/add-debit-card
Paypal.me
, ensure that route issettings/wallet/add-paypal-me
Alternatively, open each of the following routes and ensure that page properly loads:
Offline tests
QA Steps
Open each of the following routes:
Ensure each of these routes loads:
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)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)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
Web
Screen.Recording.2023-07-19.at.1.42.41.PM.mov
Mobile Web - Chrome
Chrome emulator won't connect to wifi
Mobile Web - Safari
Screen.Recording.2023-07-19.at.1.40.42.PM.mov
Desktop
Screen.Recording.2023-07-19.at.1.36.22.PM.mov
iOS
Screen.Recording.2023-07-19.at.1.39.59.PM.mov
Android
Screen.Recording.2023-07-19.at.1.37.19.PM.mov