spacemeshos / smapp-lite

The light wallet application for Spacemesh network
2 stars 1 forks source link

Implement edit/remove networks/accounts/keys. #77

Closed brusherru closed 3 days ago

brusherru commented 2 weeks ago

It closes #42, closes #43, closes #44.

I had to refactor some input components (related to key and account selection). So please, check it out carefully, just in case I broke something and didn't find :) Actually more refactoring is needed, but I decided to finish it later.

Also, I moved adding of the new network inside the special "Edit networks" drawer (mb better to rename it into "Manage networks"?). Hopefully it's fine from the UX perspective.

github-actions[bot] commented 2 weeks ago

You can preview the changes at : https://ca5d83e0.smapp-lite-prod.pages.dev

github-actions[bot] commented 1 week ago

You can preview the changes at : https://765f10ab.smapp-lite-prod.pages.dev

brusherru commented 1 week ago

@monikasmolarek thanks a lot! I have:

  1. updated texts,
  2. fixed these two bugs (when no keys / accounts),
  3. tweaked GUI for the case when there is no account in the wallet.
image

Also I can confirm that there is a bug as you recorded on the video above. I checked that we have the same bug in the main branch. So I will fix it within a separate PR. Created an issue for it: https://github.com/spacemeshos/smapp-lite/issues/79

monikasmolarek commented 1 week ago

Also, the current possibilities open a new use case.

For example: What if someone deletes both - all accounts, all keys -> The wallet is empty, we can of course create a new account - derive it from a foreign key, but the modal still has the "local key" drop-down, which is now empty. Should we allow to put there a hex and inform that it imports this key automatically while creating the account? Or should we hide the local key option? WDYT? Worth creating a feature request?

Screenshot 2024-09-10 at 10 01 23

The transactions' modals/management handle view-only perfectly well, not mentioning the local key to sign when the key isn't there. 👌 So I thought managing accounts could also be more robust.

brusherru commented 1 week ago

Should we allow to put there a hex and inform that it imports this key automatically while creating the account? Or should we hide the local key option?

It allows to put there a public key, but you need to switch to the foreign key, yeah. However, this won't be an issue when https://github.com/spacemeshos/smapp-lite/issues/82 will be implemented: Local Key dropdown will always have "Create a new key" option. So I think we don't need a separate issue for it.

github-actions[bot] commented 1 week ago

You can preview the changes at : https://7eb429d1.smapp-lite-prod.pages.dev

dioexul commented 1 week ago
Screenshot 2024-09-11 at 21 30 55

Do we need "templateAddress" field here?

Also for me it is strange that I can replace the key for an existing account.

github-actions[bot] commented 6 days ago

You can preview the changes at : https://fbeb1312.smapp-lite-prod.pages.dev

brusherru commented 6 days ago

@dioexul I've added a yellow text under the name input saying that changing anything below will affect the account's address and change it only on your own risk. I think now it will be more clear that in a common case you should not change anything. But if you really have a mistake there — it will be possible to change anything.

Sounds good?

dioexul commented 5 days ago

@brusherru yes, having a warning is better. I'm still thinking do we really need this functionality for all users, for developers - we definitely need it.

github-actions[bot] commented 3 days ago

You can preview the changes at : https://19b5255a.smapp-lite-prod.pages.dev