Add zIndex API #243
Add zIndex API #243
Conversation
WalkthroughThis pull request introduces a new view layer ordering feature into the OpenSwiftUICore framework. A new file extends the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant V as View Extension
participant TC as ViewTraitCollection
participant TK as ZIndexTraitKey
Dev->>V: call zIndex(value)
V->>TC: update trait (using TK)
TC-->>V: trait updated
V-->>Dev: return modified view with new z-index
Poem
✨ 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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Sources/OpenSwiftUICore/Layout/ZIndex.swift(1 hunks)Tests/OpenSwiftUICoreTests/Layout/ZIndexTests.swift(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
Tests/OpenSwiftUICoreTests/Layout/ZIndexTests.swift (1)
Sources/OpenSwiftUICore/Layout/ZIndex.swift (1)
zIndex(36-39)
🪛 SwiftLint (0.57.0)
Tests/OpenSwiftUICoreTests/Layout/ZIndexTests.swift
[Warning] 19-19: TODOs should be resolved (Add a test helper to hook into...)
(todo)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Execute tests on Ubuntu (6.0.1)
- GitHub Check: Execute tests on iOS (macos-15, 16.0, 2021, 18.0)
- GitHub Check: Execute tests on iOS (macos-15, 16.0, 2024, 18.0)
- GitHub Check: Execute compatibility tests for iOS (macos-15, 16.0, 2024, 18.0)
- GitHub Check: Execute tests on macOS (macos-15, 16.0, 2021)
- GitHub Check: Execute compatibility tests for macOS (macos-15, 16.0, 2024)
🔇 Additional comments (6)
Sources/OpenSwiftUICore/Layout/ZIndex.swift (6)
8-41: Clean, well-documented implementation of zIndex API.The View extension and its documentation are excellent. The documentation clearly explains the purpose, provides a practical example with visual reference, and specifies the default value. The implementation follows the trait pattern used elsewhere in the framework.
36-39: Implementation uses trait system correctly.The zIndex implementation properly uses the trait system to store the value, with appropriate annotations for inlining and actor isolation.
42-46: Trait key implementation is appropriate.The ZIndexTraitKey implementation provides the correct default value (0.0) matching the documented behavior.
48-49: Correctly marked as unavailable for Sendable.This prevents potential concurrent access issues with the trait key.
51-56: ViewTraitCollection extension provides clean accessor.The computed property offers a convenient way to access the zIndex value in the trait collection.
36-39: Consider validating the zIndex implementation with comprehensive tests.While the implementation looks solid, the associated test file (ZIndexTests.swift) doesn't yet verify the behavior of this API. Consider implementing the test helper mentioned in the TODO to ensure the functionality works as expected.
| func indexOrder() { | ||
| struct ContentView: View { | ||
| var body: some View { | ||
| Color.red | ||
| .zIndex(0.5) | ||
| } | ||
| } | ||
| // TODO: Add a test helper to hook into makeViewList and retrieve the zIndex value to verify it | ||
| } |
There was a problem hiding this comment.
Incomplete test implementation.
The test defines a view with a zIndex modifier but doesn't include any assertions to verify the functionality. This means the test isn't actually testing anything yet.
Consider implementing the test helper mentioned in the TODO comment or using a different approach to verify that the zIndex value is correctly applied to the view. Without assertions, this test doesn't provide any validation of the functionality.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 19-19: TODOs should be resolved (Add a test helper to hook into...)
(todo)
| .zIndex(0.5) | ||
| } | ||
| } | ||
| // TODO: Add a test helper to hook into makeViewList and retrieve the zIndex value to verify it |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Address TODO comment before merging.
TODOs in committed code can lead to forgotten tasks and technical debt.
Either implement the test helper mentioned in the TODO, create a tracked issue to address it later, or add more context about why it can't be implemented yet.
🌐 Web query:
What are best practices for testing SwiftUI view modifiers?
💡 Result:
To ensure SwiftUI view modifiers are thoroughly tested and maintainable, follow these best practices:
Leverage ViewInspector for Hierarchy Inspection
Use ViewInspector to verify modifiers are applied correctly by inspecting the view hierarchy:
func testButtonStyle() throws {
let view = Button("Test", action: {})
.buttonStyle(PrimaryButtonStyle())
let button = try view.inspect().button()
XCTAssertEqual(try button.label().text().string(), "Test")
XCTAssertTrue(try button.actualButtonStyle() is PrimaryButtonStyle) [1][5]
}This validates both the presence of the modifier and its concrete type.
Test Modifier Side Effects
Verify visual and behavioral impacts through:
- State changes: Check if modifiers alter @State/@binding
XCTAssertTrue(viewModel.items.first!.isCompleted) [1]
- View updates: Confirm layout changes after modifiers
XCTAssertEqual(try list.count, 1) [1]
Structure Modifiers for Testability
- Isolate pure modifiers without external dependencies
- Create test targets specifically for modifier validation [5]
- Use @testable imports to access internal components
@testable import YourAppModule [5]
Validate Custom ViewModifier Conformance
Test custom modifiers by:
- Checking protocol conformance
- Verifying modifier composition
struct TitleModifier: ViewModifier {
func body(content: Content) -> some View {
content
.font(.largeTitle)
.foregroundColor(.blue)
}
Performance and Safety
- Profile with Instruments to detect rendering bottlenecks from complex modifiers [8]
- Prevent recursion using safety attributes:
@warn_unqualified_access func customModifier() -> some View [10]
Test Modifier Combinations
Validate interactions between multiple modifiers:
Text("Hello")
.titleStyle()
.watermarked("TEST")
// Verify both modifiers apply correctly [4][7]Asynchronous Testing
Handle modifiers with async operations using XCTest expectations:
let expectation = self.expectation(description: "Async modifier")
// Test async code
waitForExpectations(timeout: 1) [5]By combining these strategies, you can create a robust testing suite that ensures view modifiers behave as expected across different states and configurations.
Citations:
- 1: https://www.kodeco.com/books/swiftui-cookbook/v1.0/chapters/9-testing-swiftui-views-with-viewinspector
- 2: https://www.kodeco.com/books/swiftui-cookbook/v1.0/chapters/8-unit-testing-strategies-in-swiftui
- 3: https://forums.swift.org/t/whats-the-best-way-to-conditionally-include-exclude-some-lines-of-code-for-performance-profiling/36532
- 4: https://www.hackingwithswift.com/books/ios-swiftui/custom-modifiers
- 5: https://www.kodeco.com/30227776-swiftui-testing-with-viewinspector-for-ios
- 6: https://developer.apple.com/documentation/swiftui/reducing-view-modifier-maintenance
- 7: https://www.avanderlee.com/swiftui/view-composition-viewmodifiers/
- 8: https://www.youtube.com/watch?v=aH15GUzk85Y
- 9: https://www.hackingwithswift.com/forums/100-days-of-swiftui/help-with-views-and-modifiers-day-24-challenge-2/4046
- 10: https://www.hackingwithswift.com/quick-start/swiftui/how-to-make-swiftui-modifiers-safer-to-use-with-warn-unqualified-access
Action Required: Implement or Track the Z‑Index Test Helper
Before merging, please address the pending TODO in Tests/OpenSwiftUICoreTests/Layout/ZIndexTests.swift at line 19. The file currently contains this placeholder:
// TODO: Add a test helper to hook into makeViewList and retrieve the zIndex value to verify itTo align with SwiftUI testing best practices, consider the following options:
- Implement the Test Helper: Use a solution such as ViewInspector to inspect the view hierarchy and verify that the expected zIndex is applied.
- Document and Track: If implementing this helper immediately isn’t feasible, add more context in the code explaining the limitation and create a tracked issue to ensure this task is addressed later.
Taking one of these actions will help prevent technical debt by ensuring that all view modifiers are properly tested.
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 19-19: TODOs should be resolved (Add a test helper to hook into...)
(todo)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #243 +/- ##
==========================================
+ Coverage 20.00% 20.03% +0.02%
==========================================
Files 325 326 +1
Lines 14583 14588 +5
==========================================
+ Hits 2918 2922 +4
- Misses 11665 11666 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Tests