Make ViewFlexContainer more like UIViewFlexContainer (#2361)

* Hold the subject view in Node.context

* Make ViewFlexContainer more like UIViewFlexContainer

* Update screenshots

* CHANGELOG

* View.measure skips measurement on its own

CHANGELOG.md

@@ -11,6 +11,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.
 
 
 ## [0.15.0] - 2024-09-30

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

@@ -44,7 +44,7 @@ internal class UIViewFlexContainer(
     container = value,
     insert = { index, widget ->
       val view = widget.value
-      val node = view.asNode()
+      val node = Node(view)
       if (widget is ResizableWidget<*>) {
         widget.sizeListener = NodeSizeListener(node, view, this@UIViewFlexContainer)
       }
@@ -111,10 +111,11 @@ internal class UIViewFlexContainer(
   }
 }
 
-private fun UIView.asNode(): Node {
-  val childNode = Node()
-  childNode.measureCallback = UIViewMeasureCallback(this)
-  return childNode
+private fun Node(view: UIView): Node {
+  val result = Node()
+  result.measureCallback = UIViewMeasureCallback
+  result.context = view
+  return result
 }
 
 private class NodeSizeListener(

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

@@ -26,14 +26,15 @@ import platform.CoreGraphics.CGSizeMake
 import platform.UIKit.UIView
 import platform.UIKit.UIViewNoIntrinsicMetric
 
-internal class UIViewMeasureCallback(val view: UIView) : MeasureCallback {
+internal object UIViewMeasureCallback : MeasureCallback {
   override fun measure(
     node: Node,
     width: Float,
     widthMode: MeasureMode,
     height: Float,
     heightMode: MeasureMode,
   ): Size {
+    val view = node.context as UIView
     val constrainedWidth = when (widthMode) {
       MeasureMode.Undefined -> UIViewNoIntrinsicMetric
       else -> width.toDouble()

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

@@ -27,6 +27,9 @@ import platform.UIKit.UIViewNoIntrinsicMetric
 
 internal class YogaUIView : UIScrollView(cValue { CGRectZero }), UIScrollViewDelegateProtocol {
   val rootNode = Node()
+    .apply {
+      this.context = this@YogaUIView
+    }
 
   var widthConstraint = Constraint.Wrap
   var heightConstraint = Constraint.Wrap
@@ -81,7 +84,7 @@ internal class YogaUIView : UIScrollView(cValue { CGRectZero }), UIScrollViewDel
 
     // Layout the nodes based on the calculatedLayouts above.
     for (childNode in rootNode.children) {
-      childNode.view.setFrame(
+      (childNode.context as UIView).setFrame(
         CGRectMake(
           x = childNode.left.toDouble(),
           y = childNode.top.toDouble(),
@@ -151,6 +154,3 @@ internal class YogaUIView : UIScrollView(cValue { CGRectZero }), UIScrollViewDel
     return rootNode.flexDirection == FlexDirection.Column
   }
 }
-
-private val Node.view: UIView
-  get() = (measureCallback as UIViewMeasureCallback).view

redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewFlexContainer.kt

@@ -21,11 +21,8 @@ import android.util.LayoutDirection
 import android.view.View
 import android.view.View.OnScrollChangeListener
 import android.view.ViewGroup
-import android.view.ViewGroup.LayoutParams.MATCH_PARENT
-import android.view.ViewGroup.LayoutParams.WRAP_CONTENT
-import android.widget.FrameLayout
 import android.widget.HorizontalScrollView
-import androidx.core.view.updateLayoutParams
+import androidx.core.view.children
 import androidx.core.widget.NestedScrollView
 import androidx.core.widget.NestedScrollView.OnScrollChangeListener as OnScrollChangeListenerCompat
 import app.cash.redwood.Modifier
@@ -57,7 +54,7 @@ internal class ViewFlexContainer(
     insert = { index, widget ->
       val view = widget.value
 
-      val node = view.asNode()
+      val node = Node(view)
       yogaLayout.rootNode.children.add(index, node)
 
       // Always apply changes *after* adding a node to its parent.
@@ -90,15 +87,11 @@ internal class ViewFlexContainer(
   }
 
   override fun width(width: Constraint) {
-    hostView.updateLayoutParams {
-      this.width = if (width == Constraint.Fill) MATCH_PARENT else WRAP_CONTENT
-    }
+    yogaLayout.widthConstraint = width
   }
 
   override fun height(height: Constraint) {
-    hostView.updateLayoutParams {
-      this.height = if (height == Constraint.Fill) MATCH_PARENT else WRAP_CONTENT
-    }
+    yogaLayout.heightConstraint = height
   }
 
   override fun overflow(overflow: Overflow) {
@@ -121,7 +114,7 @@ internal class ViewFlexContainer(
     yogaLayout.requestLayout()
   }
 
-  private inner class HostView : FrameLayout(context) {
+  private inner class HostView : ViewGroup(context) {
     var scrollEnabled = false
       set(new) {
         val old = field
@@ -135,10 +128,26 @@ internal class ViewFlexContainer(
     private var onScrollListener: Any? = null
 
     init {
-      layoutParams = LayoutParams(WRAP_CONTENT, WRAP_CONTENT)
       updateViewHierarchy()
     }
 
+    override fun onLayout(changed: Boolean, left: Int, top: Int, right: Int, bottom: Int) {
+      for (child in children) {
+        child.layout(0, 0, right - left, bottom - top)
+      }
+    }
+
+    override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) {
+      var maxWidth = 0
+      var maxHeight = 0
+      for (child in children) {
+        child.measure(widthMeasureSpec, heightMeasureSpec)
+        maxWidth = maxOf(maxWidth, child.measuredWidth)
+        maxHeight = maxOf(maxHeight, child.measuredHeight)
+      }
+      setMeasuredDimension(maxWidth, maxHeight)
+    }
+
     fun attachOrDetachScrollListeners() {
       val child = getChildAt(0)
       if (child is NestedScrollView) {
@@ -183,8 +192,9 @@ internal class ViewFlexContainer(
   }
 }
 
-private fun View.asNode(): Node {
-  return Node().apply {
-    measureCallback = ViewMeasureCallback(this@asNode)
-  }
+private fun Node(view: View): Node {
+  val result = Node()
+  result.measureCallback = ViewMeasureCallback
+  result.context = view
+  return result
 }

redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewMeasureCallback.kt

@@ -22,14 +22,15 @@ import app.cash.redwood.yoga.Node
 import app.cash.redwood.yoga.Size
 import kotlin.math.roundToInt
 
-internal class ViewMeasureCallback(val view: View) : MeasureCallback {
+internal object ViewMeasureCallback : MeasureCallback {
   override fun measure(
     node: Node,
     width: Float,
     widthMode: MeasureMode,
     height: Float,
     heightMode: MeasureMode,
   ): Size {
+    val view = node.context as View
     val safeWidth = if (width.isFinite()) width.roundToInt() else 0
     val safeHeight = if (height.isFinite()) height.roundToInt() else 0
     val widthSpec = View.MeasureSpec.makeMeasureSpec(safeWidth, widthMode.toAndroid())

redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/YogaLayout.kt

@@ -10,88 +10,100 @@ import android.annotation.SuppressLint
 import android.content.Context
 import android.view.View
 import android.view.ViewGroup
+import app.cash.redwood.layout.api.Constraint
 import app.cash.redwood.yoga.Node
 import app.cash.redwood.yoga.Size
 import kotlin.math.roundToInt
 
 @SuppressLint("ViewConstructor")
 internal class YogaLayout(context: Context) : ViewGroup(context) {
   val rootNode = Node()
+    .apply {
+      this.context = this@YogaLayout
+    }
+
+  internal var widthConstraint = Constraint.Wrap
+  internal var heightConstraint = Constraint.Wrap
 
   private fun applyLayout(node: Node, xOffset: Float, yOffset: Float) {
-    val view = node.view
-    if (view != null && view !== this) {
+    val view = node.context as View
+    if (view !== this) {
       if (view.visibility == GONE) return
 
-      val width = node.width.roundToInt()
-      val height = node.height.roundToInt()
-      val widthSpec = MeasureSpec.makeMeasureSpec(width, MeasureSpec.EXACTLY)
-      val heightSpec = MeasureSpec.makeMeasureSpec(height, MeasureSpec.EXACTLY)
-      view.measure(widthSpec, heightSpec)
-
       val left = (xOffset + node.left).roundToInt()
       val top = (yOffset + node.top).roundToInt()
-      val right = left + view.measuredWidth
-      val bottom = top + view.measuredHeight
+      val right = left + node.width.roundToInt()
+      val bottom = top + node.height.roundToInt()
+
+      // We already know how big we want this view to be. But we measure it to trigger side-effects
+      // that the view needs to render itself correctly. In particular, `TextView` needs this
+      // otherwise it won't apply gravity correctly.
+      val width = right - left
+      val height = bottom - top
+      view.measure(
+        MeasureSpec.makeMeasureSpec(width, MeasureSpec.EXACTLY),
+        MeasureSpec.makeMeasureSpec(height, MeasureSpec.EXACTLY),
+      )
       view.layout(left, top, right, bottom)
     }
 
-    if (view === this) {
-      for (child in node.children) {
-        applyLayout(child, xOffset, yOffset)
-      }
-    } else if (view !is YogaLayout) {
-      for (child in node.children) {
-        val left = xOffset + node.left
-        val top = yOffset + node.top
-        applyLayout(child, left, top)
-      }
+    for (child in node.children) {
+      applyLayout(
+        node = child,
+        xOffset = xOffset + node.left,
+        yOffset = yOffset + node.top,
+      )
     }
   }
 
   override fun onLayout(changed: Boolean, left: Int, top: Int, right: Int, bottom: Int) {
-    val widthSpec = MeasureSpec.makeMeasureSpec(right - left, MeasureSpec.EXACTLY)
-    val heightSpec = MeasureSpec.makeMeasureSpec(bottom - top, MeasureSpec.EXACTLY)
-    calculateLayout(widthSpec, heightSpec)
-    applyLayout(rootNode, 0f, 0f)
+    calculateLayout(
+      requestedWidth = (right - left).toFloat(),
+      requestedHeight = (bottom - top).toFloat(),
+    )
+    applyLayout(rootNode, left.toFloat(), top.toFloat())
   }
 
   override fun onMeasure(widthSpec: Int, heightSpec: Int) {
-    calculateLayout(widthSpec, heightSpec)
+    val widthSize = MeasureSpec.getSize(widthSpec)
+    val widthMode = MeasureSpec.getMode(widthSpec)
+    val heightSize = MeasureSpec.getSize(heightSpec)
+    val heightMode = MeasureSpec.getMode(heightSpec)
+
+    calculateLayout(
+      requestedWidth = when {
+        widthMode == MeasureSpec.EXACTLY -> widthSize.toFloat()
+        widthConstraint == Constraint.Fill -> widthSize.toFloat()
+        else -> Size.UNDEFINED
+      },
+      requestedHeight = when {
+        heightMode == MeasureSpec.EXACTLY -> heightSize.toFloat()
+        heightConstraint == Constraint.Fill -> heightSize.toFloat()
+        else -> Size.UNDEFINED
+      },
+    )
+
     val width = rootNode.width.roundToInt()
     val height = rootNode.height.roundToInt()
     setMeasuredDimension(width, height)
   }
 
-  private fun calculateLayout(widthSpec: Int, heightSpec: Int) {
-    rootNode.requestedWidth = Size.UNDEFINED
+  private fun calculateLayout(
+    requestedWidth: Float,
+    requestedHeight: Float,
+  ) {
+    rootNode.requestedWidth = requestedWidth
     rootNode.requestedMaxWidth = Size.UNDEFINED
-    rootNode.requestedHeight = Size.UNDEFINED
+    rootNode.requestedHeight = requestedHeight
     rootNode.requestedMaxHeight = Size.UNDEFINED
 
-    val widthSize = MeasureSpec.getSize(widthSpec).toFloat()
-    when (MeasureSpec.getMode(widthSpec)) {
-      MeasureSpec.EXACTLY -> rootNode.requestedWidth = widthSize
-      MeasureSpec.AT_MOST -> rootNode.requestedMaxWidth = widthSize
-      MeasureSpec.UNSPECIFIED -> {}
-    }
-    val heightSize = MeasureSpec.getSize(heightSpec).toFloat()
-    when (MeasureSpec.getMode(heightSpec)) {
-      MeasureSpec.EXACTLY -> rootNode.requestedHeight = heightSize
-      MeasureSpec.AT_MOST -> rootNode.requestedMaxHeight = heightSize
-      MeasureSpec.UNSPECIFIED -> {}
-    }
-
     // Sync widget layout requests to the Yoga node tree.
     for (node in rootNode.children) {
-      if (node.view?.isLayoutRequested == true) {
+      if ((node.context as View).isLayoutRequested) {
         node.markDirty()
       }
     }
 
     rootNode.measureOnly(Size.UNDEFINED, Size.UNDEFINED)
   }
 }
-
-private val Node.view: View?
-  get() = (measureCallback as ViewMeasureCallback?)?.view