Repository: cashapp/redwood
Pull request #388 (2022-09-26 19:41:33)

Rename composition local for the display version
This brings it in line with convention of those in Compose UI. Also expose it in the docs as it could be useful for testing (and ONLY for testing).


Patch:
diff --git a/redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/widgetVersion.kt b/redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/widgetVersion.kt
index 50a079b36..1d0aac50a 100644
--- a/redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/widgetVersion.kt
+++ b/redwood-compose/src/commonMain/kotlin/app/cash/redwood/compose/widgetVersion.kt
@@ -20,8 +20,14 @@ import androidx.compose.runtime.ProvidableCompositionLocal
 import androidx.compose.runtime.ReadOnlyComposable
 import androidx.compose.runtime.compositionLocalOf
 
-/** @suppress */
-public val WidgetVersionValue: ProvidableCompositionLocal<UInt> = compositionLocalOf {
+/**
+ * Provide the version of widgets in use.
+ * This value will be bound automatically when a protocol-based display is used.
+ * Custom values should only be provided into a composition for testing purposes!
+ *
+ * @see WidgetVersion
+ */
+public val LocalWidgetVersion: ProvidableCompositionLocal<UInt> = compositionLocalOf {
   // A real value is always provided to a protocol-based composition.
   // When connected directly (i.e., without the protocol indirection) always assume latest.
   UInt.MAX_VALUE
@@ -44,4 +50,4 @@ public val WidgetVersionValue: ProvidableCompositionLocal<UInt> = compositionLoc
 public val WidgetVersion: UInt
   @Composable
   @ReadOnlyComposable
-  get() = WidgetVersionValue.current
+  get() = LocalWidgetVersion.current
diff --git a/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/FlexboxEngine.kt b/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/FlexboxEngine.kt
index 4910dd609..950edc715 100644
--- a/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/FlexboxEngine.kt
+++ b/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/FlexboxEngine.kt
@@ -264,7 +264,7 @@ public class FlexboxEngine {
           mode = mainMode,
           maxSize = mainSize,
           currentLength = flexLine.mainSize,
-          childLength = direction.mainMeasuredSize(child) + direction.mainMarginStart(child) + direction.mainMarginEnd(child),
+          childLength = direction.mainMeasuredSize(child) + direction.mainMarginStart(child) + direction.mainMarginEnd(child, ),
           flexItem = child,
           flexLinesSize = flexLines.size,
         )
diff --git a/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/Node.kt b/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/Node.kt
index 09b99cb32..2716e84ab 100644
--- a/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/Node.kt
+++ b/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/Node.kt
@@ -16,86 +16,89 @@
 package app.cash.redwood
 
 /**
- * A node with properties that can be measured and laid out inside a flexbox.
+ * An interface that has the common behavior as a node contained in a flexbox.
  */
-public class Node(
+public interface Node {
 
   /**
-   * The requested width of the node.
+   * The width attribute of the node.
    *
    * The attribute is about how wide the view wants to be. Can be one of the
    * constants [MatchParent] or [WrapContent], or an exact size.
    */
-  public val width: Int = WrapContent,
+  public val width: Int
 
   /**
-   * The requested height of the node.
+   * The height attribute of the node.
    *
    * The attribute is about how wide the view wants to be. Can be one of the
    * constants [MatchParent] or [WrapContent], or an exact size.
    */
-  public val height: Int = WrapContent,
+  public val height: Int
 
   /**
    * The minimum width attribute of the node.
    *
    * The attribute determines the minimum width the child can shrink to.
    */
-  public val minWidth: Int = 0,
+  public val minWidth: Int
 
   /**
    * The minimum height attribute of the node.
    *
    * The attribute determines the minimum height the child can shrink to.
    */
-  public val minHeight: Int = 0,
+  public val minHeight: Int
 
   /**
    * The maximum width attribute of the node.
    *
    * The attribute determines the maximum width the child can expand to.
    */
-  public val maxWidth: Int = Int.MAX_VALUE,
+  public val maxWidth: Int
 
   /**
    * The maximum height attribute of the node.
    */
-  public val maxHeight: Int = Int.MAX_VALUE,
+  public val maxHeight: Int
 
   /**
    * True if this item is visible and should be laid out.
    */
-  public val visible: Boolean = true,
+  public val visible: Boolean
 
   /**
    * The baseline used for [AlignItems.Baseline] and [AlignSelf.Baseline].
+   * If not specified, -1 is set as a default value.
    */
-  public val baseline: Int = -1,
+  public val baseline: Int
 
   /**
    * The order attribute of the node.
    *
    * The attribute can change the order in which the children are laid out.
    * By default, children are displayed and laid out in the same order as they added to the
-   * [FlexboxEngine].
+   * [FlexboxEngine]. If not specified, [DefaultOrder] is set as a default value.
    */
-  public val order: Int = DefaultOrder,
+  public val order: Int
 
   /**
    * The flex grow attribute of the node.
    *
    * The attribute determines how much this child will grow if positive free space is
    * distributed relative to the rest of other nodes included in the same flex line.
+   * If not specified, [DefaultFlexGrow] is set as a default value.
    */
-  public val flexGrow: Float = DefaultFlexGrow,
+  public val flexGrow: Float
 
   /**
    * The flex shrink attribute of the node.
    *
    * The attribute determines how much this child will shrink if negative free space is
    * distributed relative to the rest of other nodes included in the same flex line.
+   * If not specified, [DefaultFlexShrink] is set as a default value.
    */
-  public val flexShrink: Float = DefaultFlexShrink,
+  public val flexShrink: Float
 
   /**
    * The flexBasisPercent attribute of the node.
@@ -109,7 +112,7 @@ public class Node(
    * This attribute is only effective when the parent's MeasureSpec mode is
    * MeasureSpec.EXACTLY. The default value is -1, which means not set.
    */
-  public val flexBasisPercent: Float = DefaultFlexBasisPercent,
+  public val flexBasisPercent: Float
 
   /**
    * The align self attribute of the node.
@@ -121,8 +124,9 @@ public class Node(
    * The value needs to be one of the values in ([AlignSelf.Auto],
    * [AlignItems.Stretch], [AlignItems.FlexStart], [AlignItems.FlexEnd],
    * [AlignItems.Center], or [AlignItems.Baseline]).
+   * If not specified, [AlignSelf.Auto] is set as a default value.
    */
-  public val alignSelf: AlignSelf = AlignSelf.Auto,
+  public val alignSelf: AlignSelf
 
   /**
    * The wrapBefore attribute of the node.
@@ -136,42 +140,33 @@ public class Node(
    * the layouts when building a grid like layout or for a situation where developers want
    * to put a new flex line to make a semantic difference from the previous one, etc.
    */
-  public val wrapBefore: Boolean = false,
+  public val wrapBefore: Boolean
 
   /**
    * The margin of the node.
    */
-  public val margin: Spacing = Spacing.Zero,
-) {
+  public val margin: Spacing
 
   /**
    * The measured width after invoking [measure].
-   *
-   * TODO: Remove this mutable attribute and use the returned [Size] from [measure].
    */
-  public var measuredWidth: Int = -1
+  public val measuredWidth: Int
 
   /**
    * The measured height after invoking [measure].
-   *
-   * TODO: Remove this mutable attribute and use the returned [Size] from [measure].
    */
-  public var measuredHeight: Int = -1
+  public val measuredHeight: Int
 
   /**
-   * A callback to to measure this node according to the `widthSpec` and `heightSpec` constraints
-   * and update [measuredWidth] and [measuredHeight] with the result.
+   * Measure this node according to the [widthSpec] and [heightSpec] constraints and update
+   * [measuredWidth] and [measuredHeight] with the result.
    */
-  public var measure: (widthSpec: MeasureSpec, heightSpec: MeasureSpec) -> Size = { widthSpec, heightSpec ->
-    measuredWidth = MeasureSpec.resolveSize(width, widthSpec)
-    measuredHeight = MeasureSpec.resolveSize(height, heightSpec)
-    Size(measuredWidth, measuredHeight)
-  }
+  public fun measure(widthSpec: MeasureSpec, heightSpec: MeasureSpec)
 
   /**
-   * A callback to place the node inside the given `left`, `top`, `right`, and `bottom` coordinates.
+   * Place the node inside the given bounding box.
    */
-  public var layout: (left: Int, top: Int, right: Int, bottom: Int) -> Unit = { _, _, _, _ -> }
+  public fun layout(left: Int, top: Int, right: Int, bottom: Int)
 
   public companion object {
     /**
diff --git a/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/properties.kt b/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/properties.kt
index 9010f2fe7..1b816d700 100644
--- a/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/properties.kt
+++ b/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/properties.kt
@@ -207,16 +207,6 @@ public value class MeasureSpec private constructor(internal val value: Int) {
       // Use the top 2 bits for the mode and use the bottom 30 bits for the size.
       return MeasureSpec((mode.ordinal shl 30) or (size and 0x3FFF))
     }
-
-    /** A convenience function to constrain [size] inside a given [measureSpec]. */
-    public fun resolveSize(size: Int, measureSpec: MeasureSpec): Int {
-      return when (measureSpec.mode) {
-        MeasureSpecMode.AtMost -> minOf(measureSpec.size, size)
-        MeasureSpecMode.Exactly -> measureSpec.size
-        MeasureSpecMode.Unspecified -> size
-        else -> throw AssertionError()
-      }
-    }
   }
 }
 
@@ -287,8 +277,4 @@ public data class Size(
       "invalid Size: [$width, $height]"
     }
   }
-
-  public companion object {
-    public val Zero: Size = Size(0, 0)
-  }
 }
diff --git a/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/utils.kt b/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/utils.kt
index f8e956018..5a34b3262 100644
--- a/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/utils.kt
+++ b/redwood-flexbox-engine/src/commonMain/kotlin/app/cash/redwood/utils.kt
@@ -30,12 +30,6 @@ internal fun unpackHigher(value: Long): Int {
   return (value shr 32).toInt()
 }
 
-/** Convenience function to use named arguments. */
-@Suppress("EXTENSION_FUNCTION_SHADOWED_BY_MEMBER_PROPERTY_WITH_INVOKE")
-internal inline fun Node.layout(left: Int, top: Int, right: Int, bottom: Int) {
-  layout(left, top, right, bottom)
-}
-
 /**
  * The number of children who are not invisible in this flex line.
  */
@@ -94,3 +88,12 @@ internal fun MeasureSpec.Companion.getChildMeasureSpec(
   }
   return from(resultSize, resultMode)
 }
