feat(ui): use the shared amount entry for contact payments - #930
Conversation
The Pay sheet took its amount through a bare `TextField` + `.decimalPad` while every other send surface uses `DashUIKit.EnterAmountView`. The system keyboard let through input the parser then had to rescue — a locale comma, a bare separator, precision DASH does not carry — so what was typed and what was sent could disagree, and there was no fiat line, no Max, and no shared validation. Adopt the standard component over the app's own keypad, as `SendAmountScreen` and `InternalTransferScreen` do. The confirmation now states the amount that was broadcast rather than echoing the typed text back: any gap between the two rendered as a truthful-looking "sent" line for an amount that never left the wallet.
|
Warning Review limit reached
Next review available in: 29 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe contact payment sheet now uses shared amount-entry controls with fiat conversion, max-send support, and parsed-amount validation. It records the exact duff amount broadcast and shows that amount in payment confirmation. ChangesContact payment amount
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ContactProfileSheet
participant EnterAmountView
participant NumericKeyboardView
participant PaymentConfirmation
NumericKeyboardView->>EnterAmountView: enters amount
EnterAmountView->>ContactProfileSheet: returns parsed amount
ContactProfileSheet->>PaymentConfirmation: provides exact broadcast amount
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactProfileSheet.swift`:
- Line 541: Remove the redundant nil initializer from the sentAmountDuffs `@State`
declaration in ContactProfileSheet, leaving it as an implicitly initialized
optional while preserving its UInt64? type and state behavior.
- Around line 582-583: Move the `@ViewBuilder` attribute onto the same line as the
form property declaration in the form computed property, preserving its existing
behavior and body.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2a71e62a-8bb6-440e-80ac-ac3c25f8ef0a
📒 Files selected for processing (1)
DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactProfileSheet.swift
| @ViewBuilder | ||
| private var form: some View { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Put @ViewBuilder on the property declaration line.
SwiftLint requires attributes on the same line as variable declarations.
Proposed fix
- `@ViewBuilder`
- private var form: some View {
+ `@ViewBuilder` private var form: some View {As per coding guidelines, Swift files must follow SwiftFormat/SwiftLint.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ViewBuilder | |
| private var form: some View { | |
| `@ViewBuilder` private var form: some View { |
🧰 Tools
🪛 SwiftLint (0.65.0)
[Warning] 583-583: Attributes should be on their own lines in functions and types, but on the same line as variables and imports
(attributes)
🤖 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 `@DashWallet/Sources/UI/DashPay/Contacts/SwiftUI/ContactProfileSheet.swift`
around lines 582 - 583, Move the `@ViewBuilder` attribute onto the same line as
the form property declaration in the form computed property, preserving its
existing behavior and body.
Sources: Coding guidelines, Linters/SAST tools
`redundant_optional_initialization` is a SwiftLint default and this config does not disable it, so a newly added `= nil` is a new violation even though its neighbours predate the rule.
2 participants
Issue being fixed or feature implemented
The contact Pay sheet took its amount through a bare
TextField+.decimalPad, while every other send surface in the app usesDashUIKit.EnterAmountView. The system keyboard let through input the parser then had to rescue — a locale comma, a bare separator, more precision than DASH carries — so what was typed and what was sent could disagree. There was also no fiat line, no Max, and none of the shared validation.The confirmation compounded it: it echoed
amountTextback rather than reporting what was broadcast, so any gap between the two rendered as a truthful-looking "sent" line for an amount that never left the wallet.What was done?
EnterAmountView+NumericKeyboardViewreplace the rawTextFieldand the system keypad, matchingSendAmountScreenandInternalTransferScreen. That brings the fiat line, a Max button, and the shared parsing/validation the rest of the app already relies on.sentAmountDuffsis captured at send time and rendered instead of the typed text..medium→.large, since the sheet now carries its own keypad and needs the room the system keyboard used to occupy.One file, +44/−33.
Note the Max button leans on
maxSendable, which #928 corrected to the wallet-wide spendable balance — so Max here is accurate on top of currentdevelopin a way it would not have been before that landed.How Has This Been Tested?
Testnet, on device. Paid an established DashPay contact from the profile sheet: the keypad rejects a second separator and over-precision instead of silently reshaping the value, the fiat line tracks the entry, Max fills the spendable balance, and the confirmation reports the amount that was actually broadcast.
This commit was written alongside #918 and tested in the same sessions; it is split out here because it is a Pay-sheet UI change rather than part of the payment-history restore, and it did not make it into that PR before it merged.
Breaking Changes
None. UI-only, single screen.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes