Skip to content

Commit 04a3609

Browse files
authored
Stop configuring the max width and height in YogaUIView (#2757)
* Stop configuring the max width and height in YogaUIView These attributes only cause us pain. In particular they aren't necessary to pass any of our layout tests, and by removing them I can fix a layout bug. Note that on Android we do not use these attributes, so this brings the two platforms closer together. Closes: #2753 * Start using systemLayoutSizeFittingSize This way we can tell the view whether a measurement is exactly (which it should honor unconditionally), or not (which it might honor if its constraint is Fill). * DS_Store
1 parent b5a6995 commit 04a3609

File tree

14 files changed

+109
-14
lines changed

14 files changed

+109
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ Fixed:
1414
- Don't measure the height of `RedwoodUIView` as zero when measured with `sizeThatFits()` or `intrinsicContentSize()`. We had been using `UIStackView` which doesn't support these functions.
1515
- Correctly signal `RedwoodUIView` size changes of to callers who measure it with `intrinsicContentSize()`.
1616
- The Redwood Gradle plugin is now compatible with Gradle 9.1.
17+
- Don't incorrectly size items to 0 when using the `Flex()` modifier on a `Wrap` container.
1718

1819

1920
## [0.18.0] - 2025-08-01
Lines changed: 3 additions & 0 deletions
Loading
Lines changed: 3 additions & 0 deletions
Loading

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

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,6 +1089,43 @@ abstract class AbstractFlexContainerTest<T : Any> {
10891089

10901090
snapshotter(row.value).snapshot()
10911091
}
1092+
1093+
/**
1094+
* We had a bug where putting `flex(1.0)` on a child of a column with `height(Constraint.Wrap)`
1095+
* would cause that child to get a height of 0.
1096+
*
1097+
* https://github.com/cashapp/redwood/issues/2753
1098+
*/
1099+
@Test
1100+
fun testFlexOnChildOfWrappingColumn() {
1101+
val root = row().apply {
1102+
width(Constraint.Fill)
1103+
height(Constraint.Fill)
1104+
}
1105+
1106+
val fixedSize = row().apply {
1107+
width(Constraint.Fill)
1108+
height(Constraint.Fill)
1109+
modifier = SizeImpl(width = 300.dp, height = 500.dp)
1110+
}.also {
1111+
root.children.insert(0, it)
1112+
}
1113+
1114+
val column = column().apply {
1115+
width(Constraint.Fill)
1116+
}.also {
1117+
fixedSize.children.insert(0, it)
1118+
}
1119+
1120+
widgetFactory.text("hello").apply {
1121+
modifier = FlexImpl(1.0)
1122+
bgColor(Blue)
1123+
}.also {
1124+
column.children.insert(0, it)
1125+
}
1126+
1127+
snapshotter(root.value).snapshot()
1128+
}
10921129
}
10931130

10941131
interface TestFlexContainer<T : Any> :
Lines changed: 3 additions & 0 deletions
Loading

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import kotlinx.cinterop.CValue
2323
import kotlinx.cinterop.useContents
2424
import platform.CoreGraphics.CGSize
2525
import platform.CoreGraphics.CGSizeMake
26+
import platform.UIKit.UILayoutPriorityDefaultLow
27+
import platform.UIKit.UILayoutPriorityRequired
2628
import platform.UIKit.UIView
2729
import platform.UIKit.UIViewNoIntrinsicMetric
2830

@@ -44,15 +46,18 @@ internal object UIViewMeasureCallback : MeasureCallback {
4446
else -> height.toDouble()
4547
}
4648

47-
// The default implementation of sizeThatFits: returns the existing size of
48-
// the view. That means that if we want to layout an empty UIView, which
49-
// already has a frame set, its measured size should be CGSizeZero, but
50-
// UIKit returns the existing size. See https://github.com/facebook/yoga/issues/606
51-
// for more information.
49+
// The default implementation of sizeThatFits: returns the existing size of the view. That means
50+
// that if we want to layout an empty UIView, which already has a frame set, its measured size
51+
// should be CGSizeZero, but UIKit returns the existing size. For more information, see
52+
// https://github.com/facebook/yoga/issues/606
5253
val sizeThatFits = if (view.isMemberOfClass(UIView.`class`()) && view.typedSubviews.isEmpty()) {
5354
Size(0f, 0f)
5455
} else {
55-
view.sizeThatFits(CGSizeMake(constrainedWidth, constrainedHeight)).toSize()
56+
view.systemLayoutSizeFittingSize(
57+
targetSize = CGSizeMake(constrainedWidth, constrainedHeight),
58+
withHorizontalFittingPriority = widthMode.toFittingPriority(),
59+
verticalFittingPriority = heightMode.toFittingPriority(),
60+
).toSize()
5661
}
5762

