feat: add core components#4
Open
romchornyi wants to merge 1 commit into
Open
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.
📝 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 |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Comment on lines
+112
to
+114
| case .toggle(let isOn): | ||
| Toggle("", isOn: isOn) | ||
| .labelsHidden() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Toggle accessory remains interactive when isEnabled is false.
When isEnabled is false, the leading icon and text colors switch to disabled variants, but the .toggle accessory renders a fully interactive Toggle with no .disabled() modifier. Users can still toggle the switch on a disabled menu item.
🔧 Proposed fix
case .toggle(let isOn):
Toggle("", isOn: isOn)
.labelsHidden()
+ .disabled(!isEnabled)📝 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.
Suggested change
| case .toggle(let isOn): | |
| Toggle("", isOn: isOn) | |
| .labelsHidden() | |
| case .toggle(let isOn): | |
| Toggle("", isOn: isOn) | |
| .labelsHidden() | |
| .disabled(!isEnabled) |
🤖 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/MenuItem.swift` around lines 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.
Comment on lines
+96
to
+117
| public enum NavigationBarElement: String { | ||
| case back = "navigationbar-back" | ||
| case close = "navigationbar-close" | ||
| case plus = "navigationbar-plus" | ||
| case info = "navigationbar-info" | ||
|
|
||
| public var icon: some View { | ||
| Image(dash: .custom(rawValue, bundle: .module)) | ||
| .resizable() | ||
| .scaledToFit() | ||
| } | ||
|
|
||
| public func button(action: @escaping () -> Void) -> some View { | ||
| Button(action: action) { | ||
| icon | ||
| .fixedSize() | ||
| .frame(width: 44, height: 44, alignment: .center) | ||
| .contentShape(Rectangle()) | ||
| } | ||
| .buttonStyle(NavigationBarButtonStyle()) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add accessibility labels to NavigationBarElement.button.
The button(action:) helper renders an icon-only Button without any accessibilityLabel. Other components in this codebase (AddressFieldView, SearchBar, SystemMessageView) consistently provide localized accessibility labels for icon-only buttons. Without labels, VoiceOver users cannot identify the back, close, plus, or info actions.
♿ Proposed fix: add accessibility labels per element case
public enum NavigationBarElement: String {
case back = "navigationbar-back"
case close = "navigationbar-close"
case plus = "navigationbar-plus"
case info = "navigationbar-info"
public var icon: some View {
Image(dash: .custom(rawValue, bundle: .module))
.resizable()
.scaledToFit()
}
+ private var accessibilityLabelKey: String {
+ switch self {
+ case .back: return "Back"
+ case .close: return "Close"
+ case .plus: return "Add"
+ case .info: return "Info"
+ }
+ }
+
public func button(action: `@escaping` () -> Void) -> some View {
Button(action: action) {
icon
.fixedSize()
.frame(width: 44, height: 44, alignment: .center)
.contentShape(Rectangle())
}
+ .accessibilityLabel(NSLocalizedString(accessibilityLabelKey, bundle: .module, comment: "DashUIKit"))
.buttonStyle(NavigationBarButtonStyle())
}
}📝 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.
Suggested change
| public enum NavigationBarElement: String { | |
| case back = "navigationbar-back" | |
| case close = "navigationbar-close" | |
| case plus = "navigationbar-plus" | |
| case info = "navigationbar-info" | |
| public var icon: some View { | |
| Image(dash: .custom(rawValue, bundle: .module)) | |
| .resizable() | |
| .scaledToFit() | |
| } | |
| public func button(action: @escaping () -> Void) -> some View { | |
| Button(action: action) { | |
| icon | |
| .fixedSize() | |
| .frame(width: 44, height: 44, alignment: .center) | |
| .contentShape(Rectangle()) | |
| } | |
| .buttonStyle(NavigationBarButtonStyle()) | |
| } | |
| } | |
| public enum NavigationBarElement: String { | |
| case back = "navigationbar-back" | |
| case close = "navigationbar-close" | |
| case plus = "navigationbar-plus" | |
| case info = "navigationbar-info" | |
| public var icon: some View { | |
| Image(dash: .custom(rawValue, bundle: .module)) | |
| .resizable() | |
| .scaledToFit() | |
| } | |
| private var accessibilityLabelKey: String { | |
| switch self { | |
| case .back: return "Back" | |
| case .close: return "Close" | |
| case .plus: return "Add" | |
| case .info: return "Info" | |
| } | |
| } | |
| public func button(action: `@escaping` () -> Void) -> some View { | |
| Button(action: action) { | |
| icon | |
| .fixedSize() | |
| .frame(width: 44, height: 44, alignment: .center) | |
| .contentShape(Rectangle()) | |
| } | |
| .accessibilityLabel(NSLocalizedString(accessibilityLabelKey, bundle: .module, comment: "DashUIKit")) | |
| .buttonStyle(NavigationBarButtonStyle()) | |
| } | |
| } |
🤖 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/NavigationBar.swift` around lines 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.
Comment on lines
+119
to
+127
| @available(iOS 14, macOS 11, *) | ||
| private struct NavigationBarButtonStyle: ButtonStyle { | ||
| func makeBody(configuration: Configuration) -> some View { | ||
| configuration.label | ||
| .scaleEffect(configuration.isPressed ? 0.88 : 1) | ||
| .opacity(configuration.isPressed ? 0.7 : 1) | ||
| .animation(.easeInOut(duration: 0.12), value: configuration.isPressed) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the declared platform minimum versions in Package.swift
rg -n -A 15 'platforms' Package.swiftRepository: dashpay/DashUIKit
Length of output: 499
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Package.swift ==\n'
cat -n Package.swift | sed -n '1,80p'
printf '\n== NavigationBar.swift (around the reported lines) ==\n'
cat -n Sources/DashUIKit/Components/NavigationBar.swift | sed -n '100,140p'
printf '\n== Search for animation(value:) uses in repo ==\n'
rg -n '\.animation\([^)]*,\s*value:\s*' Sources || trueRepository: dashpay/DashUIKit
Length of output: 2364
🌐 Web query:
SwiftUI View.animation(_:value:) availability iOS 15 Apple documentation
💡 Result:
The SwiftUI modifier animation(_:value:) was introduced in iOS 15 [1]. It serves as the recommended replacement for the deprecated parameterless animation modifier [1]. By requiring a value to monitor for changes, this modifier ensures that animations are only triggered when the specified state value changes, providing more predictable and controlled animation behavior in SwiftUI views [2][3]. It is available on iOS 15.0 and later, iPadOS 15.0 and later, macOS 12.0 and later, tvOS 15.0 and later, and watchOS 8.0 and later [4][2].
Citations:
- 1: https://stackoverflow.com/questions/69443588/how-to-replace-deprecated-animation-in-swiftui
- 2: https://developer.apple.com/documentation/swiftui/view/animation(_:value:)?changes=latest_minor
- 3: https://developer.apple.com/tutorials/swiftui/animating-views-and-transitions
- 4: https://developer.apple.com/documentation/swiftui/view/animation(_:value:)
animation(_:value:) is iOS 15+, but this package targets iOS 14. Use the older animation(_:) form here, or gate this modifier behind if #available(iOS 15, *).
🤖 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/NavigationBar.swift` around lines 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.
Comment on lines
+54
to
+101
| public var body: some View { | ||
| Button(action: action) { | ||
| HStack(spacing: 16) { | ||
| if let icon = icon { | ||
| Image(dash: icon) | ||
| .resizable() | ||
| .scaledToFit() | ||
| .frame(width: 30, height: 30) | ||
| } | ||
|
|
||
| VStack(alignment: .leading, spacing: 0) { | ||
| Text(title) | ||
| .dashFont(.subheadMedium) | ||
| .foregroundColor(Color.dash.primaryText) | ||
|
|
||
| if let subtitle = subtitle { | ||
| Text(subtitle) | ||
| .dashFont(.caption1) | ||
| .foregroundColor(Color.dash.secondaryText) | ||
| } | ||
| } | ||
|
|
||
| Spacer() | ||
|
|
||
| if let trailingText = trailingText { | ||
| Text(trailingText) | ||
| .dashFont(.subheadMedium) | ||
| .foregroundColor(Color.dash.primaryText) | ||
| } | ||
|
|
||
| switch style { | ||
| case .radio: | ||
| Circle() | ||
| .stroke(isSelected ? Color.dash.blue : Color.dash.gray300.opacity(0.5), lineWidth: isSelected ? 6 : 2) | ||
| .frame(width: isSelected ? 21 : 24, height: isSelected ? 21 : 24) | ||
| .padding(.trailing, isSelected ? 2 : 0) | ||
| case .checkbox: | ||
| Image(dash: .custom(isSelected ? "icon_checkbox_square_checked" : "icon_checkbox_square", bundle: .dashUIKit)) | ||
| .resizable() | ||
| .frame(width: 24, height: 24) | ||
| } | ||
| } | ||
| .padding(.horizontal, 16) | ||
| .contentShape(Rectangle()) | ||
| .frame(minHeight: subtitle != nil ? 60 : 54) | ||
| } | ||
| .buttonStyle(PlainButtonStyle()) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add accessibility traits for selection state.
The Button announces as a button to VoiceOver but does not communicate whether the row is selected or not. For a radio/checkbox component, this is essential information for accessibility users. Since the component targets iOS 14+, .accessibilityAddTraits(isSelected ? .isSelected : []) is available and should be applied.
♿ Proposed accessibility fix
}
.buttonStyle(PlainButtonStyle())
+ .accessibilityAddTraits(isSelected ? .isSelected : [])
}
}📝 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.
Suggested change
| public var body: some View { | |
| Button(action: action) { | |
| HStack(spacing: 16) { | |
| if let icon = icon { | |
| Image(dash: icon) | |
| .resizable() | |
| .scaledToFit() | |
| .frame(width: 30, height: 30) | |
| } | |
| VStack(alignment: .leading, spacing: 0) { | |
| Text(title) | |
| .dashFont(.subheadMedium) | |
| .foregroundColor(Color.dash.primaryText) | |
| if let subtitle = subtitle { | |
| Text(subtitle) | |
| .dashFont(.caption1) | |
| .foregroundColor(Color.dash.secondaryText) | |
| } | |
| } | |
| Spacer() | |
| if let trailingText = trailingText { | |
| Text(trailingText) | |
| .dashFont(.subheadMedium) | |
| .foregroundColor(Color.dash.primaryText) | |
| } | |
| switch style { | |
| case .radio: | |
| Circle() | |
| .stroke(isSelected ? Color.dash.blue : Color.dash.gray300.opacity(0.5), lineWidth: isSelected ? 6 : 2) | |
| .frame(width: isSelected ? 21 : 24, height: isSelected ? 21 : 24) | |
| .padding(.trailing, isSelected ? 2 : 0) | |
| case .checkbox: | |
| Image(dash: .custom(isSelected ? "icon_checkbox_square_checked" : "icon_checkbox_square", bundle: .dashUIKit)) | |
| .resizable() | |
| .frame(width: 24, height: 24) | |
| } | |
| } | |
| .padding(.horizontal, 16) | |
| .contentShape(Rectangle()) | |
| .frame(minHeight: subtitle != nil ? 60 : 54) | |
| } | |
| .buttonStyle(PlainButtonStyle()) | |
| } | |
| public var body: some View { | |
| Button(action: action) { | |
| HStack(spacing: 16) { | |
| if let icon = icon { | |
| Image(dash: icon) | |
| .resizable() | |
| .scaledToFit() | |
| .frame(width: 30, height: 30) | |
| } | |
| VStack(alignment: .leading, spacing: 0) { | |
| Text(title) | |
| .dashFont(.subheadMedium) | |
| .foregroundColor(Color.dash.primaryText) | |
| if let subtitle = subtitle { | |
| Text(subtitle) | |
| .dashFont(.caption1) | |
| .foregroundColor(Color.dash.secondaryText) | |
| } | |
| } | |
| Spacer() | |
| if let trailingText = trailingText { | |
| Text(trailingText) | |
| .dashFont(.subheadMedium) | |
| .foregroundColor(Color.dash.primaryText) | |
| } | |
| switch style { | |
| case .radio: | |
| Circle() | |
| .stroke(isSelected ? Color.dash.blue : Color.dash.gray300.opacity(0.5), lineWidth: isSelected ? 6 : 2) | |
| .frame(width: isSelected ? 21 : 24, height: isSelected ? 21 : 24) | |
| .padding(.trailing, isSelected ? 2 : 0) | |
| case .checkbox: | |
| Image(dash: .custom(isSelected ? "icon_checkbox_square_checked" : "icon_checkbox_square", bundle: .dashUIKit)) | |
| .resizable() | |
| .frame(width: 24, height: 24) | |
| } | |
| } | |
| .padding(.horizontal, 16) | |
| .contentShape(Rectangle()) | |
| .frame(minHeight: subtitle != nil ? 60 : 54) | |
| } | |
| .buttonStyle(PlainButtonStyle()) | |
| .accessibilityAddTraits(isSelected ? .isSelected : []) | |
| } | |
| } |
🤖 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/RadioButtonRow.swift` around lines 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.
Comment on lines
+75
to
+95
| if buttonName != nil || secondaryButtonName != nil { | ||
| HStack(spacing: 10) { | ||
| if let buttonName, let onAction { | ||
| DashUIKit.DashButton( | ||
| text: buttonName, | ||
| size: .small, | ||
| style: .filledBlue, | ||
| action: onAction | ||
| ) | ||
| } | ||
| if let secondaryButtonName, let onSecondaryAction { | ||
| DashUIKit.DashButton( | ||
| text: secondaryButtonName, | ||
| size: .small, | ||
| style: .tintedBlue, | ||
| action: onSecondaryAction | ||
| ) | ||
| } | ||
| } | ||
| .padding(.bottom, 14) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Button container renders with padding even when no buttons are shown.
The outer condition buttonName != nil || secondaryButtonName != nil renders the HStack with .padding(.bottom, 14), but the inner conditions require both name and action to be non-nil (if let buttonName, let onAction). If a caller passes buttonName: "Renew" without onAction, the container renders empty with 14pt bottom padding.
🐛 Proposed fix: align container condition with actual rendering
- if buttonName != nil || secondaryButtonName != nil {
+ if (buttonName != nil && onAction != nil)
+ || (secondaryButtonName != nil && onSecondaryAction != nil) {
HStack(spacing: 10) {
if let buttonName, let onAction {📝 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.
Suggested change
| if buttonName != nil || secondaryButtonName != nil { | |
| HStack(spacing: 10) { | |
| if let buttonName, let onAction { | |
| DashUIKit.DashButton( | |
| text: buttonName, | |
| size: .small, | |
| style: .filledBlue, | |
| action: onAction | |
| ) | |
| } | |
| if let secondaryButtonName, let onSecondaryAction { | |
| DashUIKit.DashButton( | |
| text: secondaryButtonName, | |
| size: .small, | |
| style: .tintedBlue, | |
| action: onSecondaryAction | |
| ) | |
| } | |
| } | |
| .padding(.bottom, 14) | |
| } | |
| if (buttonName != nil && onAction != nil) | |
| || (secondaryButtonName != nil && onSecondaryAction != nil) { | |
| HStack(spacing: 10) { | |
| if let buttonName, let onAction { | |
| DashUIKit.DashButton( | |
| text: buttonName, | |
| size: .small, | |
| style: .filledBlue, | |
| action: onAction | |
| ) | |
| } | |
| if let secondaryButtonName, let onSecondaryAction { | |
| DashUIKit.DashButton( | |
| text: secondaryButtonName, | |
| size: .small, | |
| style: .tintedBlue, | |
| action: onSecondaryAction | |
| ) | |
| } | |
| } | |
| .padding(.bottom, 14) | |
| } |
🤖 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/SystemMessageView.swift` around lines 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.
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
1 participant
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.
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