fix(ui): amount paste decimal separator#783
Open
romchornyi wants to merge 18 commits into
Open
Conversation
- Normalize pasted number strings to locale-independent format (using "." as decimal separator) - Accept both "." and "," as decimal separators when pasting - Use rightmost separator as decimal separator when both are present - Parse normalized string with en_US_POSIX locale to avoid misinterpretation - Re-emit using current locale's decimal separator for proper display - Add PastedAmountNormalizationTests for decimal separator handling - Fixes issue where pasted amounts with locale-specific separators were incorrectly parsed
- Remove local DashPickerView, EnterAmountView, MayaAmountView components - Add references to DashUIKit components - Update file references for new formatters and screens
…eparators - Support locale-specific decimal separators (. vs , vs ·) - Validate and normalize pasted amounts based on active locale - Add comprehensive test suite for locale-aware input validation
…imal handling - Add DashAmountFormatter for consistent amount formatting across app - Update AmountObject and BaseAmountModel to use new formatter - Support locale-specific decimal separators in amount display - Add comprehensive test coverage for amount formatting edge cases
…w controllers - Replace local EnterAmountView with DashUIKit version - Update SendAmountViewController and ProvideAmountViewController - Add SendAmountScreen SwiftUI view for modern amount input - Improve keyboard handling and decimal separator validation
…ormatting - Update DashSpendAmountView to use new DashAmountFormatter - Improve panel layout and amount display consistency - Enhance DashSpendUserAuthScreen with better state handling - Refine DashSpendPayViewModel for amount normalization
- Remove local DashPickerView, EnterAmountView, MayaAmountView - Update MayaConvertView to use DashUIKit EnterAmountView - Refine MayaConvertViewModel amount handling - Improve FrameReader for better geometry calculations
- Update AmountObjectTests to work with new formatting logic - Improve Tools.swift utilities for amount handling
- Remove DashPickerView, EnterAmountView, MayaAmountView from Maya folder - These components have been migrated to DashUIKit for reuse across the app
…l separators - Update AmountObject and BaseAmountModel to validate pasted input - Add MayaAmountFormatter for consistent amount formatting - Add comprehensive test suite for pasted amount normalization - Support locale-specific decimal separators (. vs ,)
- Update SendAmountScreen and SpecifyAmountViewController to use DashUIKit - Integrate new amount formatting with MayaConvertView - Improve MayaConvertViewModel for proper amount handling - Support pasted amounts with locale-specific decimal separators
- Update DashSpendAmountView to use consistent amount formatting - Enhance DashSpendFlexibleContent with better layout - Refine DashSpendPayViewModel for proper amount normalization - Support locale-aware decimal separators in gift card purchases
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR adds locale-aware formatting and parsing for amount inputs, updates keyboard handling to use locale-specific separators, migrates send/specify/provide amount screens to SwiftUI hosting, revises Maya convert formatting, and refreshes related localized strings and tests. ChangesLocale-aware formatting and SwiftUI amount flows
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
DashWallet/zh-Hans.lproj/Localizable.strings (1)
595-596: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new Maya portal description in zh-Hans.
This value is still English, so
MayaPortalView.swift:78-89will render untranslated copy for Simplified Chinese users.🤖 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/zh-Hans.lproj/Localizable.strings` around lines 595 - 596, The new Maya Portal localized string in Localizable.strings is still in English, so Simplified Chinese users will see untranslated text in MayaPortalView. Update the value for the Maya Portal description entry in the zh-Hans strings file with the proper Simplified Chinese translation, keeping the existing key intact so MayaPortalView uses the localized copy.DashWallet/zh-Hant-TW.lproj/Localizable.strings (1)
595-596: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new Maya portal description in zh-Hant-TW.
This value is still English, so
MayaPortalView.swift:78-89will render untranslated copy for Traditional Chinese users.🤖 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/zh-Hant-TW.lproj/Localizable.strings` around lines 595 - 596, Translate the new Maya Portal string in Localizable.strings for zh-Hant-TW so Traditional Chinese users don’t see English copy; update the value for the key used by MayaPortalView to a proper Traditional Chinese translation while keeping the same key and localization file entry.
🧹 Nitpick comments (3)
DashWallet/Sources/UI/Payments/Amount/Model/Send/SendAmountModel.swift (1)
71-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve locale injection on
SendAmountModel.Now that
BaseAmountModelacceptsinputLocale, this subclass should forward it so send-amount screens/tests can use non-current locales without losing the new locale-threading path.Proposed fix
- init() { - super.init() + init(inputLocale: Locale = .current) { + super.init(inputLocale: inputLocale)🤖 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/Payments/Amount/Model/Send/SendAmountModel.swift` around lines 71 - 85, `SendAmountModel` is not forwarding the new locale dependency to its base class. Update `SendAmountModel.init` to accept and pass through `inputLocale` when calling `BaseAmountModel`’s initializer so send-amount flows and tests can preserve non-current locale behavior. Keep the change localized to `SendAmountModel` and ensure any call sites still work with the new initializer signature.DashWalletTests/AmountObjectTests.swift (2)
353-446: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe parser matrix still skips the Arabic-separator path.
The PR objective explicitly calls out Arabic decimal separators, but the acceptance coverage here only exercises US, European, Swiss, and Ukrainian formats. Please add at least one
ar_*locale case so that regression surface is covered by automation instead of only manual verification.🤖 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 `@DashWalletTests/AmountObjectTests.swift` around lines 353 - 446, Add acceptance coverage for an Arabic locale in AmountObjectTests so the parser’s Arabic decimal-separator handling is exercised by automation. Extend the existing locale matrix in the test suite by adding at least one ar_* case alongside testUnitedStatesAcceptanceCases, testGermanAcceptanceCases, testFrenchAndSwissGrouping, and testUkrainianGrouping, and verify assertParsed against the locale’s expected editable and display formatting.
175-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one test that hits
pasteFromClipboard()end-to-end.This suite only exercises
updateKeyboardInputString(_), butDashWallet/Sources/UI/Payments/Amount/Model/BaseAmountModel.swiftLines 358-374 added separate clipboard logic: parse withinputLocale, round down to the active amount type’s fraction limit, then feed the editable string back into the model. A focused paste test on a local-currency amount would lock down the new clamping path that these keyboard-only cases currently miss.🤖 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 `@DashWalletTests/AmountObjectTests.swift` around lines 175 - 266, The keyboard-input tests in BaseAmountModelKeyboardInputTests do not cover the new clipboard flow, so add one end-to-end test for pasteFromClipboard() on BaseAmountModel. Use a local-currency amount case and verify the pasted string is parsed with inputLocale, clamped/rounded down to the active amount type’s fraction limit, and then reflected through the model’s editable/current input state. Keep the test focused on the clipboard-specific path in BaseAmountModel so it exercises the new logic beyond updateKeyboardInputString(_).
🤖 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/Categories/Numbers`+Dash.swift:
- Around line 163-175: The formatter helpers in DashAmountFormatter currently
use force casts and force unwraps, which violates SwiftLint/SwiftFormat rules.
Update formattedDashAmount(_:locale:) and
formattedDashAmountWithoutCurrencySymbol(_:locale:) to avoid copy() as!
NumberFormatter and string(from:)! by using safe casting/guard handling or by
reusing the existing locale-aware formatter helpers, while preserving the same
formatting behavior.
In `@DashWallet/Sources/UI/Explore`
Dash/Views/DashSpend/DashSpendPayViewModel.swift:
- Around line 109-118: The input parsing in DashSpendPayViewModel’s amount
setter still uses a locale-independent Decimal conversion, so values accepted by
the validation logic can fail to parse. Update the conversion path around the
input handling in the DashSpendPayViewModel setter to use the current locale
consistently, such as through input.decimal(locale: Locale.current), or
normalize the string before conversion so separators like commas and
locale-specific decimals are handled correctly.
In `@DashWallet/Sources/UI/Maya/Convert/MayaAmountFormatter.swift`:
- Around line 38-45: The coin formatting in
MayaAmountFormatter.coinDisplayString is truncating crypto amounts to 5
fractional digits instead of the expected 8. Update the maximumFractionDigits
passed into decimalString so coinDisplayString preserves 8 decimals, and keep
the existing locale, grouping, and rounding behavior unchanged.
In `@DashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift`:
- Around line 170-176: Normalize locale-specific decimal separators in
MayaConvertViewModel before storing or parsing pasted/input values, since
pasteFromClipboard and the input path still only handle “,” and “.”. Update the
parsing/canonicalization flow around pasteFromClipboard, setInput, and the
formatter helpers used by MayaAmountFormatter and PastedAmountParser so the
active locale separator (for example “٫”) is converted to the internal canonical
“.” form, while display formatting remains locale-aware. This should keep
inputValue parseable across locale-aware keyboard, currency switch, and paste
flows.
In `@DashWallet/Sources/UI/Payment` Controller/Enter
Amount/ProvideAmountViewController.swift:
- Around line 173-181: Guard the accumulation in updateInitialAmount() inside
ProvideAmountViewController so external payment-request output amounts cannot
overflow UInt64 before validation runs. Update the reduce over
details.outputAmounts to use a checked addition approach (for example, validate
each NSNumber.uint64Value before adding or use reporting/optional overflow
handling) and only call sendAmountModel.updateCurrentAmountObject(with:) when
the summed total is safely representable; keep the fix localized to
updateInitialAmount() and the totalAmount calculation.
In `@DashWallet/Sources/UI/Payments/Amount/Model/AmountObject.swift`:
- Around line 221-270: The parsing logic in AmountObject.parse currently only
recognizes ASCII digits with dot/comma separators, so it misses locale-specific
formats like Arabic decimal/grouping separators and localized numerals. Update
parse(_ string:locale:) to normalize localized digits and accept
locale.decimalSeparator and locale.groupingSeparator before running the existing
validation/SeparatorDecision flow, and make sure helper paths like
trimmedAmountBody(from:), isSupportedBodyCharacter(_:), and digitsOnly(_) work
with the normalized input.
- Around line 49-58: The AmountObject formatting/parsing paths still use
force-unwraps and a force-cast on user-facing locale input, so replace those
with safe optional handling in the AmountObject initializer and the related
formatting helpers. In particular, guard Decimal(string:locale:), the
NumberFormatter.dashFormatter copy(), and any other new cast/unwrap sites in the
affected AmountObject methods so invalid input returns nil or a safe fallback
instead of crashing. Apply the same pattern to the other flagged
locale-formatting blocks in this type, keeping the existing method names and
behavior consistent with SwiftLint/SwiftFormat rules.
In `@DashWallet/Sources/UI/Payments/Amount/Model/BaseAmountModel.swift`:
- Around line 327-344: The issue is the force cast used when cloning the DASH
formatter in validatedInputString(from:range:replacementText:), which violates
SwiftLint’s force_cast rule. Replace the forced cast on
NumberFormatter.dashFormatter.copy() with a safe cast/guard path or use an
existing locale-aware formatter helper before passing the formatter into
validatedString(fromLastInputString:range:replacementString:numberFormatter:).
In `@DashWallet/Sources/UI/Payments/Amount/SendAmountViewController.swift`:
- Around line 27-31: The user-facing validation text in SendAmountViewController
is hardcoded and should be localized, and the shared flow still contains the
CrowdNode typo. Update the message built around present(message:level:) to use
the app’s localization mechanism instead of a fixed English string, and correct
any “CowdNode” occurrences in the related shared-flow strings/methods (including
the other affected block in this controller) to “CrowdNode”.
In `@DashWallet/Sources/UI/SwiftUI` Components/Geometry Readers/FrameReader.swift:
- Line 47: The `.readingFrame` modifier is still used in the SwiftUI views, so
removing the local implementation from FrameReader makes those call sites fail
unless DashUIKit is available. Update the affected files, especially FrameReader
and LocalCurrencyView, to import DashUIKit (or ensure it is re-exported)
wherever `.readingFrame` is referenced so the provided modifier remains in
scope.
In `@DashWallet/zh-Hans.lproj/Localizable.strings`:
- Around line 3410-3412: The new swap-lock warning string is still left in
English in the zh-Hans localization, so Simplified Chinese users will see the
untranslated message. Update the Localizable.strings entry for the swap warning
key used by DashSpendError to provide a proper Simplified Chinese translation,
keeping the same key and matching the existing style of nearby Maya/SwapKit
strings.
In `@DashWallet/zh-Hant-TW.lproj/Localizable.strings`:
- Around line 3410-3412: The new swap-lock warning string is still in English in
the zh-Hant-TW localization, so it will surface untranslated in the Traditional
Chinese flow. Update the existing entry in Localizable.strings for the swap
warning key used by DashSpendError to provide a proper Traditional Chinese
translation, and keep the key unchanged so the lookup continues to work.
---
Outside diff comments:
In `@DashWallet/zh-Hans.lproj/Localizable.strings`:
- Around line 595-596: The new Maya Portal localized string in
Localizable.strings is still in English, so Simplified Chinese users will see
untranslated text in MayaPortalView. Update the value for the Maya Portal
description entry in the zh-Hans strings file with the proper Simplified Chinese
translation, keeping the existing key intact so MayaPortalView uses the
localized copy.
In `@DashWallet/zh-Hant-TW.lproj/Localizable.strings`:
- Around line 595-596: Translate the new Maya Portal string in
Localizable.strings for zh-Hant-TW so Traditional Chinese users don’t see
English copy; update the value for the key used by MayaPortalView to a proper
Traditional Chinese translation while keeping the same key and localization file
entry.
---
Nitpick comments:
In `@DashWallet/Sources/UI/Payments/Amount/Model/Send/SendAmountModel.swift`:
- Around line 71-85: `SendAmountModel` is not forwarding the new locale
dependency to its base class. Update `SendAmountModel.init` to accept and pass
through `inputLocale` when calling `BaseAmountModel`’s initializer so
send-amount flows and tests can preserve non-current locale behavior. Keep the
change localized to `SendAmountModel` and ensure any call sites still work with
the new initializer signature.
In `@DashWalletTests/AmountObjectTests.swift`:
- Around line 353-446: Add acceptance coverage for an Arabic locale in
AmountObjectTests so the parser’s Arabic decimal-separator handling is exercised
by automation. Extend the existing locale matrix in the test suite by adding at
least one ar_* case alongside testUnitedStatesAcceptanceCases,
testGermanAcceptanceCases, testFrenchAndSwissGrouping, and
testUkrainianGrouping, and verify assertParsed against the locale’s expected
editable and display formatting.
- Around line 175-266: The keyboard-input tests in
BaseAmountModelKeyboardInputTests do not cover the new clipboard flow, so add
one end-to-end test for pasteFromClipboard() on BaseAmountModel. Use a
local-currency amount case and verify the pasted string is parsed with
inputLocale, clamped/rounded down to the active amount type’s fraction limit,
and then reflected through the model’s editable/current input state. Keep the
test focused on the clipboard-specific path in BaseAmountModel so it exercises
the new logic beyond updateKeyboardInputString(_).
🪄 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: 783493f9-3268-45bd-b068-a6e3c6bb464b
📒 Files selected for processing (71)
DashWallet.xcodeproj/project.pbxprojDashWallet/Sources/Application/Tools.swiftDashWallet/Sources/Categories/Numbers+Dash.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/Components/DashSpendAmountView.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/Components/DashSpendFlexibleContent.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/Components/DashSpendSinglePanel.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendUserAuthScreen.swiftDashWallet/Sources/UI/Maya/Convert/Components/DashPickerView.swiftDashWallet/Sources/UI/Maya/Convert/Components/EnterAmountView.swiftDashWallet/Sources/UI/Maya/Convert/Components/MayaAmountView.swiftDashWallet/Sources/UI/Maya/Convert/MayaAmountFormatter.swiftDashWallet/Sources/UI/Maya/Convert/MayaConvertView.swiftDashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swiftDashWallet/Sources/UI/Maya/OrderPreview/OrderPreviewViewModel.swiftDashWallet/Sources/UI/Payment Controller/Enter Amount/ProvideAmountViewController.swiftDashWallet/Sources/UI/Payments/Amount/Model/AmountObject.swiftDashWallet/Sources/UI/Payments/Amount/Model/BaseAmountModel.swiftDashWallet/Sources/UI/Payments/Amount/Model/Send/SendAmountModel.swiftDashWallet/Sources/UI/Payments/Amount/SendAmountScreen.swiftDashWallet/Sources/UI/Payments/Amount/SendAmountViewController.swiftDashWallet/Sources/UI/Payments/Amount/SpecifyAmountViewController.swiftDashWallet/Sources/UI/SwiftUI Components/Geometry Readers/FrameReader.swiftDashWallet/Sources/UI/SwiftUI Components/NumericKeyboardView.swiftDashWallet/ar.lproj/Localizable.stringsDashWallet/bg.lproj/Localizable.stringsDashWallet/ca.lproj/Localizable.stringsDashWallet/cs.lproj/Localizable.stringsDashWallet/da.lproj/Localizable.stringsDashWallet/de.lproj/Localizable.stringsDashWallet/el.lproj/Localizable.stringsDashWallet/en.lproj/Localizable.stringsDashWallet/eo.lproj/Localizable.stringsDashWallet/es.lproj/Localizable.stringsDashWallet/et.lproj/Localizable.stringsDashWallet/fa.lproj/Localizable.stringsDashWallet/fi.lproj/Localizable.stringsDashWallet/fil.lproj/Localizable.stringsDashWallet/fr.lproj/Localizable.stringsDashWallet/hr.lproj/Localizable.stringsDashWallet/hu.lproj/Localizable.stringsDashWallet/id.lproj/Localizable.stringsDashWallet/it.lproj/Localizable.stringsDashWallet/ja.lproj/Localizable.stringsDashWallet/ko.lproj/Localizable.stringsDashWallet/mk.lproj/Localizable.stringsDashWallet/ms.lproj/Localizable.stringsDashWallet/nb.lproj/Localizable.stringsDashWallet/nl.lproj/Localizable.stringsDashWallet/pl.lproj/Localizable.stringsDashWallet/pt.lproj/Localizable.stringsDashWallet/ro.lproj/Localizable.stringsDashWallet/ru.lproj/Localizable.stringsDashWallet/sk.lproj/Localizable.stringsDashWallet/sl.lproj/Localizable.stringsDashWallet/sl_SI.lproj/Localizable.stringsDashWallet/sq.lproj/Localizable.stringsDashWallet/sr.lproj/Localizable.stringsDashWallet/sv.lproj/Localizable.stringsDashWallet/th.lproj/Localizable.stringsDashWallet/tr.lproj/Localizable.stringsDashWallet/uk.lproj/Localizable.stringsDashWallet/vi.lproj/Localizable.stringsDashWallet/zh-Hans.lproj/Localizable.stringsDashWallet/zh-Hant-TW.lproj/Localizable.stringsDashWallet/zh.lproj/Localizable.stringsDashWallet/zh_TW.lproj/Localizable.stringsDashWalletTests/AmountObjectTests.swiftDashWalletTests/DashAmountFormatterTests.swiftDashWalletTests/NumericKeyboardLocaleSupportTests.swiftDashWalletTests/PastedAmountNormalizationTests.swift
💤 Files with no reviewable changes (3)
- DashWallet/Sources/UI/Maya/Convert/Components/MayaAmountView.swift
- DashWallet/Sources/UI/Maya/Convert/Components/EnterAmountView.swift
- DashWallet/Sources/UI/Maya/Convert/Components/DashPickerView.swift
- Update AmountObject to validate and normalize pasted amounts - Enhance BaseAmountModel with locale-aware formatting - Add Numbers+Dash utilities for amount handling - Add comprehensive test coverage for amount normalization
- Update SendAmountViewController with better amount validation - Enhance BaseAmountViewController for consistent amount handling - Improve ProvideAmountViewController keyboard integration - Refine FrameReader for accurate geometry calculations
- Update MayaConvertViewModel to normalize pasted amounts - Improve DashSpendPayViewModel for gift card purchases - Enhance LocalCurrencyView with better formatting - Support locale-specific decimal separators throughout
There was a problem hiding this comment.
🧹 Nitpick comments (1)
DashWalletTests/AmountObjectTests.swift (1)
352-385: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the canonical pasted string here as well.
editableString(from: parsed.decimalValue, locale:)only re-renders the parsed number, so this helper won't catch regressions incanonicalEditableString(from:locale:)such as preserving a pasted trailing decimal separator. Since pasted normalization is part of this PR, add that assertion in this shared helper.Proposed test adjustment
XCTAssertEqual( - PastedAmountParser.editableString(from: parsed.decimalValue, locale: locale), + PastedAmountParser.canonicalEditableString(from: pasted, locale: locale), expectedEditable, file: file, line: line )🤖 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 `@DashWalletTests/AmountObjectTests.swift` around lines 352 - 385, The shared test helper assertParsed currently verifies only the parsed numeric value, editableString(from:locale:), and grouped display output, so it misses regressions in canonicalEditableString(from:locale:). Update assertParsed in AmountObjectTests to also assert the canonical pasted string produced by PastedAmountParser for the parsed decimal value and locale, alongside the existing editable and display assertions. Use the existing PastedAmountParser, assertParsed helper, and locale-based normalization logic to keep pasted normalization covered.
🤖 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.
Nitpick comments:
In `@DashWalletTests/AmountObjectTests.swift`:
- Around line 352-385: The shared test helper assertParsed currently verifies
only the parsed numeric value, editableString(from:locale:), and grouped display
output, so it misses regressions in canonicalEditableString(from:locale:).
Update assertParsed in AmountObjectTests to also assert the canonical pasted
string produced by PastedAmountParser for the parsed decimal value and locale,
alongside the existing editable and display assertions. Use the existing
PastedAmountParser, assertParsed helper, and locale-based normalization logic to
keep pasted normalization covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c35eb8b9-4060-41d6-9d35-0bf690953bf4
📒 Files selected for processing (12)
DashWallet/Sources/Categories/Numbers+Dash.swiftDashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swiftDashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swiftDashWallet/Sources/UI/Menu/Settings/LocalCurrency/LocalCurrencyView.swiftDashWallet/Sources/UI/Payment Controller/Enter Amount/ProvideAmountViewController.swiftDashWallet/Sources/UI/Payments/Amount/BaseAmountViewController.swiftDashWallet/Sources/UI/Payments/Amount/Model/AmountObject.swiftDashWallet/Sources/UI/Payments/Amount/Model/BaseAmountModel.swiftDashWallet/Sources/UI/Payments/Amount/SendAmountViewController.swiftDashWallet/Sources/UI/SwiftUI Components/Geometry Readers/FrameReader.swiftDashWalletTests/AmountObjectTests.swiftDashWalletTests/PastedAmountNormalizationTests.swift
🚧 Files skipped from review as they are similar to previous changes (8)
- DashWalletTests/PastedAmountNormalizationTests.swift
- DashWallet/Sources/UI/Payments/Amount/SendAmountViewController.swift
- DashWallet/Sources/Categories/Numbers+Dash.swift
- DashWallet/Sources/UI/Explore Dash/Views/DashSpend/DashSpendPayViewModel.swift
- DashWallet/Sources/UI/Payments/Amount/Model/BaseAmountModel.swift
- DashWallet/Sources/UI/Maya/Convert/MayaConvertViewModel.swift
- DashWallet/Sources/UI/Payment Controller/Enter Amount/ProvideAmountViewController.swift
- DashWallet/Sources/UI/Payments/Amount/Model/AmountObject.swift
- Enhance DWDecimalInputValidator to handle locale-specific decimal separators - Support pasting amounts with comma (,) in European locales - Improve validation logic for consistent amount input
…ests - Enhance AmountObject with better locale-aware decimal handling - Improve PastedAmountNormalizationTests with additional test cases - Better validation for pasted amounts from different locales
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
2 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
Fixes the issue where pasting amounts with locale-specific decimal separators (e.g., comma in European locales) would fail or display incorrectly. Additionally, consolidates amount input components by migrating from local implementations to centralized DashUIKit versions.
What was done?
How Has This Been Tested?
- Tested pasting amounts with various decimal separators:
- Period (.) — US, UK, most programming contexts
- Comma (,) — European locales (DE, FR, IT, etc.)
- Arabic decimal separator (٫) — Arabic locales
- Verified amount input in SendAmount and Maya Convert screens
- Tested dual-swap animation in DashUIKit (Xcode previews)
- Confirmed DashSpend gift card purchases work with new formatter
- NumericKeyboardLocaleSupportTests — validates locale-specific separator handling
- DashAmountFormatterTests — edge cases (0, very large amounts, etc.)
- PastedAmountNormalizationTests — pasted input from various locales
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes
Documentation