Skip to content

Commit

Permalink
improve onPreferenceChange closure MainActor handling (#51)
Browse files Browse the repository at this point in the history
# improve `onPreferenceChange` closure MainActor handling

## ♻️ Current situation & Problem
In response to Apple changing the `onPreferenceChange` API to no longer
MainActor-constrain its closure parameter,
#48 and
#50 updated all
`onPreferenceChange` calls to either schedule the change onto the
MainActor, or (in some cases) check if the closure is being called on
the MainActor, and then run it directly.

This PR fixes this, by using the `runOrScheduleOnMainActor` function
from SpeziFoundation


## ⚙️ Release Notes 
- Improve MainActor interactions in `onPreferenceChange` closures.


## 📚 Documentation
n/a


## ✅ Testing
There are no dedicated new tests but these changes do in fact fix tests
elsewhere.

## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).

---------

Co-authored-by: Andreas Bauer <[email protected]>
  • Loading branch information
lukaskollmer and Supereg authored Jan 19, 2025
1 parent 69b0857 commit 540010e
Show file tree
Hide file tree
Showing 11 changed files with 20 additions and 51 deletions.
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 {
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 {
@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 {
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 @@ final class ViewsTests: XCTestCase {

#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 @@ final class ViewsTests: XCTestCase {
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 @@ final class ViewsTests: XCTestCase {
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

0 comments on commit 540010e

Please sign in to comment.