Fix an issue where resizing scroll views could result in their content offset being changed erroneously (#557)

### Bug

https://github.com/user-attachments/assets/7f93f74d-0b54-4131-ac52-8970ec623452

### Notes
🐛 If you have a partial modal (or any other modal that resizes), and you
resize the modal while the keyboard is focused and scrolled away from
the top, to be taller (eg by adding content), the scroll view within
would jump up towards the top.

Westin and I had dug in a while ago and thought it was just a UIKit bug,
but it turns out it's an order of operations thing: You need to apply
the new `contentSize` to the scroll view before you make its frame
taller, otherwise UIKit's automatic `contentOffset` adjustment kicks you
back towards the top.

To fix this, I've added a `applyBeforeLayout` addition to
`ViewDescription.Config`, which will be applied before the
`LayoutAttributes` (except on initial view allocation – we still apply
the attributes first so the view's properties are sensical).

Author: kyleve

BlueprintUI/Sources/BlueprintView/BlueprintView.swift

@@ -709,6 +709,7 @@ extension BlueprintView {
                         layoutTransition = .inherited
                     }
                     layoutTransition.perform {
+                        child.viewDescription.applyBeforeLayout(to: controller.view)
                         child.layoutAttributes.apply(to: controller.view)
 
                         if pathsChanged {
@@ -729,6 +730,8 @@ extension BlueprintView {
                     UIView.performWithoutAnimation {
                         controller = NativeViewController(node: child)
                         child.layoutAttributes.apply(to: controller.view)
+                        // So the view has a reasonable size during creation/allocation, do this afterwards.
+                        child.viewDescription.applyBeforeLayout(to: controller.view)
 
                         contentView.insertSubview(controller.view, at: index)
 

BlueprintUI/Sources/ViewDescription/ViewDescription.swift

@@ -43,6 +43,7 @@ public struct ViewDescription {
     private let _viewType: UIView.Type
     private let _build: () -> UIView
     private let _apply: (UIView) -> Void
+    private let _applyBeforeLayout: (UIView) -> Void
     private let _contentView: (UIView) -> UIView
 
     private let _layoutTransition: LayoutTransition
@@ -86,6 +87,16 @@ public struct ViewDescription {
             }
         }
 
+        _applyBeforeLayout = { view in
+            let typedView = configuration.typeChecked(view: view)
+            for update in configuration.updatesBeforeLayout {
+                update(typedView)
+            }
+            for binding in configuration.bindings {
+                binding.value.apply(to: typedView)
+            }
+        }
+
         _contentView = { view in
             let typedView = configuration.typeChecked(view: view)
             return configuration.contentView(typedView)
@@ -109,6 +120,10 @@ public struct ViewDescription {
         _build()
     }
 
+    public func applyBeforeLayout(to view: UIView) {
+        _applyBeforeLayout(view)
+    }
+
     public func apply(to view: UIView) {
         _apply(view)
     }
@@ -154,9 +169,12 @@ extension ViewDescription {
         /// The default value instantiates the view using `init(frame:)`.
         public var builder: () -> View
 
-        /// An array of update closures.
+        /// An array of update closures that are applied **after** the `LayoutAttributes` is applied to the view.
         public var updates: [Update]
 
+        /// An array of update closures that are applied **before** the `LayoutAttributes` is applied to the view.
+        public var updatesBeforeLayout: [Update]
+
         /// A closure that takes a native view instance as the single argument, and
         /// returns a subview of that view into which child views should be added
         /// and managed.
@@ -201,6 +219,7 @@ extension ViewDescription {
         public init() {
             builder = { View(frame: .zero) }
             updates = []
+            updatesBeforeLayout = []
             contentView = { $0 }
         }
 
@@ -217,11 +236,24 @@ extension ViewDescription {
 
 extension ViewDescription.Configuration {
 
-    /// Adds the given update closure to the `updates` array.
+    /// Adds the given update closure to the `updates` array,
+    /// and applies them  **after** the `LayoutAttributes` is applied to the view.
     public mutating func apply(_ update: @escaping Update) {
         updates.append(update)
     }
 
+    /// Adds the given update closure to the `updates` array,
+    /// and applies them  **before** the `LayoutAttributes` is applied to the view.
+    ///
+    /// > During view allocation and creation, this closure is applied after initial `LayoutAttributes`
+    /// are applied, so the view has reasonable default values.
+    ///
+    /// > You should rarely need to use this method, unless you need to apply an update to the view
+    /// before one of the properties on set by `LayoutAttributes` is set.
+    public mutating func applyBeforeLayout(_ update: @escaping Update) {
+        updatesBeforeLayout.append(update)
+    }
+
     /// Subscript for values that are not optional. We must represent these values as optional so that we can
     /// return nil from the subscript in the case where no value has been assigned for the given keypath.
     ///

BlueprintUI/Tests/ViewDescriptionTests.swift

@@ -31,6 +31,24 @@ class ViewDescriptionTests: XCTestCase {
         XCTAssertEqual(view.mutableProperty, "123")
     }
 
+    func test_applyBeforeLayout() {
+        let description = TestView.describe { config in
+            config.applyBeforeLayout {
+                $0.mutableProperty = "testing"
+            }
+        }
+
+        let view = description.build() as! TestView
+        description.applyBeforeLayout(to: view)
+        XCTAssertEqual(view.mutableProperty, "testing")
+
+        let secondDescription = TestView.describe { config in
+            config.applyBeforeLayout { $0.mutableProperty = "123" }
+        }
+        secondDescription.applyBeforeLayout(to: view)
+        XCTAssertEqual(view.mutableProperty, "123")
+    }
+
     func test_bind() {
         let description = TestView.describe { config in
             config[\.mutableProperty] = "testing"

BlueprintUICommonControls/Sources/ScrollView.swift

@@ -57,8 +57,34 @@ public struct ScrollView: Element {
 
             config.contentView = { $0.scrollView }
 
-            config.apply {
-                $0.apply(scrollView: self, contentFrame: context.subtreeExtent ?? .zero)
+            let contentFrame = context.subtreeExtent ?? .zero
+
+            config.applyBeforeLayout { view in
+
+                /// We apply the `contentSize` before the `LayoutAttributes` because order of operations matters
+                /// when adjusting the size of the scroll view's frame and the content size. If we set the frame before
+                /// setting the content size – in particular making the frame taller before adjusting the content size,
+                /// the scroll view will internally adjust the `contentOffset`, resulting in the on-screen content
+                /// jumping around.
+                ///
+                /// This is most visible and annoying when you have a scroll view in a resizable modal, which is scrolled away
+                /// from the top of its content. If the size of the scroll view grows before the `contentSize` is adjusted,
+                /// the visible content will shift.
+
+                let contentSize = switch contentSize {
+                case .fittingWidth, .fittingHeight, .fittingContent:
+                    CGSize(width: contentFrame.maxX, height: contentFrame.maxY)
+                case .custom(let custom):
+                    custom
+                }
+
+                if view.scrollView.contentSize != contentSize {
+                    view.scrollView.contentSize = contentSize
+                }
+            }
+
+            config.apply { view in
+                view.apply(scrollView: self, contentFrame: contentFrame)
             }
         }
     }
@@ -346,15 +372,6 @@ fileprivate final class ScrollerWrapperView: UIView {
             }
         }
 
-        let contentSize: CGSize
-
-        switch scrollView.contentSize {
-        case .fittingWidth, .fittingHeight, .fittingContent:
-            contentSize = CGSize(width: contentFrame.maxX, height: contentFrame.maxY)
-        case .custom(let customSize):
-            contentSize = customSize
-        }
-
         if self.scrollView.alwaysBounceHorizontal != scrollView.alwaysBounceHorizontal {
             self.scrollView.alwaysBounceHorizontal = scrollView.alwaysBounceHorizontal
         }
@@ -363,10 +380,6 @@ fileprivate final class ScrollerWrapperView: UIView {
             self.scrollView.alwaysBounceVertical = scrollView.alwaysBounceVertical
         }
 
-        if self.scrollView.contentSize != contentSize {
-            self.scrollView.contentSize = contentSize
-        }
-
         if self.scrollView.showsVerticalScrollIndicator != scrollView.showsVerticalScrollIndicator {
             self.scrollView.showsVerticalScrollIndicator = scrollView.showsVerticalScrollIndicator
         }

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Fixed bounding rects for VoiceOver when an attributed label's link spans more than one line.
+- Fixed an issue where resizing a `ScrollView` could result in its scroll position being adjusted incorrectly.
 
 ### Added