Allow removing loose accounts

[danfinlay]

While deleting accounts from an HD keychain is a weird concept, deleting loose (imported) accounts should be easy to add.

[danfinlay]

If we make removing also delete that account's transaction history, it could help developers alleviate pain from #1999.

[cjeria]

This is related to hardware support PR #4625

[MoMannn]

+1

[leafcutterant]

This functionality would help me. Would resetting account (in Metamask settings) achieve the same?

[brunobar79]

The reset account under settings will remove all the transactions from your account history but won’t forget the account. We will add this specific feature soon.

On Fri, Jul 6, 2018 at 14:50 leafcutterant ***@***.***> wrote: This functionality would help me. Would resetting account (in Metamask settings) achieve the same? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#2638 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABMKWkGm2IU5nSdJAzLFyb0AnqIgf0y9ks5uD7F4gaJpZM4Qx_0q> .

-- *Bruno Barbieri* // Senior Javascript Engineer @ Metamask 49 Bogart St, Suite 22, Brooklyn NY 11206 Web <https://consensys.net/> | Twitter <https://twitter.com/brunobar79> | Linkedin <https://www.linkedin.com/in/brunobar79/> | Github <https://github.com/brunobar79>

[bdresser]

@cjeria what do you think? how bout something super simple for the base case, like:

[brunobar79]

@bdresser +100!

The only consideration is that we might need to ask for confirmation to avoid "accidents".

Something like this with the right copy might do the trick:

[bdresser]

For sure, definitely should show the confirm modal to make sure

[cjeria]

@bdresser That works. The only concern though is that long account names won't be as readable especially when the "import" label is present.

Another option would be to add the remove button within the account balance view.

Another ux consideration and something I anticipate users requesting is the ability to show accounts after they've removed them. We might want to think through this use case as well? Although, this feature is specifically for imported account and not for all other account types.

[bdresser]

@cjeria I prefer in the top-right menu because (1) that seems like the home for overall management features and (2) because it'll be much quicker for someone to clean up their accounts (rather than clicking in to each account, then removing, then getting dumped on another account screen, etc)

I hear you on the long names. We could leave the IMPORTED tag as-is and put the X just below, or just to the right? Or the X could go all the way to the left, but that could get weird with the check mark that indicates the account is selected.

[cjeria]

I agree. The x in the account dropdown for quick access is nice. Let's go with the "x" on the far right with the imported label next to it also aligned right. Confirm message could say something like "Remove account?" w/subtext "This account will be removed from your wallet. You can import or create accounts again from the account dropdown. Learn more (link)"

[bdresser]

sounds good!

let's also include a reminder to make sure the user has the seed for the imported account, can't be too safe

This account will be removed from your wallet. Please make sure you have the original seed phrase or private key for this imported account before continuing. You can import or create accounts again from the account drop-down.

"Learn more" can link to this helpdesk article: https://consensys.zendesk.com/hc/en-us/articles/360004180111-What-are-imported-accounts-New-UI-

[cjeria]

Let's also make the "x" visible on hovering over the account row. This'll make the dropdown visually less busy.

[brunobar79]

@cjeria @bdresser This is done in #4625. Check it out:

[brunobar79]

Just discussed this with @danfinlay and he brought up a good point:

• Loose accounts don't have a seed phrase (just a private key)
• For trezor accounts you only need your device PIN to unlock it
• We are not allowing to remove accounts from the HD keyring so there's no need to mention "seed phrase"

With that said we should rethink the copy to something more generic and probably change the "Learn more" link to something else.

CC: @bdresser @cjeria

[bdresser]

Good call on the copy @brunobar79 @danfinlay . How about just saying "access to the account's private key"? Also on re-read, the last sentence seems unnecessary since the user is already on the accounts menu...

How about:

This account will be removed from your wallet. Please make sure you have another means of accessing this account's private key before continuing. You can import accounts or connect hardware again from the account drop-down menu. Learn more.

Would it be insane to make users double-confirm?

If you're cool with it, let's leave the "Learn More" link as-is and I can edit the support article to include a mention of hardware wallets (or to include a link to a separate article on hardware wallets once we get one up). Sound good?

Also, random - does this modal fit okay in the extension view?

[brunobar79]

@bdresser I think "private key" doesn't apply for the trezor (or maybe it does? 😕 )

About double confirm I don't think it's necessary since:

• User needs to click twice already (first on the X, then confirm their action on the modal
• In my opinion private keys are less likely to be lost since the user already had it when it imported the account to metamask. It's different when they generate the seed words using metamask cause we provide it.

I'd like to hear @danfinlay and @cjeria comments on this too.

About keeping the learn more there, sounds good to me.
About the modal fitting on the extension, that's not a problem.

Thanks for all the input @bdresser!

[cjeria]

I agree with @brunobar79 no need for a double confirm given that there are already two clicks to remove the account.

I'd suggest adding a tooltip on the "X" with "Remove" copy on hover as well. Everything else sounds good to me.

[cjeria]

And hopefully with the simplification of the copy, the modal will fit within the bounds of the extension?

[brunobar79]

@cjeria Sounds good! About the popup layout + modal, it fits just fine:

About the tooltip, is there any specific place in the new UI where we a similar tooltip?

[cjeria]

@brunobar79 @alextsg has implemented a tooltip in various places such as the top bar with address to copy/paste and it's in a few other places too. Not sure how modular it is, but you can reference this one for now to make it look consistent with other tooltips.

[cjeria]

Also, looking at the remove confirm modal, do you guys think it would be helpful to add more context about where this account was imported from? (E.I Trezor, Ledger etc.)? Could help if a user has a few different brands of hardware wallets and they forgot which one they used.

[brunobar79]

I like the idea of adding more context! I think it would be more relevant to show the account label (which in the TREZOR case it's called TREZOR 1, 2, 3, etc. ) but also works for accounts that were imported with the private key. WDYT?

[cjeria]

@brunobar79 Here's an option with a little more ui design polish. The little arrow icon is button to view the account on etherscan, much like you had in the connect to hardware flow. thoughts?

[brunobar79]

@cjeria beatiful! I'll do that

[brunobar79]

Fixed by #4625