+
+internal fun MeasureSpec.Companion.resolveSize(size: Int, measureSpec: MeasureSpec): Int {
+  return when (measureSpec.mode) {
+    MeasureSpecMode.AtMost -> minOf(measureSpec.size, size)
+    MeasureSpecMode.Exactly -> measureSpec.size
+    MeasureSpecMode.Unspecified -> size
+    else -> throw AssertionError()
+  }
+}
diff --git a/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolRedwoodComposition.kt b/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolRedwoodComposition.kt
index f6bd7d8c2..3008299da 100644
--- a/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolRedwoodComposition.kt
+++ b/redwood-protocol-compose/src/commonMain/kotlin/app/cash/redwood/protocol/compose/ProtocolRedwoodComposition.kt
@@ -23,7 +23,7 @@ import androidx.compose.runtime.Recomposer
 import androidx.compose.runtime.currentComposer
 import androidx.compose.runtime.snapshots.ObserverHandle
 import androidx.compose.runtime.snapshots.Snapshot
-import app.cash.redwood.compose.WidgetVersionValue
+import app.cash.redwood.compose.LocalWidgetVersion
 import app.cash.redwood.protocol.DiffSink
 import app.cash.redwood.protocol.Event
 import app.cash.redwood.protocol.EventSink
@@ -127,7 +127,7 @@ private class DiffProducingRedwoodComposition(
     //    %24 = call %struct.ObjHeader* @"kfun:kotlin.collections#copyInto__at__kotlin.Array<out|0:0>(kotlin.Array<0:0>;kotlin.Int;kotlin.Int;kotlin.Int){0\C2\A7<kotlin.Any?>}kotlin.Array<0:0>"(%struct.ObjHeader* %21, %struct.ObjHeader* %22, i32 %23, i32 0, i32 %20, %struct.ObjHeader** %13)
     //  inlinable function call in a function with debug info must have a !dbg location
     //    call void @"kfun:androidx.compose.runtime#CompositionLocalProvider(kotlin.Array<out|androidx.compose.runtime.ProvidedValue<*>>...;kotlin.Function2<androidx.compose.runtime.Composer,kotlin.Int,kotlin.Unit>;androidx.compose.runtime.Composer?;kotlin.Int){}"(%struct.ObjHeader* %27, %struct.ObjHeader* %1, %struct.ObjHeader* %3, i32 %28)
-    val providers = arrayOf(WidgetVersionValue provides widgetVersion)
+    val providers = arrayOf(LocalWidgetVersion provides widgetVersion)
     composition.setContent {
       currentComposer.startProviders(providers)
       content()


Test patch:
diff --git a/redwood-flexbox-engine/src/commonTest/kotlin/app/cash/redwood/FakeNode.kt b/redwood-flexbox-engine/src/commonTest/kotlin/app/cash/redwood/FakeNode.kt
new file mode 100644
index 000000000..202c24052
--- /dev/null
+++ b/redwood-flexbox-engine/src/commonTest/kotlin/app/cash/redwood/FakeNode.kt
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2022 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package app.cash.redwood
+
+import app.cash.redwood.Node.Companion.DefaultFlexBasisPercent
+import app.cash.redwood.Node.Companion.DefaultFlexGrow
+import app.cash.redwood.Node.Companion.DefaultFlexShrink
+import app.cash.redwood.Node.Companion.DefaultOrder
+import app.cash.redwood.Node.Companion.WrapContent
+
+class FakeNode(
+  override val width: Int = WrapContent,
+  override val height: Int = WrapContent,
+  override val minWidth: Int = 0,
+  override val minHeight: Int = 0,
+  override val maxWidth: Int = Int.MAX_VALUE,
+  override val maxHeight: Int = Int.MAX_VALUE,
+  override val visible: Boolean = true,
+  override val baseline: Int = -1,
+  override val order: Int = DefaultOrder,
+  override val flexGrow: Float = DefaultFlexGrow,
+  override val flexShrink: Float = DefaultFlexShrink,
+  override val flexBasisPercent: Float = DefaultFlexBasisPercent,
+  override val alignSelf: AlignSelf = AlignSelf.Auto,
+  override val wrapBefore: Boolean = false,
+  override val margin: Spacing = Spacing.Zero,
+) : Node {
+
+  override var measuredWidth: Int = -1
+  override var measuredHeight: Int = -1
+
+  override fun measure(widthSpec: MeasureSpec, heightSpec: MeasureSpec) {
+    measuredWidth = MeasureSpec.resolveSize(width, widthSpec)
+    measuredHeight = MeasureSpec.resolveSize(height, heightSpec)
+  }
+
+  override fun layout(left: Int, top: Int, right: Int, bottom: Int) {}
+}
diff --git a/redwood-flexbox-engine/src/commonTest/kotlin/app/cash/redwood/FlexboxEngineTest.kt b/redwood-flexbox-engine/src/commonTest/kotlin/app/cash/redwood/FlexboxEngineTest.kt
index f2a4811d6..1868fe7cf 100644
--- a/redwood-flexbox-engine/src/commonTest/kotlin/app/cash/redwood/FlexboxEngineTest.kt
+++ b/redwood-flexbox-engine/src/commonTest/kotlin/app/cash/redwood/FlexboxEngineTest.kt
@@ -31,10 +31,10 @@ class FlexboxEngineTest {
 
   @Test
   fun testCalculateHorizontalFlexLines() {
-    val node1 = Node(100, 100)
-    val node2 = Node(200, 100)
-    val node3 = Node(300, 100)
-    val node4 = Node(400, 100)
+    val node1 = FakeNode(100, 100)
+    val node2 = FakeNode(200, 100)
+    val node3 = FakeNode(300, 100)
+    val node4 = FakeNode(400, 100)
     engine.nodes += node1
     engine.nodes += node2
     engine.nodes += node3
@@ -73,10 +73,10 @@ class FlexboxEngineTest {
 
   @Test
   fun testCalculateVerticalFlexLines() {
-    val node1 = Node(100, 100)
-    val node2 = Node(100, 200)
-    val node3 = Node(100, 300)
-    val node4 = Node(100, 400)
+    val node1 = FakeNode(100, 100)
+    val node2 = FakeNode(100, 200)
+    val node3 = FakeNode(100, 300)
+    val node4 = FakeNode(100, 400)
     engine.nodes += node1
     engine.nodes += node2
     engine.nodes += node3
@@ -116,10 +116,10 @@ class FlexboxEngineTest {
 
   @Test
   fun testDetermineMainSize_direction_row_flexGrowSet() {
-    val node1 = Node(100, 100)
-    val node2 = Node(200, 100, flexGrow = 1.0f)
-    val node3 = Node(300, 100)
-    val node4 = Node(400, 100, flexGrow = 2.0f)
+    val node1 = FakeNode(100, 100)
+    val node2 = FakeNode(200, 100, flexGrow = 1.0f)
+    val node3 = FakeNode(300, 100)
+    val node4 = FakeNode(400, 100, flexGrow = 2.0f)
     engine.nodes += node1
     engine.nodes += node2
     engine.nodes += node3
@@ -145,10 +145,10 @@ class FlexboxEngineTest {
 
   @Test
   fun testDetermineMainSize_direction_column_flexGrowSet() {
-    val node1 = Node(100, 100)
-    val node2 = Node(100, 200, flexGrow = 1.0f)
-    val node3 = Node(100, 300)
-    val node4 = Node(100, 400, flexGrow = 2.0f)
+    val node1 = FakeNode(100, 100)
+    val node2 = FakeNode(100, 200, flexGrow = 1.0f)
+    val node3 = FakeNode(100, 300)
+    val node4 = FakeNode(100, 400, flexGrow = 2.0f)
     engine.nodes += node1
     engine.nodes += node2
     engine.nodes += node3
@@ -174,10 +174,10 @@ class FlexboxEngineTest {
 
   @Test
   fun testDetermineMainSize_direction_row_flexShrinkSet() {
-    val node1 = Node(200, 100)
-    val node2 = Node(200, 100)
-    val node3 = Node(200, 100)
-    val node4 = Node(200, 100)
+    val node1 = FakeNode(200, 100)
+    val node2 = FakeNode(200, 100)
+    val node3 = FakeNode(200, 100)
+    val node4 = FakeNode(200, 100)
     engine.nodes += node1
     engine.nodes += node2
     engine.nodes += node3
@@ -203,10 +203,10 @@ class FlexboxEngineTest {
 
   @Test
   fun testDetermineMainSize_direction_column_flexShrinkSet() {
-    val node1 = Node(100, 200)
-    val node2 = Node(100, 200)
-    val node3 = Node(100, 200)
-    val node4 = Node(100, 200)
+    val node1 = FakeNode(100, 200)
+    val node2 = FakeNode(100, 200)
+    val node3 = FakeNode(100, 200)
+    val node4 = FakeNode(100, 200)
     engine.nodes += node1
     engine.nodes += node2
     engine.nodes += node3
@@ -232,8 +232,8 @@ class FlexboxEngineTest {
 
   @Test
   fun testDetermineMainSize_directionRow_fixedSizeViewAndShrinkable_doNotExceedMaxMainSize() {
-    val node1 = Node(100, 100, flexShrink = 0f)
-    val node2 = Node(2000, 2000) // simulate a very long text view
+    val node1 = FakeNode(100, 100, flexShrink = 0f)
+    val node2 = FakeNode(2000, 2000) // simulate a very long text view
     engine.nodes += node1
     engine.nodes += node2
     engine.flexWrap = FlexWrap.NoWrap
@@ -250,9 +250,9 @@ class FlexboxEngineTest {
 
   @Test
   fun testDetermineMainSize_directionRow_twoFixedSizeViewsAndShrinkable_doNotExceedMaxMainSize() {
-    val node1 = Node(100, 100, flexShrink = 0f)
-    val node2 = Node(2000, 2000) // simulate a very long text view
-    val node3 = Node(100, 100, flexShrink = 0f)
+    val node1 = FakeNode(100, 100, flexShrink = 0f)
+    val node2 = FakeNode(2000, 2000) // simulate a very long text view
+    val node3 = FakeNode(100, 100, flexShrink = 0f)
     engine.nodes += node1
     engine.nodes += node2
     engine.nodes += node3
@@ -271,10 +271,10 @@ class FlexboxEngineTest {
 
   @Test
   fun testDetermineCrossSize_direction_row_alignContent_stretch() {
-    val node1 = Node(100, 100)
-    val node2 = Node(200, 100)
-    val node3 = Node(300, 100)
-    val node4 = Node(400, 100)
+    val node1 = FakeNode(100, 100)
+    val node2 = FakeNode(200, 100)
+    val node3 = FakeNode(300, 100)
+    val node4 = FakeNode(400, 100)
     engine.nodes += node1
     engine.nodes += node2
     engine.nodes += node3
@@ -300,10 +300,10 @@ class FlexboxEngineTest {
 
   @Test
   fun testDetermineCrossSize_direction_column_alignContent_stretch() {
-    val node1 = Node(100, 100)
-    val node2 = Node(100, 200)
-    val node3 = Node(100, 300)
-    val node4 = Node(100, 400)
+    val node1 = FakeNode(100, 100)
+    val node2 = FakeNode(100, 200)
+    val node3 = FakeNode(100, 300)
+    val node4 = FakeNode(100, 400)
     engine.nodes += node1
     engine.nodes += node2
     engine.nodes += node3
@@ -365,10 +365,10 @@ class FlexboxEngineTest {
 
   @Test
   fun testFlexLine_anyItemsHaveFlexGrow() {
-    val node1 = Node(100, 100, flexGrow = 1.0f)
-    val node2 = Node(100, 200)
-    val node3 = Node(100, 300)
-    val node4 = Node(100, 400, flexGrow = 2.0f)
+    val node1 = FakeNode(100, 100, flexGrow = 1.0f)
+    val node2 = FakeNode(100, 200)
+    val node3 = FakeNode(100, 300)
+    val node4 = FakeNode(100, 400, flexGrow = 2.0f)
     engine.apply {
       nodes += node1
       nodes += node2