fix(deposits): stop support-chat clicks from cancelling deposits + confirm-before-cancel + uniform reference#2416
Conversation
A user cancelled a funded bank deposit while trying to report a problem: the cancel button sits next to the support link and fired instantly with no confirmation (PostHog: 'Issues with this transaction?' at 09:03:00, 'Cancel deposit' 11s later), making the arriving wire unmatchable. The drawer also showed the FULL deposit reference while Add Money shows the intentionally shortened 10-char form (some banks cap reference fields; Bridge matches on the partial) — the two-different-codes confusion that sent the user hunting in the drawer to begin with.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughChangesThe PR standardizes deposit-reference shortening, adds confirmation before deposit or request cancellation, and explicitly manages SupportDrawer pointer events during open and closed states. Deposit reference formatting
Cancellation confirmation
SupportDrawer pointer events
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Code-analysis diffPainscore total: 6109.87 → 6111.31 (+1.44) 🆕 New findings (13)
✅ Resolved (13)
📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
…a vaul drawer Root cause of the phantom deposit cancels: vaul sets pointer-events:none on body while the transaction drawer is open, and the SupportDrawer's backdrop/panel never re-enabled pointer events — the whole support overlay was click-transparent, so taps on it fell through to the receipt underneath, where 'Cancel deposit' sits adjacent to the support link and fired irreversibly (PostHog: support link 15:05:47.7 → fall-through click on Cancel deposit 15:05:48.8 → BE cancel 15:05:49.4). Also per code review: single shortDepositReference() helper for the 6 reference sites across both screens, separate confirm-modal visibility from the armed action (noun flashed mid-fade), drop no-op icon props.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/Global/SupportDrawer/__tests__/SupportDrawer.test.tsx (1)
159-174: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the Vaul regression state in this test.
This only checks class strings while the drawer is open; it never simulates Vaul’s
document.body.style.pointerEvents = 'none'state or verifies that closed elements retainpointer-events-none. Add assertions for both open and closed states, ideally with the body pointer-events condition set before rendering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/Global/SupportDrawer/__tests__/SupportDrawer.test.tsx` around lines 159 - 174, Update the SupportDrawer test to set document.body.style.pointerEvents to none before rendering, then assert the open backdrop and panel explicitly use pointer-events-auto. Also exercise the closed state and verify the relevant elements retain pointer-events-none, covering both Vaul’s regression condition and normal closed behavior.src/components/TransactionDetails/provider-actions/CancelDepositActions.tsx (1)
70-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSynchronize the error message with the cancelled item.
Since this PR introduces dynamic terminology (
noun) for requests vs. deposits, you can use it here to ensure the error text matches the user's intended action instead of hardcoding "deposit".✨ Proposed adjustment
- setError("We couldn't cancel this deposit. Please try again or contact support.") + setError(`We couldn't cancel this ${pendingCancel?.noun ?? 'deposit'}. Please try again or contact support.`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TransactionDetails/provider-actions/CancelDepositActions.tsx` at line 70, Update the error message in the cancel action to use the existing dynamic noun terminology instead of hardcoding “deposit,” ensuring the text matches whether the cancelled item is a request or deposit. Preserve the current retry and support guidance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/TransactionDetails/provider-actions/CancelDepositActions.tsx`:
- Line 47: Update the pendingCancel state type and confirmThenRun flow to allow
an absent run action. In confirmThenRun, capture the current action, immediately
clear run while preserving noun, then execute the captured action only once so
repeated confirmation taps cannot start concurrent cancellations.
---
Nitpick comments:
In `@src/components/Global/SupportDrawer/__tests__/SupportDrawer.test.tsx`:
- Around line 159-174: Update the SupportDrawer test to set
document.body.style.pointerEvents to none before rendering, then assert the open
backdrop and panel explicitly use pointer-events-auto. Also exercise the closed
state and verify the relevant elements retain pointer-events-none, covering both
Vaul’s regression condition and normal closed behavior.
In `@src/components/TransactionDetails/provider-actions/CancelDepositActions.tsx`:
- Line 70: Update the error message in the cancel action to use the existing
dynamic noun terminology instead of hardcoding “deposit,” ensuring the text
matches whether the cancelled item is a request or deposit. Preserve the current
retry and support guidance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ad421249-6980-41e0-899d-faba29946e97
📒 Files selected for processing (8)
src/components/AddMoney/components/AddMoneyBankDetails.tsxsrc/components/Global/SupportDrawer/__tests__/SupportDrawer.test.tsxsrc/components/Global/SupportDrawer/index.tsxsrc/components/TransactionDetails/provider-actions/CancelDepositActions.tsxsrc/components/TransactionDetails/provider-actions/__tests__/CancelDepositActions.test.tsxsrc/components/TransactionDetails/provider-rows/BridgeDepositInstructions.tsxsrc/components/TransactionDetails/provider-rows/__tests__/BridgeDepositInstructions.test.tsxsrc/utils/format.utils.ts
… twice CodeRabbit finding: two taps during the modal fade both see the armed action and start concurrent cancellations. Ref guard, not state — a same-frame double-tap lands before any re-render.
Summary
The bug, explained for a stupid 5-year-old 🖍️
The transaction screen has two buttons near each other:
You press the green button and a big support-chat window slides up in front of everything. Looks great. Except the chat window is a hologram: the page underneath had told the browser "ignore fingers" (that's what the transaction drawer does while it's open), and the chat window forgot to say "except me — fingers stop HERE!". So it looks solid, but your finger goes straight through the glass.
Now you tap the chat to type your message… and the browser delivers that tap to whatever is behind the hologram at that exact spot. And what's behind it, right there? The red shred button. Click. Deposit cancelled. You never saw it, you never meant it, and the money that arrives at the bank can no longer be matched to anything.
That's why the fix is not "disconnect the green button from cancel" — there was never a wire between them. The green button was innocent. The fix is two words,
pointer-events-auto: "this glass is solid." Plus a seatbelt: the shred button now always asks "are you REALLY sure?" — so no stray tap can ever shred money in one touch again.Replaces #2415 (closed — wrong diagnosis). Three fixes, grounded in the Valentin case (Crisp session_27a67f97) and reproduced by @abalinda (intent cbc0fd7d-14e9-4a6c-b5be-c54dbe0c49ee):
ROOT CAUSE — "Issues with this transaction?" cancelled deposits via click fall-through. vaul sets
pointer-events: noneon<body>while the transaction drawer is open; theSupportDrawer's backdrop and panel never re-enabled pointer events, so the entire support overlay inheritednoneand was click-transparent. Taps on the visually-open support chat fell through to the receipt underneath — whereCancel depositrenders adjacent to the support link — and fired the un-guarded cancel. PostHog element chains prove it: support link clicked 15:05:47.742 → fall-through click autocaptured onCancel deposit15:05:48.798 (1.06s — not a human aim) → BEuser_cancelled15:05:49.444, withbody[style="pointer-events: none"]captured in both chains. Fixed with explicitpointer-events-autoon the backdrop and panel while open, plus a regression test.Confirmation before any deposit/request cancel (defense in depth — the cancel is irreversible money-state). All three cancel variants (Bridge onramp, Manteca onramp, bank request) now arm a shared
ActionModalconfirmation warning that a sent transfer can no longer be matched after cancelling. Even a fall-through click can now only open the modal, never cancel.Uniform 10-char deposit reference. The Add Money screen intentionally shortens the Bridge reference to 10 chars (some banks, e.g. Wise, cap reference fields; Bridge matches deposits on the partial reference) — but the transaction drawer showed the full 20-char form. Seeing 'two different codes' is what sent the user into the drawer in a panic in the first place. Both screens now route through a single
shortDepositReference()helper (6 call sites), so the lengths can't drift apart again.Before (drawer 20 char; deposit flow transaction screen 10 chars)
Now - 10 chars everywhere + cancel confirmation popup + "Need help with this transaction" doesn't trigger auto-cancel
this is after clicking on the explicit "Cancel deposit"

Design notes / accepted trade-offs
CancelDepositActionsreusing the globalActionModal(same pattern asCancelSendLinkModal) — one modal serves all three cancel branches, parameterized by noun (deposit/request).CancelSendLinkModal,TokenAndNetworkConfirmationModal) — extracting a shared confirm-in-drawer wrapper is a follow-up, out of hotfix scope.Risks / breaking changes
main→ back-merge debt main→dev after merge.QA
CancelDepositActions.test.tsx(3 tests: click ≠ cancel, confirm → cancel, dismiss → untouched),BridgeDepositInstructions.test.tsx(shortened ref in display + copy), and aSupportDrawerregression test asserting backdrop + panel carrypointer-events-autowhile open (the vaul-inheritance trap).