Fix measurement of Row and Column on iOS inside of UIStackView

UIStackView uses UIView.intrinsicContentSize on its subviews, but
our Row and Column implementations don't have a natural implementation
of this API. In particular, if any subview wraps its contents (so
its height depends on its available width), there's no single
intrinsicContentSize().

We work around this by following some online advice: invalidate
the intrinsic size when the bounds change, and then use those
bounds when computing the new intrinsic size.

This is more fragile and stateful than I'd like, but it seems
like a common practice for working around UIStackView's API.

CHANGELOG.md

@@ -13,6 +13,7 @@ Changed:
 Fixed:
 - Fix a layout bug where children of fixed-with `Row` containers were assigned the wrong width.
 - Fix inconsistencies between iOS and Android for `Column` and `Row` layouts.
+- Fix a layout bug where `Row` and `Column` layouts reported the wrong dimensions if their subviews could wrap.
 - Correctly update the layout when a Box's child's modifiers are removed.
 - Fix a layout bug where children of `Box` containers were not measured properly.
 

redwood-layout-shared-test/src/commonMain/kotlin/app/cash/redwood/layout/AbstractFlexContainerTest.kt

@@ -885,6 +885,63 @@ abstract class AbstractFlexContainerTest<T : Any> {
 
     snapshotter(root.value).snapshot()
   }
+
+  @Test fun testIntrinsicContentSizeWhenSubviewsWrap() {
+    val fullWidthParent = widgetFactory.column()
+
+    flexContainer(FlexDirection.Column)
+      .apply {
+        width(Constraint.Fill)
+        height(Constraint.Wrap)
+        margin(Margin(horizontal = 30.dp))
+        add(widgetFactory.text("A ".repeat(50)))
+        add(widgetFactory.text("B ".repeat(50)))
+        onEndChanges()
+      }
+      .also { fullWidthParent.add(it.value) }
+
+    flexContainer(FlexDirection.Column)
+      .apply {
+        width(Constraint.Fill)
+        height(Constraint.Wrap)
+        margin(Margin(horizontal = 40.dp))
+        add(widgetFactory.text("C ".repeat(50)))
+        add(widgetFactory.text("D ".repeat(50)))
+        onEndChanges()
+      }
+      .also { fullWidthParent.add(it.value) }
+
+    flexContainer(FlexDirection.Column)
+      .apply {
+        width(Constraint.Fill)
+        height(Constraint.Wrap)
+        margin(Margin(horizontal = 50.dp))
+        add(widgetFactory.text("E ".repeat(50)))
+        add(widgetFactory.text("F ".repeat(50)))
+        onEndChanges()
+      }
+      .also { fullWidthParent.add(it.value) }
+
+    val scrollWrapper = widgetFactory.scrollWrapper()
+    scrollWrapper.content = fullWidthParent.value
+    snapshotter(scrollWrapper.value).snapshot(scrolling = true)
+  }
+
+  @Test fun testIntrinsicContentSizeWhenSubviewsRequireScrolling() {
+    val column = flexContainer(FlexDirection.Column)
+      .apply {
+        width(Constraint.Fill)
+        height(Constraint.Wrap)
+        margin(Margin(horizontal = 50.dp))
+        add(widgetFactory.text("AAAAA BBBB CCC DD E ".repeat(50)))
+        add(widgetFactory.text("FFFFF GGGG HHH II J ".repeat(50)))
+        onEndChanges()
+      }
+
+    val scrollWrapper = widgetFactory.scrollWrapper()
+    scrollWrapper.content = column.value
+    snapshotter(scrollWrapper.value).snapshot(scrolling = true)
+  }
 }
 
 interface TestFlexContainer<T : Any> :

redwood-layout-uiview/src/commonMain/kotlin/app/cash/redwood/layout/uiview/YogaUIView.kt

@@ -16,6 +16,7 @@ import kotlinx.cinterop.CValue
 import kotlinx.cinterop.cValue
 import kotlinx.cinterop.useContents
 import platform.CoreGraphics.CGFloat
+import platform.CoreGraphics.CGRect
 import platform.CoreGraphics.CGRectMake
 import platform.CoreGraphics.CGRectZero
 import platform.CoreGraphics.CGSize
@@ -37,6 +38,9 @@ internal class YogaUIView : UIScrollView(cValue { CGRectZero }), UIScrollViewDel
 
   var onScroll: ((Px) -> Unit)? = null
 
+  private var widthForIntrinsicSize = UIViewNoIntrinsicMetric
+  private var heightForIntrinsicSize = UIViewNoIntrinsicMetric
+
   init {
     // TODO: Support scroll indicators.
     scrollEnabled = false
@@ -45,7 +49,48 @@ internal class YogaUIView : UIScrollView(cValue { CGRectZero }), UIScrollViewDel
     contentInsetAdjustmentBehavior = UIScrollViewContentInsetAdjustmentNever
   }
 
-  override fun intrinsicContentSize(): CValue<CGSize> = calculateLayout()
+  /**
+   * The intrinsic size is broken by design if any subview's height depends on its width (or
+   * vice-versa). For example, if a subview is UILabel that wraps, we need to know how wide the
+   * label is before we can compute that label's height.
+   *
+   * We work around this by:
+   *
+   *  1. Making [intrinsicContentSize] depend on the mostly-recently applied bounds
+   *  2. Invalidating it each time the bounds change
+   *
+   * This will result in an additional layout pass when the parent view uses [intrinsicContentSize].
+   */
+  override fun setBounds(bounds: CValue<CGRect>) {
+    val newWidth = bounds.useContents { size.width }
+    val newHeight = bounds.useContents { size.height }
+    if (isColumn()) {
+      if (widthForIntrinsicSize != newWidth) {
+        invalidateIntrinsicContentSize()
+        this.widthForIntrinsicSize = newWidth
+      }
+    } else {
+      if (heightForIntrinsicSize != newHeight) {
+        invalidateIntrinsicContentSize()
+        this.heightForIntrinsicSize = newHeight
+      }
+    }
+
+    super.setBounds(bounds)
+  }
+
+  override fun invalidateIntrinsicContentSize() {
+    super.invalidateIntrinsicContentSize()
+    this.widthForIntrinsicSize = UIViewNoIntrinsicMetric
+    this.heightForIntrinsicSize = UIViewNoIntrinsicMetric
+  }
+
+  override fun intrinsicContentSize(): CValue<CGSize> {
+    return calculateLayout(
+      width = widthForIntrinsicSize.toYogaWithWidthConstraint(),
+      height = heightForIntrinsicSize.toYogaWithWidthConstraint(),
+    )
+  }
 
   override fun sizeThatFits(size: CValue<CGSize>): CValue<CGSize> {
     return size.useContents<CGSize, CValue<CGSize>> {