feat: remove slider from manage UTXO custom amount flow#691
feat: remove slider from manage UTXO custom amount flow#691Vineet1101 wants to merge 2 commits intobitcoinppl:masterfrom
Conversation
📝 WalkthroughWalkthroughRemoved the slider-based input mechanism and associated state management from the UTXO custom amount sheet view. The amount input is now driven exclusively by a text field, with simplified dispatch logic triggered only by text field changes and focus loss events. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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. Comment |
Greptile SummaryThis PR simplifies the manage-UTXO custom amount flow by replacing the Confidence Score: 5/5Safe to merge; only P2 dead-code cleanup remains. All findings are P2 style issues (unused No files require special attention beyond the minor dead-code cleanup in SendFlowUtxoCustomAmountSheetView.swift. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User opens UTXO custom amount sheet] --> B[TextField shown with current amount]
B --> C{User taps TextField?}
C -- Yes --> D[isFocused = true]
D --> E[User types amount]
E --> F[displayAmountBinding.set fires]
F --> G[enteringAmount updated]
G --> H[manager.dispatch notifyCoinControlEnteredAmountChanged]
C -- No --> I[Amount stays at customAmount]
D --> J{User dismisses keyboard?}
J -- Yes --> K[isFocused = false]
K --> L[onChange isFocused: reset customAmount from manager.amount]
H --> M{manager.amount changes?}
M -- Yes --> N[onChange manager.amount: update customAmount]
N --> O[Display refreshed via displayAmount]
|
|
Hey @Vineet1101 lgtm. A couple of things before this can move forward: The Summary section is empty could you add a brief description of what was wrong with the slider (referencing #457) and why a text field alone is the better UX? Thanks! |
|
@Vineet1101 what about android? should be done together |
praveenperera
force-pushed
the
feature/remove-utxo-slider
branch
from
3510c66 to
e79828f
Compare
|
also please make sure CI is passing before requesting review again thank you |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift (1)
174-179:⚠️ Potential issue | 🟠 MajorDispatch the final amount when the field loses focus.
After removing the slider/editing-end path, the backend may never receive
.notifyCoinControlEnteredAmountChanged(..., false)when the user taps away without changing text again. That can leave focus-loss validation/commit logic untriggered.🐛 Proposed fix
.onChange(of: isFocused, initial: false) { old, new in // lost focus if old == true, new == false { + if let enteringAmount { + manager.dispatch(.notifyCoinControlEnteredAmountChanged(enteringAmount, false)) + self.enteringAmount = nil + } self.customAmount = manager.amount.map(amountToDouble) ?? maxSend } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift` around lines 174 - 179, When the text field loses focus the code resets self.customAmount but does not dispatch the final entered amount to the backend, so add a call to manager.notifyCoinControlEnteredAmountChanged(...) (or the project's equivalent) inside the isFocused onChange branch where old == true and new == false; compute the final amount the same way you set self.customAmount (use manager.amount.map(amountToDouble) ?? maxSend or the resolved Double) and call notifyCoinControlEnteredAmountChanged(finalAmount, false) so the backend receives the final committed value; update the block in SendFlowUtxoCustomAmountSheetView where customAmount, manager, amountToDouble, maxSend are used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift`:
- Around line 174-179: When the text field loses focus the code resets
self.customAmount but does not dispatch the final entered amount to the backend,
so add a call to manager.notifyCoinControlEnteredAmountChanged(...) (or the
project's equivalent) inside the isFocused onChange branch where old == true and
new == false; compute the final amount the same way you set self.customAmount
(use manager.amount.map(amountToDouble) ?? maxSend or the resolved Double) and
call notifyCoinControlEnteredAmountChanged(finalAmount, false) so the backend
receives the final committed value; update the block in
SendFlowUtxoCustomAmountSheetView where customAmount, manager, amountToDouble,
maxSend are used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08c12828-6ae0-48b6-ae41-3fda2f04d038
📒 Files selected for processing (1)
ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift
praveenperera
force-pushed
the
feature/remove-utxo-slider
branch
from
e79828f to
72b9345
Compare
praveenperera
force-pushed
the
feature/remove-utxo-slider
branch
from
72b9345 to
49a740d
Compare
|
@Vineet1101 get ci passing |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift (2)
28-32:⚠️ Potential issue | 🟡 MinorRemove unused
dividercomputed property — leftover from slider UI removal.The
dividerproperty (lines 28–32) is defined but never referenced. The inlineDivider()on line 124 is separate. This dead code can be removed along with the surrounding blank lines.Suggested cleanup
- - - private var divider: some View { - Divider() - .padding(.vertical, 28) - .foregroundStyle(.red) - } - - - private var maxSend: Double {Additionally, the
softMaxSendcomputed property (lines 44–47) also appears unused after slider removal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift` around lines 28 - 32, Remove the dead computed properties `divider` and `softMaxSend` from the SendFlowUtxoCustomAmountSheetView type: delete the `private var divider: some View { Divider() ... }` block and the `private var softMaxSend: Decimal { ... }` block (and any adjacent blank lines), then run a build to ensure nothing references them; if any references remain, replace them with the inline `Divider()` or the appropriate value logic where used.
162-179:⚠️ Potential issue | 🟡 MinorClarify dust-UTXO safeguard enforcement:
softMaxSendis calculated but not used for input validation.The
softMaxSendproperty is defined at line 44 but never used to constrain theTextFieldor validateenteringAmountbefore dispatch. Unlike the Android equivalent (which coerces input againstsoftMaxSend), the iOS view sends raw user input directly to Rust vianotifyCoinControlEnteredAmountChangedwithout client-side validation. The dust-UTXO guardrail is enforced downstream on the Rust side throughmaxSendMinusFeesAndSmallUtxo()(which calls the actual dust-check logic incove-bdk/src/util.rs:72), so amounts that would create dust UTXOs should still be rejected during coin selection.Confirm on a real device that:
- Typing remains smooth and the input field doesn't rewrite mid-edit as the
onChangehandlers fire- Fee/total fields don't visibly thrash during rapid keystroke input
- The Rust-side validation correctly rejects amounts in the dust range when the transaction is confirmed
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift` around lines 162 - 179, The view computes softMaxSend but never applies it before notifying Rust; fix by clamping user-entered amounts to softMaxSend before dispatching to notifyCoinControlEnteredAmountChanged (and/or when committing on focus loss) instead of altering the live TextField while typing. Locate references to softMaxSend, notifyCoinControlEnteredAmountChanged, manager.amount, customAmount, isFocused and amountToDouble: on focus-loss handler (the .onChange for isFocused) and in the confirm/send action that calls notifyCoinControlEnteredAmountChanged, coerce the value with min(customAmount, softMaxSend) (convert units using amountToDouble / selectedUnit or manager.amount helpers) and then send the clamped value; keep the per-keystroke handlers unchanged to avoid mid-edit rewrites so typing remains smooth.
🧹 Nitpick comments (1)
ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift (1)
44-47:softMaxSendappears unused after slider removal — remove dead code.The doc comment explicitly describes a slider-selection constraint ("the next biggest amount below maxSend that can be selected"), and the property is no longer referenced anywhere in the file now that the slider is gone. Leaving it (and the dependency on
maxSendMinusFeesAndSmallUtxo) increases surface area and may also be tripping the CI warnings/lints@praveenpereraflagged.♻️ Proposed cleanup
- /// softMaxSend is the next biggest amount below maxSend that can be selected - /// any amount between softMaxSend and maxSend can NOT be selected, because that would create a dust UTXO - private var softMaxSend: Double { - let amount = manager.rust.maxSendMinusFeesAndSmallUtxo() ?? minSendAmount - return amountToDouble(amount) - } -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift` around lines 44 - 47, Remove the now-unused softMaxSend computed property and its internal call to manager.rust.maxSendMinusFeesAndSmallUtxo() (and any reliance on amountToDouble/minSendAmount only used by that property) from SendFlowUtxoCustomAmountSheetView, since the slider was removed and softMaxSend is no longer referenced; delete the softMaxSend declaration and any imports or unused helpers that exist solely to support it to eliminate dead code and related lint/CI warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift`:
- Around line 28-32: Remove the dead computed properties `divider` and
`softMaxSend` from the SendFlowUtxoCustomAmountSheetView type: delete the
`private var divider: some View { Divider() ... }` block and the `private var
softMaxSend: Decimal { ... }` block (and any adjacent blank lines), then run a
build to ensure nothing references them; if any references remain, replace them
with the inline `Divider()` or the appropriate value logic where used.
- Around line 162-179: The view computes softMaxSend but never applies it before
notifying Rust; fix by clamping user-entered amounts to softMaxSend before
dispatching to notifyCoinControlEnteredAmountChanged (and/or when committing on
focus loss) instead of altering the live TextField while typing. Locate
references to softMaxSend, notifyCoinControlEnteredAmountChanged,
manager.amount, customAmount, isFocused and amountToDouble: on focus-loss
handler (the .onChange for isFocused) and in the confirm/send action that calls
notifyCoinControlEnteredAmountChanged, coerce the value with min(customAmount,
softMaxSend) (convert units using amountToDouble / selectedUnit or
manager.amount helpers) and then send the clamped value; keep the per-keystroke
handlers unchanged to avoid mid-edit rewrites so typing remains smooth.
---
Nitpick comments:
In `@ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift`:
- Around line 44-47: Remove the now-unused softMaxSend computed property and its
internal call to manager.rust.maxSendMinusFeesAndSmallUtxo() (and any reliance
on amountToDouble/minSendAmount only used by that property) from
SendFlowUtxoCustomAmountSheetView, since the slider was removed and softMaxSend
is no longer referenced; delete the softMaxSend declaration and any imports or
unused helpers that exist solely to support it to eliminate dead code and
related lint/CI warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d841c0dc-167b-4621-b6b2-4f157b40db7c
📒 Files selected for processing (1)
ios/Cove/Flows/SendFlow/Common/SendFlowUtxoCustomAmountSheetView.swift
3 participants
Summary
Testing
Platform Coverage
Checklist
Fixes #457
Summary by CodeRabbit
UI Changes
Behavior