Fix Constraint.Wrap layouts when a view is larger than its parent (#2337

)

* Fix Constraint.Wrap layouts when a view is larger than its parent

These were incorrectly overflowing the parent container
because we were applying the target size at the wrong layer.

This fixes an ugly inconsistency between iOS and Android,
and makes the iOS layouts less surprising.

#2128

* CHANGELOG

CHANGELOG.md

@@ -16,6 +16,7 @@ Fixed:
 - Breaking the last remaining retain cycle in `UIViewLazyList`.
 - Don't leak the `DisplayLink` when a `TreehouseApp` is stopped on iOS.
 - Correctly handle dynamic size changes for child widgets of `Box`, `Column`, and `Row`.
+- Don't clip elements of `Column` and `Row` layouts whose unbounded size exceeds the container size.
 - Correctly implement margins for `Box` on iOS.
 - Correctly handle dynamic updates to modifiers on `Column` and `Row`.
 

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

@@ -782,6 +782,41 @@ abstract class AbstractFlexContainerTest<T : Any> {
     rowA2.text("A-TWO ".repeat(5))
     snapshotter.snapshot("v2")
   }
+
+  /**
+   * When a child widget's intrinsic size won't fit in the available space, what happens? We can
+   * either let it have its requested size anyway (and overrun the available space) or we confine it
+   * to the space available.
+   */
+  @Test fun testChildIsConstrainedToParentWidth() {
+    // Wrap in a parent column to let us configure an exact width for our subject flex container.
+    // Otherwise we're relying on the platform-specific snapshot library's unspecified frame width.
+    val fullWidthParent = column().apply {
+      width(Constraint.Fill)
+      height(Constraint.Fill)
+    }
+
+    val alignStart = HorizontalAlignmentImpl(CrossAxisAlignment.Start)
+    flexContainer(FlexDirection.Column)
+      .apply {
+        modifier = WidthImpl(25.dp)
+        add(widgetFactory.text("ok", alignStart)) // This is under 25.dp in width.
+        add(widgetFactory.text("1 2 3 4", alignStart)) // Each character is under 25.dp in width.
+        onEndChanges()
+      }
+      .also { fullWidthParent.children.insert(0, it) }
+
+    flexContainer(FlexDirection.Column)
+      .apply {
+        modifier = WidthImpl(25.dp)
+        add(widgetFactory.text("overflows parent", alignStart)) // This is over 25.dp in width.
+        add(widgetFactory.text("1 2 3 4", alignStart)) // Each character is under 25.dp in width.
+        onEndChanges()
+      }
+      .also { fullWidthParent.children.insert(1, it) }
+
+    snapshotter(fullWidthParent.value).snapshot()
+  }
 }
 
 interface TestFlexContainer<T : Any> :

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

@@ -76,11 +76,11 @@ internal class UIViewFlexContainer(
   }
 
   override fun width(width: Constraint) {
-    yogaView.width = width
+    yogaView.widthConstraint = width
   }
 
   override fun height(height: Constraint) {
-    yogaView.height = height
+    yogaView.heightConstraint = height
   }
 
   override fun overflow(overflow: Overflow) {

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

@@ -30,8 +30,8 @@ internal class YogaUIView(
 ) : UIScrollView(cValue { CGRectZero }), UIScrollViewDelegateProtocol {
   val rootNode = Node()
 
-  var width = Constraint.Wrap
-  var height = Constraint.Wrap
+  var widthConstraint = Constraint.Wrap
+  var heightConstraint = Constraint.Wrap
 
   var onScroll: ((Px) -> Unit)? = null
 
@@ -44,12 +44,11 @@ internal class YogaUIView(
   }
 
   override fun intrinsicContentSize(): CValue<CGSize> {
-    return calculateLayoutWithSize(CGSizeMake(Size.UNDEFINED.toDouble(), Size.UNDEFINED.toDouble()))
+    return calculateLayoutWithSize(Size.UNDEFINED, Size.UNDEFINED)
   }
 
   override fun sizeThatFits(size: CValue<CGSize>): CValue<CGSize> {
-    val constrainedSize = size.useContents { sizeForConstraints(this) }
-    return calculateLayoutWithSize(constrainedSize)
+    return calculateLayoutWithSize(size)
   }
 
   override fun layoutSubviews() {
@@ -58,7 +57,7 @@ internal class YogaUIView(
     // Based on the constraints of Fill or Wrap, we
     // calculate a size that the container should fit in.
     val bounds = bounds.useContents {
-      sizeForConstraints(size)
+      CGSizeMake(size.width, size.height)
     }
 
     if (scrollEnabled) {
@@ -96,6 +95,24 @@ internal class YogaUIView(
   }
 
   private fun calculateLayoutWithSize(size: CValue<CGSize>): CValue<CGSize> {
+    return size.useContents {
+      calculateLayoutWithSize(width.toYoga(), height.toYoga())
+    }
+  }
+
+  private fun calculateLayoutWithSize(width: Float, height: Float): CValue<CGSize> {
+    rootNode.requestedWidth = when (widthConstraint) {
+      Constraint.Fill -> width
+      else -> Size.UNDEFINED
+    }
+    rootNode.requestedMaxWidth = width
+
+    rootNode.requestedHeight = when (heightConstraint) {
+      Constraint.Fill -> height
+      else -> Size.UNDEFINED
+    }
+    rootNode.requestedMaxHeight = height
+
     for ((index, node) in rootNode.children.withIndex()) {
       applyModifier(node, index)
     }
@@ -104,23 +121,16 @@ internal class YogaUIView(
       rootNode.markEverythingDirty()
     }
 
-    size.useContents { rootNode.measureOnly(width.toFloat(), height.toFloat()) }
+    rootNode.measureOnly(width, height)
 
     return CGSizeMake(rootNode.width.toDouble(), rootNode.height.toDouble())
   }
 
-  private fun sizeForConstraints(size: CGSize): CValue<CGSize> {
-    return CGSizeMake(
-      width = sizeForConstraintsDimension(width, size.width),
-      height = sizeForConstraintsDimension(height, size.height),
-    )
-  }
-
-  private fun sizeForConstraintsDimension(constraint: Constraint, dimension: Double): Double {
-    if (constraint == Constraint.Wrap || dimension == UIViewNoIntrinsicMetric) {
-      return Size.UNDEFINED.toDouble()
-    } else {
-      return dimension
+  /** Convert a UIView dimension (a double) to a Yoga dimension (a float). */
+  private fun Double.toYoga(): Float {
+    return when (this) {
+      UIViewNoIntrinsicMetric -> Size.UNDEFINED
+      else -> this.toFloat()
     }
   }