feat: add primitives and layout utilities#3
Conversation
Add the low-level building blocks on top of Foundation: - Button/DashButton - ViewModifiers/MenuViewModifier - Components/Geometry (FrameReader, LocationReader, ScaleToFitWidth, ScrollViewWithOnScrollChanged) - Components/Icons/XmarkIcon - Components/Illustrations (Error, Loading, Success)
|
Warning Review limit reached
Next review available in: 43 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)
📝 WalkthroughWalkthroughAdds reusable SwiftUI buttons, geometry readers, scroll offset reporting, width scaling, icons, illustrations, loading indicators, and menu styling to DashUIKit. ChangesSwiftUI controls and layout utilities
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ScrollViewWithOnScrollChanged
participant LocationReader
participant Callback
ScrollViewWithOnScrollChanged->>LocationReader: place offset probe in named coordinate space
LocationReader->>Callback: report probe center
Callback->>ScrollViewWithOnScrollChanged: update offset state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 4
🧹 Nitpick comments (4)
Sources/DashUIKit/Components/Geometry/ScaleToFitWidth.swift (1)
37-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
letovervarforminScale.
minScaleis set at initialization and never mutated. Usingletbetter expresses intent and prevents accidental mutation.Proposed fix
`@available`(iOS 14, macOS 11, *) public struct ScaleToFitWidth: ViewModifier { - public var minScale: CGFloat = 0.35 + public let minScale: CGFloat `@State` private var naturalSize: CGSize = .zero🤖 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/Geometry/ScaleToFitWidth.swift` around lines 37 - 39, Change the minScale property in ScaleToFitWidth from var to let, preserving its existing default value and initialization behavior.Sources/DashUIKit/Components/Geometry/LocationReader.swift (1)
53-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider making
readingLocationpublic for consistency withreadingFrame.
readingFrameinFrameReader.swiftis declared in apublic extension View, butreadingLocationhere is in a non-publicextension View(internal). The doc comment onLocationReadersays "Reach for it throughreadingLocation", suggesting it's intended for general use. If external consumers should have access, addpublic.Proposed access level fix
`@available`(iOS 14, macOS 11, *) -public extension View { +extension View { /// Reports this view's center point — in `coordinateSpace` — on appear and on every change. /// Installed as a zero-sized, transparent background so it never affects layout. + public func readingLocation( - func readingLocation( coordinateSpace: CoordinateSpace = .global, onChange: `@escaping` (_ location: CGPoint) -> Void ) -> some View { background(LocationReader(coordinateSpace: coordinateSpace, onChange: onChange)) } }🤖 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/Geometry/LocationReader.swift` around lines 53 - 64, Make readingLocation publicly accessible to match readingFrame and its documented intended use. Update the View extension containing readingLocation to a public extension, or explicitly mark readingLocation as public while preserving its existing API and availability.Sources/DashUIKit/ViewModifiers/MenuViewModifier.swift (1)
21-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a convenience View extension for ergonomic usage.
Applying the modifier currently requires
.modifier(MenuViewModifier(...)). A short extension method would improve call-site readability.✨ Optional convenience extension
+extension View { + public func menuStyle(shadowRadius: CGFloat = 10, innerPadding: CGFloat = 6) -> some View { + modifier(MenuViewModifier(shadowRadius: shadowRadius, innerPadding: innerPadding)) + } +}🤖 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/ViewModifiers/MenuViewModifier.swift` around lines 21 - 39, Add a public View extension near MenuViewModifier with a convenience method that applies MenuViewModifier, exposing shadowRadius and innerPadding with the same defaults and returning the modified view. This should allow callers to use the modifier directly through the extension instead of invoking modifier(MenuViewModifier(...)).Sources/DashUIKit/Components/Illustrations/ErrorIllustration.swift (1)
21-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared illustration base to reduce duplication.
ErrorIllustrationandSuccessIllustrationare structurally identical except for the background color and asset name. A parameterized base view would eliminate the copy-paste and make adding future illustrations trivial.♻️ Proposed shared base view
+@available(iOS 14, macOS 11, *) +struct CircleIllustration: View { + let background: Color + let iconName: String + let iconHeight: CGFloat + + var body: some View { + ZStack { + background + Image(dash: .custom(iconName, bundle: .dashUIKit)) + .resizable() + .scaledToFit() + .frame(maxHeight: iconHeight) + } + .frame(width: 90, height: 90) + .clipShape(Circle()) + } +} `@available`(iOS 14, macOS 11, *) public struct ErrorIllustration: View { public init() {} public var body: some View { - ZStack { - Color.dash.red - Image(dash: .custom("illustration-xmark", bundle: .dashUIKit)) - .resizable() - .scaledToFit() - .frame(maxHeight: 29) - } - .frame(width: 90, height: 90) - .clipShape(Circle()) + CircleIllustration(background: .dash.red, iconName: "illustration-xmark", iconHeight: 29) } }🤖 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/Illustrations/ErrorIllustration.swift` around lines 21 - 37, Extract the duplicated layout from ErrorIllustration and SuccessIllustration into a shared parameterized base View, accepting the background color and illustration asset name while preserving the existing sizing, clipping, and image configuration. Refactor both illustrations to delegate to this base, keeping their public initializers and existing visual behavior unchanged.
🤖 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/Button/DashButton.swift`:
- Around line 170-181: Add .background(style.backgroundColor(isEnabled:
isEnabled)) to the strokeGray branch of styledContent, positioned after the
frame modifier and before the overlay (or clipShape as appropriate), so the
disabled background is rendered consistently with the else branch.
- Line 175: Replace both .clipShape(.rect(cornerRadius: size.radius)) calls in
DashButton with RoundedRectangle(cornerRadius: size.radius, style: .continuous)
to preserve iOS 14 and macOS 11 compatibility.
In `@Sources/DashUIKit/Components/Illustrations/LoadingIllustration.swift`:
- Around line 76-84: Validate and clamp spokeCount in LoadingIllustration.init
to at least 1 before assigning it, so the normalized value is used by both the
ForEach range and onReceive phase modulo; update the timer interval to use the
stored clamped count rather than independently guarding only the divisor.
- Line 74: Refactor LoadingIllustration’s timer lifecycle so
Timer.publish/autoconnect is not created or started during initialization.
Update the timer properties and body’s onReceive/onAppear/onDisappear handling
to explicitly start the timer only when the view appears and stop or cancel it
when the view disappears, ensuring recreated view structs do not leave active
timers.
---
Nitpick comments:
In `@Sources/DashUIKit/Components/Geometry/LocationReader.swift`:
- Around line 53-64: Make readingLocation publicly accessible to match
readingFrame and its documented intended use. Update the View extension
containing readingLocation to a public extension, or explicitly mark
readingLocation as public while preserving its existing API and availability.
In `@Sources/DashUIKit/Components/Geometry/ScaleToFitWidth.swift`:
- Around line 37-39: Change the minScale property in ScaleToFitWidth from var to
let, preserving its existing default value and initialization behavior.
In `@Sources/DashUIKit/Components/Illustrations/ErrorIllustration.swift`:
- Around line 21-37: Extract the duplicated layout from ErrorIllustration and
SuccessIllustration into a shared parameterized base View, accepting the
background color and illustration asset name while preserving the existing
sizing, clipping, and image configuration. Refactor both illustrations to
delegate to this base, keeping their public initializers and existing visual
behavior unchanged.
In `@Sources/DashUIKit/ViewModifiers/MenuViewModifier.swift`:
- Around line 21-39: Add a public View extension near MenuViewModifier with a
convenience method that applies MenuViewModifier, exposing shadowRadius and
innerPadding with the same defaults and returning the modified view. This should
allow callers to use the modifier directly through the extension instead of
invoking modifier(MenuViewModifier(...)).
🪄 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: 6409b66f-d215-4f3d-9d97-00ca4a09ef96
📒 Files selected for processing (10)
Sources/DashUIKit/Button/DashButton.swiftSources/DashUIKit/Components/Geometry/FrameReader.swiftSources/DashUIKit/Components/Geometry/LocationReader.swiftSources/DashUIKit/Components/Geometry/ScaleToFitWidth.swiftSources/DashUIKit/Components/Geometry/ScrollViewWithOnScrollChanged.swiftSources/DashUIKit/Components/Icons/XmarkIcon.swiftSources/DashUIKit/Components/Illustrations/ErrorIllustration.swiftSources/DashUIKit/Components/Illustrations/LoadingIllustration.swiftSources/DashUIKit/Components/Illustrations/SuccessIllustration.swiftSources/DashUIKit/ViewModifiers/MenuViewModifier.swift
- DashButton: apply background in the strokeGray branch so the disabled background color is rendered (was omitted) - DashButton: use RoundedRectangle(cornerRadius:style:.continuous) instead of .rect(cornerRadius:) (iOS 17+) to keep the iOS 14 availability surface - LoadingSpinner: clamp spokeCount to >= 1 to avoid division-by-zero / negative range crashes - LoadingSpinner: connect the timer on appear and cancel it on disappear instead of autoconnecting in init
Second migration PR — low-level building blocks that sit on top of Foundation.
What's included
Button/DashButtonViewModifiers/MenuViewModifierComponents/Geometry/— FrameReader, LocationReader, ScaleToFitWidth, ScrollViewWithOnScrollChangedComponents/Icons/XmarkIconComponents/Illustrations/— Error, Loading, SuccessNotes
master(from feat: add package scaffold and Foundation (design tokens & assets) #1).swift buildpasses locally.Summary by CodeRabbit