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: improve wallet control in settings #325

Merged
merged 7 commits into from
Jun 15, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jun 14, 2022

Resolves #323. Lays some groundwork for #228.

Adds actions for locking the wallet and creating a new wallet to the settings. We're not at a single wallet flow yet but this lays some groundwork for it and makes features available through the settings screen, which were previously accessible only on the wallets screen.

Also updates the UI of the modal used to warn the user before locking a wallet where a service is running.

📸

Screenshot 2022-06-14 at 14 48 01

Screenshot 2022-06-14 at 14 48 21

@ghost ghost added enhancement New feature or request UI/UX Issue related to cosmetics, design, or user experience labels Jun 14, 2022
@ghost ghost requested a review from theborakompanioni June 14, 2022 12:54
@ghost ghost self-assigned this Jun 14, 2022
@ghost ghost mentioned this pull request Jun 14, 2022
Copy link
Collaborator

@theborakompanioni theborakompanioni left a comment

Choose a reason for hiding this comment

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

Sorry for repeating myself, but I think TypeScript would be cool – especially for such small and new components.

The modal looks slick : D
Great work. Approved.

Daniel and others added 3 commits June 15, 2022 09:51
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 Jun 15, 2022

Sorry for repeating myself, but I think TypeScript would be cool – especially for such small and new components.

I refrained from using it here for such a light component with no real API and complex logic. Mostly because I feel like it adds a lot of complexity everywhere. But I won't die on that hill. I can update it if you feel like we should use it more consistently?

@theborakompanioni
Copy link
Collaborator

I refrained from using it here for such a light component with no real API and complex logic. Mostly because I feel like it adds a lot of complexity everywhere. But I won't die on that hill. I can update it if you feel like we should use it more consistently?

No, it's okay. 👍
I don't want to create any additional work.

I think IDE support improves a lot with TypeScript. Will do it myself once we are polishing things up, if it pays off at all.

@theborakompanioni theborakompanioni merged commit 9d00212 into master Jun 15, 2022
@theborakompanioni theborakompanioni deleted the settings-wallet-control branch June 15, 2022 11:33
@ghost
Copy link
Author

ghost commented Jun 15, 2022

I think IDE support improves a lot with TypeScript.

Good point! 👍

@editwentyone
Copy link

new proposal that I came up with right now:

image

instead of "create wallet" directly on settings page, we could remove that, and rename "switch wallet" to "manage wallets", there the user can directly lock, unlock and create stuff.

and in the future it makes more sense because there the user could also import and maybe export wallets. what do you think?

@ghost
Copy link
Author

ghost commented Jun 20, 2022

I feel like what we have implemented atm is good enough for now. It's a rather niche feature -- I wouldn't want to spend too much time on it right now. We can definitely keep the idea in the backlog for now, though. Feel free to create an issue for it. :)

@editwentyone
Copy link

editwentyone commented Jun 20, 2022

thanks, you are right, I will open a new issue #337

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request UI/UX Issue related to cosmetics, design, or user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question: Lock wallet flow
2 participants