input-output-hk / nami

Nami Wallet is a browser based wallet extension to interact with the Cardano blockchain. Support requests: https://iohk.zendesk.com/hc/en-us/requests/new
https://namiwallet.io
Apache License 2.0
372 stars 167 forks source link

Broadcast onDisconnect() & onNetworkChange() injected API events #17

Open DEADPXLZ opened 3 years ago

DEADPXLZ commented 3 years ago

Callbacks on Disconnect (aka remove from Whitelist) and on switching between Testnet & Mainnet would be very helpful, consider adding these? Functionality should be similar to the onAccountChange() injected API call.

alessandrokonrad commented 3 years ago

onNetworkChange is a good idea. Gonna add that one. With onDisconnect I'm not sure, because as soon as you remove the page from the whitelist, in going and outgoing connections to the webpage are blocked. Also I don't think this would be a typical event or where do you see this as useful?

DEADPXLZ commented 3 years ago

100% on the onNetworkChange(), thank you very much!

Regarding the onDisconnect() event, it's useful for updating the UI on apps that use the wallet during runtime. Metamask also does this and they actually go the extra mile where you can actually trigger a disconnection from within the 3rd party website. LMKWYT

alessandrokonrad commented 3 years ago

So onDisconnect would probably work best then if an endpoint cardano.disable() exists as well right? I'll think about the onDisconnect event.

DEADPXLZ commented 3 years ago

Agreed! Listening for disconnections is important as well as it enables more interaction via 3rd party apps and gets users closer to the classic 'account' user journey.

cjkoepke commented 3 years ago

closer to the classic 'account' user journey.

This is the main point, user's should be able to interface with the wallet without necessarily opening the extension.

alessandrokonrad commented 3 years ago

I get the idea, but the reason why you connect the wallet in first place is because you trust the website you visit. Why would you want to disconnect it again?

cjkoepke commented 3 years ago

@alessandrokonrad For instance, I am building an interface that uses NFTs within your wallet to "login" to the website. If the user wants to log out (privacy or other), disconnecting your wallet could theoretically act as a catalyst or action to do so.

Also, what if you change your mind about trusting a website?

rooooooooob commented 3 years ago

Perhaps your view changed of the website, or you only wanted to check it out but would prefer it not have automatic access from now on, depending on how you're implementing your whitelist. It is probably something good to have. Could we start the discussion in the cardano-foundation/CIPs#88 over which events we need to put in the standard? It was an oversight I think as I had included a disconnect event in the Ergo EIP-0012 standard we based the CIP on.

rooooooooob commented 2 years ago

As the initial CIP-30 PR cardano-foundation/CIPs#88 was merged without an events API (since it wasn't finalized), we have put up a new PR cardano-foundation/CIPs#151 to amend it for events. Events-related discussion should take place in that PR.