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.

Sign up for GitHub

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

Jump to bottom

Wallet Groups #6314

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

Wallet Groups #6314

wants to merge 16 commits into from
+248 −104

Conversation

greg-schrammel
Copy link
Contributor

@greg-schrammel greg-schrammel commented Dec 10, 2024 •
edited
Loading

Fixes APP-####

What changed (plus any additional context for devs)

adds ability to choose a mnemonic to derive a new account from (wallet groups)

Screen recordings / screenshots

Screen.Recording.2024-12-20.at.14.58.14.mov

What to test

test you can create new wallet groups, new accounts in these groups, and these wallets actually work (sign something)

@linear Linear
Copy link

linear bot commented Dec 10, 2024

APP-1382 Prompt user to choose Wallet Group when creating a wallet

@greg-schrammel greg-schrammel marked this pull request as draft December 10, 2024 18:15
@brunobar79
Copy link
Member

Launch in simulator or device for 48bcd19

@greg-schrammel greg-schrammel marked this pull request as ready for review December 10, 2024 23:23
@brunobar79
Copy link
Member

Launch in simulator or device for 6e32be5

walmat
walmat reviewed Dec 11, 2024
View reviewed changes
Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

Can you test the entry point from backups v2 creating a new wallet?

walmat
walmat reviewed Dec 11, 2024
View reviewed changes
Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

great work just a couple questions and we need to test entry point from backups too

@brunobar79
Copy link
Member

Launch in simulator or device for acb99a8

@derHowie derHowie marked this pull request as draft December 19, 2024 16:39
@derHowie
Copy link
Member

figma has like 3 designs, I just picked one and made the components in a way it's easy to switch between them if I got the wrong one

bumped this to draft @greg-schrammel. I think it's important that we confirm final design so that reviewers know what they're testing

@brunobar79 brunobar79 added the release for release blockers and release candidate branches label Dec 19, 2024
@greg-schrammel greg-schrammel marked this pull request as ready for review December 20, 2024 17:59
@brunobar79
Copy link
Member

Launch in simulator or device for b36e13c

@jinchung jinchung removed the release for release blockers and release candidate branches label Dec 20, 2024
@walmat walmat requested review from derHowie and maxbbb and removed request for benisgold January 3, 2025 18:09
@brunobar79
Copy link
Member

Launch in simulator or device for 0f46f0a

maxbbb
maxbbb reviewed Jan 6, 2025
View reviewed changes
Copy link
Contributor

@maxbbb maxbbb left a comment

Choose a reason for hiding this comment

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

Lgtm other than a few minor things

src/assets/CreateNewWalletGroup.png Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason this was needed over the existing CreateNewWallet.png?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that one has a builtin shadow

src/navigation/ChooseWalletGroup.tsx Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

The other screens of the AddWalletNavigator are in the screens folder, I'd assume the ChooseWalletGroup sheet should be too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes good call

src/navigation/Routes.ios.tsx Outdated Show resolved Hide resolved
src/navigation/Routes.android.tsx Outdated Show resolved Hide resolved
@brunobar79
Copy link
Member

Launch in simulator or device for 12d1019

@brunobar79 brunobar79 added the release for release blockers and release candidate branches label Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@walmat walmat walmat left review comments

@maxbbb maxbbb maxbbb approved these changes

@derHowie derHowie Awaiting requested review from derHowie

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
release for release blockers and release candidate branches
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@greg-schrammel @brunobar79 @derHowie @walmat @maxbbb @jinchung