Skip to content

Commit

Permalink
Drop support for non-incremental Row and Column (#2353)
Browse files Browse the repository at this point in the history
  • Loading branch information
squarejesse authored Oct 2, 2024
1 parent 631cdad commit 9f28c22
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 79 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ New:
- Nothing yet!

Changed:
- Nothing yet!
- Drop support for non-incremental layouts in `Row` and `Column`.

Fixed:
- Fix a layout bug where children of fixed-with `Row` containers were assigned the wrong width.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,6 @@ import kotlin.test.assertEquals
import kotlin.test.assertTrue

abstract class AbstractFlexContainerTest<T : Any> {
/**
* Returns true if the FlexContainer implementation implements incremental layouts. This is
* currently opt-in, but will soon be the only supported mode.
*/
open val incremental: Boolean
get() = true

abstract val widgetFactory: TestWidgetFactory<T>

abstract fun flexContainer(
Expand Down Expand Up @@ -683,23 +676,21 @@ abstract class AbstractFlexContainerTest<T : Any> {
val aMeasureCountV2 = a.measureCount
val bMeasureCountV2 = b.measureCount
val cMeasureCountV2 = c.measureCount
if (incremental) {
// Only 'b' is measured again.
assertEquals(aMeasureCountV1, aMeasureCountV2)
assertTrue(bMeasureCountV1 <= bMeasureCountV2)
assertEquals(cMeasureCountV1, cMeasureCountV2)
}

// Only 'b' is measured again.
assertEquals(aMeasureCountV1, aMeasureCountV2)
assertTrue(bMeasureCountV1 <= bMeasureCountV2)
assertEquals(cMeasureCountV1, cMeasureCountV2)

snapshotter.snapshot("v3")
val aMeasureCountV3 = a.measureCount
val bMeasureCountV3 = b.measureCount
val cMeasureCountV3 = c.measureCount
if (incremental) {
// Nothing is measured again.
assertEquals(aMeasureCountV2, aMeasureCountV3)
assertEquals(bMeasureCountV2, bMeasureCountV3)
assertEquals(cMeasureCountV2, cMeasureCountV3)
}

// Nothing is measured again.
assertEquals(aMeasureCountV2, aMeasureCountV3)
assertEquals(bMeasureCountV2, bMeasureCountV3)
assertEquals(cMeasureCountV2, cMeasureCountV3)
}

@Test fun testRecursiveLayoutIsIncremental() {
Expand Down Expand Up @@ -747,23 +738,21 @@ abstract class AbstractFlexContainerTest<T : Any> {
val aMeasureCountV2 = a.measureCount
val bMeasureCountV2 = b.measureCount
val cMeasureCountV2 = c.measureCount
if (incremental) {
// Only 'b' is measured again.
assertEquals(aMeasureCountV1, aMeasureCountV2)
assertTrue(bMeasureCountV1 <= bMeasureCountV2)
assertEquals(cMeasureCountV1, cMeasureCountV2)
}

// Only 'b' is measured again.
assertEquals(aMeasureCountV1, aMeasureCountV2)
assertTrue(bMeasureCountV1 <= bMeasureCountV2)
assertEquals(cMeasureCountV1, cMeasureCountV2)

snapshotter.snapshot("v3")
val aMeasureCountV3 = a.measureCount
val bMeasureCountV3 = b.measureCount
val cMeasureCountV3 = c.measureCount
if (incremental) {
// Nothing is measured again.
assertEquals(aMeasureCountV2, aMeasureCountV3)
assertEquals(bMeasureCountV2, bMeasureCountV3)
assertEquals(cMeasureCountV2, cMeasureCountV3)
}

// Nothing is measured again.
assertEquals(aMeasureCountV2, aMeasureCountV3)
assertEquals(bMeasureCountV2, bMeasureCountV3)
assertEquals(cMeasureCountV2, cMeasureCountV3)
}

/** Confirm that child element size changes propagate up the view hierarchy. */
Expand Down
1 change: 0 additions & 1 deletion redwood-layout-uiview/api/redwood-layout-uiview.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
// Library unique name: <app.cash.redwood:redwood-layout-uiview>
final class app.cash.redwood.layout.uiview/UIViewRedwoodLayoutWidgetFactory : app.cash.redwood.layout.widget/RedwoodLayoutWidgetFactory<platform.UIKit/UIView> { // app.cash.redwood.layout.uiview/UIViewRedwoodLayoutWidgetFactory|null[0]
constructor <init>() // app.cash.redwood.layout.uiview/UIViewRedwoodLayoutWidgetFactory.<init>|<init>(){}[0]
constructor <init>(kotlin/Boolean) // app.cash.redwood.layout.uiview/UIViewRedwoodLayoutWidgetFactory.<init>|<init>(kotlin.Boolean){}[0]

final fun Box(): app.cash.redwood.layout.widget/Box<platform.UIKit/UIView> // app.cash.redwood.layout.uiview/UIViewRedwoodLayoutWidgetFactory.Box|Box(){}[0]
final fun Column(): app.cash.redwood.layout.widget/Column<platform.UIKit/UIView> // app.cash.redwood.layout.uiview/UIViewRedwoodLayoutWidgetFactory.Column|Column(){}[0]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,13 @@ import platform.darwin.NSInteger

internal class UIViewFlexContainer(
direction: FlexDirection,
private val incremental: Boolean,
) : YogaFlexContainer<UIView>,
ResizableWidget<UIView>,
ChangeListener {
private val yogaView: YogaUIView = YogaUIView(
applyModifier = { node, _ ->
node.applyModifier(node.context as Modifier, Density.Default)
},
incremental = incremental,
)
override val rootNode: Node get() = yogaView.rootNode
override val density: Density get() = Density.Default
Expand Down Expand Up @@ -96,21 +94,16 @@ internal class UIViewFlexContainer(
}

internal fun invalidateSize() {
if (incremental) {
if (rootNode.markDirty()) {
// The node was newly-dirty. Propagate that up the tree.
val sizeListener = this.sizeListener
if (sizeListener != null) {
value.setNeedsLayout()
sizeListener.invalidateSize()
} else {
value.invalidateIntrinsicContentSize() // Tell the enclosing view that our size changed.
value.setNeedsLayout() // Update layout of subviews.
}
if (rootNode.markDirty()) {
// The node was newly-dirty. Propagate that up the tree.
val sizeListener = this.sizeListener
if (sizeListener != null) {
value.setNeedsLayout()
sizeListener.invalidateSize()
} else {
value.invalidateIntrinsicContentSize() // Tell the enclosing view that our size changed.
value.setNeedsLayout() // Update layout of subviews.
}
} else {
value.invalidateIntrinsicContentSize() // Tell the enclosing view that our size changed.
value.setNeedsLayout() // Update layout of subviews.
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,9 @@ import app.cash.redwood.yoga.FlexDirection
import platform.UIKit.UIView

@ObjCName("UIViewRedwoodLayoutWidgetFactory", exact = true)
public class UIViewRedwoodLayoutWidgetFactory(
private val incremental: Boolean,
) : RedwoodLayoutWidgetFactory<UIView> {
public constructor() : this(false)

public class UIViewRedwoodLayoutWidgetFactory : RedwoodLayoutWidgetFactory<UIView> {
override fun Box(): Box<UIView> = UIViewBox()
override fun Column(): Column<UIView> = UIViewFlexContainer(FlexDirection.Column, incremental)
override fun Row(): Row<UIView> = UIViewFlexContainer(FlexDirection.Row, incremental)
override fun Column(): Column<UIView> = UIViewFlexContainer(FlexDirection.Column)
override fun Row(): Row<UIView> = UIViewFlexContainer(FlexDirection.Row)
override fun Spacer(): Spacer<UIView> = UIViewSpacer()
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import platform.UIKit.UIViewNoIntrinsicMetric

internal class YogaUIView(
private val applyModifier: (Node, Int) -> Unit,
private val incremental: Boolean,
) : UIScrollView(cValue { CGRectZero }), UIScrollViewDelegateProtocol {
val rootNode = Node()

Expand Down Expand Up @@ -106,10 +105,6 @@ internal class YogaUIView(
applyModifier(node, index)
}

if (!incremental) {
rootNode.markEverythingDirty()
}

rootNode.measureOnly(Size.UNDEFINED, Size.UNDEFINED)

return CGSizeMake(rootNode.width.toDouble(), rootNode.height.toDouble())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class UIViewFlexContainerTest(
direction: FlexDirection,
backgroundColor: Int,
): UIViewTestFlexContainer {
return UIViewTestFlexContainer(UIViewFlexContainer(direction, incremental)).apply {
return UIViewTestFlexContainer(UIViewFlexContainer(direction)).apply {
value.backgroundColor = backgroundColor.toUIColor()

// Install a default SizeListener that doesn't do anything. Otherwise the test subject
Expand Down
3 changes: 1 addition & 2 deletions redwood-layout-view/api/redwood-layout-view.api
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
public final class app/cash/redwood/layout/view/ViewRedwoodLayoutWidgetFactory : app/cash/redwood/layout/widget/RedwoodLayoutWidgetFactory {
public fun <init> (Landroid/content/Context;Z)V
public synthetic fun <init> (Landroid/content/Context;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Landroid/content/Context;)V
public fun Box ()Lapp/cash/redwood/layout/widget/Box;
public fun Column ()Lapp/cash/redwood/layout/widget/Column;
public fun Row ()Lapp/cash/redwood/layout/widget/Row;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,13 @@ import app.cash.redwood.yoga.isHorizontal
internal class ViewFlexContainer(
private val context: Context,
private val direction: FlexDirection,
incremental: Boolean = false,
) : YogaFlexContainer<View>,
ChangeListener {
private val yogaLayout: YogaLayout = YogaLayout(
context,
applyModifier = { node, index ->
node.applyModifier(children.widgets[index].modifier, density)
},
incremental = incremental,
)
override val rootNode: Node get() = yogaLayout.rootNode
override val density = Density(context.resources)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ import app.cash.redwood.yoga.FlexDirection

public class ViewRedwoodLayoutWidgetFactory(
private val context: Context,
private val incremental: Boolean = true,
) : RedwoodLayoutWidgetFactory<View> {
override fun Box(): Box<View> = ViewBox(context)

override fun Column(): Column<View> =
ViewFlexContainer(context, FlexDirection.Column, incremental)
override fun Column(): Column<View> = ViewFlexContainer(context, FlexDirection.Column)

override fun Row(): Row<View> = ViewFlexContainer(context, FlexDirection.Row)

override fun Row(): Row<View> = ViewFlexContainer(context, FlexDirection.Row, incremental)
override fun Spacer(): Spacer<View> = ViewSpacer(context)
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import kotlin.math.roundToInt
internal class YogaLayout(
context: Context,
private val applyModifier: (Node, Int) -> Unit,
private val incremental: Boolean,
) : ViewGroup(context) {
val rootNode = Node()

Expand Down Expand Up @@ -92,14 +91,10 @@ internal class YogaLayout(
}

// Sync widget layout requests to the Yoga node tree.
if (incremental) {
for (node in rootNode.children) {
if (node.view?.isLayoutRequested == true) {
node.markDirty()
}
for (node in rootNode.children) {
if (node.view?.isLayoutRequested == true) {
node.markDirty()
}
} else {
rootNode.markEverythingDirty()
}

rootNode.measureOnly(Size.UNDEFINED, Size.UNDEFINED)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class ViewFlexContainerTest(
direction: FlexDirection,
backgroundColor: Int,
): ViewTestFlexContainer {
val delegate = ViewFlexContainer(paparazzi.context, direction, incremental).apply {
val delegate = ViewFlexContainer(paparazzi.context, direction).apply {
value.setBackgroundColor(backgroundColor)
}
return ViewTestFlexContainer(delegate)
Expand Down

0 comments on commit 9f28c22

Please sign in to comment.