[HOLD for payment 2024-08-19] [$250] Update the Wallets page to put bank accounts at top

danielrvidal commented 1 month ago

Problem: Users are trying to setup their reimbursement accounts and running into confusion. They actually are setting up the reimbursement account successfully, but then find the Wallets page where there is a big green Enable Wallet button above their Bank accounts section. On top of that, they are trying to enable their wallet and then run into confusing errors setting up their wallet, which are expected because we haven't cleaned up the wallet flow, nor error messages (background)

Solution: Put Bank accounts above Expensify Wallet and emphasize bank accounts more.

Let's put the Bank accounts section/enablement above the Expensify wallet section
Let's add an image as part of bank accounts to the Bank accounts section
Update the text for each section to be:

Bank accounts

[Bank accounts image]

*Bank accounts*
Adding a bank account allows you to get paid back for expenses you submit to a workspace.

[+ Add bank account]

Wallet

*Send and receive money with friends*
Enable your wallet to send and receive money with friends. 

Balance
$X.XX

[:wallet: Enable wallet]

You'll notice a different title and we've emphasized the big green button for the wallet.

Here is a screenshot of the updated page @shawnborton mocked up. We updated the copy a bit so please use the copy above. He will get you the image we're going to use for the bank accounts section.

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01dbcb038228b4c6a0
Upwork Job ID: 1816609222716969047
Last Price Increase: 2024-07-25
Automatic offers:

paultsimura | Reviewer | 103283186
Krishna2323 | Contributor | 103283188

Issue Owner

Current Issue Owner: @paultsimura

melvin-bot[bot] commented 1 month ago

Job added to Upwork: https://www.upwork.com/jobs/~01dbcb038228b4c6a0

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @paultsimura (External)

Krishna2323 commented 1 month ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Update the Wallets page to put bank accounts at top

What is the root cause of that problem?

New update

What changes do you think we should make in order to solve the problem?

We need to move this section above this line.
Update the translation for bank accounts & translation for wallet section in both en and es.

Update the button to use menuitem

<MenuItem
ref={buttonRef as ForwardedRef<View>}
onPress={() => Navigation.navigate(ROUTES.SETTINGS_ENABLE_PAYMENTS)}
disabled={network.isOffline}
title={translate('walletPage.enableWallet')}
icon={Expensicons.Wallet}
hoverAndPressStyle={styles.hoveredComponentBG}
wrapperStyle={[
    styles.transferBalance,
    shouldUseNarrowLayout ? styles.mhn5 : styles.mhn8,
    shouldUseNarrowLayout ? styles.ph5 : styles.ph8,
]}
/>

Pass the new illustration to the section component.

We also need to create translation for Balance text and replace CurrentWalletBalance with MenuItemWithTopDescription. https://github.com/Expensify/App/blob/14be949a53eeb42a3d208af61b83a682726b5e8d/src/pages/settings/Wallet/WalletPage/WalletPage.tsx#L392

<MenuItemWithTopDescription
description={translate('common.transferBalance')}
title={CurrencyUtils.convertToDisplayString(userWallet?.currentBalance ?? 0)}
titleStyle={styles.walletBalance}
interactive={false}
wrapperStyle={styles.sectionMenuItemTopDescription}
/>

We also need to decrease the font size in walletBalance object. https://github.com/Expensify/App/blob/6d2cf34314913c71f69dd4ed997f4d3935c7dbf9/src/styles/index.ts#L4749-L4751
We might also want to take off some margin from balance display container because the font size is decresed. https://github.com/Expensify/App/blob/14be949a53eeb42a3d208af61b83a682726b5e8d/src/pages/settings/Wallet/WalletPage/WalletPage.tsx#L390
What alternative solutions did you explore? (Optional)

Result

Monosnap (187) New Expensify 2024-07-26 06-05-54

Note: Translations have been not updated in the screenshot

Krishna2323 commented 1 month ago

Proposal updated

Added more details in solution section and added a POC.

nyomanjyotisa commented 1 month ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Update the Wallets page to put bank accounts at top

What is the root cause of that problem?

Update request

What changes do you think we should make in order to solve the problem?

Update the WalletPage base on the request above

I have created a branch for POC with the following changes:

Move Bank Account section to the top
Pass the illustration to Section component (need to wait for the illustration asset)
Update addBankAccountToSendAndReceive translation (both en & es)
add new translation key balance and add text with that translation above Balance Amount
Decrease Wallet Balance Amount font size to match the mockup (if needed)
Change Expensify Wallet text to Send and receive money with friends, use sendAndReceiveMoney translation key (Might need to remove the dot (.) from the translation value)
Change Enable Wallet button to MenuItem

RESULT

What alternative solutions did you explore? (Optional)

cretadn22 commented 1 month ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Update the Wallets page to put bank accounts at top

What is the root cause of that problem?

