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: update create wallet flow #62

Merged
15 commits merged into from
Feb 8, 2022
Merged

feat: update create wallet flow #62

15 commits merged into from
Feb 8, 2022

Conversation

ghost
Copy link

@ghost ghost commented Feb 7, 2022

Updates the "create wallet" flow according to the designs in Figma.

It's just a UI refresh more or less. The only difference in business logic is that the wallet doesn't get opened until the seed and password have been confirmed by the user.

One thing I wanted to add is that the seed and password are hidden by default and the user needs to click to unveil them. Similar to how we hide the balance by default. Thoughts on that?

📸

Screenshot 2022-02-07 at 17 43 03

Screenshot 2022-02-07 at 17 43 12

@ghost ghost self-requested a review February 7, 2022 17:07
@ghost ghost self-assigned this Feb 7, 2022
@ghost ghost mentioned this pull request Feb 7, 2022
@ghost ghost linked an issue Feb 7, 2022 that may be closed by this pull request
@dergigi
Copy link
Contributor

dergigi commented Feb 7, 2022

One thing I wanted to add is that the seed and password are hidden by default and the user needs to click to unveil them. Similar to how we hide the balance by default. Thoughts on that?

We should make it considerably difficult to show the seed and show various warnings, since it is a fatal action, potentially. Phoenix does this well, for example, we could do it in a similar way.

Edit: Just to be clear, I think it's fine for now as is, the above could be a future improvement.

@dergigi dergigi added the enhancement New feature or request label Feb 7, 2022
@dergigi dergigi added this to the v0.0.3 - Join the Market milestone Feb 7, 2022
@dergigi dergigi changed the title feat: updated create wallet flow feat: update create wallet flow Feb 7, 2022
@theborakompanioni
Copy link
Collaborator

theborakompanioni commented Feb 7, 2022

One thing I wanted to add is that the seed and password are hidden by default and the user needs to click to unveil them. Similar to how we hide the balance by default. Thoughts on that?

Just to be clear, I think it's fine for now as is, the above could be a future improvement.

I agree with that.

Tested on Firefox/Brave ✔️ mobile/desktop ✔️ darkmode/lightmode ✔️ .
Looks superb yet again. Pixel-perfect.

Daniel and others added 3 commits February 8, 2022 09:18
Co-authored-by: Thebora Kompanioni <[email protected]>
Co-authored-by: Thebora Kompanioni <[email protected]>
Co-authored-by: Thebora Kompanioni <[email protected]>
@ghost
Copy link
Author

ghost commented Feb 8, 2022

We should make it considerably difficult to show the seed and show various warnings, since it is a fatal action, potentially. Phoenix does this well, for example, we could do it in a similar way.

Totally agree. The one difference from us to Phoenix is that we only show the seed once immediately after wallet creation. Still, if someone sees that seed they can still steal you funds later on after you funded your wallet. I tracked #65 to hide the seed and password and make the user actively reveal it. I'll try to get this in soon—I think we should already have that for the first release.

@ghost
Copy link
Author

ghost commented Feb 8, 2022

Thanks for the reviews guys! I'll merge this later today unless there's still some feedback to be addressed?

@dergigi
Copy link
Contributor

dergigi commented Feb 8, 2022

Totally agree. The one difference from us to Phoenix is that we only show the seed once immediately after wallet creation. Still, if someone sees that seed they can still steal you funds later on after you funded your wallet.

That's right. What's usually done in that case is that the user is quizzed, to make sure that the seed was written down. We should definitely make sure to do this and get it right, because loss of funds can occur if the user fails to write it down.

It say it's not super-duper critical since the first release will be an early beta release anyway, but still.

I think we should already have that for the first release.

I agree!

Thanks for the reviews guys! I'll merge this later today

Go from my side ✅

@ghost ghost merged commit 8ed27ae into master Feb 8, 2022
@ghost ghost deleted the create-wallet branch February 8, 2022 11:45
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Polish Create Wallet Flow
2 participants