fix(transaction): non-blocking boarding settlement UX#556
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughAdjusted LoadingIcon small sizing and removed the WaitingForRound component, replacing its usages with LoadingLogo/Info banners across wallet and settings screens; transaction settling flow was simplified (new "Settling" banner, gated status UI, and updated status computation). Changes
Sequence Diagram(s)(omitted — changes are UI component replacements and do not introduce multi-actor sequential flows requiring visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 42 minutes and 14 seconds.Comment |
Deploying wallet-bitcoin with
|
| Latest commit: |
ef431f3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://17e15612.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://wt-nonblocking-boarding-2026.wallet-bitcoin.pages.dev |
Deploying wallet-mutinynet with
|
| Latest commit: |
ef431f3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a5deb5e3.arkade-wallet.pages.dev |
| Branch Preview URL: | https://wt-nonblocking-boarding-2026.arkade-wallet.pages.dev |
![arkanaai[bot]](https://avatars.githubusercontent.com/u/170820085?s=60&v=4){kind=small}
There was a problem hiding this comment.
Arkana Code Review — wallet#556
Verdict: Approve ✅
This is a well-scoped UX improvement. The changes are UI-only — no protocol logic, no VTXO handling, no signing paths touched. The settlement itself still goes through the same settlePreconfirmed() → settleVtxos() path in the wallet provider. Safe to merge.
What this PR does right
-
Removes the
setTxInfooptimistic update (Transaction.tsx:96→removed) — this was the most important fix. The old code didsetTxInfo({ ...tx, preconfirmed: false, settled: true })after an asyncsettlePreconfirmed()+sleep(2000). If the user navigated to a different transaction during that ~3+ second window, this would corrupt the displayed transaction by overwritingtxInfoinFlowContextwith stale data. Good catch. -
hideStatusBannersguard (Transaction.tsx:55) — prevents "Settling" and "Pending boarding" / "Expired" banners from rendering simultaneously. Clean approach. -
Adds
defaultButtonLabelto useEffect deps (Transaction.tsx:57) — fixes a React lint warning / stale closure. The old code only depended on[settling], so ifdefaultButtonLabelchanged (theoretically possible ifboardingTxstate changed), the button text would be stale. -
Status priority reorder (Transaction.tsx:112-124) — putting
settleSuccess || tx.settledfirst ensures the "Settled" status shows immediately after settlement completes, rather than still showing "Pending boarding" becausetx.preconfirmedhasn't been cleared yet (since we no longer optimistically updatetxInfo). This is the correct priority order.
Minor observations (non-blocking)
-
LoadingIconsmall size change (Loading.tsx:2, 32→20) — this is a global change to all<LoadingIcon small />consumers. I checked: there are currently zero other call sites usingsmallprop — the only existing consumer isLoading.tsxitself, and this PR introduces the first usage. So this is safe. But worth noting: if anyone adds<LoadingIcon small />elsewhere in the future, they'll get 20px, not 32px. -
WaitingForRoundimport left in master (Transaction.tsx:16) — the PR removes the import, which is correct. The component is still imported inVtxos.tsxfor the rollover flow, so the component file itself shouldn't be deleted. No dead code issue. -
No error reset on success — after
setSettleSuccess(true), if there was a previous error from a failed attempt, both the error banner and the success banner could render simultaneously. ThesetError('')at the top ofhandleSettlehandles this for retries, but if somehow the flow gets intosettleSuccess=trueANDerror!=''(shouldn't happen with current code), both would show. Non-issue in practice. -
No tests —
Transaction.tsxhas no test file. This is pre-existing tech debt, not introduced by this PR.
Protocol safety
settlePreconfirmed()is unchanged — still callssettleVtxos()in the wallet provider- No changes to signing, VTXO selection, forfeit paths, or round lifecycle
- The removal of
setTxInfois strictly safer (removes a race condition) - This is purely UI/UX — no protocol-critical review needed
LGTM. Ship it.
🤖 Reviewed by Arkana
Deploying tmp-boltz-upstream-mainnet-arkade-wallet with
|
| Latest commit: |
ef431f3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://25d65892.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
| Branch Preview URL: | https://wt-nonblocking-boarding-2026.tmp-boltz-upstream-mainnet-arkade-wallet.pages.dev |
|
@sahilc0 we should do the same on the other usage of WaitingForRound (i.e. Settings > Coin Control) and remove the WaitingForRound component after that. |
bordalix
left a comment
bordalix
left a comment
There was a problem hiding this comment.
Do the same on the other usage of WaitingForRound and remove component.
Extends the non-blocking settlement UX to remaining usages: - Vtxos.tsx (Coin Control): Replace blocking WaitingForRound with inline "Renewing" Info banner during rollover - Send/Details.tsx: Replace WaitingForRound with LoadingLogo for mainnet payments (matches Lightning/Ark payment patterns) - Delete WaitingForRound component now that all usages are removed Addresses bordalix's review comment on PR #556.
|
@bordalix Done! Extended the non-blocking UX to Coin Control (Vtxos.tsx) and cleaned up Send/Details.tsx. The WaitingForRound component is now deleted entirely. Changes in this commit:
Let me know if this looks good! |
|
@sahilc0 please rebase. This PR is still using ionic. |
…ng banner The full-screen WaitingForRound animation blocks all interaction for 1+ minute when completing boarding. This replaces it with an inline purple Info banner that shows "Processing your boarding transaction..." while settlement runs, allowing users to navigate freely. Changes: - Remove WaitingForRound blocking overlay during settlement - Add inline settling banner using Info component with LoadingIcon - Fix LoadingIcon small size (32px -> 20px) to match other inline icons - Add guards to prevent conflicting UI states (settled + pending banners) - Remove stale setTxInfo callback that could corrupt navigation state
Extends the non-blocking settlement UX to remaining usages: - Vtxos.tsx (Coin Control): Replace blocking WaitingForRound with inline "Renewing" Info banner during rollover - Send/Details.tsx: Replace WaitingForRound with LoadingLogo for mainnet payments (matches Lightning/Ark payment patterns) - Delete WaitingForRound component now that all usages are removed Addresses bordalix's review comment on PR #556.
sahilc0
force-pushed
the
wt-nonblocking-boarding-20260422
branch
from
946928f to
ef431f3
Compare
|
@bordalix Rebased this onto current master and pushed the updated branch. The WaitingForRound usage in Coin Control is removed, the component is deleted, and local lint/unit/build are passing. Cloudflare + normal CI are green; the Playwright regtest job was still running last I checked. Ready for another look. |
2 participants
Summary
When users press "Complete boarding" on a pending boarding transaction, the app currently shows a full-screen
WaitingForRoundanimation that blocks all interaction for 1+ minute. This PR replaces that blocking UI with an inline settling banner, allowing users to navigate freely while settlement processes in background.Changes:
WaitingForRoundfull-screen blocking overlayInfocomponent withLoadingIconLoadingIconsmall size from 32px to 20px to match other inline iconssetTxInfocallback that could corrupt navigation if user views another tx during settlementTest plan
Files changed
src/icons/Loading.tsx- Fix small icon sizesrc/screens/Wallet/Transaction.tsx- Replace blocking UX with inline bannerSummary by CodeRabbit
Bug Fixes
Improvements