Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update/wallet connect #493

Merged
merged 6 commits into from
Dec 20, 2023
Merged

Update/wallet connect #493

merged 6 commits into from
Dec 20, 2023

Conversation

ccali11
Copy link
Contributor

@ccali11 ccali11 commented Dec 19, 2023

Made a few meaningful updates to our WalletConnect integration:

  • removed unnecessary code
  • handle multiple edge cases (such as user starts a session but isn't signed in)
  • handle account change in the app
  • gracefully end connected sessions on log out
  • assure a session is active when attempting to Stake / Withdraw (prompt to connect if not)
  • etc.

Made these changes in mvp. They can be ported over to app when we're ready to enable the WalletConnect integration there.

@ccali11
Copy link
Contributor Author

ccali11 commented Dec 20, 2023

Completes #494

@@ -40,7 +40,7 @@ const { getEthersLedgerAddresses } = useLedger()
const { getEthersTrezorAddresses } = useTrezor()
const { user } = useUser()
const { detectActiveNetwork, switchEthersNetwork } = useWallets()
const { connectWalletConnectV2 } = useWalletConnect()
const { connectWalletConnectV2, walletConnectSelectedAccount } = useWalletConnect()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally curious, would it ever make sense to do something like:

const walletConnect = useWalletConnect()
...
watch(walletConnect.selectedAccount, () => {
...
}

composableName.someVariableOrMethodName might be nice for organization (and less redundancy).

Not suggesting this for the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool pattern.

apps/mvp/src/composables/walletConnectV2.ts Outdated Show resolved Hide resolved
@ccali11 ccali11 merged commit 19bf2e9 into develop Dec 20, 2023
1 check passed
@ccali11 ccali11 deleted the update/wallet-connect branch December 20, 2023 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants