-
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
Rename PaymentsPage to WalletPage #23798
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] |
@allroundexperts are you able to review this today? the merge conflicts are not fun with the name changes and I already remade this PR once because of them |
@allroundexperts bump on review, if you're unable to prioritize, please let me know and I'll assign someone else |
On it today @grgia! |
Sorry for the delay here. I just missed this PR. |
@allroundexperts thank you! I'm going OOO next week so I'm hoping to get this merged 😁🤞 |
Reviewer Checklist
Screenshots/VideosWebMobile Web - ChromeMobile Web - SafariDesktopiOSAndroid |
@grgia Facing following issues:
|
I was OOO last week, will update this PR today or tomorrow |
@allroundexperts are you around today for a PR review? It's easier to just recreate a new PR than try to merge these kinds of conflicts. But I want to hold off on recreating it again until you are able to review |
Hi @grgia! |
Closing in favor of #24632 |
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