-
Notifications
You must be signed in to change notification settings - Fork 987
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
base: develop
Are you sure you want to change the base?
Conversation
Jenkins BuildsClick to see older builds (30)
|
ca748e5
to
b479ecf
Compare
b479ecf
to
8818311
Compare
8818311
to
0bda6cc
Compare
100% of end-end tests have passed
Passed tests (8)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestWalletOneDevice:
|
48c8881
to
3036c4d
Compare
:flow-id :wallet-bridge-flow}]]]})) | ||
(rf/reg-event-fx | ||
:wallet/start-bridge | ||
(fn [{:keys [db]} [{:keys [navigate-forward?]}]] |
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.
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?
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.
Absolutely, I've updated the code removing this parameter
Signed-off-by: Brian Sztamfater <brian@status.im>
3036c4d
to
06d3f3e
Compare
@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? 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. 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? |
Hey @Horupa-Olena :) The buttons on the token list are to help people with no funds so they have diffferent purpose. 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:
video_2025-01-29_15-57-23.mp4
Here’s an example: video_2025-01-29_16-05-02.mp4In 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. |
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. |
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. |
@Horupa-Olena here is Phantom, the worlds most popular wallet where they do exactly this. Empty/new wallet: Populated wallet: |
@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. |
But phantom does not have a global home like we do. This is one of our selling points. 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. |
We will recreate the flow for swaps so there is no doubts. @xAlisher |
Testing is on hold, and the PR is blocked until the design is clarified. сс @briansztamfater |
@briansztamfater Hi! Sorry for such a long delay. |
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
Areas that may be impacted
Steps to test
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