safe-global / safe-wallet-web

Safe{Wallet} – multisig EVM wallet
https://app.safe.global
GNU General Public License v3.0
349 stars 414 forks source link

Refactor `CheckWallet` to split up conditions #4210

Open usame-algan opened 1 month ago

usame-algan commented 1 month ago

What is the feature about

The CheckWallet component is a critical component that wraps almost every button in the app to decide whether that button should be enabled or not. It checks for several conditions like the user wallet, the safe setup and multiple permissions like delegates and spending limits. This component grew over time, has a high complexity and is hard to extend and maintain.

In some cases we have buttons where we only want to ensure that any wallet is connected. In order to achieve this we have additional flags that are passed to CheckWallet like allowNonOwner

The list of requirements

I suggest to split the condition up into multiple hooks that are composed and conditionally used inside CheckWallet e.g.

const CheckWallet = ({
  checkWalletConnection = true,
  checkSafeStatus = true,
  checkPermissions = true,
}) => {
  const walletMessage = useWalletCheck(checkWalletConnection)
  const safeStatusMessage = useSafeStatusCheck(checkSafeStatus)
  const permissionMessage = usePermissionCheck(checkPermissions)

  const message = walletMessage || safeStatusMessage || permissionMessage

  const isDisabled = Boolean(message)

  return ...
}

With the above and wanting to skip permission and safe status check it would look like this:

<CheckWallet checkSafeStatus={false} checkPermissions={false}>...</CheckWallet>

Furthermore, having to change something about permissions would only touch a single hook instead of the whole component where the rest of the logic lies.

Designs/sketches

Current conditions

Screenshot 2024-09-19 at 13 49 09
usame-algan commented 1 month ago

Another idea: If we can determine that there are a fix amount of variants we could also split up CheckWallet into multiple components like CheckWalletConnected etc. and use those more specific components instead of one big one.