5863
return Size(
@@ -65,3 +70,8 @@ internal object UIViewMeasureCallback : MeasureCallback {
6570
private fun CValue<CGSize>.toSize() = useContents {
6671
Size(width.toFloat(), height.toFloat())
6772
}
73+
74+
private fun MeasureMode.toFittingPriority() = when (this) {
75+
MeasureMode.Exactly -> UILayoutPriorityRequired
76+
else -> UILayoutPriorityDefaultLow
77+
}

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

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import platform.CoreGraphics.CGRectMake
2121
import platform.CoreGraphics.CGRectZero
2222
import platform.CoreGraphics.CGSize
2323
import platform.CoreGraphics.CGSizeMake
24+
import platform.UIKit.UILayoutPriority
25+
import platform.UIKit.UILayoutPriorityRequired
2426
import platform.UIKit.UIScrollView
2527
import platform.UIKit.UIScrollViewContentInsetAdjustmentBehavior.UIScrollViewContentInsetAdjustmentNever
2628
import platform.UIKit.UIScrollViewDelegateProtocol
@@ -94,19 +96,36 @@ internal class YogaUIView : UIScrollView(cValue { CGRectZero }), UIScrollViewDel
9496
override fun intrinsicContentSize(): CValue<CGSize> {
9597
return calculateLayout(
9698
width = fillWidth.toYogaWithWidthConstraint(),
97-
maxWidth = fillWidth.toYoga(),
9899
height = fillHeight.toYogaWithWidthConstraint(),
99-
maxHeight = fillHeight.toYoga(),
100100
)
101101
}
102102

103103
override fun sizeThatFits(size: CValue<CGSize>): CValue<CGSize> {
104104
return size.useContents<CGSize, CValue<CGSize>> {
105105
calculateLayout(
106106
width = width.toYogaWithWidthConstraint(),
107-
maxWidth = width.toYoga(),
108107
height = height.toYogaWithHeightConstraint(),
109-
maxHeight = height.toYoga(),
108+
)
109+
}
110+
}
111+
112+
override fun systemLayoutSizeFittingSize(
113+
targetSize: CValue<CGSize>,
114+
withHorizontalFittingPriority: UILayoutPriority,
115+
verticalFittingPriority: UILayoutPriority,
116+
): CValue<CGSize> {
117+
return targetSize.useContents<CGSize, CValue<CGSize>> {
118+
calculateLayout(
119+
width = when {
120+
withHorizontalFittingPriority == UILayoutPriorityRequired -> width.toYoga()
121+
widthConstraint == Constraint.Fill -> width.toYoga()
122+
else -> Size.UNDEFINED
123+
},
124+
height = when {
125+
verticalFittingPriority == UILayoutPriorityRequired -> height.toYoga()
126+
heightConstraint == Constraint.Fill -> height.toYoga()
127+
else -> Size.UNDEFINED
128+
},
110129
)
111130
}
112131
}
@@ -160,14 +179,12 @@ internal class YogaUIView : UIScrollView(cValue { CGRectZero }), UIScrollViewDel
160179

161180
private fun calculateLayout(
162181
width: Float = Size.UNDEFINED,
163-
maxWidth: Float = Size.UNDEFINED,
164182
height: Float = Size.UNDEFINED,
165-
maxHeight: Float = Size.UNDEFINED,
166183
): CValue<CGSize> {
167184
rootNode.requestedWidth = width
168-
rootNode.requestedMaxWidth = maxWidth
185+
rootNode.requestedMaxWidth = Size.UNDEFINED
169186
rootNode.requestedHeight = height
170-
rootNode.requestedMaxHeight = maxHeight
187+
rootNode.requestedMaxHeight = Size.UNDEFINED
171188

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

Lines changed: 3 additions & 0 deletions
Loading
Lines changed: 3 additions & 0 deletions
Loading
Lines changed: 3 additions & 0 deletions
Loading

0 commit comments

Comments
 (0)