Add Spacing implementation #245
Conversation
WalkthroughThe changes update the Spacing module within OpenSwiftUICore. The Changes
Sequence Diagram(s)sequenceDiagram
participant V as View
participant S as Spacing
participant L as LayoutChecker
V->>S: distanceToSuccessorView(axis, layoutDirection, nextPreference)
S->>S: Retrieve minima & default value
S->>L: Check layout direction symmetry (isLayoutDirectionSymmetric)
alt Symmetric Layout
S-->>V: Return calculated spacing value
else Asymmetric Layout
S-->>V: Return adjusted spacing value (or nil)
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code Definitions (1)Sources/OpenSwiftUICore/Layout/Geometry/Spacing.swift (1)
⏰ Context from checks skipped due to timeout of 90000ms (6)
🔇 Additional comments (18)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Tests/OpenSwiftUICoreTests/Layout/Geometry/SpacingTests.swift (1)
130-211: Consider adding negative spacing tests.While the current tests are extensive, you could include a scenario with negative or zero distances to confirm correct behavior under edge cases.
Sources/OpenSwiftUICore/Layout/Geometry/Spacing.swift (1)
138-150: Clarify the note regardingbottom.leading.The comment at line 145 indicates potential confusion about using
bottom.leading. Consider revisiting or explaining the logic to avoid confusion.
🛑 Comments failed to post (1)
Sources/OpenSwiftUICore/Layout/Geometry/Spacing.swift (1)
249-251: 🛠️ Refactor suggestion
Address unused closure parameter.
Static analysis flagged the unused parameter
valuein the.filter { key, value in ... }closure. Replacevaluewith_to resolve the warning.-.filter { key, value in - edges.contains(key.edge) +.filter { key, _ in + edges.contains(key.edge) }📝 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..filter { key, _ in edges.contains(key.edge) }🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 249-249: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Sources/OpenSwiftUICore/Layout/Geometry/Spacing.swift (2)
10-10: Check nonstandard Swift package syntax.
“package import Foundation” is unusual in mainstream Swift. Confirm that your build setup supports the “package” keyword; otherwise, regularimport Foundationshould suffice.
234-253: Minor SwiftLint warning: Unused closure parameter “value”.
You only usekeyin the.filterclosure at line 249. SwiftLint suggests using_in place of the unused parameter.- .filter { key, value in edges.contains(key.edge) } + .filter { key, _ in edges.contains(key.edge) }🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 249-249: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Sources/OpenSwiftUICore/Layout/Geometry/Spacing.swift(1 hunks)Tests/OpenSwiftUICoreTests/Layout/Geometry/SpacingTests.swift(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Tests/OpenSwiftUICoreTests/Layout/Geometry/SpacingTests.swift
🧰 Additional context used
🧬 Code Definitions (1)
Sources/OpenSwiftUICore/Layout/Geometry/Spacing.swift (1)
Tests/OpenSwiftUICoreTests/Layout/Geometry/SpacingTests.swift (9)
lineSpacing(105-109)isAlmostEqual(85-93)spacing(95-103)distance(153-175)incorporate(215-256)clear(258-307)reset(309-360)distanceToSuccessorView(402-473)description(545-610)
🪛 SwiftLint (0.57.0)
Sources/OpenSwiftUICore/Layout/Geometry/Spacing.swift
[Warning] 249-249: Unused parameter in a closure should be replaced with _
(unused_closure_parameter)
🔇 Additional comments (13)
Sources/OpenSwiftUICore/Layout/Geometry/Spacing.swift (13)
3-3: Header updates look good.
The bump in iOS audit version to 18.0 and status completion markers appear consistent with your versioning approach. No issues here.Also applies to: 5-8
12-13: Validate default spacing source.
ReferencingCoreGlue.shared.defaultSpacingis fine if you are certainCoreGlueis initialized before use. Ensure that no race condition or lazy property needs special handling.
17-23: Well-documented struct introduction.
Your doc comments effectively describeSpacingusage and context. Great clarity here.
24-25: Organized MARK usage.
Using “// MARK: -” improves code readability.
26-72:Spacing.CategoryandSpacing.Keylook sound.
These structs are succinct, aligning categories and edges with minimal overhead. Their hashable conformance also looks consistent.
74-154:TextMetricsimplementation is logically consistent.
The properties (ascend, descend, leading, pixelLength) and related methods (includingisAlmostEqualandspacing) appear correct. However, note the comment at line 145 (“Actually this is still bottom.leading 🤔”). Please verify that the arithmetic accurately matches your intended spacing rules.
157-232:Valueenum captures spacing variants well.
Supporting both distances and text metrics is a flexible design. Pattern matching indistance(to:)is clearly structured.
255-311:clearandresetmethods are thoughtfully designed.
They handle edges appropriately for both absolute and direction-based sets. The approach of removing preexisting values before optionally resetting them to zero is cohesive with the rest of your spacing system.
313-331: Constructors provide flexible initialization.
You offer a default constructor with standard minimal edges, plus an override for explicitly setting the dictionary. This is a helpful combination.
333-394:distanceToSuccessorView&_distancelogic.
These methods correctly combine matching categories between twoSpacinginstances. The fallback to uncategorized edges is a good safeguard. Good job ensuring different layout directions are handled.
397-468:CustomStringConvertibleandisLayoutDirectionSymmetric.
Providing a descriptive string output is great for debugging. Checking for symmetric layout direction is also helpful. This entire extension is nicely organized.
471-516: Convenient static constructors.
zero,all(_:),horizontal(_:), andvertical(_:)improve readability and usage clarity for typical spacing needs.
519-556: Additional text-related categories.
Defining category-specific enums (e.g.,.edgeBelowText,.edgeRightText) is a neat way to keep specialized spacing rules well-structured.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #245 +/- ##
==========================================
+ Coverage 20.24% 21.44% +1.20%
==========================================
Files 326 326
Lines 14609 14793 +184
==========================================
+ Hits 2957 3173 +216
+ Misses 11652 11620 -32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Spacingstruct, now supporting complex spacing definitions and calculations.spacing()method in the layout system to return an emptySpacingobject.Tests
Spacingfunctionality, validating various aspects such as category uniqueness, spacing calculations, and method behaviors.