feat: add core components - #4
Conversation
Add the core presentational components on top of Foundation + primitives: SearchBar, Toast, BottomSheet, NavigationBar, MenuItem, RadioButtonRow, DashSwitch, SystemMessageView, AddressFieldView, DashAmount, List1View, plus the assets they use (menu-*.disabled, info-rect, toast-no-wifi). iOS 14 compatibility fixes applied during migration: - SystemMessageView: add missing @available(iOS 14, macOS 11, *) (was absent, broke the build) - SystemMessageView / AddressFieldView / SearchBar / Toast: replace iOS 16+ .rect(cornerRadius:) and .contentShape(.rect) with RoundedRectangle(..., style: .continuous) / Rectangle() so the declared iOS 14/15 surface holds.
|
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 (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds 13 public SwiftUI components for address/search input, navigation, sheets, menus, amounts, switches, messages, toasts, selection rows, and labeled values, plus four image-set asset definitions and debug previews. ChangesDashUIKit component suite
Estimated code review effort: 5 (Critical) | ~90 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
Sources/DashUIKit/Table List/List1View.swift (1)
20-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
letinstead ofvarfor view properties.SwiftUI view properties should be immutable. Declaring
labelandvalueaspublic varallows external mutation, which is non-idiomatic for a value-typeView. Additionally, the default placeholder values"Label"and"Value"could accidentally surface in production if a caller omits them.♻️ Proposed refactor
- public var label: String - public var value: String + public let label: String + public let value: String - public init(label: String = "Label", value: String = "Value") { + public init(label: String, value: String) { self.label = label self.value = value }🤖 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 `@Sources/DashUIKit/Table` List/List1View.swift around lines 20 - 29, In List1View, make the label and value stored properties immutable by changing both public var declarations to public let, and remove the placeholder default arguments from its initializer so callers must provide explicit values.Sources/DashUIKit/Components/SearchBar.swift (1)
48-214: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
magnifyingGlassandclearButtonare duplicated betweenSearchBarFocusedandSearchBarLegacy.Both private structs define identical
magnifyingGlassandclearButtoncomputed properties. Extracting these into a shared private helper or a shared protocol would reduce maintenance burden if the icons or styling ever change.♻️ Suggested approach: extract shared views
+// Shared between both variants +private struct SearchBarMagnifyingGlass: View { + var body: some View { + Image(dash: .custom("searchbar-magnifyingglass-icon", bundle: .dashUIKit)) + .resizable() + .scaledToFit() + .frame(maxHeight: 15) + } +} + +private struct SearchBarClearButton: View { + `@Binding` var text: String + var body: some View { + if !text.isEmpty { + Button(action: { text = "" }) { + Image(dash: .custom("searchbar-xmark-icon", bundle: .dashUIKit)) + .resizable() + .scaledToFit() + .frame(maxHeight: 15) + .frame(width: 44, height: 44) + .contentShape(Rectangle()) + } + .accessibilityLabel(Text(NSLocalizedString("Clear", bundle: .module, comment: "DashUIKit"))) + } + } +}Then replace the inline computed properties in both
SearchBarFocusedandSearchBarLegacywithSearchBarMagnifyingGlass()andSearchBarClearButton(text: $text).🤖 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 `@Sources/DashUIKit/Components/SearchBar.swift` around lines 48 - 214, Extract the duplicated magnifyingGlass and clearButton implementations from SearchBarFocused and SearchBarLegacy into shared private views, such as SearchBarMagnifyingGlass and SearchBarClearButton(text:), preserving the existing icons, sizing, accessibility label, and binding behavior; replace both structs’ computed properties with those shared components.Sources/DashUIKit/Components/DashAmount.swift (1)
37-45: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStatic
NumberFormattercaptures.currentlocale at first access.The formatter is initialized once as a static property, so
f.locale = .currentis locked to whatever locale was active whenDashAmountFormatwas first referenced. If the user changes their locale/language while the app is running, amounts will continue to display in the stale locale until the process restarts.Consider using
Formatter.withLocale(.current)per call (iOS 15+) or recreating the formatter whenLocale.currentchanges:♻️ Suggested approach: recompute locale per call
private enum DashAmountFormat { static let duffsPerDash: Decimal = 100_000_000 - static let numberFormatter: NumberFormatter = { - let f = NumberFormatter() - f.numberStyle = .decimal - f.locale = .current - f.minimumFractionDigits = 0 - f.maximumFractionDigits = 5 - f.usesGroupingSeparator = true - return f - }() + static func makeFormatter() -> NumberFormatter { + let f = NumberFormatter() + f.numberStyle = .decimal + f.locale = .current + f.minimumFractionDigits = 0 + f.maximumFractionDigits = 5 + f.usesGroupingSeparator = true + return f + } static func string(forDuffs duffs: Int64) -> String { let value = Decimal(duffs) / duffsPerDash - return numberFormatter.string(from: value as NSNumber) ?? "\(value)" + return makeFormatter().string(from: value as NSNumber) ?? "\(value)" } }🤖 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 `@Sources/DashUIKit/Components/DashAmount.swift` around lines 37 - 45, DashAmountFormat’s static numberFormatter captures Locale.current only on first access, causing stale formatting after runtime locale changes. Update the formatting logic to obtain or configure a NumberFormatter with Locale.current for each formatting call, using Formatter.withLocale(.current) where supported, and remove reliance on the permanently cached locale in numberFormatter.Sources/DashUIKit/Components/Toast.swift (1)
24-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
BackgroundBlurViewhas no macOS fallback, yetToastdeclares macOS 11 availability.The entire file is guarded by
#if canImport(UIKit), so on macOS neitherBackgroundBlurViewnorToastwill compile. The@available(iOS 14, macOS 11, *)annotations onToastStyleandToastare misleading — API consumers may expect macOS support that doesn't exist. Either remove the macOS availability target or provide a SwiftUI-native blur fallback (e.g.,.material(.ultraThinvia.background(.ultraThinMaterial)).♻️ Proposed refactor — use SwiftUI material instead of UIKit blur (iOS 15+, or keep UIKit for iOS 14)
If iOS 15+ is acceptable for blur, replace
BackgroundBlurViewwith the built-in SwiftUI modifier:- .background( - ZStack { - BackgroundBlurView() - Color.dash.toastBackground - } - ) + .background( - ZStack { - BackgroundBlurView() - Color.dash.toastBackground - } + )Alternatively, if iOS 14 support is required, keep
BackgroundBlurViewbut removemacOS 11from the@availableannotations to avoid confusion.🤖 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 `@Sources/DashUIKit/Components/Toast.swift` around lines 24 - 31, Resolve the platform mismatch between BackgroundBlurView and Toast’s declared availability: either add a SwiftUI-native macOS blur implementation and ensure Toast compiles outside the UIKit guard, or remove macOS 11 from the `@available` annotations on ToastStyle and Toast. Keep the existing UIKit blur for supported iOS versions if iOS 14 compatibility is required.
🤖 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 `@Sources/DashUIKit/Components/MenuItem.swift`:
- Around line 112-114: Add the disabled state to the Toggle accessory in the
menu item’s accessory rendering logic: update the `.toggle(let isOn)` case to
apply `.disabled(!isEnabled)`, ensuring the switch cannot be changed when the
surrounding menu item is disabled.
In `@Sources/DashUIKit/Components/NavigationBar.swift`:
- Around line 96-117: Add localized accessibility labels to the icon-only button
created by NavigationBarElement.button(action:), providing distinct labels for
the back, close, plus, and info cases. Reuse the project’s established
localization/accessibility-label pattern and attach the label to the Button so
VoiceOver identifies each action.
- Around line 119-127: NavigationBarButtonStyle.makeBody uses the iOS 15-only
animation(_:value:) API despite the iOS 14 deployment target. Replace it with
the backward-compatible animation(_:) modifier, or conditionally apply the
value-based modifier with an iOS 15 availability check while preserving the
pressed-state animation.
In `@Sources/DashUIKit/Components/RadioButtonRow.swift`:
- Around line 54-101: Add the selection accessibility trait to the row’s Button
in RadioButtonRow.body by applying accessibilityAddTraits with .isSelected when
isSelected is true and no additional traits otherwise, so VoiceOver announces
the current selection state for both radio and checkbox styles.
In `@Sources/DashUIKit/Components/SystemMessageView.swift`:
- Around line 75-95: Update the button container condition in SystemMessageView
to render the HStack only when at least one complete button pair exists: button
name with its corresponding action, or secondary button name with its
corresponding action. Keep the existing inner DashButton conditions and padding
unchanged.
---
Nitpick comments:
In `@Sources/DashUIKit/Components/DashAmount.swift`:
- Around line 37-45: DashAmountFormat’s static numberFormatter captures
Locale.current only on first access, causing stale formatting after runtime
locale changes. Update the formatting logic to obtain or configure a
NumberFormatter with Locale.current for each formatting call, using
Formatter.withLocale(.current) where supported, and remove reliance on the
permanently cached locale in numberFormatter.
In `@Sources/DashUIKit/Components/SearchBar.swift`:
- Around line 48-214: Extract the duplicated magnifyingGlass and clearButton
implementations from SearchBarFocused and SearchBarLegacy into shared private
views, such as SearchBarMagnifyingGlass and SearchBarClearButton(text:),
preserving the existing icons, sizing, accessibility label, and binding
behavior; replace both structs’ computed properties with those shared
components.
In `@Sources/DashUIKit/Components/Toast.swift`:
- Around line 24-31: Resolve the platform mismatch between BackgroundBlurView
and Toast’s declared availability: either add a SwiftUI-native macOS blur
implementation and ensure Toast compiles outside the UIKit guard, or remove
macOS 11 from the `@available` annotations on ToastStyle and Toast. Keep the
existing UIKit blur for supported iOS versions if iOS 14 compatibility is
required.
In `@Sources/DashUIKit/Table` List/List1View.swift:
- Around line 20-29: In List1View, make the label and value stored properties
immutable by changing both public var declarations to public let, and remove the
placeholder default arguments from its initializer so callers must provide
explicit values.
🪄 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: 152be8f6-976d-4edf-800e-c2e33c91afb0
⛔ Files ignored due to path filters (12)
Sources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Menu/menu-receive.disabled.imageset/receive-disabled.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Menu/menu-receive.disabled.imageset/receive-disabled@2x.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Menu/menu-receive.disabled.imageset/receive-disabled@3x.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Menu/menu-send.disabled.imageset/send-disabled.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Menu/menu-send.disabled.imageset/send-disabled@2x.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Menu/menu-send.disabled.imageset/send-disabled@3x.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/System message/info-rect.imageset/info-rect.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/System message/info-rect.imageset/info-rect@2x.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/System message/info-rect.imageset/info-rect@3x.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Toast/toast-no-wifi.imageset/toast-no-wifi.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Toast/toast-no-wifi.imageset/toast-no-wifi@2x.pngis excluded by!**/*.pngSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Toast/toast-no-wifi.imageset/toast-no-wifi@3x.pngis excluded by!**/*.png
📒 Files selected for processing (15)
Sources/DashUIKit/Components/AddressFieldView.swiftSources/DashUIKit/Components/BottomSheet.swiftSources/DashUIKit/Components/DashAmount.swiftSources/DashUIKit/Components/DashSwitch.swiftSources/DashUIKit/Components/MenuItem.swiftSources/DashUIKit/Components/NavigationBar.swiftSources/DashUIKit/Components/RadioButtonRow.swiftSources/DashUIKit/Components/SearchBar.swiftSources/DashUIKit/Components/SystemMessageView.swiftSources/DashUIKit/Components/Toast.swiftSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Menu/menu-receive.disabled.imageset/Contents.jsonSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Menu/menu-send.disabled.imageset/Contents.jsonSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/System message/info-rect.imageset/Contents.jsonSources/DashUIKit/Resources/Media.xcassets/Icons & Illustrations/Toast/toast-no-wifi.imageset/Contents.jsonSources/DashUIKit/Table List/List1View.swift
- MenuItem: disable the toggle accessory when the row is disabled - NavigationBar: add localized accessibility labels to back/close/plus/info icon-only buttons - NavigationBar: gate animation(_:value:) (iOS 15+) behind if #available with an iOS 14 fallback - RadioButtonRow: expose selection state to VoiceOver via accessibilityAddTraits - SystemMessageView: only render the button container when a button actually has both a name and an action
Fourth migration PR — core presentational components on top of Foundation + primitives.
Components (11)
SearchBar, Toast, BottomSheet, NavigationBar, MenuItem, RadioButtonRow, DashSwitch, SystemMessageView, AddressFieldView, DashAmount,
Table List/List1ViewAssets added
menu-receive.disabled,menu-send.disabled(MenuItem),info-rect(SystemMessage),toast-no-wifi(Toast)iOS 14 compat fixes applied during migration⚠️
The source versions had latent issues that either broke the build or the declared availability:
@available(iOS 14, macOS 11, *)(was absent → build error)..rect(cornerRadius:)/.contentShape(.rect)withRoundedRectangle(cornerRadius:style:.continuous)/Rectangle()to honor the declared iOS 14/15 support.Notes
swift buildpasses.Summary by CodeRabbit