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

feat: elevate and highlight wallet CTAs on wallet home screen #21985

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

briansztamfater
Copy link
Member

@briansztamfater briansztamfater commented Jan 24, 2025

fixes #21976

Summary

This PR adds wallet CTAs to our wallet home screen, and handle the case for each flow when they are initiated from the wallet home without any preselected token.

Platforms

  • Android
  • iOS

Areas that may be impacted

Steps to test

  • Open Status
  • Open wallet
  • Tap Send / Bridge / Swap / Buy / Receive options
  • Verify flows are working fine

We should also verify the current Send / Bridge / Swap / Buy / Receive flows where not broken by initiating them from different screens and actions.

status: ready

@briansztamfater briansztamfater added request-manual-qa wallet-core Issues for mobile wallet team labels Jan 24, 2025
@briansztamfater briansztamfater added this to the 2.33.0 milestone Jan 24, 2025
@briansztamfater briansztamfater self-assigned this Jan 24, 2025
@status-im-auto
Copy link
Member

status-im-auto commented Jan 24, 2025

Jenkins Builds

Click to see older builds (30)
Commit #️⃣ Finished (UTC) Duration Platform Result
0ad3a88 #1 2025-01-24 13:53:22 ~3 min tests 📄log
✔️ 0ad3a88 #1 2025-01-24 13:59:03 ~8 min android-e2e 🤖apk 📲
✔️ 0ad3a88 #1 2025-01-24 13:59:37 ~9 min android 🤖apk 📲
✔️ 0ad3a88 #1 2025-01-24 14:00:40 ~10 min ios 📱ipa 📲
ac7043d #2 2025-01-24 14:24:12 ~3 min tests 📄log
✔️ ac7043d #2 2025-01-24 14:29:35 ~8 min android-e2e 🤖apk 📲
✔️ ac7043d #2 2025-01-24 14:29:56 ~9 min android 🤖apk 📲
✔️ ac7043d #2 2025-01-24 14:32:42 ~11 min ios 📱ipa 📲
a2675af #3 2025-01-24 15:30:29 ~3 min tests 📄log
✔️ a2675af #3 2025-01-24 15:34:24 ~7 min android-e2e 🤖apk 📲
d26afe3 #4 2025-01-24 15:43:32 ~8 min tests 📄log
✔️ d26afe3 #4 2025-01-24 15:45:34 ~10 min ios 📱ipa 📲
✔️ d26afe3 #4 2025-01-24 15:48:26 ~13 min android-e2e 🤖apk 📲
✔️ d26afe3 #4 2025-01-24 15:48:47 ~14 min android 🤖apk 📲
81ad779 #5 2025-01-24 21:30:30 ~2 min tests 📄log
✔️ 81ad779 #5 2025-01-24 21:35:29 ~7 min android-e2e 🤖apk 📲
✔️ 81ad779 #5 2025-01-24 21:35:58 ~8 min android 🤖apk 📲
✔️ 81ad779 #5 2025-01-24 21:38:54 ~11 min ios 📱ipa 📲
✔️ ca748e5 #6 2025-01-24 21:49:20 ~4 min tests 📄log
✔️ ca748e5 #6 2025-01-24 21:53:07 ~8 min android-e2e 🤖apk 📲
✔️ ca748e5 #6 2025-01-24 21:53:58 ~9 min android 🤖apk 📲
✔️ ca748e5 #6 2025-01-24 21:55:53 ~11 min ios 📱ipa 📲
✔️ 8818311 #8 2025-01-24 22:05:39 ~5 min tests 📄log
✔️ 8818311 #8 2025-01-24 22:09:27 ~8 min android-e2e 🤖apk 📲
✔️ 8818311 #8 2025-01-24 22:10:19 ~9 min android 🤖apk 📲
✔️ 8818311 #8 2025-01-24 22:14:27 ~13 min ios 📱ipa 📲
✔️ 0bda6cc #9 2025-01-28 10:47:20 ~5 min tests 📄log
✔️ 0bda6cc #9 2025-01-28 10:50:52 ~8 min android 🤖apk 📲
✔️ 0bda6cc #9 2025-01-28 10:52:35 ~10 min ios 📱ipa 📲
✔️ 0bda6cc #9 2025-01-28 10:52:45 ~10 min android-e2e 🤖apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 3036c4d #11 2025-01-28 16:41:44 ~4 min tests 📄log
✔️ 3036c4d #11 2025-01-28 16:43:58 ~6 min android-e2e 🤖apk 📲
✔️ 3036c4d #11 2025-01-28 16:45:10 ~7 min android 🤖apk 📲
✔️ 3036c4d #11 2025-01-28 16:46:58 ~9 min ios 📱ipa 📲
✔️ 06d3f3e #12 2025-01-28 18:46:43 ~4 min tests 📄log
✔️ 06d3f3e #12 2025-01-28 18:51:10 ~8 min android-e2e 🤖apk 📲
✔️ 06d3f3e #12 2025-01-28 18:51:39 ~9 min android 🤖apk 📲
✔️ 06d3f3e #12 2025-01-28 18:55:35 ~13 min ios 📱ipa 📲

