From c1bf3c87885a3a5eb7534915365e7bb1d03ad783 Mon Sep 17 00:00:00 2001 From: Andrew Watt Date: Mon, 27 Jan 2025 18:01:13 -0800 Subject: [PATCH 1/3] feat: flexible swiftui preferredContentSize calc --- .../Sources/ObservableScreen.swift | 66 +++++++++---------- .../Sources/SwiftUIScreen.swift | 64 ++++++++---------- 2 files changed, 59 insertions(+), 71 deletions(-) diff --git a/WorkflowSwiftUI/Sources/ObservableScreen.swift b/WorkflowSwiftUI/Sources/ObservableScreen.swift index a888a6d5..40c85b06 100644 --- a/WorkflowSwiftUI/Sources/ObservableScreen.swift +++ b/WorkflowSwiftUI/Sources/ObservableScreen.swift @@ -97,14 +97,15 @@ private final class ModeledHostingController: UIHostingCon var swiftUIScreenSizingOptions: SwiftUIScreenSizingOptions { didSet { updateSizingOptionsIfNeeded() - - if !hasLaidOutOnce { + if isViewLoaded { setNeedsLayoutBeforeFirstLayoutIfNeeded() } } } private var hasLaidOutOnce = false + private var maxFrameWidth: CGFloat = 0 + private var maxFrameHeight: CGFloat = 0 init( setModel: @escaping (Model) -> Void, @@ -133,7 +134,7 @@ private final class ModeledHostingController: UIHostingCon super.viewDidLoad() // `UIHostingController`'s provides a system background color by default. In order to - // support `ObervableModelScreen`s being composed in contexts where it is composed within another + // support `SwiftUIScreen`s being composed in contexts where it is composed within another // view controller where a transparent background is more desirable, we set the background // to clear to allow this kind of flexibility. view.backgroundColor = .clear @@ -146,21 +147,31 @@ private final class ModeledHostingController: UIHostingCon defer { hasLaidOutOnce = true } - if #available(iOS 16.0, *) { - // Handled in initializer, but set it on first layout to resolve a bug where the PCS is - // not updated appropriately after the first layout. - // UI-5797 - if !hasLaidOutOnce, - swiftUIScreenSizingOptions.contains(.preferredContentSize) - { - let size = view.sizeThatFits(view.frame.size) - - if preferredContentSize != size { - preferredContentSize = size - } - } - } else if swiftUIScreenSizingOptions.contains(.preferredContentSize) { - let size = view.sizeThatFits(view.frame.size) + if swiftUIScreenSizingOptions.contains(.preferredContentSize) { + // Use the largest frame ever laid out in as a constraint for preferredContentSize + // measurements. + let width = max(view.frame.width, maxFrameWidth) + let height = max(view.frame.height, maxFrameHeight) + + maxFrameWidth = width + maxFrameHeight = height + + let fixedSize = CGSize(width: width, height: height) + + // Measure a few different ways to account for ScrollView behavior. ScrollViews will + // always greedily fill the space available, but will report the natural content size + // when given an infinite size. By combining the results of these measurements we can + // deduce the natural size of content that scrolls in either direction, or both, or + // neither. + + let fixedResult = view.sizeThatFits(fixedSize) + let unboundedHorizontalResult = view.sizeThatFits(CGSize(width: .infinity, height: fixedSize.height)) + let unboundedVerticalResult = view.sizeThatFits(CGSize(width: fixedSize.width, height: .infinity)) + + let size = CGSize( + width: min(fixedResult.width, unboundedHorizontalResult.width), + height: min(fixedResult.height, unboundedVerticalResult.height) + ) if preferredContentSize != size { preferredContentSize = size @@ -175,10 +186,6 @@ private final class ModeledHostingController: UIHostingCon } private func updateSizingOptionsIfNeeded() { - if #available(iOS 16.0, *) { - self.sizingOptions = swiftUIScreenSizingOptions.uiHostingControllerSizingOptions - } - if !swiftUIScreenSizingOptions.contains(.preferredContentSize), preferredContentSize != .zero { @@ -187,7 +194,7 @@ private final class ModeledHostingController: UIHostingCon } private func setNeedsLayoutBeforeFirstLayoutIfNeeded() { - if swiftUIScreenSizingOptions.contains(.preferredContentSize) { + if swiftUIScreenSizingOptions.contains(.preferredContentSize), !hasLaidOutOnce { // Without manually calling setNeedsLayout here it was observed that a call to // layoutIfNeeded() immediately after loading the view would not perform a layout, and // therefore would not update the preferredContentSize in viewDidLayoutSubviews(). @@ -203,17 +210,4 @@ private final class ModeledHostingController: UIHostingCon } } -extension SwiftUIScreenSizingOptions { - @available(iOS 16.0, *) - fileprivate var uiHostingControllerSizingOptions: UIHostingControllerSizingOptions { - var options = UIHostingControllerSizingOptions() - - if contains(.preferredContentSize) { - options.insert(.preferredContentSize) - } - - return options - } -} - #endif diff --git a/WorkflowSwiftUIExperimental/Sources/SwiftUIScreen.swift b/WorkflowSwiftUIExperimental/Sources/SwiftUIScreen.swift index b00d33dd..79fecaee 100644 --- a/WorkflowSwiftUIExperimental/Sources/SwiftUIScreen.swift +++ b/WorkflowSwiftUIExperimental/Sources/SwiftUIScreen.swift @@ -98,14 +98,15 @@ private final class ModeledHostingController: UIHostingCon var swiftUIScreenSizingOptions: SwiftUIScreenSizingOptions { didSet { updateSizingOptionsIfNeeded() - - if !hasLaidOutOnce { + if isViewLoaded { setNeedsLayoutBeforeFirstLayoutIfNeeded() } } } private var hasLaidOutOnce = false + private var maxFrameWidth: CGFloat = 0 + private var maxFrameHeight: CGFloat = 0 init( modelSink: Sink, @@ -144,21 +145,31 @@ private final class ModeledHostingController: UIHostingCon defer { hasLaidOutOnce = true } - if #available(iOS 16.0, *) { - // Handled in initializer, but set it on first layout to resolve a bug where the PCS is - // not updated appropriately after the first layout. - // UI-5797 - if !hasLaidOutOnce, - swiftUIScreenSizingOptions.contains(.preferredContentSize) - { - let size = view.sizeThatFits(view.frame.size) - - if preferredContentSize != size { - preferredContentSize = size - } - } - } else if swiftUIScreenSizingOptions.contains(.preferredContentSize) { - let size = view.sizeThatFits(view.frame.size) + if swiftUIScreenSizingOptions.contains(.preferredContentSize) { + // Use the largest frame ever laid out in as a constraint for preferredContentSize + // measurements. + let width = max(view.frame.width, maxFrameWidth) + let height = max(view.frame.height, maxFrameHeight) + + maxFrameWidth = width + maxFrameHeight = height + + let fixedSize = CGSize(width: width, height: height) + + // Measure a few different ways to account for ScrollView behavior. ScrollViews will + // always greedily fill the space available, but will report the natural content size + // when given an infinite size. By combining the results of these measurements we can + // deduce the natural size of content that scrolls in either direction, or both, or + // neither. + + let fixedResult = view.sizeThatFits(fixedSize) + let unboundedHorizontalResult = view.sizeThatFits(CGSize(width: .infinity, height: fixedSize.height)) + let unboundedVerticalResult = view.sizeThatFits(CGSize(width: fixedSize.width, height: .infinity)) + + let size = CGSize( + width: min(fixedResult.width, unboundedHorizontalResult.width), + height: min(fixedResult.height, unboundedVerticalResult.height) + ) if preferredContentSize != size { preferredContentSize = size @@ -173,10 +184,6 @@ private final class ModeledHostingController: UIHostingCon } private func updateSizingOptionsIfNeeded() { - if #available(iOS 16.0, *) { - self.sizingOptions = swiftUIScreenSizingOptions.uiHostingControllerSizingOptions - } - if !swiftUIScreenSizingOptions.contains(.preferredContentSize), preferredContentSize != .zero { @@ -185,7 +192,7 @@ private final class ModeledHostingController: UIHostingCon } private func setNeedsLayoutBeforeFirstLayoutIfNeeded() { - if swiftUIScreenSizingOptions.contains(.preferredContentSize) { + if swiftUIScreenSizingOptions.contains(.preferredContentSize), !hasLaidOutOnce { // Without manually calling setNeedsLayout here it was observed that a call to // layoutIfNeeded() immediately after loading the view would not perform a layout, and // therefore would not update the preferredContentSize in viewDidLayoutSubviews(). @@ -201,17 +208,4 @@ private final class ModeledHostingController: UIHostingCon } } -extension SwiftUIScreenSizingOptions { - @available(iOS 16.0, *) - fileprivate var uiHostingControllerSizingOptions: UIHostingControllerSizingOptions { - var options = UIHostingControllerSizingOptions() - - if contains(.preferredContentSize) { - options.insert(.preferredContentSize) - } - - return options - } -} - #endif From 7ee2295822f9196add02c0d25ec873adededd5a5 Mon Sep 17 00:00:00 2001 From: Andrew Watt Date: Wed, 29 Jan 2025 17:00:36 -0800 Subject: [PATCH 2/3] test: swiftui preferredcontentsize --- .../Tests/PreferredContentSizeTests.swift | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 WorkflowSwiftUI/Tests/PreferredContentSizeTests.swift diff --git a/WorkflowSwiftUI/Tests/PreferredContentSizeTests.swift b/WorkflowSwiftUI/Tests/PreferredContentSizeTests.swift new file mode 100644 index 00000000..d044abb3 --- /dev/null +++ b/WorkflowSwiftUI/Tests/PreferredContentSizeTests.swift @@ -0,0 +1,123 @@ +#if canImport(UIKit) + +import SwiftUI +import Workflow +import WorkflowSwiftUI +import XCTest + +final class PreferredContentSizeTests: XCTestCase { + func test_preferredContentSize() { + let maxWidth: CGFloat = 50 + let maxHeight: CGFloat = 50 + + func assertPreferredContentSize(in axes: Axis.Set) { + let screen = TestScreen(model: .constant(state: State(axes: axes))) + let viewController = screen.buildViewController(in: .empty) + viewController.view.frame = CGRect(x: 0, y: 0, width: maxWidth, height: maxHeight) + viewController.view.layoutIfNeeded() + + func assertContentSize( + _ contentSize: CGSize, + expected: CGSize? = nil, + file: StaticString = #filePath, + line: UInt = #line + ) { + let state = State(width: contentSize.width, height: contentSize.height, axes: axes) + let screen = TestScreen(model: .constant(state: state)) + screen.update(viewController: viewController, with: .empty) + + viewController.view.layoutIfNeeded() + let pcs = viewController.preferredContentSize + + XCTAssertEqual( + pcs, + expected ?? contentSize, + "Axes: \(axes.testDescription)", + file: file, + line: line + ) + } + + assertContentSize(CGSize(width: 20, height: 20)) + assertContentSize(CGSize(width: 40, height: 20)) + assertContentSize(CGSize(width: 20, height: 40)) + assertContentSize( + CGSize(width: 100, height: 100), + expected: CGSize( + width: axes.contains(.horizontal) ? maxWidth : 100, + height: axes.contains(.vertical) ? maxHeight : 100 + ) + ) + } + + assertPreferredContentSize(in: []) + assertPreferredContentSize(in: .horizontal) + assertPreferredContentSize(in: .vertical) + assertPreferredContentSize(in: [.horizontal, .vertical]) + } +} + +extension Axis.Set { + var testDescription: String { + switch self { + case .horizontal: "[horizontal]" + case .vertical: "[vertical]" + case [.horizontal, .vertical]: "[horizontal, vertical]" + default: "[]" + } + } +} + +@ObservableState +private struct State { + var width: CGFloat = 0 + var height: CGFloat = 0 + var axes: Axis.Set = [] +} + +private struct TestWorkflow: Workflow { + typealias Rendering = StateAccessor + + func makeInitialState() -> State { + State() + } + + func render(state: State, context: RenderContext) -> Rendering { + context.makeStateAccessor(state: state) + } +} + +private struct TestScreen: ObservableScreen { + typealias Model = StateAccessor + + var model: Model + + var sizingOptions: SwiftUIScreenSizingOptions = .preferredContentSize + + @ViewBuilder + static func makeView(store: Store) -> some View { + TestView(store: store) + } +} + +private struct TestView: View { + var store: Store> + + var body: some View { + WithPerceptionTracking { + if store.axes.isEmpty { + box + } else { + ScrollView(store.axes) { + box + } + } + } + } + + var box: some View { + Color.red.frame(width: store.width, height: store.height) + } +} + +#endif From 47cc3b68c12f0dfa01bc33e4fb2fff0103fe4f79 Mon Sep 17 00:00:00 2001 From: Andrew Watt Date: Thu, 30 Jan 2025 16:46:28 -0800 Subject: [PATCH 3/3] chore: update comment about background color --- WorkflowSwiftUI/Sources/ObservableScreen.swift | 7 +++---- WorkflowSwiftUIExperimental/Sources/SwiftUIScreen.swift | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/WorkflowSwiftUI/Sources/ObservableScreen.swift b/WorkflowSwiftUI/Sources/ObservableScreen.swift index 40c85b06..fc000d79 100644 --- a/WorkflowSwiftUI/Sources/ObservableScreen.swift +++ b/WorkflowSwiftUI/Sources/ObservableScreen.swift @@ -133,10 +133,9 @@ private final class ModeledHostingController: UIHostingCon override func viewDidLoad() { super.viewDidLoad() - // `UIHostingController`'s provides a system background color by default. In order to - // support `SwiftUIScreen`s being composed in contexts where it is composed within another - // view controller where a transparent background is more desirable, we set the background - // to clear to allow this kind of flexibility. + // `UIHostingController` provides a system background color by default. We set the + // background to clear to support contexts where it is composed within another view + // controller. view.backgroundColor = .clear setNeedsLayoutBeforeFirstLayoutIfNeeded() diff --git a/WorkflowSwiftUIExperimental/Sources/SwiftUIScreen.swift b/WorkflowSwiftUIExperimental/Sources/SwiftUIScreen.swift index 79fecaee..a673b39e 100644 --- a/WorkflowSwiftUIExperimental/Sources/SwiftUIScreen.swift +++ b/WorkflowSwiftUIExperimental/Sources/SwiftUIScreen.swift @@ -131,10 +131,9 @@ private final class ModeledHostingController: UIHostingCon override func viewDidLoad() { super.viewDidLoad() - // `UIHostingController`'s provides a system background color by default. In order to - // support `SwiftUIScreen`s being composed in contexts where it is composed within another - // view controller where a transparent background is more desirable, we set the background - // to clear to allow this kind of flexibility. + // `UIHostingController` provides a system background color by default. We set the + // background to clear to support contexts where it is composed within another view + // controller. view.backgroundColor = .clear setNeedsLayoutBeforeFirstLayoutIfNeeded()