search

safe-global / safe-wallet-web

Safe{Wallet} – smart contract wallet for Ethereum (ex-Gnosis Safe multisig)
https://app.safe.global
GNU General Public License v3.0
314 stars 372 forks source link

Incomplete Disconnect Functionality on Safe Global Web UI #3918

Open mshakeg opened 1 week ago

mshakeg commented 1 week ago

Bug description

When clicking the "Disconnect" button on the Safe Global web UI, the account does not fully disconnect. Instead, it only clears the connected account from the UI. Upon attempting to reconnect via MetaMask, the previously disconnected account immediately shows up without prompting MetaMask to select an account, leading to a poor user experience.

Environment

Steps to reproduce

  1. Go to the Safe Global web UI at https://app.safe.global
  2. Connect your wallet using MetaMask.
  3. Click the "Disconnect" button.
  4. Attempt to reconnect via MetaMask.

Expected result

Clicking the "Disconnect" button should fully disconnect the account from the UI. Upon attempting to reconnect, MetaMask should prompt the user with the "Connect with MetaMask" popup, allowing the user to reselect the account(s) to connect to Safe Global.

The Uniswap web UI at https://app.uniswap.org provides a good working example of this desired behavior. When clicking "Disconnect," it fully disconnects the account on the UI. Upon reconnection, MetaMask prompts the "Connect with MetaMask" popup, enabling the user to select the account(s) to connect.

Obtained result

Clicking the "Disconnect" button only clears the connected account from the UI. When attempting to reconnect via MetaMask, the previously disconnected account immediately appears without the MetaMask prompt, requiring the user to manually delete app.safe.global from MetaMask's connected sites list to reconnect a different account.

katspaugh commented 1 day ago

I briefly looked into it, and this seems to be an inherent feature of onboard.js and is not configurable on our side. This might change with the new Metamask onboard module (#3841).

Update: #3946 does solve this. ✅

mshakeg commented 1 day ago

@katspaugh thanks for looking it to it, but why does the Uniswap web ui not have this issue?

katspaugh commented 1 day ago

They use a different wallet connector library. Like I said #3946 fixes it, so please feel free to follow that.