argentlabs / argent-x

ArgentX browser extension for StarkNet - made with ❤️ by Argent
https://github.com/argentlabs/argent-x
Other
625 stars 276 forks source link

Disconnect feature #262

Closed delaaxe closed 2 years ago

delaaxe commented 2 years ago
jgresham commented 2 years ago

taking a look

janek26 commented 2 years ago

should be just a change in the example dapp jediswap already has this: https://app.testnet.jediswap.xyz/#/swap

jgresham commented 2 years ago

After disconnecting, and then reconnecting, should the user be prompted with this again? (This is not how JediSwap works currently. JediSwap reconnects without requiring this.)

reconnect

Related, but could be in a new issue, I think with #90 implemented, allowing disconnect to a dapp from extension, there needs to be an event passed to the dapp to know that the user disconnected the dapp from the extension.

juniset commented 2 years ago

Yes, if the user disconnects and reconnects from a dapp he should see the "Connect to a dapp" prompt.

Agree that when the user disconnects from a dapp in the extension an event should be passed to the dapp. We already have the accountsChanged event so we should use the same event but with an empty array. I'll create a separate issue.

delaaxe commented 2 years ago

@juniset We should probably implement the rest of the standard events like 'chainChanged', 'connect', 'disconnect': https://docs.metamask.io/guide/ethereum-provider.html#events

jgresham commented 2 years ago

It seems like the dapp whitelist and if a dapp is connected are separate concepts in current metamask specs.

delaaxe commented 2 years ago

I agree with @jgresham here, on JediSwap clicking on disconnect doesn't remove it from the whitelist

jasonzhouu commented 2 years ago

How about this:

So, when the user disconnects from a dapp, the dapp should stay in the whitelist, and ArgentX record that this dapp has disconnected in the last time.

delaaxe commented 2 years ago

yes this should make more sense

dhruvkelawala commented 2 years ago

I agree with @jgresham here, on JediSwap clicking on disconnect doesn't remove it from the whitelist

Hi, I am leading frontend at JediSwap and I can confirm that we are not "disconnecting" from ArgentX in a proper way. We are just doing a soft disconnect, meaning just removing the currently connected account from the frontend.

We were waiting for a method on Starknet Window Object to implement this method, such as window.starknet.disconnect() which can support removing from the whitelist.

EDIT: SushiSwap also does the same thing as JediSwap when clicking disconnect.

janek26 commented 2 years ago

should be solved by https://github.com/argentlabs/argent-x/pull/378 please reopen if that's not the case