fix: copy button copies unified BIP21 URI immediately#579
Conversation
Fixes #573. The Copy button now copies the BIP21 URI to clipboard on click, rather than only opening the sheet. The sheet still opens so users can explicitly tap a specific address to override. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Deploying tmp-boltz-upstream-mainnet-arkade-wallet with
|
| Latest commit: |
7429078
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://12f58a2c.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
| Branch Preview URL: | https://fix-copy-button-copies-bip21.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
Deploying wallet-bitcoin with
|
| Latest commit: |
7429078
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3e8fcc95.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://fix-copy-button-copies-bip21.wallet-bitcoin.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
7429078
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e7b207de.arkade-wallet.pages.dev |
| Branch Preview URL: | https://fix-copy-button-copies-bip21.arkade-wallet.pages.dev |
There was a problem hiding this comment.
Code Review — #579
🐛 Bug: Sheet race condition — copy sheet opens then immediately closes
src/screens/Wallet/Receive/QrCode.tsx:462
The new onClick handler:
onClick={() => { handleCopy(bip21Uri || qrCodeValue); setShowCopySheet(true); }}handleCopy is async (line 324). It awaits copyToClipboard(value), then unconditionally calls setShowCopySheet(false) at line 328. The execution order is:
handleCopy(...)begins, hitsawait copyToClipboard(value), suspendssetShowCopySheet(true)runs → sheet openscopyToClipboardresolves (near-instant — clipboard API is a microtask)handleCopyresumes →setShowCopySheet(false)→ sheet closes
The sheet flashes open then immediately closes. This directly contradicts the PR description: "The address selection sheet still opens so users can explicitly tap a specific address to override." Users will see a flicker, not a usable sheet.
Fix: Don't call handleCopy from the Copy button. Instead, copy the BIP21 URI directly without going through handleCopy (which was designed for the sheet's individual address rows and closes the sheet on completion):
onClick={async () => {
if (!prefersReducedMotion) hapticSubtle()
await copyToClipboard(bip21Uri || qrCodeValue)
toast('Copied to clipboard')
setCopied(bip21Uri || qrCodeValue)
setShowCopySheet(true)
}}Or refactor handleCopy to accept an option for whether to close the sheet.
⚠️ Minor: Copy button can copy empty string
src/screens/Wallet/Receive/QrCode.tsx:462
The <ButtonsOnBottom> block is rendered unconditionally — it's outside the loading/error conditional (lines 402-455). If addresses haven't loaded yet, both bip21Uri and qrCodeValue are '' (initialized on lines 88-89). Tapping Copy would copy an empty string to clipboard and show a "Copied to clipboard" toast, which is misleading.
Low severity, but worth a disabled={!bip21Uri && !qrCodeValue} guard on the Copy button (similar to how shareDisabled works for the Share button).
✅ Test coverage
The new test (line 342-354) correctly asserts the clipboard receives a bitcoin: URI containing the ark address. However, it does not verify the sheet remains open after the copy, which is exactly the bug above. Consider adding:
expect(screen.getByText('Copy address')).toBeInTheDocument() // sheet titleafter the copy assertion to catch the race.
✅ No protocol-critical concerns
This PR only touches UI copy-to-clipboard behavior. No VTXO handling, signing, forfeit paths, or round lifecycle code is affected. No cross-repo API surface changes.
Summary: One functional bug (sheet race), one minor UX issue (empty copy). The intent is good — copying BIP21 on tap is the right UX — but handleCopy's setShowCopySheet(false) creates a race that defeats the stated behavior. Requesting changes on the race condition fix.
Fixes #573.
Summary
Test plan
bitcoin:)pnpm test— new unit test passes🤖 Generated with Claude Code