Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve onPreferenceChange closure MainActor handling #51

Merged
merged 9 commits into from
Jan 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ let package = Package(
],
dependencies: [
.package(url: "https://github.com/StanfordSpezi/Spezi.git", from: "1.8.0"),
.package(url: "https://github.com/StanfordSpezi/SpeziFoundation.git", from: "2.0.1"),
.package(url: "https://github.com/apple/swift-collections.git", from: "1.1.0"),
.package(url: "https://github.com/pointfreeco/swift-snapshot-testing.git", from: "1.17.0")
] + swiftLintPackage(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,10 @@
// SPDX-License-Identifier: MIT
//

import SpeziFoundation
import SwiftUI


@MainActor
private struct IsolatedValidationBinding: Sendable {
let state: ValidationState.Binding

init(_ state: ValidationState.Binding) {
self.state = state
}
}


/// Provide access to validation state to the parent view.
///
/// The internal preference key to provide parent views access to all configured ``ValidationEngine`` and input
Expand Down Expand Up @@ -46,11 +37,9 @@ extension View {
/// - Parameter state: The binding to the ``ValidationState``.
/// - Returns: The modified view.
public func receiveValidation(in state: ValidationState.Binding) -> some View {
let binding = IsolatedValidationBinding(state)

return onPreferenceChange(CapturedValidationStateKey.self) { entries in
Task { @Sendable @MainActor in
binding.state.wrappedValue = ValidationContext(entries: entries)
onPreferenceChange(CapturedValidationStateKey.self) { entries in
runOrScheduleOnMainActor {
state.wrappedValue = ValidationContext(entries: entries)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import SwiftUI
///
/// You can use this structure to retrieve the state of all ``ValidationEngine``s of a subview or manually
/// initiate validation by calling ``validateSubviews(switchFocus:)``. E.g., when pressing on a submit button of a form.
public struct ValidationContext {
public struct ValidationContext: Sendable {
Supereg marked this conversation as resolved.
Show resolved Hide resolved
private let entries: [CapturedValidationState]


Expand Down
4 changes: 2 additions & 2 deletions Sources/SpeziValidation/ValidationState/ValidationState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ public struct ValidationState: DynamicProperty {
extension ValidationState {
/// A binding to a ``ValidationState``.
@propertyWrapper
public struct Binding {
private let binding: SwiftUI.Binding<ValidationContext>
public struct Binding: Sendable {
lukaskollmer marked this conversation as resolved.
Show resolved Hide resolved
@MainActor private let binding: SwiftUI.Binding<ValidationContext>

/// The validation context.
public var wrappedValue: ValidationContext {
Expand Down
15 changes: 3 additions & 12 deletions Sources/SpeziViews/Views/Layout/HorizontalGeometryReader.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// SPDX-License-Identifier: MIT
//

import SpeziFoundation
import SwiftUI


Expand Down Expand Up @@ -39,18 +40,8 @@ public struct HorizontalGeometryReader<Content: View>: View {
}
)
.onPreferenceChange(WidthPreferenceKey.self) { width in
// The `onPreferenceChange` view modfier now takes a `@Sendable` closure, therefore we cannot capture `@MainActor` isolated properties
// on the `View` directly anymore: https://developer.apple.com/documentation/swiftui/view/onpreferencechange(_:perform:)?changes=latest_minor
// However, as the `@Sendable` closure is still run on the MainActor (at least in my testing on 18.2 RC SDKs), we can use `MainActor.assumeIsolated`
// to avoid scheduling a `MainActor` `Task`, which could delay execution and cause unexpected UI behavior.
if Thread.isMainThread {
MainActor.assumeIsolated {
self.width = width
}
} else {
Task { @MainActor in
self.width = width
}
runOrScheduleOnMainActor {
lukaskollmer marked this conversation as resolved.
Show resolved Hide resolved
self.width = width
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions Tests/SpeziViewsTests/SnapshotTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ final class SnapshotTests: XCTestCase {
Text(verbatim: "20 °C, Sunny")
}
}



let largeRow = row
.dynamicTypeSize(.accessibility3)

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 3 additions & 9 deletions Tests/UITests/TestApp/ViewsTests/CanvasTestView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#if canImport(PencilKit) && !os(macOS)
import PencilKit
import SpeziFoundation
import SpeziViews
import SwiftUI

Expand Down Expand Up @@ -47,15 +48,8 @@ struct CanvasTestView: View {
}
.navigationBarTitleDisplayMode(.inline)
.onPreferenceChange(CanvasView.CanvasSizePreferenceKey.self) { size in
// See `HorizontalGeometryReader.swift`
if Thread.isMainThread {
MainActor.assumeIsolated {
self.receivedSize = size
}
} else {
Task { @MainActor in
self.receivedSize = size
}
runOrScheduleOnMainActor {
self.receivedSize = size
}
}
}
Expand Down
15 changes: 5 additions & 10 deletions Tests/UITests/TestAppUITests/SpeziViews/ViewsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,17 @@

#if os(visionOS)
// visionOS doesn't have the image anymore, this should be enough to check
let paletteView = app.scrollViews.otherElements["Pen, black"]
let penView = app.scrollViews.otherElements["Pen, black"]
#else
let paletteView = app.images["palette_tool_pencil_base"]
let penView = app.buttons["Pen"]
#endif


XCTAssert(app.collectionViews.buttons["Canvas"].waitForExistence(timeout: 2))
app.collectionViews.buttons["Canvas"].tap()

XCTAssert(app.staticTexts["Did Draw Anything: false"].waitForExistence(timeout: 2))
XCTAssertFalse(paletteView.exists)
XCTAssertFalse(penView.exists)

let canvasView = app.scrollViews.firstMatch
canvasView.swipeRight()
Expand All @@ -55,7 +55,7 @@
XCTAssert(app.buttons["Show Tool Picker"].waitForExistence(timeout: 2))
app.buttons["Show Tool Picker"].tap()

XCTAssertTrue(paletteView.waitForExistence(timeout: 5))
XCTAssertTrue(penView.waitForExistence(timeout: 5))
canvasView.swipeLeft()

XCTAssertTrue(canvasView.waitForExistence(timeout: 2.0))
Expand All @@ -65,12 +65,7 @@
return // the pencilKit toolbar cannot be hidden anymore on visionOS
#endif

#if compiler(>=6)
XCTAssertTrue(paletteView.waitForNonExistence(timeout: 15))
#else
sleep(15) // waitForExistence will otherwise return immediately
XCTAssertFalse(paletteView.exists)
#endif
XCTAssertTrue(penView.waitForNonExistence(timeout: 15))

Check warning on line 68 in Tests/UITests/TestAppUITests/SpeziViews/ViewsTests.swift

View workflow job for this annotation

GitHub Actions / Build and Test UI Tests visionOS / Test using xcodebuild or run fastlane

code after 'return' will never be executed
canvasView.swipeUp()
}

Expand Down
Loading