fix: polish trezor send loading states - #708
Conversation
Greptile SummaryThis PR separates disabled and submitting states in the swipe control and adds explicit hardware-wallet funding-source and preview loading state across the send flow.
Confidence Score: 4/5The hardware-source loading failure path should be fixed before merging because a fee-estimation failure can trap the send screen with no usable source-selection or Continue action. Hardware selection now starts an externally visible loading state that is cleared only after a refresh or preview, but the amount view can return before starting either operation when its fee rate is unavailable. Files Needing Attention: Bitkit/Views/Wallets/Send/SendAmountView.swift; Bitkit/ViewModels/HwFundingSigner.swift
|
| Filename | Overview |
|---|---|
| Bitkit/ViewModels/HwFundingSigner.swift | Adds request-scoped funding-source and preview loading flags, including stale-request invalidation and cancellation cleanup. |
| Bitkit/Views/Wallets/Send/SendAmountView.swift | Connects hardware loading to source and Continue controls, but can leave both unavailable when fee resolution fails before balance refresh. |
| Bitkit/Views/Wallets/Send/SendConfirmationView.swift | Presents hardware preview loading on the fee icon and disables source, fee, and swipe actions until preview readiness. |
| Bitkit/Components/SwipeButton.swift | Separates temporarily disabled presentation from the post-swipe spinner while retaining unified interaction blocking. |
| BitkitTests/HwFundingSignerTests.swift | Covers successful funding-source refresh and preview-loading transitions but not unavailable fee-rate or refresh-failure recovery. |
Sequence Diagram
sequenceDiagram
participant U as User
participant V as SendAmountView
participant C as HwSendCoordinator
participant F as Fee estimation
U->>V: Select Trezor
V->>C: selectWallet(showsLoading: true)
C-->>V: "isFundingSourceLoading = true"
V->>F: Resolve selected fee rate
alt Fee rate available
V->>C: refreshAvailable(...)
C-->>V: "Balance and loading=false"
else Fee estimation fails
V-->>V: Refresh guard returns
Note over V,C: Loading remains true and source control stays disabled
end
Reviews (1): Last reviewed commit: "fix: polish trezor send loading states" | Re-trigger Greptile
ovitrif
left a comment
There was a problem hiding this comment.
Confirm still traps source, fee, and swipe after a missing fee rate, and the Amount → Confirm reload blanks the fee Amount just prepared.
- Switching to Trezor on Confirm sets
isFundingSourceLoadingthroughselectWallet(..., showsLoading: true), thencalculateTransactionFee()returns whenselectedFeeRateSatsPerVByteis nil, so nothing settles the flag. Source, fee, and swipe stay disabled until the sheet is dismissed. displayedTransactionFeeonly preserves a fee already stored in localtransactionFee. Confirm's firstpreparePreviewzerospreviewFeeSatswhiletransactionFeeis still 0, so the fee from Amount disappears until the duplicate preview returns.testCoordinatorTracksPreviewPreparationonly covers a successful estimate. Nothing would fail ifpreparePreview's catch stopped clearingisPreviewLoading.
|
@ben-kaufman Can we please include media in PR description (screens / recs) 🙏🏻 ? |
|
@ovitrif Added four screenshots and a recording to the PR description covering the funding-source refresh, Continue preparation, Confirm preview refresh, and transition to Sign With Device. Thanks! |
ovitrif
left a comment
There was a problem hiding this comment.
QA Notes
iPhone 17 simulator, iOS 26.5, regtest.
- Amount: switching the source to Trezor refreshed available funds; source and Continue were usable after settle.
- Confirm: the previous fee stayed visible across Trezor and Savings switches; swipe proceeded once the preview was ready.
- Swipe after the fee was ready reached Sign With Device.
- Switching back to Savings restored source, fee, Continue, and swipe.
Approve.
|
LGTM, tested all cases from QA notes. Before merge we still need e2e green, maybe merge with master so it retriggers properly? cc. @ben-kaufman 🙏🏻 Meanwhile @piotr-iohk can have a look to confirm if the e2e failure would likely succeed next run: thanks 🙏🏻 |
Fixes #696
Description
This PR polishes the transient loading states in the Trezor send flow:
This is stacked on #688. Review this PR against
feat/trezor-send; it contains only the loading-state polish for that flow.Linked Issues/Tasks
Screenshot / Video
pr-708-trezor-send-polish.mov
QA Notes
Manual Tests
regression:switch back to Spending or Savings: the normal source, fee, Continue, and swipe behavior remains available.Automated Checks
HwFundingSignerTests: 26 tests passed, including source-refresh, unavailable-fee, and preview-loading coverage.git diff --checkpassed.