New update

What changes do you think we should make in order to solve the problem?

Currently, we have 3 section in this page
- Wallet Section
- Assigned Card Section
- Bank Account Section

We need to move the bank account section to the top (maybe need to confirm the position of assigned card section)

Add illustration prop to the Section Component of Bank Account section (waiting for design)
Update the Enable Wallet button to use MenuItem component
Update translation https://github.com/Expensify/App/blob/6d2cf34314913c71f69dd4ed997f4d3935c7dbf9/src/pages/settings/Wallet/WalletPage/WalletPage.tsx#L373

--> Send and receive money with friends.

But there is a problem, if hasActivatedWallet is true, we will display Send and receive money with friends. for both title and subtitle (need to confirm again)

https://github.com/Expensify/App/blob/6d2cf34314913c71f69dd4ed997f4d3935c7dbf9/src/pages/settings/Wallet/WalletPage/WalletPage.tsx#L503

--> Adding a bank account allows you to get paid back for expenses you submit to a workspace.

Add new translation to display "Balance" text above this line https://github.com/Expensify/App/blob/6d2cf34314913c71f69dd4ed997f4d3935c7dbf9/src/pages/settings/Wallet/WalletPage/WalletPage.tsx#L392

What alternative solutions did you explore? (Optional)

cretadn22 commented 1 month ago

@danielrvidal @shawnborton as mentioned in my proposal, we have a problem in wallet section if hasActivatedWallet is true

The same text is displayed for both title and subtitle. Could you please provide a new design for this case

Krishna2323 commented 1 month ago

Proposal Updated

Missed the change for Balance text above amount text in my initial proposal. Now updated

shawnborton commented 1 month ago

It would use the same design, we just need some updated copy for that which shouldn't be a huge deal.

paultsimura commented 1 month ago

This is quite a straightforward task, the proposal by @Krishna2323 covers it and is the first one (with insignificant edits).

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @neil-marcellini, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

nyomanjyotisa commented 1 month ago

I believe my proposal is the first complete proposal 🙏

paultsimura commented 1 month ago

Hmm, I think you're right @nyomanjyotisa – thanks for pointing this out. Apparently, Krishna updated the proposal with a few points present in your proposal but initially missing in his – I must have misread that last update.

In this case, I think it's fair to go with @nyomanjyotisa's proposal.

🎀👀🎀 C+ reviewed (again)

melvin-bot[bot] commented 1 month ago

Current assignee @neil-marcellini is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

Krishna2323 commented 1 month ago

@paultsimura, I only updated for adding balance text and reduce some font size. I believe my initial proposal was mostly complete for this straight forward issue. The other proposals are almost similar to my proposal, only balance text part was missing in my proposal and I believe that could be ignored looking at the simplicity of the issue.

Also my solution for showing the balance is different, can you pls check that once🙏🏻. No problem if you don't want to :)

cretadn22 commented 1 month ago

@paultsimura what do you think about this problem https://github.com/Expensify/App/issues/46242#issuecomment-2251617966

neil-marcellini commented 1 month ago

I'm going to go with @Krishna2323's proposal because it was first, and his edits based on other proposals were minimal. I'm sure all of your are very capable of handling this kind of work, and the proposals don't need to be super detailed, so it seems most fair to go with the person that posted first.

melvin-bot[bot] commented 1 month ago

📣 @paultsimura 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 1 month ago

📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

paultsimura commented 1 month ago

Sounds reasonable, thanks for stepping in @neil-marcellini

danielrvidal commented 1 month ago

@Krishna2323 to address this problem. For the wallet section if hasActivatedWallet is true, could you make the subtext: "Your wallet has been enabled to send and receive money with friends."

danielrvidal commented 4 weeks ago

It was merged to staging yesterday so it should be done soon.

melvin-bot[bot] commented 3 weeks ago

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot[bot] commented 3 weeks ago

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.18-10 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

https://github.com/Expensify/App/pull/46380

If no regressions arise, payment will be issued on 2024-08-19. :confetti_ball:

For reference, here are some details about the assignees on this issue:

@paultsimura requires payment automatic offer (Reviewer)
@Krishna2323 requires payment automatic offer (Contributor)

paultsimura commented 3 weeks ago

BZ checklist: This was a feature request, therefore there are no offending PR or Regression tests to suggest.

melvin-bot[bot] commented 2 weeks ago

Issue is ready for payment but no BZ is assigned. @alexpensify you are the lucky winner! Please verify the payment summary looks correct and complete the checklist. Thanks!

alexpensify commented 2 weeks ago

Payouts due: 2024-08-19

[X] Contributor: $250 @Krishna2323
[X] Contributor+: $250 @paultsimura

Upwork job is here.

I've completed the payment process via Upwork and will close this issue for now. Later this week, I'll create the regression test request.

Expensify / App