@briansztamfater briansztamfater force-pushed the feat/elevate-wallet-ctas branch 6 times, most recently from ca748e5 to b479ecf Compare January 24, 2025 21:59
@briansztamfater briansztamfater changed the title [wip] feat: elevate wallet CTAs feat: elevate and highlight wallet CTAs on wallet home screen Jan 24, 2025
@briansztamfater briansztamfater force-pushed the feat/elevate-wallet-ctas branch from b479ecf to 8818311 Compare January 24, 2025 22:00
@briansztamfater briansztamfater marked this pull request as ready for review January 24, 2025 22:00
@Horupa-Olena Horupa-Olena self-assigned this Jan 28, 2025
@Horupa-Olena Horupa-Olena force-pushed the feat/elevate-wallet-ctas branch from 8818311 to 0bda6cc Compare January 28, 2025 10:41
@status-im-auto
Copy link
Member

100% of end-end tests have passed

Total executed tests: 8
Failed tests: 0
Expected to fail tests: 0
Passed tests: 8

Passed tests (8)

Click to expand

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
Device sessions

2. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
2. test_wallet_send_eth, id: 727229

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Device sessions

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231
2. test_wallet_balance_mainnet, id: 740490

@briansztamfater briansztamfater force-pushed the feat/elevate-wallet-ctas branch 2 times, most recently from 48c8881 to 3036c4d Compare January 28, 2025 16:37
:flow-id :wallet-bridge-flow}]]]}))
(rf/reg-event-fx
:wallet/start-bridge
(fn [{:keys [db]} [{:keys [navigate-forward?]}]]
Copy link
Contributor

Choose a reason for hiding this comment

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

I tend to think that it is better to make decision regarding navigation inside the event itself. Because we probably have all the information in the rf-db - current screen, state of the accounts.
So maybe we can avoid making this decision inside different views and concentrate logic here, wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

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

Absolutely, I've updated the code removing this parameter

Signed-off-by: Brian Sztamfater <brian@status.im>
@briansztamfater briansztamfater force-pushed the feat/elevate-wallet-ctas branch from 3036c4d to 06d3f3e Compare January 28, 2025 18:42
@Horupa-Olena
Copy link

Horupa-Olena commented Jan 29, 2025

@briansztamfater Hi! Thanks for your work and PR. I have some questions about the design first, and then I can share my testing results.

@xAlisher @pedro-et Hi! I have a few questions regarding the design and how this task should be implemented.

Since the parent task specifically focused on the Buy and Swap buttons, does it make sense to place all the buttons on the main wallet screen?
In my opinion, they might get lost among other elements, which would diminish the purpose of this feature.

Secondly, we already worked on a task to add Buy/Receive buttons (I can't find the design and task, so I highlighted them with a red frame on the screenshot) to the main wallet screen for new users with empty accounts.
The current implementation would simply override this feature and negate the effort spent on it.

Would it perhaps make more sense to keep this design and make a small upgrade instead—replacing the Receive button with Swap for accounts that already have some balance?

photo_2025-01-29_11-43-53

@pedro-et
Copy link

Hey @Horupa-Olena :)
This was a special request from Volo as an attempt to increase revenue.

The buttons on the token list are to help people with no funds so they have diffferent purpose.
So if people have no tokens they can receive or buy to populate the wallet.

Swaps is what will increase revenue but there is no point in swaping for new new joiners. Does it make sense?

@Horupa-Olena
Copy link

Horupa-Olena commented Jan 29, 2025

Hey @Horupa-Olena :)
This was a special request from Volo as an attempt to increase revenue.

The buttons on the token list are to help people with no funds so they have diffferent purpose.
So if people have no tokens they can receive or buy to populate the wallet.

Swaps is what will increase revenue but there is no point in swaping for new new joiners. Does it make sense?

@pedro-et To avoid any miscommunication, I'll rephrase my questions more precisely:

  1. The parent task states that the Buy and Swap buttons should be added to the main screen. However, in the design, all five buttons (Buy, Swap, Receive, Send, Bridge) have been added. Why? cc @shivekkhurana
    photo_2025-01-29_15-58-32

  2. As a result of point 1, new users with empty balances will see duplicated Buy and Receive buttons. What should the design look like in this case?
    photo_2025-01-29_15-57-18

  3. Following up on point 2, if a user is new and has no balance, they won’t be able to perform Swap/Send/Bridge actions. However, since the buttons are active, the user can start the flow, but it will get stuck on the asset selection step. How should this work for such accounts?

video_2025-01-29_15-57-23.mp4
  1. The Swap/Send/Bridge flows have multiple steps that may or may not be displayed depending on different conditions (From page, asset selection, and network selection). [#21557] feat: show from account page in swap flow #21611 - implemented here.
  • Which accounts do we display on the From page when the user taps Swap/Send/Bridge on the main wallet screen?
  • Should accounts with zero balances (empty accounts) be displayed there?

Here’s an example:
I have three accounts—one empty and two with tokens. When I click the Send button, I see a list of all three accounts and can select the empty account to start the send flow.

video_2025-01-29_16-05-02.mp4

In the previous video (point 3), I can click Send button on home page and both empty accounts are displayed on the From page, allowing me to select them too.

@pedro-et
Copy link

We have flows designed for when the user has no balance and triggers send for example.

One thing does not invalidate the other.

I do agree it could be a little duplicative but most people will not have an empty wallet.

@pedro-et
Copy link

Think of the bottom cards as promotional cards. They won't be there most of the time and we need to ensure the user is familiar with the main actions in the wallet.

@pedro-et
Copy link

@Horupa-Olena here is Phantom, the worlds most popular wallet where they do exactly this.

Empty/new wallet:

image

Populated wallet:

image

@Horupa-Olena
Copy link

@Horupa-Olena here is Phantom, the worlds most popular wallet where they do exactly this.

Empty/new wallet:

image

Populated wallet:

image

@pedro-et Thank you. But if I understand correctly, the screenshot shows a specific Account 4, meaning it’s the view inside that account.

However, in our case, we are discussing the main wallet screen, where all accounts are displayed. These accounts may differ in balance and type.

These are different cases, and the design should reflect that accordingly.

@pedro-et
Copy link

But phantom does not have a global home like we do.

This is one of our selling points.
we have flows designed for when any of the wallet flows are triggered from the home.

Essentially the first step is to select an account.

https://www.figma.com/design/xLs1KYmF4e6WwRTZVJKeUK/Wallet?node-id=15725-31935&t=j7V7rJx1xpvIslVT-1

While this was triggered differently, the flows are the same. You can find all the necessary flows there

@Horupa-Olena
Copy link

But phantom does not have a global home like we do.

This is one of our selling points. we have flows designed for when any of the wallet flows are triggered from the home.

Essentially the first step is to select an account.

https://www.figma.com/design/xLs1KYmF4e6WwRTZVJKeUK/Wallet?node-id=15725-31935&t=j7V7rJx1xpvIslVT-1

While this was triggered differently, the flows are the same. You can find all the necessary flows there

@pedro-et I tested these flows and know all the steps. However, the current PR follows a different logic, and the flow initiation differs from starting it via the Send button inside an account or through the context menu when long-pressing an asset.

Here #21611 (comment) we approved how the logic for the "From" page should work, but we don’t fully understand how it should behave in this new process. I don’t have a clear description of this case, so I can’t confirm whether the current behavior—showing an empty account on the "From" page—is correct or if it’s a bug.

@pedro-et
Copy link

We will recreate the flow for swaps so there is no doubts. @xAlisher

@Horupa-Olena
Copy link

Horupa-Olena commented Jan 31, 2025

Testing is on hold, and the PR is blocked until the design is clarified. сс @briansztamfater

@Horupa-Olena
Copy link

@briansztamfater Hi! Sorry for such a long delay.
Please resolve the conflicts, and I will perform the final testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request-manual-qa wallet-core Issues for mobile wallet team
Projects
Status: IN TESTING
Development

Successfully merging this pull request may close these issues.

✨ Elevate and Highlight Swap & Buy Buttons
8 participants