fix(ui): add Coinbase transfer amount screen and converter UI components#785
fix(ui): add Coinbase transfer amount screen and converter UI components#785romchornyi wants to merge 13 commits into
Conversation
- Add TransferAmountView and ViewModel for Coinbase transfer input - Add TransferAmountHostingController for UIKit integration - Update IntegrationViewController+Coinbase with new screen routing - Support custom transfer amounts for Coinbase operations
- Update Coinbase.storyboard with transfer amount screen integration - Remove old TransferAmountViewController references - Add new hosting controller wiring for SwiftUI TransferAmountView
…ut control - Enhance FrameReader with improved geometry calculations - Improve ScaleToFitWidth for responsive text scaling - Support better layout adaptability across screen sizes
- Enhance MayaAmountView with better amount formatting - Improve LocalCurrencyView for better currency selection UX - Support new converter card integration
- Update project.pbxproj with new Coinbase transfer amount files - Remove old TransferAmountViewController references - Add new converter card and receive estimate components - Update asset references for arrow and diagonal icons
- Remove legacy UIKit TransferAmountViewController - Replaced by new SwiftUI TransferAmountView with TransferAmountHostingController - Complete migration to modern SwiftUI-based approach
- Update MayaConvertView to use new DashUIKit ConverterCard - Improve exchange rate display with new component - Better layout for amount input and conversion preview
…e management - Enhance TransferAmountHostingController with better lifecycle handling - Improve TransferAmountViewModel for state persistence and validation - Better error handling and user feedback during transfer
|
Warning Review limit reached
Next review available in: 51 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 Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe Coinbase transfer flow now uses SwiftUI with a dedicated view model and UIKit hosting controller. Coinbase storyboard modules and project wiring were updated, DashUIKit components replaced duplicated UI helpers, and currency conversion and authentication continuation handling were corrected. ChangesCoinbase Transfer Amount SwiftUI Migration
DashUIKit Adoption and Helper Relocation
Conversion, Authentication, and Gift Card Updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TransferAmountView
participant TransferAmountViewModel
participant TransferAmountHostingController
participant PaymentController
User->>TransferAmountView: Enter amount and select transfer
TransferAmountView->>TransferAmountViewModel: transfer()
TransferAmountViewModel->>TransferAmountHostingController: onInitiatePayment
TransferAmountHostingController->>PaymentController: Start payment
PaymentController-->>TransferAmountHostingController: Payment result
TransferAmountHostingController-->>TransferAmountView: Show transfer status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/Coinbase/Transfer` Amount/TransferAmountView.swift:
- Around line 35-37: The NavigationBar call in TransferAmountView uses a
trailing comma after the leading closure argument, which breaks compilation on
the Swift 5.0 toolchain. Update the DashUIKit.NavigationBar invocation to remove
that trailing comma while keeping the existing leading: {
NavigationBarElement.back.button { onBack?() } } argument intact.
In `@DashWallet/Sources/UI/Maya/Convert/Components/MayaAmountView.swift`:
- Line 19: The new DashUIKit import in MayaAmountView should be reordered to
match the project’s sorted_imports rule. Move the DashUIKit import into the
correct alphabetical/configured import group alongside the existing imports, and
run SwiftFormat/SwiftLint to ensure the file matches the expected import
ordering.
In `@DashWallet/Sources/UI/Maya/Convert/MayaConvertView.swift`:
- Line 19: The new DashUIKit import is out of the configured sort order and is
triggering sorted_imports. Update the import block in MayaConvertView so
DashUIKit is placed according to the project’s import ordering rules, and re-run
SwiftFormat/SwiftLint to verify the imports are sorted correctly.
🪄 Autofix (Beta)
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
Run ID: 4bb1098f-7013-4b7b-9027-dc6234ae74bb
📒 Files selected for processing (12)
DashWallet.xcodeproj/project.pbxprojDashWallet/Sources/UI/Coinbase/Base.lproj/Coinbase.storyboardDashWallet/Sources/UI/Coinbase/CoinbaseEntryPoint/IntegrationViewController+Coinbase.swiftDashWallet/Sources/UI/Coinbase/Transfer Amount/TransferAmountHostingController.swiftDashWallet/Sources/UI/Coinbase/Transfer Amount/TransferAmountView.swiftDashWallet/Sources/UI/Coinbase/Transfer Amount/TransferAmountViewController.swiftDashWallet/Sources/UI/Coinbase/Transfer Amount/TransferAmountViewModel.swiftDashWallet/Sources/UI/Maya/Convert/Components/MayaAmountView.swiftDashWallet/Sources/UI/Maya/Convert/MayaConvertView.swiftDashWallet/Sources/UI/Menu/Settings/LocalCurrency/LocalCurrencyView.swiftDashWallet/Sources/UI/SwiftUI Components/Geometry Readers/FrameReader.swiftDashWallet/Sources/UI/SwiftUI Components/Geometry Readers/ScaleToFitWidth.swift
💤 Files with no reviewable changes (1)
- DashWallet/Sources/UI/Coinbase/Transfer Amount/TransferAmountViewController.swift
- Enhance TransferAmountView with better amount formatting and validation - Improve MayaAmountView for better exchange rate display - Update MayaConvertView layout with ConverterCard improvements - Better visual consistency across amount input screens
|
@coderabbitai review |
✅ Action performedReview finished.
|
…nfig - Improve TransferAmountViewModel with better amount validation - Update project.pbxproj for build configuration consistency - Better error handling and user feedback
|
@coderabbitai review |
✅ Action performedReview finished.
|
Resolve conflicts against master (post #780 About Dash redesign + DashUIKit): - Keep our Coinbase Transfer Amount screen files (auto-merged) - FrameReader.swift / ScaleToFitWidth.swift: keep ours — this branch still needs the local FrameReader/LocationReader structs (DashUIKit's are internal); only the View extensions were removed to avoid clashing with DashUIKit's public readingFrame/scaleToFitWidth - Restore LocationReader.swift and ScrollViewWithOnScrollChanged.swift that master deleted but this branch still uses - project.pbxproj: take master's structure (DashUIKit remote, 14 DashSpend files, groups); drop our duplicate/explicit geometry entries; re-add "Geometry Readers" and "ScrollViews" synchronized groups to the dashwallet and dashpay targets so the restored files compile via file-system sync Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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)
DashWallet/Sources/Infrastructure/Currency Exchanger/CurrencyExchanger.swift (1)
192-197: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBypass conversion when source and target currencies are identical.
If
amountCurrencyandcurrencyare the same (e.g., converting DASH to DASH), the method currently proceeds toconvertDash, which attempts to fetch the exchange rate. If the rate provider does not explicitly store a 1:1 rate for the currency, this will unexpectedly throw aratesNotAvailableerror.Adding an early return prevents this exception and eliminates unnecessary computations.
💡 Proposed fix
public func convert(to currency: String, amount: Decimal, amountCurrency: String) throws -> Decimal { if amount.isZero { return 0 } + if currency == amountCurrency { return amount } if amountCurrency == kDashCurrency { return try convertDash(amount: amount, to: currency) }🤖 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/Infrastructure/Currency` Exchanger/CurrencyExchanger.swift around lines 192 - 197, Update convert(to:amount:amountCurrency:) to return the input amount immediately when amountCurrency equals currency, before the convertDash path or other rate lookups. Preserve the existing zero-amount behavior and all conversion logic for differing currencies.DashWallet.xcodeproj/project.pbxproj (1)
3597-3597: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMissing
PBXBuildFiledefinition forDashUIKitindashpaytarget.The
dashpaytarget'sPBXFrameworksBuildPhasereferences the build file ID51760D812FF7C2CF00E70F0B, but this identifier is missing from thePBXBuildFilesection. This will cause project parsing issues or linker errors fordashpay.Please add the missing
PBXBuildFiledefinition to thePBXBuildFilesection.🛠 Proposed fix (add to PBXBuildFile section)
/* Begin PBXBuildFile section */ ... + 51760D812FF7C2CF00E70F0B /* DashUIKit in Frameworks */ = {isa = PBXBuildFile; productRef = 5149DA072FF3DA5200207AE5 /* DashUIKit */; };🤖 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.xcodeproj/project.pbxproj` at line 3597, Add the missing PBXBuildFile entry for identifier 51760D812FF7C2CF00E70F0B in the PBXBuildFile section, referencing the existing DashUIKit file and preserving the dashpay target’s PBXFrameworksBuildPhase reference.
🤖 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.
Outside diff comments:
In `@DashWallet.xcodeproj/project.pbxproj`:
- Line 3597: Add the missing PBXBuildFile entry for identifier
51760D812FF7C2CF00E70F0B in the PBXBuildFile section, referencing the existing
DashUIKit file and preserving the dashpay target’s PBXFrameworksBuildPhase
reference.
In `@DashWallet/Sources/Infrastructure/Currency`
Exchanger/CurrencyExchanger.swift:
- Around line 192-197: Update convert(to:amount:amountCurrency:) to return the
input amount immediately when amountCurrency equals currency, before the
convertDash path or other rate lookups. Preserve the existing zero-amount
behavior and all conversion logic for differing currencies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: adfa32d1-0388-49e9-80be-3e419b316cd1
📒 Files selected for processing (6)
DashWallet.xcodeproj/project.pbxprojDashWallet/Sources/Infrastructure/Currency Exchanger/CurrencyExchanger.swiftDashWallet/Sources/Models/Coinbase/Auth/CBAuth.swiftDashWallet/Sources/UI/Coinbase/Transfer Amount/TransferAmountView.swiftDashWallet/Sources/UI/Coinbase/Transfer Amount/TransferAmountViewModel.swiftDashWallet/Sources/UI/Maya/Convert/MayaConvertView.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- DashWallet/Sources/UI/Coinbase/Transfer Amount/TransferAmountView.swift
- DashWallet/Sources/UI/Maya/Convert/MayaConvertView.swift
- DashWallet/Sources/UI/Coinbase/Transfer Amount/TransferAmountViewModel.swift
Issue being fixed or feature implemented
Addresses the issue where there is no summary "You will receive.." for Coinbase operations. This feature adds a dedicated transfer amount screen for Coinbase with proper amount input, validation, and receive summary display. Additionally, integrates new converter card component from DashUIKit for better exchange rate visualization across the app.
What was done?
Coinbase Transfer Amount Screen
Services & Models
How Has This Been Tested?
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit