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

Introduce AddWalletButton and connect it to the AddWallet flow #418

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

johnny9
Copy link
Contributor

@johnny9 johnny9 commented Aug 24, 2024

These commits enable the "Add Wallet" button at the bottom of the Wallet Select menu on Desktop

Copy link
Contributor

@MarnixCroes MarnixCroes left a comment

Choose a reason for hiding this comment

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

4b79351
the space between the icon and the text is different than from master

Master

Screenshot from 2024-08-24 11-06-36

PR

Screenshot from 2024-08-24 11-06-48


maybe for follow up:
At the AddWallet page, the Skip button in in the right top should get a condition to only have it when in the onboarding flow. Now in the app when you click add wallet button -> Add Wallet page has a skip button, which makes no sense. a cancel would be more appropriate

@hebasto
Copy link
Member

hebasto commented Sep 2, 2024

cc @GBKS

@GBKS
Copy link
Contributor

GBKS commented Sep 11, 2024

Nice work adding this functionality. I did a review of the full flow (MacOS) and added my notes in the visual below. Some of them will not be relevant to the focus of this PR and should probably be better handled in a separate one. Let me know how you want to handle that.

Bitcoin Core App PR 418 review 240911

And in written form:

1. Wallet selector dropdown
1.1 All font sizes should be 15px (not 13, 14, and 15)
1.2 Reduce spacing between "+" icon and label in the "+ Add Wallet" button

2. Add a wallet screen
2.1 In the transition to this screen, the new screen should slide in from below (not from the right)
2.2 Replace "Skip" button in the top-right, but a "Cancel" button in the top-left (keep "Skip" on this screen when it is shown in the onboarding sequence)

3. Choose a wallet name screen
3.1 Replace "Continue" button label with "Next"

4. Choose a password screen
All good

5. Your wallet has been created screen
5.1 Do we want to offer a "Back" button here? It is convenient for users, yet also creates a new wallet every time they go back and forth - is that a problem?

6. Back up your wallet screen
6.1 Same question about the "Back" button as above
6.2 "View file" button label should be white
6.3 Reduce spacing between buttons to 20px
6.4 When exiting this screen, it should slide down

7. Activity screen post wallet creation
7.1 The newly created wallet should now be active

@jarolrod jarolrod added enhancement New feature or request Wallet labels Sep 11, 2024
@rabbitholiness
Copy link

On macOS, the wallet name screen would not allow me to add spaces to the wallet name
Screenshot 2024-09-18 at 10 38 10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Wallet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants