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

Wallet Groups #6314

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

Conversation

greg-schrammel
Copy link
Contributor

@greg-schrammel greg-schrammel commented Dec 10, 2024

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)

Copy link

linear bot commented Dec 10, 2024

@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

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?

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

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

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants