Skip to content

Commit

Permalink
Fix a bug where Box's margins weren't included in the measurement (#2429
Browse files Browse the repository at this point in the history
)

We included it in layout but not measurement, which led to
bad renders.

Also fix UIViewBox to override intrinsicContentSize() so that
it can be used as a child of a UIStackView.
  • Loading branch information
squarejesse authored Nov 6, 2024
1 parent 5b11767 commit 19bae88
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 18 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,49 @@ abstract class AbstractBoxTest<T : Any> {
snapshotter(widget.value).snapshot()
}

@Test
fun testBoxMeasurementIncludesMargins() {
val container = widgetFactory.column()
container.add(
box().apply {
children.insert(0, widgetFactory.text("box 1"))
}.value,
)

container.add(
box().apply {
horizontalAlignment(CrossAxisAlignment.End)
margin(Margin(start = 10.dp, top = 20.dp, end = 30.dp, bottom = 40.dp))
children.insert(0, widgetFactory.text("box 2"))
}.value,
)

container.add(
box().apply {
horizontalAlignment(CrossAxisAlignment.Center)
children.insert(0, widgetFactory.text("box 3"))
}.value,
)

container.add(
box().apply {
margin(Margin(start = 10.dp, top = 20.dp, end = 30.dp, bottom = 40.dp))
children.insert(0, widgetFactory.text("box 4"))
}.value,
)

container.add(
box().apply {
horizontalAlignment(CrossAxisAlignment.End)
children.insert(0, widgetFactory.text("box 5"))
}.value,
)

val scrollWrapper = widgetFactory.scrollWrapper()
scrollWrapper.content = container.value
snapshotter(scrollWrapper.value).snapshot()
}

@Test
fun testMarginsAndAlignment() {
val widget = box().apply {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ import kotlinx.cinterop.convert
import kotlinx.cinterop.readValue
import kotlinx.cinterop.useContents
import platform.CoreGraphics.CGFloat
import platform.CoreGraphics.CGRect
import platform.CoreGraphics.CGRectMake
import platform.CoreGraphics.CGRectZero
import platform.CoreGraphics.CGSize
import platform.CoreGraphics.CGSizeMake
import platform.UIKit.UIView
import platform.UIKit.UIViewNoIntrinsicMetric
import platform.darwin.NSInteger

internal class UIViewBox :
Expand Down Expand Up @@ -118,6 +118,9 @@ internal class UIViewBox :
}
}

override fun intrinsicContentSize() =
sizeThatFits(CGSizeMake(UIViewNoIntrinsicMetric, UIViewNoIntrinsicMetric))

override fun layoutSubviews() {
super.layoutSubviews()

Expand All @@ -126,7 +129,8 @@ internal class UIViewBox :
boxHorizontalAlignment = horizontalAlignment,
boxVerticalAlignment = verticalAlignment,
boxMargin = margin,
boxFrame = frame,
boxWidth = frame.useContents { size.width },
boxHeight = frame.useContents { size.height },
)

for (widget in children.widgets) {
Expand All @@ -141,7 +145,8 @@ internal class UIViewBox :
boxHorizontalAlignment = horizontalAlignment,
boxVerticalAlignment = verticalAlignment,
boxMargin = margin,
boxFrame = frame,
boxWidth = size.useContents { width },
boxHeight = size.useContents { height },
)

var maxWidth = 0.0
Expand All @@ -152,7 +157,10 @@ internal class UIViewBox :
maxHeight = maxOf(maxHeight, measurer.height + measurer.marginHeight)
}

return CGSizeMake(maxWidth, maxHeight)
val boxMarginWidth = measurer.boxMarginStart + measurer.boxMarginEnd
val boxMarginHeight = measurer.boxMarginTop + measurer.boxMarginBottom

return CGSizeMake(maxWidth + boxMarginWidth, maxHeight + boxMarginHeight)
}
}
}
Expand Down Expand Up @@ -201,7 +209,8 @@ private class Measurer {
boxHorizontalAlignment: CrossAxisAlignment,
boxVerticalAlignment: CrossAxisAlignment,
boxMargin: Margin,
boxFrame: CValue<CGRect>,
boxWidth: CGFloat,
boxHeight: CGFloat,
) {
this.boxDensity = boxDensity
this.boxHorizontalAlignment = boxHorizontalAlignment
Expand All @@ -212,9 +221,13 @@ private class Measurer {
boxMarginTop = boxMargin.top.toPx()
boxMarginBottom = boxMargin.bottom.toPx()
}
boxFrame.useContents {
frameWidth = (size.width - boxMarginStart - boxMarginEnd).coerceAtLeast(0.0)
frameHeight = (size.height - boxMarginTop - boxMarginBottom).coerceAtLeast(0.0)
this.frameWidth = when {
boxWidth == UIViewNoIntrinsicMetric -> UIViewNoIntrinsicMetric
else -> (boxWidth - boxMarginStart - boxMarginEnd).coerceAtLeast(0.0)
}
this.frameHeight = when {
boxHeight == UIViewNoIntrinsicMetric -> UIViewNoIntrinsicMetric
else -> (boxHeight - boxMarginTop - boxMarginBottom).coerceAtLeast(0.0)
}
}

Expand Down Expand Up @@ -253,8 +266,14 @@ private class Measurer {
}
}

val availableWidth = (frameWidth - marginWidth).coerceAtLeast(0.0)
val availableHeight = (frameHeight - marginHeight).coerceAtLeast(0.0)
val availableWidth = when {
frameWidth == UIViewNoIntrinsicMetric -> UIViewNoIntrinsicMetric
else -> (frameWidth - marginWidth).coerceAtLeast(0.0)
}
val availableHeight = when {
frameHeight == UIViewNoIntrinsicMetric -> UIViewNoIntrinsicMetric
else -> (frameHeight - marginHeight).coerceAtLeast(0.0)
}

val fitWidth = when {
!requestedWidth.isNaN() -> requestedWidth
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,18 @@ private class BoxViewGroup(
maxHeight = maxOf(maxHeight, measurer.height + measurer.marginHeight)
}

val resultWidth = when {
widthMode == MeasureSpec.EXACTLY -> widthSize
widthMode == MeasureSpec.AT_MOST -> maxWidth.coerceAtMost(widthSize)
else -> maxWidth
val boxMarginWidth = measurer.boxMarginStart + measurer.boxMarginEnd
val boxMarginHeight = measurer.boxMarginTop + measurer.boxMarginBottom

val resultWidth = when (widthMode) {
MeasureSpec.EXACTLY -> widthSize
MeasureSpec.AT_MOST -> (maxWidth + boxMarginWidth).coerceAtMost(widthSize)
else -> maxWidth + boxMarginWidth
}
val resultHeight = when {
heightMode == MeasureSpec.EXACTLY -> heightSize
heightMode == MeasureSpec.AT_MOST -> maxHeight.coerceAtMost(heightSize)
else -> maxHeight
val resultHeight = when (heightMode) {
MeasureSpec.EXACTLY -> heightSize
MeasureSpec.AT_MOST -> (maxHeight + boxMarginHeight).coerceAtMost(heightSize)
else -> maxHeight + boxMarginHeight
}

setMeasuredDimension(resultWidth, resultHeight)
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.

0 comments on commit 19bae88

Please sign in to comment.