feat(ui): rework the external send flow - #951
Closed
romchornyi wants to merge 19 commits into
Closed
Conversation
SendScreen.swift held every step of the external send — the address screen, the source picker, the amount keypad, the confirm sheet — plus their shared chrome and helpers. Each now has its own file under Send/, with the pieces two or more steps share in Send/Components/. Nothing changed but location. The helpers and the two chrome views were file-scope `private`, which the split cannot preserve, so they are internal now; SendViewModel moves in beside the screens it drives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SendViewModel could not be built in a canvas: its initializer registers an observer on SyncingActivityMonitor and subscribes to two balance publishers, and isChainSynced read the monitor from a property initializer, which runs before any flag could stop it. That read moves into init, and an unwired initializer skips the registrations entirely — the screens only render published state, so an unwired model draws them exactly as a live one does. Previews cover what each step actually looks like: an empty address field and a decoded one, the sync gate, the header-less embed, a pinned source, and a shielded source whose route leaves the classic payment processor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The step header, the address summary in each of its three destination forms, and the chain-styling helpers rendered side by side — that last one exists so the source and destination label sets cannot drift apart unnoticed, which is only visible when they are shown together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SendStepHeader drew its own back circle and centred title with hardcoded colours and metrics — a second implementation of the bar the rest of the redesign already uses. The source and amount steps now take DashUIKit.NavigationBar with the standard back element, which carries its own height and insets, so the manual padding goes with it. The file that held it kept only SendAddressSummary, so it is named after that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The hand-rolled field carried its own label row, TextField styling, error copy and a separate full-width "Scan QR" button. AddressFieldView already has all of it, including the scan control inside the field — so the button below goes, and the two mutually exclusive error messages collapse into the one error slot the component offers. The badge naming the decoded destination moves into the component's new label accessory, so the label is drawn once instead of the screen printing its own row above the field. Two things stay here. The collapsed card a decoded address folds into has no counterpart in the component, so the field keeps both branches. And the clipboard chip stays instead of the component's Paste button: it shows which address it would paste and what kind it is, and only appears when the clipboard actually holds a valid one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The field folded into a truncated one-line card once an address decoded, and tapping it reopened the editor. That was the reason the screen kept two branches, a locked flag and an editing flag, and three onChange handlers to move between them. The address now stays in AddressFieldView, always. One behaviour survives: focus still drops when the address becomes valid, so Continue is reachable without a second tap. It no longer has anything to do with locking, and the comment says so. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The clipboard suggestion card is replaced by AddressFieldView's own Paste button, which the component shows only while the field is empty. Paste now takes whatever is on the clipboard rather than only a pre-detected address: the previous action came from the suggestion, so with unparseable content the button would have sat there doing nothing. The field's validation already explains what is wrong with what landed in it. The detection itself stays — the "Send to Address" shortcut still opens the screen prefilled when the clipboard holds a valid address. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The source and amount steps opened with a boxed Address control carrying a pencil, but by then the address is settled and neither step can change it — tapping only went back, which the navigation bar's back button already does. It read as a form where it was context, and cost the top of the screen. It is now a single line, the way the gift-card screens head themselves with the merchant: "to <address>" with the badge naming what kind of address it turned out to be. Truncation stays in the middle — the tail is what tells two addresses of the same form apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The amount step showed where the money was going but not where it was coming out of — the source was chosen a screen earlier and then never mentioned again, so the amount had nothing to be measured against without going back. SendAddressSummary gains an optional source line: which balance, and how much is in it. Optional because the source step below it already says both while the choice is still open; only the amount step, where it is settled, turns it on. The number honours DWGlobalOptions.balanceHidden — the same flag the home balance reads — so hiding in one place hides in both, and the eye here toggles that one flag rather than a second local notion of hidden. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ep too The source step sent Transparent → Transparent straight to the classic L1 amount screen and every other route to the redesigned one, so the route most people use was the only one that looked different. Nothing new was needed to route it: the amount step's Send already hands a core route to `continueCore`, and the view model's `canContinue` already has its Transparent branch, gating on balance alone because the L1 fee rides on top and the payment processor refuses an unfundable send itself. Only the shortcut in the navigation stood in the way. The processor still finishes the send and still owns the fee math — it just receives an amount now instead of zero, so it opens at confirmation rather than at an amount screen the user has already filled in. Comments on all three steps described the old split; they describe this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Continue let a Core-funded amount through on `coreBalanceDuffs` — the wallet total, which counts unconfirmed and immature coins and leaves nothing for the fee. Max has always filled in something stricter: spendable minus the send fee reserve. So amounts existed that Max would never offer, that armed the button anyway, and that only failed later at the payment processor. Reaching that window used to need the shielded route; routing Transparent through this step opened it to the common case. Both Core-funded routes now gate on the number Max uses, and Transparent gets the dust floor the classic amount screen enforces — below it the network drops the output whatever the balance says. The validation message follows the same number. Its own comment promises a button disabled on affordability is never left unexplained, and quoting the total there would have reported an amount as affordable while Continue stayed dead. Balances shown to the user are unchanged: the cards and the header still say what is in the wallet. Only what may be spent right now moved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Transparent → Transparent still left the redesigned flow at the last step: it handed the amount to the legacy payment processor, which then showed its own confirmation. Three screens looked one way and the fourth another, on the route most people use. It now presents SendConfirmSheet like every other route. A small controller drives the Core leg rather than ShieldedTransferCoordinator, whose phases — locking, proving, asset-lock polling — a plain L1 send never passes through; its checklist is the two stages this send actually has. The sheet prepares on appear through WalletSendService, which runs the auth gate and signs without broadcasting, and Confirm calls broadcast() on the signed transaction. That ordering is not new: DWPaymentProcessor already prepares before showing its confirmation, so the fee shown is the real one off a signed transaction rather than an estimate — the only route here that can say that. Retrying a failed broadcast re-sends the same signed transaction; a result the relay accepted but could not confirm stays terminal, as PreparedStandardSend's own claim state and every other route already treat it. continueCore had one caller and is gone with it. Scan routing and the "Send to Address" shortcut prefill the view model and come through the amount step, so neither reached it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every Transparent send failed on the first attempt. The sheet prepared in its own `.task`, so the auth gate asked for the PIN while the sheet was still animating in; UIKit refuses that presentation and returns nothing, and `PinPromptPresenter`'s watchdog resolved it as an authentication failure. The log reads: preparing standard send → presentation rejected → authentication failed. Tapping "Try again" then worked, because by then the sheet had settled. The amount step signs first and presents the sheet only once it holds a signed transaction — the order DWPaymentProcessor has always used, and there is no sheet in the way when the PIN goes up. The sheet still shows the real fee off that transaction and still broadcasts only on Confirm. Backing out of the PIN now returns to idle rather than failed: it is a decision, not an error, and it left the amount step showing "Authentication cancelled" as though something had gone wrong. A signature that genuinely could not be produced does say so there, since Continue would otherwise look broken. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Receive / Internal / Send switcher was fifty lines inside PaymentsLandingScreen's body, reaching into the view model for both the tab list and the selection. It takes the tabs and a binding now. Which tabs are offered is the presentation's decision — the full landing shows three, the balance-row receive sheet narrows to two — and the switcher has no reason to know why. That also makes it previewable: all three, narrowed, and dark, where the selected pill switches fill so the label stays legible. Metrics move into a Layout enum rather than staying loose in the body. The screen's colorScheme environment value went with the switcher — nothing else was reading it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Was a segmented control: three equal cells in a filled box, the selected one a white pill with a shadow, all three labelled and all three the same size. The design is a row of capsules. The active tab takes the space that is left, fills with its own colour — green for Receive, light blue for Internal transfer, blue for Send — and is the only one that carries a label; the other two shrink to a fixed 60pt icon. Colours are existing tokens: gray300Alpha20, green, lightBlue, blue map straight onto the ones the design names. The glyphs are not — the icon set has an arrow-down but no arrow-up, and the design's are solid variants — so the three are exported from Figma as template assets, with fill-opacity stripped since a baked one survives tinting and washes the glyph out. Each keeps its own width and height: the arrows and the transfer glyph have different aspect ratios and a shared square frame would squash them. Icon and accent move onto PaymentsLandingTab, next to the title, so a tab describes itself in one place. Internal's label becomes "Internal transfer", which the screen header already said — and the header now reads the tab's own title rather than repeating the switch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The gap was written for the boxed segmented control, which brought its own padding and a visible edge; the capsules that replaced it have neither, so twelve points left them sitting against the close button. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two things the screenshots showed. Figma exports one arrow glyph and flips it vertically for the up variant (-scale-y-100 in the reference). Downloading it under the name arrow-up left the flip behind, so Send pointed down — on its own blue pill and on the collapsed one beside Receive. The flip is baked into the asset rather than applied at the call site, so the name says what the file draws. The stack's spacing was chosen by the active tab — 12 where a tab embeds a full form, 20 otherwise — which spaced the header from the pills too. Switching tabs therefore moved both by 8pt. The tightening belongs below the selector, where the form that needs the room actually is; above it the gap is fixed and reads the same on every tab. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Switching tabs cut instantly — the active pill jumped to its new width and colour and the content underneath was replaced in the same frame, so a change that has an obvious direction had none. The pill morphs: width, fill and the label that only the active one carries, which fades in while the capsule is still growing rather than landing at full width against a moving edge. The animation is scoped to the selector instead of wrapping the binding write, so it does not silently animate everything else that observes the selection. The content cross-fades, and briefly — two of these tabs carry a keypad and a scroll view, where a long fade reads as slow rather than smooth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
llbartekll
requested changes
Aug 11, 2026
llbartekll
left a comment
llbartekll
left a comment
Contributor
There was a problem hiding this comment.
Two functional issues in the new Core → Core confirmation path:
-
CoreSendConfirmController.retryreusespreparedafter any non-unknown broadcast failure. That object captures a single-shotFinalizedCoreTransaction; oncebroadcastActionhas run, a rejection/throw consumes it (the contract inPreparedStandardSendandSwiftDashSDKTransactionSender.broadcastexplicitly says it must be rebuilt). As a result, tapping “Try again” after a relay rejection/local broadcast error just hits the SDK’s already-consumed error. Please retain the prepared send only for failures before broadcast (such as the online precheck), or re-prepare after a consumed attempt. Unknown outcomes should remain terminal. -
For Core → Core, the confirmation sheet shows the exact fee but
Totalis still onlydashDuffs. Core sends charge the fee on top, so this understates the wallet debit. The previous confirmation path deliberately usedamount + preparedSend.feefor Total; this route should preserve that behavior.
`CoreSendConfirmController.retry` re-broadcast the prepared send after any non-unknown failure, but `PreparedStandardSend` captures a single-shot `FinalizedCoreTransaction`: once the broadcast has reached the SDK, a retry can only surface its already-consumed error. The prepared object now reports whether its broadcast ran, and the controller drops it when it did, so "Try again" re-prepares (auth + build + sign). A failure raised before the broadcast — the online precheck — still re-broadcasts the same signed transaction, and an unknown outcome stays terminal. A Core send charges the network fee on top of the amount, so the confirm sheet's Total now adds the exact fee off the signed transaction: the same all-in wallet debit the legacy confirm screen showed.
romchornyi
marked this pull request as draft
August 18, 2026 15:31
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
The external send lived in one 1156-line file holding every step, its shared
chrome and its helpers, none of it previewable —
SendViewModel's initializerregisters a sync-monitor observer and subscribes to two balance publishers, so
a canvas could not build one.
Two things were also wrong beyond the structure. Transparent → Transparent
jumped from the source step straight to the classic L1 amount screen, so the
route most people use was the only one that looked different. And Continue
gated Core-funded amounts on the wallet total, which counts unconfirmed and
immature coins and leaves nothing for the fee — amounts existed that Max would
never offer, that armed the button anyway, and that only failed later at the
payment processor.
What was done?
Split into
Pay/Send/— a file per step (address, source, amount, confirm),with what two or more steps share under
Send/Components/. Nothing moved butlocation; the file-scope
privatehelpers had to become internal to surviveit.
Replaced two hand-rolled controls with the design system:
DashUIKit.NavigationBarfor the step header, and
AddressFieldViewfor the address entry, which bringsits own scan control, clear button, Paste and error slot.
AddressFieldViewgained an optional label accessory upstream so the badge naming the decoded
destination has somewhere to live; the collapsed address card and the separate
scan row are gone.
The destination now reads as a heading rather than a form field — the address
is settled by then and neither step can change it — and the amount step says
which balance it is spending from, masked behind the app-wide
balanceHiddenflag the home balance already uses.Every route takes the same amount step now, Transparent included. Nothing new
was needed to route it: the step already handed Core routes to
continueCore,which still finishes through the payment processor and still owns the fee math
— it just receives an amount instead of zero.
Both Core-funded routes gate on the number Max fills in (spendable minus the
send fee reserve), and Transparent gets the dust floor the classic amount
screen enforces. The validation message follows the same number, so a button
disabled on affordability is never left unexplained.
SendViewModelgained an unwired initializer for previews, and each step andcomponent has previews for the states it can be in.
Separately, the payments landing's tab selector was pulled out of the screen
into its own view and rebuilt to the Android design: capsule pills where only
the active one carries a label, filled in that tab's own accent. Each tab now
describes its own icon and accent instead of the selector switching over them.
Switching a tab animates. It used to cut — the active pill jumped to its new
width and colour and the content underneath was replaced in the same frame, so
a change with an obvious direction had none. The pill morphs, its label fading
in while the capsule is still growing, and the content cross-fades briefly.
The animation is scoped to the selector rather than wrapping the binding write,
so it does not silently animate everything else observing the selection.
Screenshots
Switching the landing's tabs — the pill morph and the content cross-fade.
Screen.Recording.2026-08-10.at.17.48.26.mov
The external send flow — address entry through to the amount step, with the
destination read as a heading and the source balance named beneath it.
Screen.Recording.2026-08-10.at.17.48.42.mov
How Has This Been Tested?
Clean
dashpaybuild after every step. Exercised on the simulator againsttestnet: address entry with paste and scan, the source picker, the amount step
for Transparent and Shielded sources, the Send button gating, and the landing's
tab switching in both directions. The recordings above are from that pass.
Not yet exercised: whether the payment processor opens at confirmation rather
than its own amount screen now that it receives a non-zero amount — worth
checking first on a device.
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only