diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a31f6699e..2c6759d496 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,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. +- Correctly update the layout when a Box's child's modifiers are removed. Breaking: - Replace `CodeListener` with new functions in `RedwoodView`. This puts the loading/error/ready state with the UI that displays that state. diff --git a/redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewBox.kt b/redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewBox.kt index de70eafcb4..bd0de8026a 100644 --- a/redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewBox.kt +++ b/redwood-layout-view/src/main/kotlin/app/cash/redwood/layout/view/ViewBox.kt @@ -16,19 +16,17 @@ package app.cash.redwood.layout.view import android.content.Context -import android.view.Gravity import android.view.View +import android.view.View.LAYOUT_DIRECTION_LTR +import android.view.View.MeasureSpec +import android.view.View.MeasureSpec.makeMeasureSpec import android.view.ViewGroup -import android.view.ViewGroup.LayoutParams.MATCH_PARENT -import android.view.ViewGroup.LayoutParams.WRAP_CONTENT -import android.widget.FrameLayout -import androidx.core.view.updateLayoutParams import app.cash.redwood.Modifier import app.cash.redwood.layout.api.Constraint import app.cash.redwood.layout.api.CrossAxisAlignment import app.cash.redwood.layout.modifier.Height import app.cash.redwood.layout.modifier.HorizontalAlignment -import app.cash.redwood.layout.modifier.Margin as MarginModifier +import app.cash.redwood.layout.modifier.Margin as RedwoodMargin import app.cash.redwood.layout.modifier.Size import app.cash.redwood.layout.modifier.VerticalAlignment import app.cash.redwood.layout.modifier.Width @@ -36,17 +34,18 @@ import app.cash.redwood.layout.widget.Box import app.cash.redwood.ui.Density import app.cash.redwood.ui.Margin import app.cash.redwood.widget.ViewGroupChildren +import app.cash.redwood.widget.Widget internal class ViewBox( context: Context, -) : FrameLayout(context), +) : ViewGroup(context), Box { private val density = Density(context.resources) private var horizontalAlignment = CrossAxisAlignment.Start private var verticalAlignment = CrossAxisAlignment.Start - private var width = Constraint.Wrap - private var height = Constraint.Wrap - private var margin: Margin? = null + private var widthConstraint = Constraint.Wrap + private var heightConstraint = Constraint.Wrap + private var margin: Margin = Margin.Zero override var modifier: Modifier = Modifier @@ -54,171 +53,269 @@ internal class ViewBox( override val children = ViewGroupChildren(this) - override fun generateDefaultLayoutParams(): LayoutParams { - return LayoutParams(WRAP_CONTENT, WRAP_CONTENT) - } + private val measurer = Measurer() override fun width(width: Constraint) { - this.width = width - invalidate() + this.widthConstraint = width + requestLayout() } override fun height(height: Constraint) { - this.height = height - invalidate() - } - - override fun setLayoutParams(params: ViewGroup.LayoutParams?) { - if (params != null) { - margin?.let { margin -> - maybeUpdateLayoutParams(margin, params) - } - } - super.setLayoutParams(params) + this.heightConstraint = height + requestLayout() } override fun margin(margin: Margin) { this.margin = margin - - layoutParams?.let { params -> - maybeUpdateLayoutParams(margin, params) - // Write instance back out to indicate it has changed. - super.setLayoutParams(params) - } - } - - private fun maybeUpdateLayoutParams(margin: Margin, params: ViewGroup.LayoutParams) { - if (params is MarginLayoutParams) { - with(density) { - params.marginStart = margin.start.toPxInt() - params.topMargin = margin.top.toPxInt() - params.marginEnd = margin.end.toPxInt() - params.bottomMargin = margin.bottom.toPxInt() - } - } + requestLayout() } override fun horizontalAlignment(horizontalAlignment: CrossAxisAlignment) { this.horizontalAlignment = horizontalAlignment - invalidate() + requestLayout() } override fun verticalAlignment(verticalAlignment: CrossAxisAlignment) { this.verticalAlignment = verticalAlignment - invalidate() + requestLayout() } - /** Flush Redwood's modifiers into FrameLayout's LayoutParams before it measures. */ override fun onMeasure(widthMeasureSpec: Int, heightMeasureSpec: Int) { - val widthMode = if (width == Constraint.Fill) { - MeasureSpec.EXACTLY - } else { - MeasureSpec.getMode(widthMeasureSpec) - } val widthSize = MeasureSpec.getSize(widthMeasureSpec) + val widthMode = MeasureSpec.getMode(widthMeasureSpec) + val heightSize = MeasureSpec.getSize(heightMeasureSpec) + val heightMode = MeasureSpec.getMode(heightMeasureSpec) - val heightMode = if (height == Constraint.Fill) { - MeasureSpec.EXACTLY - } else { - MeasureSpec.getMode(heightMeasureSpec) + // Short circuit if this box's dimensions are already fully specified. + if (widthMode == MeasureSpec.EXACTLY && heightMode == MeasureSpec.EXACTLY) { + setMeasuredDimension(widthSize, heightSize) + return } - val heightSize = MeasureSpec.getSize(heightMeasureSpec) - for (child in children.widgets) { - child.value.updateLayoutParams { - setFrom(child.modifier) - } + measurer.box( + layoutDirection = layoutDirection, + boxDensity = density, + boxHorizontalAlignment = horizontalAlignment, + boxVerticalAlignment = verticalAlignment, + boxMargin = margin, + boxWidth = widthSize, + boxWidthUnspecified = widthMode == MeasureSpec.UNSPECIFIED, + boxHeight = heightSize, + boxHeightUnspecified = heightMode == MeasureSpec.UNSPECIFIED, + ) + + var maxWidth = 0 + var maxHeight = 0 + for (widget in children.widgets) { + measurer.measure(widget = widget) + maxWidth = maxOf(maxWidth, measurer.width + measurer.marginWidth) + maxHeight = maxOf(maxHeight, measurer.height + measurer.marginHeight) + } + + val resultWidth = when { + widthMode == MeasureSpec.EXACTLY -> widthSize + widthMode == MeasureSpec.AT_MOST -> maxWidth.coerceAtMost(widthSize) + else -> maxWidth + } + val resultHeight = when { + heightMode == MeasureSpec.EXACTLY -> heightSize + heightMode == MeasureSpec.AT_MOST -> maxHeight.coerceAtMost(heightSize) + else -> maxHeight } - super.onMeasure( - MeasureSpec.makeMeasureSpec(widthSize, widthMode), - MeasureSpec.makeMeasureSpec(heightSize, heightMode), + setMeasuredDimension(resultWidth, resultHeight) + } + + override fun onLayout(changed: Boolean, left: Int, top: Int, right: Int, bottom: Int) { + measurer.box( + layoutDirection = layoutDirection, + boxDensity = density, + boxHorizontalAlignment = horizontalAlignment, + boxVerticalAlignment = verticalAlignment, + boxMargin = margin, + boxWidth = (right - left).coerceAtLeast(0), + boxHeight = (bottom - top).coerceAtLeast(0), ) + + for (widget in children.widgets) { + measurer.measure(widget) + measurer.layout(widget) + } } +} - private fun LayoutParams.setFrom(modifier: Modifier) { - var horizontalAlignment = horizontalAlignment - var verticalAlignment = verticalAlignment - var requestedWidth = Int.MIN_VALUE - var requestedHeight = Int.MIN_VALUE - - modifier.forEachScoped { childModifier -> - // Check for modifier overrides in the children, otherwise default to the Box's alignment - // values. - when (childModifier) { - is HorizontalAlignment -> { - horizontalAlignment = childModifier.alignment - } +/** + * Measures and lays out one child view at a time. + * + * This class is mutable and reused to avoid object allocation. + */ +private class Measurer { + // Inputs from the box. + var layoutDirection = LAYOUT_DIRECTION_LTR + var boxDensity = Density(1.0) + var boxHorizontalAlignment = CrossAxisAlignment.Start + var boxVerticalAlignment = CrossAxisAlignment.Start + var boxMarginStart = 0 + var boxMarginEnd = 0 + var boxMarginTop = 0 + var boxMarginBottom = 0 + var boxWidthUnspecified = false + var boxHeightUnspecified = false - is VerticalAlignment -> { - verticalAlignment = childModifier.alignment - } + // The available space for the child view and its margins. + var frameWidth = -1 + var frameHeight = -1 - is Width -> { - requestedWidth = with(density) { childModifier.width.toPxInt() } - } + // Inputs from the child widget. + var horizontalAlignment = CrossAxisAlignment.Start + var verticalAlignment = CrossAxisAlignment.Start + var marginStart = 0 + var marginEnd = 0 + var marginTop = 0 + var marginBottom = 0 + var requestedWidth = -1 + var requestedHeight = -1 - is Height -> { - requestedHeight = with(density) { childModifier.height.toPxInt() } - } + // Measurement results. + var width = -1 + var height = -1 - is Size -> { - requestedWidth = with(density) { childModifier.width.toPxInt() } - requestedHeight = with(density) { childModifier.height.toPxInt() } - } + val marginWidth: Int + get() = marginStart + marginEnd + val marginHeight: Int + get() = marginTop + marginBottom + + /** Configure the enclosing box. */ + fun box( + layoutDirection: Int, + boxDensity: Density, + boxHorizontalAlignment: CrossAxisAlignment, + boxVerticalAlignment: CrossAxisAlignment, + boxMargin: Margin, + boxWidth: Int, + boxWidthUnspecified: Boolean = false, + boxHeight: Int, + boxHeightUnspecified: Boolean = false, + ) { + this.layoutDirection = layoutDirection + this.boxDensity = boxDensity + this.boxHorizontalAlignment = boxHorizontalAlignment + this.boxVerticalAlignment = boxVerticalAlignment + with(boxDensity) { + boxMarginStart = boxMargin.start.toPxInt() + boxMarginEnd = boxMargin.end.toPxInt() + boxMarginTop = boxMargin.top.toPxInt() + boxMarginBottom = boxMargin.bottom.toPxInt() + } + this.frameWidth = (boxWidth - boxMarginStart - boxMarginEnd).coerceAtLeast(0) + this.boxWidthUnspecified = boxWidthUnspecified + this.frameHeight = (boxHeight - boxMarginTop - boxMarginBottom).coerceAtLeast(0) + this.boxHeightUnspecified = boxHeightUnspecified + } - is MarginModifier -> { - with(density) { - marginStart = childModifier.margin.start.toPxInt() - marginEnd = childModifier.margin.end.toPxInt() - topMargin = childModifier.margin.top.toPxInt() - bottomMargin = childModifier.margin.bottom.toPxInt() + /** Measure [widget]. Always call [box] first. */ + fun measure(widget: Widget) { + this.horizontalAlignment = boxHorizontalAlignment + this.verticalAlignment = boxVerticalAlignment + this.marginStart = 0 + this.marginEnd = 0 + this.marginTop = 0 + this.marginBottom = 0 + this.requestedWidth = -1 + this.requestedHeight = -1 + + with(boxDensity) { + widget.modifier.forEachScoped { childModifier -> + when (childModifier) { + is HorizontalAlignment -> horizontalAlignment = childModifier.alignment + is VerticalAlignment -> verticalAlignment = childModifier.alignment + is Width -> requestedWidth = childModifier.width.toPxInt() + is Height -> requestedHeight = childModifier.height.toPxInt() + is Size -> { + requestedWidth = childModifier.width.toPxInt() + requestedHeight = childModifier.height.toPxInt() + } + + is RedwoodMargin -> { + marginStart = maxOf(marginStart, childModifier.margin.start.toPxInt()) + marginEnd = maxOf(marginEnd, childModifier.margin.end.toPxInt()) + marginTop = maxOf(marginTop, childModifier.margin.top.toPxInt()) + marginBottom = maxOf(marginBottom, childModifier.margin.bottom.toPxInt()) } } } } - width = if (requestedWidth != Int.MIN_VALUE) { - requestedWidth - } else { - horizontalAlignment.toDimension() + val availableWidth = (frameWidth - marginWidth).coerceAtLeast(0) + val availableHeight = (frameHeight - marginHeight).coerceAtLeast(0) + + val widthMeasureSpec = when { + requestedWidth != -1 -> makeMeasureSpec(requestedWidth, MeasureSpec.EXACTLY) + horizontalAlignment == CrossAxisAlignment.Stretch -> + makeMeasureSpec(availableWidth, MeasureSpec.EXACTLY) + boxWidthUnspecified -> makeMeasureSpec(availableWidth, MeasureSpec.UNSPECIFIED) + else -> makeMeasureSpec(availableWidth, MeasureSpec.AT_MOST) } - height = if (requestedHeight != Int.MIN_VALUE) { - requestedHeight - } else { - verticalAlignment.toDimension() + + val heightMeasureSpec = when { + requestedHeight != -1 -> makeMeasureSpec(requestedHeight, MeasureSpec.EXACTLY) + verticalAlignment == CrossAxisAlignment.Stretch -> + makeMeasureSpec(availableHeight, MeasureSpec.EXACTLY) + boxHeightUnspecified -> makeMeasureSpec(availableHeight, MeasureSpec.UNSPECIFIED) + else -> makeMeasureSpec(availableHeight, MeasureSpec.AT_MOST) } - gravity = toGravity(horizontalAlignment, verticalAlignment) - } - private fun toGravity( - horizontalAlignment: CrossAxisAlignment, - verticalAlignment: CrossAxisAlignment, - ): Int { - val horizontalGravity = when (horizontalAlignment) { - CrossAxisAlignment.Start -> Gravity.START - CrossAxisAlignment.Center -> Gravity.CENTER_HORIZONTAL - CrossAxisAlignment.End -> Gravity.END - CrossAxisAlignment.Stretch -> Gravity.FILL_HORIZONTAL - else -> throw AssertionError() + widget.value.measure(widthMeasureSpec, heightMeasureSpec) + + width = when { + requestedWidth != -1 -> requestedWidth + horizontalAlignment == CrossAxisAlignment.Stretch -> availableWidth + else -> widget.value.measuredWidth } - val verticalGravity = when (verticalAlignment) { - CrossAxisAlignment.Start -> Gravity.TOP - CrossAxisAlignment.Center -> Gravity.CENTER_VERTICAL - CrossAxisAlignment.End -> Gravity.BOTTOM - CrossAxisAlignment.Stretch -> Gravity.FILL_VERTICAL - else -> throw AssertionError() + + height = when { + requestedHeight != -1 -> requestedHeight + verticalAlignment == CrossAxisAlignment.Stretch -> availableHeight + else -> widget.value.measuredHeight } - return horizontalGravity or verticalGravity } - private fun CrossAxisAlignment.toDimension(): Int { - return when (this) { - CrossAxisAlignment.Start -> WRAP_CONTENT - CrossAxisAlignment.Center -> WRAP_CONTENT - CrossAxisAlignment.End -> WRAP_CONTENT - CrossAxisAlignment.Stretch -> MATCH_PARENT - else -> throw AssertionError() + /** Layout [widget]. Always call [measure] first. */ + fun layout(widget: Widget) { + val marginLeft: Int + val marginRight: Int + val alignRight: Boolean + + if (layoutDirection == View.LAYOUT_DIRECTION_RTL) { + marginLeft = marginEnd + boxMarginEnd + marginRight = marginStart + boxMarginStart + alignRight = horizontalAlignment == CrossAxisAlignment.Start + } else { + marginLeft = marginStart + boxMarginStart + marginRight = marginEnd + boxMarginEnd + alignRight = horizontalAlignment == CrossAxisAlignment.End + } + + val left = when { + horizontalAlignment == CrossAxisAlignment.Center -> { + marginLeft + (frameWidth - width) / 2 + } + alignRight -> { + (boxMarginStart + frameWidth + boxMarginEnd) - marginRight - width + } + else -> marginLeft } + + val top = when (verticalAlignment) { + CrossAxisAlignment.Center -> { + boxMarginTop + marginTop + (frameHeight - height) / 2 + } + CrossAxisAlignment.End -> { + boxMarginTop + frameHeight - marginBottom - height + } + else -> boxMarginTop + marginTop + } + + widget.value.layout(left, top, left + width, top + height) } } diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testChildrenModifierChanges_empty.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testChildrenModifierChanges_empty.png index ca2a08de20..0673865974 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testChildrenModifierChanges_empty.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testChildrenModifierChanges_empty.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:50b9c85846f850d18a937a524b9c5110bbc8f7788c813d701b523029510f4a16 -size 43147 +oid sha256:804ef6f58c103cb616ad2ab610c4287eba85e4225228330aa9cfac9b40eee6ba +size 42200 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMargins.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMargins.png index 0c1a3e1c2b..007e79bd1a 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMargins.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMargins.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:c23c04e5cf84ac043a05f101bffedd61dab17d87876a3ac38d78927bf29c36fe -size 4237 +oid sha256:40fed9e52c8241f10518017ad53f90c9f3e2c183bb4bfb925af373c6be511247 +size 4175 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMarginsAndAlignment.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMarginsAndAlignment.png index 56eed696e2..58f739c1cf 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMarginsAndAlignment.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_RTL_testMarginsAndAlignment.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:e6d995a87ee3eb14794c8056b0750b5a6d340b5c28e8931d134655ca046d0c72 -size 9473 +oid sha256:3204ac7fd815cfa3f5f078c4ac79538a6646e16281a9cb06842f8349b88462d5 +size 9474 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testChildrenModifierChanges_empty.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testChildrenModifierChanges_empty.png index ae24faa6b9..924fcdcc39 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testChildrenModifierChanges_empty.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testChildrenModifierChanges_empty.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:cd12803cda1cfaec124d2de3931c13a097aa6472d8c494ca7862ca1875f95803 -size 42875 +oid sha256:5ed76589be96daa6352e17401d40dc2fb826547c2bd7726e0a6731bcf531fcb6 +size 42322 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMargins.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMargins.png index 147628e9a6..20aee469b4 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMargins.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMargins.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:4de42288244c75fcf904f566bb10f86042c37e90b5302ce75c1150ecb4b18ebd -size 4253 +oid sha256:aecf04b9e93f7e5058908cd38122af8049cba17b4cade2945cd34886acfd84d6 +size 4194 diff --git a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMarginsAndAlignment.png b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMarginsAndAlignment.png index 75be3f73f5..6bd387d011 100644 --- a/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMarginsAndAlignment.png +++ b/redwood-layout-view/src/test/snapshots/images/app.cash.redwood.layout.view_ViewBoxTest_testMarginsAndAlignment.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:a7bf7a96c20b583f844fe8c8354adce35494cf080e1998fcef1c112f9f12fa7b -size 9515 +oid sha256:bb2dc0c8bddaa5591399db9a3fcbbf3ae66567737b230347968dc9849066febb +size 9481