Concordium / concordium-dapp-libraries

A coherent set of building blocks for making it as easy as possible for developers to build web-based dApps.
Apache License 2.0
7 stars 5 forks source link

Update wc modal #65

Closed soerenbf closed 5 months ago

soerenbf commented 5 months ago

Purpose

Deep linking does not work with the deprecated @walletconnect/qrcode-modal. This PR aims to update the dependency. Additionally, I've added a method on the wallet connect connector for defining the scope of the connection made to a wallet (connectWithScope), as it seems there is not feature parity with regards to the supported wallet connect methods.

This is already used in the published alpha version in the gc voting application (added in https://github.com/Concordium/concordium-governance-committee-voting/pull/115).

Checklist

soerenbf commented 5 months ago

I strongly believe that adding another connect method is the wrong way to solve this. Now clients have to care what concrete type of connector they're interacting with, which is exactly what the WalletConnector interface is there to hide.

There's never a need of being able to use the same connector to connect in different ways; just create another connector then. So it would be much simpler IMHO if WalletConnectConnector kept WalletConnectModalConfig etc. as its state, passed via its constructor. Then the existing connect method just uses that state.

Constants, convenience functions etc. can then be used to create these values in standardized ways before creating the connector.

This is just like how WalletConnectConnector.create already accepts SignClientTypes.Options.

Good point, will revise the implementation to follow this instead 🙂