Skip to content
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

Add New payments page #3727

Merged
merged 60 commits into from
Jul 2, 2021
Merged

Add New payments page #3727

merged 60 commits into from
Jul 2, 2021

Conversation

stitesExpensify
Copy link
Contributor

@stitesExpensify stitesExpensify commented Jun 22, 2021

Details

This PR:

  1. Refactors <CreateMenu> to <PopoverMenu>
  2. Moves the current <PaymentsPage> to <AddPayPalMePage> because that's all it was
  3. Creates the new <PaymentsPage>
  4. Creates the new <PaymentMethodsList> component/file which gets and displays all bank accounts, cards, your wallet, and your paypal.me

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/166278

Tests/QA

  1. Click your profile picture in the top left
  2. Click payments
  3. Make sure you see all of your payment accounts (bank accounts, cards, etc)
  4. Click "Add payment account" and make sure a menu pops up
  5. Click paypal.me and make sure you get set to the paypal.me page
  6. Test the back/close buttons on both the new payments page, and the paypal page. Make sure they do what you expect them to

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

2021-06-23_16-58-52.mp4

Mobile Web

2021-06-23_18-06-06.mp4

Desktop

2021-06-23_17-14-17.mp4

iOS

Uploading 2021-06-23_18-04-05.mp4…

Android

2021-06-23_19-27-17.mp4

@stitesExpensify stitesExpensify self-assigned this Jun 22, 2021
@stitesExpensify stitesExpensify marked this pull request as ready for review June 28, 2021 23:03
@stitesExpensify stitesExpensify requested a review from a team as a code owner June 28, 2021 23:03
@stitesExpensify
Copy link
Contributor Author

Updated

luacmartins
luacmartins previously approved these changes Jun 30, 2021
@stitesExpensify
Copy link
Contributor Author

If we could get this merged today that would be great :D

@marcaaron
Copy link
Contributor

@stitesExpensify I'll re-review and merge if you fix those conflicts!

@stitesExpensify
Copy link
Contributor Author

gah new conflicts!? Fixing rn

@stitesExpensify
Copy link
Contributor Author

I un-praise git. It's bad.
2021-07-01_13-02-44

src/components/MenuItem.js Outdated Show resolved Hide resolved
src/languages/en.js Outdated Show resolved Hide resolved
src/libs/getPaymentMethodScreenLocation/index.js Outdated Show resolved Hide resolved
@stitesExpensify
Copy link
Contributor Author

Updated for all comments! Thank you again for looking through so many times

@marcaaron marcaaron merged commit 039b671 into main Jul 2, 2021
@marcaaron marcaaron deleted the stites-newPaymentsPage branch July 2, 2021 01:17
@OSBotify
Copy link
Contributor

OSBotify commented Jul 2, 2021

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@stitesExpensify stitesExpensify changed the title Stites new payments page Add New payments page Jul 2, 2021
@roryabraham
Copy link
Contributor

Unfortunately the deploy comments are not working right now. This was deployed to staging yesterday.

enterYourUsernameToGetPaidViaPayPal: 'Enter your username to get paid back via PayPal.',
payPalMe: 'PayPal.me/',
yourPayPalUsername: 'Your PayPal username',
addPayPalAccount: 'Add PayPal Account',
growlMessageOnSave: 'Your PayPal username was successfully added',
},
paymentMethodList: {
addPaymentMethod: 'Add Payment Method',
accountLastFour: 'Account ending in',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI – we've ended up shortening this to just "Ending in" to save some space in #47104

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants