Skip to content

fix(connect-modal): close stuck dapp-kit WC QR modal after handshake#630

Merged
Agilulfo1820 merged 1 commit into
mainfrom
fix/wallet-connect-qr-modal-stuck-open
May 25, 2026
Merged

fix(connect-modal): close stuck dapp-kit WC QR modal after handshake#630
Agilulfo1820 merged 1 commit into
mainfrom
fix/wallet-connect-qr-modal-stuck-open

Conversation

@Agilulfo1820

@Agilulfo1820 Agilulfo1820 commented May 25, 2026

Copy link
Copy Markdown
Member

Summary

Fixes a WalletConnect bug introduced with the custom connect modal in #611.

When a user picked WalletConnect from vechain-kit's own modal, scanned the QR with their phone and approved, the kit reflected the connection in the background (button showed the connected address/domain) — but dapp-kit-ui's QR modal stayed open on top, and clicking its X disconnected the user we'd just connected.

Root cause

setSource('wallet-connect') + connect() causes dapp-kit's signer to call CustomWalletConnectModal.openModal({ uri }), which fires the vdk-open-wc-qrcode event and pops dapp-kit-ui's own <vdk-connect-modal> to display the QR.

That modal only auto-closes when the user clicked through dapp-kit-ui's own onSourceClick handler — it calls n.modal.close() after wallet.connect() resolves (gated by alwaysShowConnect, which we have on). Because we drive the connect from useConnectWithDappKitSource, that path never runs and the QR modal stays up.

And the modal's X button triggers:

this.handleClose = () => {
    n.modal.close(),
    this.walletConnectQRcode && setTimeout(() => {
        this.handleBack(),
        this.wallet.disconnect()   // ← disconnects the user
    }, 500),
    ...
};

Since walletConnectQRcode is still set after a successful handshake, the X disconnects the freshly-connected wallet.

Using useDAppKitWalletModal() directly avoids this because the user-click goes through dapp-kit-ui's onSourceClick, which does call n.modal.close() post-handshake.

Fix

After dappKitConnect() resolves for wallet-connect, call useDAppKitWalletModal().close(). That dispatches vdk-close-wallet-modal, whose only listener sets open = false — no disconnect side effect, since the disconnect path lives inside handleClose, not the close-event listener.

Test plan

  • WalletConnect: open vechain-kit connect modal → click WalletConnect → QR modal appears → scan with VeWorld mobile → approve → both modals close automatically, button shows connected address
  • WalletConnect: open vechain-kit connect modal → click WalletConnect → close the QR modal with X before scanning → modal closes, no connection established (existing behavior preserved)
  • VeWorld: unchanged flow still works (extension prompt, no dapp-kit-ui modal involved)
  • Sync2: unchanged flow still works
  • loginMethods=['dappkit'] legacy path still routes through useDAppKitWalletModal() directly and behaves as before

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where the QR modal for WalletConnect would remain open after a successful connection attempt, which could interfere with disconnect functionality.

Review Change Stack

When connecting via WalletConnect through vechain-kit's custom modal,
`setSource('wallet-connect') + connect()` causes dapp-kit's signer to
call `CustomWalletConnectModal.openModal({uri})`, which fires
`vdk-open-wc-qrcode` and pops dapp-kit-ui's own `<vdk-connect-modal>`
with the QR on top of ours. That modal only auto-closes when the user
clicks through dapp-kit-ui's own source picker (its `onSourceClick`
calls `n.modal.close()` after `wallet.connect()` resolves). Since we
drive the connect from our hook, that path never runs and the QR
modal stays up post-handshake. Worse, the X button's `handleClose`
calls `wallet.disconnect()` whenever `walletConnectQRcode` is set —
so the user's only out also disconnects them.

Fix: after `dappKitConnect()` resolves for `wallet-connect`, call
`useDAppKitWalletModal().close()`. That dispatches
`vdk-close-wallet-modal` (only sets `open=false`; no disconnect side
effect, since disconnect lives inside `handleClose`, not the
close-event listener).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 25, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a22c081c-aa53-45ed-86f9-47cce39e46c7

📥 Commits

Reviewing files that changed from the base of the PR and between 66b6ab2 and ea92487.

📒 Files selected for processing (1)
  • packages/vechain-kit/src/hooks/login/useConnectWithDappKitSource.ts

📝 Walkthrough

Walkthrough

The hook useConnectWithDappKitSource now retrieves the dapp-kit modal close function and calls it immediately after initiating a WalletConnect connection. This prevents the QR modal from remaining open after connection, which was affecting subsequent disconnect behavior. The useCallback dependency array was updated to include the new modal-closing capability.

Changes

WalletConnect QR Modal Closing

Layer / File(s) Summary
Modal close implementation and dependency wiring
packages/vechain-kit/src/hooks/login/useConnectWithDappKitSource.ts
closeDappKitModal is extracted from useDAppKitWalletModal at the hook level, then called within the WalletConnect connection branch to close the QR modal after the connection attempt, with the dependency array updated to reflect this new capability.

🎯 2 (Simple) | ⏱️ ~15 minutes

Possibly related PRs

  • vechain/vechain-kit#611: Both PRs touch the same dapp-kit connection flow behind the ConnectModal; this PR adds explicit QR modal closing to useConnectWithDappKitSource, while PR #611 introduces/uses this hook as part of the new custom ConnectModal UI for WalletConnect.

Suggested reviewers

  • victorkl400
  • mikeredmond

Poem

🐰 A modal that lingered when it should have fled,
Now closes with grace after WalletConnect's thread,
The QR flickers out, just as intended to be—
One hook, one branch, one fix: modal harmony! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'fix(connect-modal): close stuck dapp-kit WC QR modal after handshake' directly and clearly summarizes the main change: closing a stuck WalletConnect QR modal after connection handshake completes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/wallet-connect-qr-modal-stuck-open

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

🚀 Playground preview deployed!

Preview URL: https://preview.vechainkit.vechain.org/fixwallet-connect-qr-modal-stuck-open/playground

@github-actions

Copy link
Copy Markdown
Contributor

Size Change: +2.48 kB (+0.03%)

Total Size: 8.84 MB

Filename Size Change
packages/vechain-kit/dist/index-BjQrND59.d.mts 0 B -5.63 kB (removed) 🏆
packages/vechain-kit/dist/index-BjQrND59.d.mts.map 0 B -2.99 kB (removed) 🏆
packages/vechain-kit/dist/index-C8Uj8ple.d.cts 0 B -5.63 kB (removed) 🏆
packages/vechain-kit/dist/index-C8Uj8ple.d.cts.map 0 B -2.99 kB (removed) 🏆
packages/vechain-kit/dist/index-DbtVLVA5.d.cts 0 B -185 kB (removed) 🏆
packages/vechain-kit/dist/index-DbtVLVA5.d.cts.map 0 B -50.3 kB (removed) 🏆
packages/vechain-kit/dist/index-DqmXn4Mz.d.mts 0 B -185 kB (removed) 🏆
packages/vechain-kit/dist/index-DqmXn4Mz.d.mts.map 0 B -49.2 kB (removed) 🏆
packages/vechain-kit/dist/index.cjs.map 2.83 MB +1.19 kB (+0.04%)
packages/vechain-kit/dist/index.mjs.map 2.77 MB +1.18 kB (+0.04%)
packages/vechain-kit/dist/index-BYfc-9J5.d.mts 5.63 kB +5.63 kB (new file) 🆕
packages/vechain-kit/dist/index-BYfc-9J5.d.mts.map 2.99 kB +2.99 kB (new file) 🆕
packages/vechain-kit/dist/index-DEhBh9oZ.d.cts 185 kB +185 kB (new file) 🆕
packages/vechain-kit/dist/index-DEhBh9oZ.d.cts.map 50.3 kB +50.3 kB (new file) 🆕
packages/vechain-kit/dist/index-dIepdPhU.d.mts 185 kB +185 kB (new file) 🆕
packages/vechain-kit/dist/index-dIepdPhU.d.mts.map 49.2 kB +49.2 kB (new file) 🆕
packages/vechain-kit/dist/index-nRf3KRll.d.cts 5.63 kB +5.63 kB (new file) 🆕
packages/vechain-kit/dist/index-nRf3KRll.d.cts.map 2.99 kB +2.99 kB (new file) 🆕
ℹ️ View Unchanged
Filename Size Change
packages/vechain-kit/dist/assets 4.1 kB 0 B
packages/vechain-kit/dist/assets-C0RHiZ9a.mjs 51.4 kB 0 B
packages/vechain-kit/dist/assets-C0RHiZ9a.mjs.map 74.6 kB 0 B
packages/vechain-kit/dist/assets-CZs6EVH8.cjs 58.9 kB 0 B
packages/vechain-kit/dist/assets-CZs6EVH8.cjs.map 76.1 kB 0 B
packages/vechain-kit/dist/assets/index.cjs 716 B 0 B
packages/vechain-kit/dist/assets/index.d.cts 973 B 0 B
packages/vechain-kit/dist/assets/index.d.mts 973 B 0 B
packages/vechain-kit/dist/assets/index.mjs 718 B 0 B
packages/vechain-kit/dist/index.cjs 1.14 MB +61 B (+0.01%)
packages/vechain-kit/dist/index.d.cts 24.5 kB 0 B
packages/vechain-kit/dist/index.d.mts 24.5 kB 0 B
packages/vechain-kit/dist/index.mjs 1.1 MB +43 B (0%)
packages/vechain-kit/dist/utils 4.1 kB 0 B
packages/vechain-kit/dist/utils-C4gc1L9t.cjs 27.4 kB 0 B
packages/vechain-kit/dist/utils-C4gc1L9t.cjs.map 68.2 kB 0 B
packages/vechain-kit/dist/utils-DPIscp9_.mjs 22.2 kB 0 B
packages/vechain-kit/dist/utils-DPIscp9_.mjs.map 67.5 kB 0 B
packages/vechain-kit/dist/utils/index.cjs 2.02 kB 0 B
packages/vechain-kit/dist/utils/index.d.cts 3.1 kB 0 B
packages/vechain-kit/dist/utils/index.d.mts 3.1 kB 0 B
packages/vechain-kit/dist/utils/index.mjs 2.04 kB 0 B

compressed-size-action

@Agilulfo1820 Agilulfo1820 merged commit 8a3ffaf into main May 25, 2026
9 checks passed
@Agilulfo1820 Agilulfo1820 deleted the fix/wallet-connect-qr-modal-stuck-open branch May 25, 2026 09:00
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.

1 participant