From 917cc8cb1846302e291db79352c376c6bc85b472 Mon Sep 17 00:00:00 2001 From: Daniel Bertoldi Date: Wed, 25 Mar 2026 13:32:37 -0300 Subject: [PATCH] [JEWEL-1075] Fix crash in ScrollableContainer when used with IntrinsicSize When a VerticallyScrollableContainer or HorizontallyScrollableContainer was placed inside a layout querying intrinsic measurements (e.g., Column(Modifier.width(IntrinsicSize.Max))), Compose's default MeasurePolicy fallback re-ran the measure block with Constraints.Infinity. The scrollbar measurement then called Constraints.fixedHeight(Int.MAX_VALUE), which is an illegal Constraints value and threw an IllegalArgumentException. Fix the crash by converting the Layout SAM lambdas in ScrollableContainer and the scrollbar measure policies in Scrollbar to explicit MeasurePolicy objects with proper intrinsic overrides. The scrollbar policies report their thumb thickness as intrinsic width/height on the fixed axis and 0 on the scroll axis. ScrollableContainer queries scrollbar intrinsic sizes first, then measures content and scrollbars with concrete constraints, avoiding any unbounded intermediate measurement. Additionally add LazyColumn-style check() assertions that throw IllegalStateException when the scroll axis has an unbounded constraint during actual layout, giving developers a clear error message instead of a cryptic crash deep in Constraints internals. Tests covering all edge cases are added to ScrollableContainerTest: intrinsic-size parents no longer crash, containers report correct intrinsic widths (including scrollbar space on macOS AlwaysVisible), scrolling still works after the fix, and unbounded-axis nesting produces the expected IllegalStateException with a descriptive message. closes https://github.com/JetBrains/intellij-community/pull/3471 (cherry picked from commit 34cca8aef7c5410917506abb57cabf71917452f1) (cherry picked from commit b9ae1bc44aa9092d91fc122d9cc7fb7ef07095ef) IJ-MR-211495 GitOrigin-RevId: 9296aece0ee973fd40f37e1a30f1f66f63ac337a --- .../ui/component/ScrollableContainerTest.kt | 323 +++++++++++++++++- .../jewel/ui/component/ScrollableContainer.kt | 294 +++++++++++----- .../jetbrains/jewel/ui/component/Scrollbar.kt | 73 +++- 3 files changed, 594 insertions(+), 96 deletions(-) diff --git a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/ScrollableContainerTest.kt b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/ScrollableContainerTest.kt index 064c8317dafd..18ad8a14a213 100644 --- a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/ScrollableContainerTest.kt +++ b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/ScrollableContainerTest.kt @@ -2,14 +2,20 @@ package org.jetbrains.jewel.ui.component import androidx.compose.foundation.ScrollState +import androidx.compose.foundation.horizontalScroll import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.IntrinsicSize import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.requiredWidth import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.widthIn @@ -17,6 +23,8 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.LazyRow import androidx.compose.foundation.lazy.itemsIndexed +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf @@ -27,10 +35,11 @@ import androidx.compose.ui.layout.Layout import androidx.compose.ui.platform.testTag import androidx.compose.ui.test.assertHeightIsEqualTo import androidx.compose.ui.test.assertWidthIsEqualTo -import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.unit.Constraints import androidx.compose.ui.unit.dp +import kotlin.test.assertFailsWith import kotlin.test.assertFalse import kotlin.test.assertTrue import kotlin.time.Duration @@ -1479,6 +1488,318 @@ class ScrollableContainerTest { rule.waitForIdle() } + @Test + fun `vertical container inside column with IntrinsicSize Max does not crash`() { + rule.setContent { + IntUiTheme { + Column(Modifier.width(IntrinsicSize.Max).height(300.dp)) { + VerticallyScrollableContainer { Column { repeat(50) { Text("Item $it") } } } + } + } + } + rule.waitForIdle() + } + + @Test + fun `horizontal container inside row with IntrinsicSize Max does not crash`() { + rule.setContent { + IntUiTheme { + Row(Modifier.height(IntrinsicSize.Max).width(300.dp)) { + HorizontallyScrollableContainer(scrollState = rememberScrollState()) { + Row { repeat(50) { Text("Item $it") } } + } + } + } + } + rule.waitForIdle() + } + + @Test + fun `vertical container reports content intrinsic width inside IntrinsicSize Max column`() { + rule.setContent { + IntUiTheme { + val style by rememberScrollbarStyle(alwaysVisible = false) + Column(Modifier.width(IntrinsicSize.Max)) { + VerticallyScrollableContainer( + modifier = Modifier.height(300.dp).testTag("container"), + style = style, + ) { + Spacer(Modifier.requiredWidth(100.dp).height(1.dp)) + } + } + } + } + rule.onNodeWithTag("container").assertWidthIsEqualTo(100.dp) + } + + @Test + fun `IntrinsicSize Max column is wide enough to fit vertical container with AlwaysVisible scrollbar(macOs only)`() { + if (hostOs != OS.MacOS) return + + rule.setContent { + IntUiTheme { + val style = alwaysVisibleScrollbarStyle() + // Tag the Column: its width is what IntrinsicSize.Max resolves to. The container's + // own layout width may be smaller (content only) when canScroll = false, but the + // column must still reserve space for the scrollbar track. + Column(Modifier.width(IntrinsicSize.Max).testTag("column")) { + VerticallyScrollableContainer(modifier = Modifier.height(300.dp), style = style) { + Spacer(Modifier.requiredWidth(100.dp).height(1.dp)) + } + } + } + } + val scrollbarWidth = ScrollbarVisibility.AlwaysVisible.default().trackThicknessExpanded + rule.onNodeWithTag("column").assertWidthIsEqualTo(100.dp + scrollbarWidth) + } + + @Test + fun `horizontal container reports content intrinsic height inside IntrinsicSize Max row`() { + rule.setContent { + IntUiTheme { + val style by rememberScrollbarStyle(alwaysVisible = false) + Row(Modifier.height(IntrinsicSize.Max)) { + HorizontallyScrollableContainer( + scrollState = rememberScrollState(), + modifier = Modifier.width(300.dp).testTag("container"), + style = style, + ) { + Spacer(Modifier.width(1.dp).height(100.dp)) + } + } + } + } + rule.onNodeWithTag("container").assertHeightIsEqualTo(100.dp) + } + + @Test + fun `vertical container inside IntrinsicSize Max column with bounded height still allows scrolling`() { + val scrollState = ScrollState(0) + rule.setContent { + IntUiTheme { + Column(Modifier.width(IntrinsicSize.Max).height(300.dp)) { + VerticallyScrollableContainer(scrollState = scrollState) { + Column { repeat(100) { Text("Item $it") } } + } + } + } + } + assertTrue(scrollState.canScrollForward) + } + + @Test + fun `horizontal container inside IntrinsicSize Max row with bounded width still allows scrolling`() { + val scrollState = ScrollState(0) + rule.setContent { + IntUiTheme { + Row(Modifier.height(IntrinsicSize.Max).width(300.dp)) { + HorizontallyScrollableContainer(scrollState = scrollState) { + Row { repeat(100) { Text("Item $it") } } + } + } + } + } + assertTrue(scrollState.canScrollForward) + } + + @Test + fun `vertical container inside column with IntrinsicSize Min does not crash`() { + rule.setContent { + IntUiTheme { + Column(Modifier.width(IntrinsicSize.Min).height(300.dp)) { + VerticallyScrollableContainer { Column { repeat(50) { Text("Item $it") } } } + } + } + } + rule.waitForIdle() + } + + @Test + fun `horizontal container inside row with IntrinsicSize Min does not crash`() { + rule.setContent { + IntUiTheme { + Row(Modifier.height(IntrinsicSize.Min).width(300.dp)) { + HorizontallyScrollableContainer(scrollState = rememberScrollState()) { + Row { repeat(50) { Text("Item $it") } } + } + } + } + } + rule.waitForIdle() + } + + @Test + fun `vertical container reports content intrinsic width inside IntrinsicSize Min column`() { + rule.setContent { + IntUiTheme { + val style by rememberScrollbarStyle(alwaysVisible = false) + Column(Modifier.width(IntrinsicSize.Min)) { + VerticallyScrollableContainer( + modifier = Modifier.height(300.dp).testTag("container"), + style = style, + ) { + Spacer(Modifier.requiredWidth(100.dp).height(1.dp)) + } + } + } + } + rule.onNodeWithTag("container").assertWidthIsEqualTo(100.dp) + } + + @Test + fun `IntrinsicSize Min column is wide enough to fit vertical container with AlwaysVisible scrollbar (macOs only)`() { + if (hostOs != OS.MacOS) return + + rule.setContent { + IntUiTheme { + val style = alwaysVisibleScrollbarStyle() + Column(Modifier.width(IntrinsicSize.Min).testTag("column")) { + VerticallyScrollableContainer(modifier = Modifier.height(300.dp), style = style) { + Spacer(Modifier.requiredWidth(100.dp).height(1.dp)) + } + } + } + } + val scrollbarWidth = ScrollbarVisibility.AlwaysVisible.default().trackThicknessExpanded + rule.onNodeWithTag("column").assertWidthIsEqualTo(100.dp + scrollbarWidth) + } + + @Test + fun `horizontal container reports content intrinsic height inside IntrinsicSize Min row`() { + rule.setContent { + IntUiTheme { + val style by rememberScrollbarStyle(alwaysVisible = false) + Row(Modifier.height(IntrinsicSize.Min)) { + HorizontallyScrollableContainer( + scrollState = rememberScrollState(), + modifier = Modifier.width(300.dp).testTag("container"), + style = style, + ) { + Spacer(Modifier.width(1.dp).height(100.dp)) + } + } + } + } + rule.onNodeWithTag("container").assertHeightIsEqualTo(100.dp) + } + + @Test + fun `IntrinsicSize Min row is tall enough to fit horizontal container with AlwaysVisible scrollbar (macOs only)`() { + if (hostOs != OS.MacOS) return + + rule.setContent { + IntUiTheme { + val style = alwaysVisibleScrollbarStyle() + Row(Modifier.height(IntrinsicSize.Min).testTag("row")) { + HorizontallyScrollableContainer( + scrollState = rememberScrollState(), + modifier = Modifier.width(300.dp), + style = style, + ) { + Spacer(Modifier.width(1.dp).height(100.dp)) + } + } + } + } + val scrollbarHeight = ScrollbarVisibility.AlwaysVisible.default().trackThicknessExpanded + rule.onNodeWithTag("row").assertHeightIsEqualTo(100.dp + scrollbarHeight) + } + + @Test + fun `vertical container throws when nested inside vertically scrollable parent`() { + assertFailsWith { + rule.setContent { + IntUiTheme { + Box(Modifier.verticalScroll(rememberScrollState())) { + VerticallyScrollableContainer(modifier = Modifier.fillMaxWidth()) { Text("content") } + } + } + } + rule.waitForIdle() + } + } + + @Test + fun `horizontal container throws when nested inside horizontally scrollable parent`() { + assertFailsWith { + rule.setContent { + IntUiTheme { + Box(Modifier.horizontalScroll(rememberScrollState())) { + HorizontallyScrollableContainer( + scrollState = rememberScrollState(), + modifier = Modifier.fillMaxHeight(), + ) { + Text("content") + } + } + } + } + rule.waitForIdle() + } + } + + @Test + fun `vertical container throws when height is unbounded via ForceConstraints`() { + assertFailsWith { + rule.setContent { + IntUiTheme { + ForceConstraints(Constraints(maxWidth = 400, maxHeight = Constraints.Infinity)) { + VerticallyScrollableContainer { Text("content") } + } + } + } + rule.waitForIdle() + } + } + + @Test + fun `horizontal container throws when width is unbounded via ForceConstraints`() { + assertFailsWith { + rule.setContent { + IntUiTheme { + ForceConstraints(Constraints(maxWidth = Constraints.Infinity, maxHeight = 400)) { + HorizontallyScrollableContainer(scrollState = rememberScrollState()) { Text("content") } + } + } + } + rule.waitForIdle() + } + } + + @Test + fun `vertical container exception message mentions component name and cause`() { + val exception = + assertFailsWith { + rule.setContent { + IntUiTheme { + ForceConstraints(Constraints(maxWidth = 400, maxHeight = Constraints.Infinity)) { + VerticallyScrollableContainer { Text("content") } + } + } + } + rule.waitForIdle() + } + assertTrue(exception.message!!.contains("VerticallyScrollableContainer")) + assertTrue(exception.message!!.contains("unbounded")) + } + + @Test + fun `horizontal container exception message mentions component name and cause`() { + val exception = + assertFailsWith { + rule.setContent { + IntUiTheme { + ForceConstraints(Constraints(maxWidth = Constraints.Infinity, maxHeight = 400)) { + HorizontallyScrollableContainer(scrollState = rememberScrollState()) { Text("content") } + } + } + } + rule.waitForIdle() + } + assertTrue(exception.message!!.contains("HorizontallyScrollableContainer")) + assertTrue(exception.message!!.contains("unbounded")) + } + @Composable private fun rememberScrollbarStyle( alwaysVisible: Boolean, diff --git a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/ScrollableContainer.kt b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/ScrollableContainer.kt index 72482bc1ff23..7ccb023813f2 100644 --- a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/ScrollableContainer.kt +++ b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/ScrollableContainer.kt @@ -25,8 +25,13 @@ import androidx.compose.ui.input.pointer.PointerEventType import androidx.compose.ui.input.pointer.PointerIcon import androidx.compose.ui.input.pointer.pointerHoverIcon import androidx.compose.ui.input.pointer.pointerInput +import androidx.compose.ui.layout.IntrinsicMeasurable +import androidx.compose.ui.layout.IntrinsicMeasureScope import androidx.compose.ui.layout.Layout -import androidx.compose.ui.layout.Placeable +import androidx.compose.ui.layout.Measurable +import androidx.compose.ui.layout.MeasurePolicy +import androidx.compose.ui.layout.MeasureResult +import androidx.compose.ui.layout.MeasureScope import androidx.compose.ui.layout.layoutId import androidx.compose.ui.unit.Constraints import androidx.compose.ui.unit.Dp @@ -824,102 +829,227 @@ private fun ScrollableContainerImpl( Box(Modifier.layoutId(ID_HORIZONTAL_SCROLLBAR)) { horizontalScrollbar() } } }, - modifier, - ) { measurables, incomingConstraints -> - val verticalScrollbarMeasurable = measurables.find { it.layoutId == ID_VERTICAL_SCROLLBAR } - val horizontalScrollbarMeasurable = measurables.find { it.layoutId == ID_HORIZONTAL_SCROLLBAR } + modifier = modifier, + measurePolicy = + object : MeasurePolicy { + override fun MeasureScope.measure( + measurables: List, + constraints: Constraints, + ): MeasureResult { + if (verticalScrollbar != null) { + check(constraints.hasBoundedHeight) { + "VerticallyScrollableContainer was measured with an unbounded maximum " + + "height, which is not supported. One of the common reasons is " + + "nesting it inside another vertically scrollable layout such as " + + "Column(Modifier.verticalScroll()) or another " + + "VerticallyScrollableContainer. Use Modifier.height(), " + + "Modifier.fillMaxHeight(), or Modifier.weight(1f) inside a Column " + + "to provide a bounded height." + } + } + if (horizontalScrollbar != null) { + check(constraints.hasBoundedWidth) { + "HorizontallyScrollableContainer was measured with an unbounded maximum " + + "width, which is not supported. One of the common reasons is " + + "nesting it inside another horizontally scrollable layout such as " + + "Row(Modifier.horizontalScroll()) or another " + + "HorizontallyScrollableContainer. Use Modifier.width(), " + + "Modifier.fillMaxWidth(), or Modifier.weight(1f) inside a Row to " + + "provide a bounded width." + } + } - // Leaving the bottom-end corner empty when both scrollbars visible at the same time - val accountForVerticalScrollbar = verticalScrollbarMeasurable != null && verticalScrollbarVisible - val accountForHorizontalScrollbar = horizontalScrollbarMeasurable != null && horizontalScrollbarVisible - val sizeOffsetWhenBothVisible = - if (accountForVerticalScrollbar && accountForHorizontalScrollbar) { - scrollbarStyle.scrollbarVisibility.trackThicknessExpanded.roundToPx() - } else { - 0 - } + val verticalScrollbarMeasurable = measurables.find { it.layoutId == ID_VERTICAL_SCROLLBAR } + val horizontalScrollbarMeasurable = measurables.find { it.layoutId == ID_HORIZONTAL_SCROLLBAR } - val verticalScrollbarPlaceable = - if (accountForVerticalScrollbar) { - val verticalScrollbarConstraints = - Constraints.fixedHeight(incomingConstraints.maxHeight - sizeOffsetWhenBothVisible) - verticalScrollbarMeasurable.measure(verticalScrollbarConstraints) - } else { - null - } + // Leaving the bottom-end corner empty when both scrollbars visible at the same time + val accountForVerticalScrollbar = verticalScrollbarMeasurable != null && verticalScrollbarVisible + val accountForHorizontalScrollbar = + horizontalScrollbarMeasurable != null && horizontalScrollbarVisible + val sizeOffsetWhenBothVisible = + if (accountForVerticalScrollbar && accountForHorizontalScrollbar) { + scrollbarStyle.scrollbarVisibility.trackThicknessExpanded.roundToPx() + } else { + 0 + } - val horizontalScrollbarPlaceable = - if (accountForHorizontalScrollbar) { - val horizontalScrollbarConstraints = - Constraints.fixedWidth(incomingConstraints.maxWidth - sizeOffsetWhenBothVisible) - horizontalScrollbarMeasurable.measure(horizontalScrollbarConstraints) - } else { - null - } + // Query scrollbar intrinsic sizes instead of measuring them upfront. This breaks + // the layout dependency cycle that would crash when incomingConstraints are + // unbounded (e.g., inside a Column with IntrinsicSize): measuring a scrollbar + // with fixedHeight/fixedWidth of Int.MAX_VALUE is illegal, but querying its + // intrinsic size is always safe. + val vScrollbarIntrinsicWidth = + if (accountForVerticalScrollbar) { + verticalScrollbarMeasurable.maxIntrinsicWidth(constraints.maxHeight) + } else { + 0 + } - val isMacOs = hostOs == OS.MacOS - val contentMeasurable = measurables.find { it.layoutId == ID_CONTENT } ?: error("Content not provided") - val contentConstraints = - computeContentConstraints( - scrollbarStyle, - isMacOs, - incomingConstraints, - verticalScrollbarPlaceable, - horizontalScrollbarPlaceable, - ) - val contentPlaceable = contentMeasurable.measure(contentConstraints) + val hScrollbarIntrinsicHeight = + if (accountForHorizontalScrollbar) { + horizontalScrollbarMeasurable.maxIntrinsicHeight(constraints.maxWidth) + } else { + 0 + } - val isAlwaysVisible = scrollbarStyle.scrollbarVisibility is AlwaysVisible - val vScrollbarWidth = - when { - !isMacOs -> 0 - isAlwaysVisible -> verticalScrollbarPlaceable?.width ?: 0 - else -> 0 - } - val width = contentPlaceable.width + vScrollbarWidth + val isMacOs = hostOs == OS.MacOS + val contentMeasurable = + measurables.find { it.layoutId == ID_CONTENT } ?: error("Content not provided") + val contentConstraints = + computeContentConstraints( + scrollbarStyle, + isMacOs, + constraints, + vScrollbarIntrinsicWidth, + hScrollbarIntrinsicHeight, + ) + val contentPlaceable = contentMeasurable.measure(contentConstraints) - val hScrollbarHeight = - when { - !isMacOs -> 0 - isAlwaysVisible -> horizontalScrollbarPlaceable?.height ?: 0 - else -> 0 - } - val height = contentPlaceable.height + hScrollbarHeight + val isAlwaysVisible = scrollbarStyle.scrollbarVisibility is AlwaysVisible + val vScrollbarWidth = + when { + !isMacOs -> 0 + isAlwaysVisible -> vScrollbarIntrinsicWidth + else -> 0 + } + val width = contentPlaceable.width + vScrollbarWidth - layout(width, height) { - contentPlaceable.placeRelative(x = 0, y = 0, zIndex = 0f) - verticalScrollbarPlaceable?.placeRelative( - x = - when (verticalScrollbarPosition) { - ScrollbarPosition.Start -> 0 - ScrollbarPosition.End -> width - verticalScrollbarPlaceable.width - }, - y = 0, - zIndex = 1f, - ) - horizontalScrollbarPlaceable?.placeRelative( - x = 0, - y = - when (horizontalScrollbarPosition) { - ScrollbarPosition.Start -> 0 - ScrollbarPosition.End -> height - horizontalScrollbarPlaceable.height - }, - zIndex = 1f, - ) - } - } + val hScrollbarHeight = + when { + !isMacOs -> 0 + isAlwaysVisible -> hScrollbarIntrinsicHeight + else -> 0 + } + val height = contentPlaceable.height + hScrollbarHeight + + // Now that content is measured and the container dimensions are known (bounded + // even if incomingConstraints were unbounded), measure the scrollbars with + // concrete sizes. + val verticalScrollbarPlaceable = + if (accountForVerticalScrollbar) { + val safeVerticalScrollbarHeightConstraint = + (height - sizeOffsetWhenBothVisible).coerceAtLeast(0) + val verticalScrollbarConstraints = + Constraints.fixedHeight(safeVerticalScrollbarHeightConstraint) + verticalScrollbarMeasurable.measure(verticalScrollbarConstraints) + } else { + null + } + + val horizontalScrollbarPlaceable = + if (accountForHorizontalScrollbar) { + val safeHorizontalScrollbarWidthConstraint = + (width - sizeOffsetWhenBothVisible).coerceAtLeast(0) + val horizontalScrollbarConstraints = + Constraints.fixedWidth(safeHorizontalScrollbarWidthConstraint) + horizontalScrollbarMeasurable.measure(horizontalScrollbarConstraints) + } else { + null + } + + return layout(width, height) { + contentPlaceable.placeRelative(x = 0, y = 0, zIndex = 0f) + verticalScrollbarPlaceable?.placeRelative( + x = + when (verticalScrollbarPosition) { + ScrollbarPosition.Start -> 0 + ScrollbarPosition.End -> width - verticalScrollbarPlaceable.width + }, + y = 0, + zIndex = 1f, + ) + horizontalScrollbarPlaceable?.placeRelative( + x = 0, + y = + when (horizontalScrollbarPosition) { + ScrollbarPosition.Start -> 0 + ScrollbarPosition.End -> height - horizontalScrollbarPlaceable.height + }, + zIndex = 1f, + ) + } + } + + // Intrinsic overrides prevent the default fallback from re-running the measure + // block with Constraints.Infinity, which would trip the bounded-constraints checks + // above. They also provide correct values for IntrinsicSize parents. + // + // Cross-axis (the axis the container sizes itself to match its content): + // - VerticallyScrollableContainer → width delegates to content + scrollbar + // - HorizontallyScrollableContainer → height delegates to content + scrollbar + // Scroll-axis (must be bounded by the parent — no natural size): + // - Always returns 0 + + // These overrides skip verticalScrollbarVisible/horizontalScrollbarVisible intentionally. + // Intrinsic queries run before the measure pass, so checking scroll state here would + // cause IntrinsicSize parents to resize as content becomes scrollable. The cost is that + // a non-scrollable container inside an IntrinsicSize parent reserves track space it + // doesn't actually need. + override fun IntrinsicMeasureScope.maxIntrinsicWidth( + measurables: List, + height: Int, + ): Int { + val contentWidth = measurables[0].maxIntrinsicWidth(height) + if (verticalScrollbar == null) return contentWidth + val isMacOs = hostOs == OS.MacOS + val isAlwaysVisible = scrollbarStyle.scrollbarVisibility is AlwaysVisible + if (!isMacOs || !isAlwaysVisible) return contentWidth + // Vertical scrollbar is always at index 1 when present + return contentWidth + measurables[1].maxIntrinsicWidth(height) + } + + override fun IntrinsicMeasureScope.minIntrinsicWidth( + measurables: List, + height: Int, + ): Int { + val contentWidth = measurables[0].minIntrinsicWidth(height) + if (verticalScrollbar == null) return contentWidth + val isMacOs = hostOs == OS.MacOS + val isAlwaysVisible = scrollbarStyle.scrollbarVisibility is AlwaysVisible + if (!isMacOs || !isAlwaysVisible) return contentWidth + return contentWidth + measurables[1].minIntrinsicWidth(height) + } + + override fun IntrinsicMeasureScope.maxIntrinsicHeight( + measurables: List, + width: Int, + ): Int { + if (verticalScrollbar != null) return 0 + val contentHeight = measurables[0].maxIntrinsicHeight(width) + if (horizontalScrollbar == null) return contentHeight + val isMacOs = hostOs == OS.MacOS + val isAlwaysVisible = scrollbarStyle.scrollbarVisibility is AlwaysVisible + if (!isMacOs || !isAlwaysVisible) return contentHeight + // Horizontal scrollbar is at index 1 when only horizontal is present + return contentHeight + measurables[1].maxIntrinsicHeight(width) + } + + override fun IntrinsicMeasureScope.minIntrinsicHeight( + measurables: List, + width: Int, + ): Int { + if (verticalScrollbar != null) return 0 + val contentHeight = measurables[0].minIntrinsicHeight(width) + if (horizontalScrollbar == null) return contentHeight + val isMacOs = hostOs == OS.MacOS + val isAlwaysVisible = scrollbarStyle.scrollbarVisibility is AlwaysVisible + if (!isMacOs || !isAlwaysVisible) return contentHeight + return contentHeight + measurables[1].minIntrinsicHeight(width) + } + }, + ) } private fun computeContentConstraints( scrollbarStyle: ScrollbarStyle, isMacOs: Boolean, incomingConstraints: Constraints, - verticalScrollbarPlaceable: Placeable?, - horizontalScrollbarPlaceable: Placeable?, + verticalScrollbarWidth: Int, + horizontalScrollbarHeight: Int, ): Constraints { val visibility = scrollbarStyle.scrollbarVisibility - val scrollbarWidth = verticalScrollbarPlaceable?.width ?: 0 - val scrollbarHeight = horizontalScrollbarPlaceable?.height ?: 0 + val scrollbarWidth = verticalScrollbarWidth + val scrollbarHeight = horizontalScrollbarHeight val maxWidth = incomingConstraints.maxWidth val maxHeight = incomingConstraints.maxHeight diff --git a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Scrollbar.kt b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Scrollbar.kt index a891ece1b4b9..6eba2a5df236 100644 --- a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Scrollbar.kt +++ b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Scrollbar.kt @@ -51,8 +51,13 @@ import androidx.compose.ui.graphics.toArgb import androidx.compose.ui.input.pointer.PointerInputScope import androidx.compose.ui.input.pointer.SuspendingPointerInputModifierNode import androidx.compose.ui.input.pointer.positionChange +import androidx.compose.ui.layout.IntrinsicMeasurable +import androidx.compose.ui.layout.IntrinsicMeasureScope import androidx.compose.ui.layout.Layout +import androidx.compose.ui.layout.Measurable import androidx.compose.ui.layout.MeasurePolicy +import androidx.compose.ui.layout.MeasureResult +import androidx.compose.ui.layout.MeasureScope import androidx.compose.ui.layout.layoutId import androidx.compose.ui.node.DelegatingNode import androidx.compose.ui.node.ModifierNodeElement @@ -471,25 +476,67 @@ private val IntRange.size get() = last + 1 - first private fun verticalMeasurePolicy(sliderAdapter: SliderAdapter, setContainerSize: (Int) -> Unit, thumbThickness: Int) = - MeasurePolicy { measurables, constraints -> - setContainerSize(constraints.maxHeight) - val pixelRange = sliderAdapter.thumbPixelRange - val placeable = - measurables.first().measure(Constraints.fixed(constraints.constrainWidth(thumbThickness), pixelRange.size)) - layout(placeable.width, constraints.maxHeight) { placeable.place(0, pixelRange.first) } + object : MeasurePolicy { + override fun MeasureScope.measure(measurables: List, constraints: Constraints): MeasureResult { + setContainerSize(constraints.maxHeight) + val pixelRange = sliderAdapter.thumbPixelRange + val placeable = + measurables + .first() + .measure(Constraints.fixed(constraints.constrainWidth(thumbThickness), pixelRange.size)) + return layout(placeable.width, constraints.maxHeight) { placeable.place(0, pixelRange.first) } + } + + // The scrollbar's cross-axis intrinsic size is simply its track thickness. Returning it + // here avoids the default fallback that re-runs the measure block with Int.MAX_VALUE as + // the height, which would crash when trying to call layout(w, Int.MAX_VALUE). + override fun IntrinsicMeasureScope.maxIntrinsicWidth(measurables: List, height: Int): Int = + thumbThickness + + override fun IntrinsicMeasureScope.minIntrinsicWidth(measurables: List, height: Int): Int = + thumbThickness + + // The vertical scrollbar has no natural height — it always fills its container. Returning + // 0 prevents the default fallback from re-running the measure block with Int.MAX_VALUE, + // which would crash at layout(w, Int.MAX_VALUE) when queried via modifier chains (e.g. + // padding) that propagate intrinsic measurements. + override fun IntrinsicMeasureScope.maxIntrinsicHeight(measurables: List, width: Int): Int = + 0 + + override fun IntrinsicMeasureScope.minIntrinsicHeight(measurables: List, width: Int): Int = + 0 } private fun horizontalMeasurePolicy( sliderAdapter: SliderAdapter, setContainerSize: (Int) -> Unit, thumbThickness: Int, -) = MeasurePolicy { measurables, constraints -> - setContainerSize(constraints.maxWidth) - val pixelRange = sliderAdapter.thumbPixelRange - val placeable = - measurables.first().measure(Constraints.fixed(pixelRange.size, constraints.constrainHeight(thumbThickness))) - layout(constraints.maxWidth, placeable.height) { placeable.place(pixelRange.first, 0) } -} +) = + object : MeasurePolicy { + override fun MeasureScope.measure(measurables: List, constraints: Constraints): MeasureResult { + setContainerSize(constraints.maxWidth) + val pixelRange = sliderAdapter.thumbPixelRange + val placeable = + measurables + .first() + .measure(Constraints.fixed(pixelRange.size, constraints.constrainHeight(thumbThickness))) + return layout(constraints.maxWidth, placeable.height) { placeable.place(pixelRange.first, 0) } + } + + // The scrollbar's cross-axis intrinsic size is simply its track thickness. + override fun IntrinsicMeasureScope.maxIntrinsicHeight(measurables: List, width: Int): Int = + thumbThickness + + override fun IntrinsicMeasureScope.minIntrinsicHeight(measurables: List, width: Int): Int = + thumbThickness + + // The horizontal scrollbar has no natural width — it always fills its container. + override fun IntrinsicMeasureScope.maxIntrinsicWidth(measurables: List, height: Int): Int = + 0 + + override fun IntrinsicMeasureScope.minIntrinsicWidth(measurables: List, height: Int): Int = + 0 + } private fun Modifier.scrollbarDrag( interactionSource: MutableInteractionSource,