Skip to content

Commit 7b090e3

Browse files
committed
Fix sheet performance on expanding content
1 parent 935d8e0 commit 7b090e3

2 files changed

Lines changed: 141 additions & 5 deletions

File tree

composeunstyled-bottom-sheet/src/commonMain/kotlin/com/composeunstyled/BottomSheet.kt

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,19 @@ class BottomSheetState internal constructor(
400400
return contentDependentDetents
401401
}
402402

403-
internal fun shouldUseContentHeightForLayout(): Boolean {
403+
internal fun shouldUseContentHeightForLayout(measuredContentHeightPx: Float): Boolean {
404+
val contentHeightIsChanging = this.measuredContentHeightPx.isNaN().not() &&
405+
this.measuredContentHeightPx.isSameValueAs(measuredContentHeightPx).not()
406+
val offsetHeight = currentOffsetHeight()
407+
if (
408+
contentHeightIsChanging &&
409+
offsetHeight.isNaN().not() &&
410+
(isDragging || isIdle.not()) &&
411+
offsetHeight > measuredContentHeightPx
412+
) {
413+
return false
414+
}
415+
404416
val isMovingToDifferentDetent = targetDetent != currentDetent &&
405417
anchoredDraggableState.offset.isNaN().not()
406418

@@ -433,6 +445,14 @@ class BottomSheetState internal constructor(
433445
}
434446
}
435447

448+
internal fun bottomAlignedOffsetPx(): Float {
449+
return if (containerHeightPx.isNaN() || measuredSheetHeightPx.isNaN()) {
450+
Float.NaN
451+
} else {
452+
(containerHeightPx - measuredSheetHeightPx).coerceAtLeast(0f)
453+
}
454+
}
455+
436456
private fun detentHeightPx(
437457
detent: SheetDetent,
438458
sheetHeightPx: Float,
@@ -522,7 +542,6 @@ class BottomSheetState internal constructor(
522542
if (measuredSheetHeightPx.isSameValueAs(measuredHeightPx)) return
523543

524544
measuredSheetHeightPx = measuredHeightPx
525-
invalidateDetents()
526545
}
527546

528547
internal fun updateContentHeight(measuredHeightPx: Float, includeMeasuredSheetHeight: Boolean) {
@@ -865,7 +884,9 @@ fun BottomSheetScope.Sheet(
865884
val contentHeight = placeables.maxOfOrNull { it.height } ?: 0
866885
val height = when {
867886
state == null -> contentHeight
868-
state.shouldUseContentHeightForLayout() -> minOf(contentHeight, layoutMaxHeight)
887+
state.shouldUseContentHeightForLayout(
888+
contentHeight.toFloat(),
889+
) -> minOf(contentHeight, layoutMaxHeight)
869890
constraints.hasBoundedHeight.not() && resolvedLayoutHeight.isNaN() -> contentHeight
870891
else -> layoutMaxHeight
871892
}.coerceIn(constraints.minHeight, constraints.maxHeight)
@@ -917,7 +938,13 @@ private fun Modifier.sheetOffset(state: BottomSheetState, offsetForIme: Boolean)
917938
}
918939

919940
else -> {
920-
val calculatedOffset = state.anchoredDraggableState.visualOffset() - imeHeight
941+
val visualOffset = state.anchoredDraggableState.visualOffset()
942+
val calculatedOffset = maxOrFallback(
943+
visualOffset,
944+
state.bottomAlignedOffsetPx(),
945+
Float.NaN,
946+
fallback = visualOffset,
947+
) - imeHeight
921948
// do not let the sheet's top go out of screen bounds
922949
val y = calculatedOffset.coerceAtLeast(0f).toInt()
923950
IntOffset(x = 0, y = y)

composeunstyled-bottom-sheet/src/commonTest/kotlin/BottomSheet.test.kt

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,115 @@ class BottomSheetCommonTest {
640640
mainClock.autoAdvance = true
641641
}
642642

643+
@Test
644+
fun fully_expanded_sheet_with_taller_percentage_detent_matches_changing_content_height() = runComposeUiTest {
645+
val peekDetent = SheetDetent("peek") { containerHeight, _ ->
646+
containerHeight * 0.6f
647+
}
648+
var contentHeight by mutableStateOf(80.dp)
649+
650+
setContent {
651+
Box(
652+
Modifier
653+
.testTag("root")
654+
.requiredSize(400.dp),
655+
) {
656+
val state = rememberBottomSheetState(
657+
initialDetent = SheetDetent.FullyExpanded,
658+
detents = listOf(SheetDetent.Hidden, peekDetent, SheetDetent.FullyExpanded),
659+
)
660+
UnstyledBottomSheet(
661+
state = state,
662+
modifier = Modifier.fillMaxSize(),
663+
) {
664+
Sheet(Modifier.testTag("sheet")) {
665+
Box(
666+
Modifier
667+
.fillMaxWidth()
668+
.height(contentHeight),
669+
)
670+
}
671+
}
672+
}
673+
}
674+
675+
waitForIdle()
676+
677+
val rootBounds = onNodeWithTag("root").fetchSemanticsNode().boundsInRoot
678+
679+
onNodeWithTag("sheet").assertHeightIsEqualTo(80.dp)
680+
assertThat(onNodeWithTag("sheet").fetchSemanticsNode().boundsInRoot.bottom).isCloseTo(
681+
rootBounds.bottom,
682+
with(density) { DensityTolerance.toPx() },
683+
)
684+
685+
contentHeight = 200.dp
686+
waitForIdle()
687+
688+
val sheetBounds = onNodeWithTag("sheet").fetchSemanticsNode().boundsInRoot
689+
690+
onNodeWithTag("sheet").assertHeightIsEqualTo(200.dp)
691+
assertThat(sheetBounds.bottom).isCloseTo(
692+
rootBounds.bottom,
693+
with(density) { DensityTolerance.toPx() },
694+
)
695+
}
696+
697+
@Test
698+
fun sheet_stays_bottom_anchored_when_content_shrinks_during_detent_animation() = runComposeUiTest {
699+
mainClock.autoAdvance = true
700+
701+
val peekDetent = SheetDetent("peek") { containerHeight, _ ->
702+
containerHeight * 0.6f
703+
}
704+
var contentHeight by mutableStateOf(320.dp)
705+
lateinit var state: BottomSheetState
706+
707+
setContent {
708+
Box(
709+
Modifier
710+
.testTag("root")
711+
.requiredSize(400.dp),
712+
) {
713+
state = rememberBottomSheetState(
714+
initialDetent = SheetDetent.FullyExpanded,
715+
detents = listOf(SheetDetent.Hidden, peekDetent, SheetDetent.FullyExpanded),
716+
animationSpec = tween(durationMillis = 300),
717+
)
718+
UnstyledBottomSheet(
719+
state = state,
720+
modifier = Modifier.fillMaxSize(),
721+
) {
722+
Sheet(Modifier.testTag("sheet")) {
723+
Box(
724+
Modifier
725+
.fillMaxWidth()
726+
.height(contentHeight),
727+
)
728+
}
729+
}
730+
}
731+
}
732+
733+
waitForIdle()
734+
mainClock.autoAdvance = false
735+
736+
state.targetDetent = peekDetent
737+
mainClock.advanceTimeBy(150)
738+
contentHeight = 80.dp
739+
mainClock.advanceTimeByFrame()
740+
741+
val sheetBounds = onNodeWithTag("sheet").fetchSemanticsNode().boundsInRoot
742+
743+
assertThat(state.isIdle).isFalse()
744+
assertThat(sheetBounds.height).isGreaterThan(
745+
state.offset - with(density) { DensityTolerance.toPx() },
746+
)
747+
assertThat(sheetBounds.height).isGreaterThan(with(density) { 80.dp.toPx() })
748+
749+
mainClock.autoAdvance = true
750+
}
751+
643752
@Test
644753
fun offset_is_zero_when_sheet_is_created_at_hidden_detent() = runComposeUiTest {
645754
lateinit var state: BottomSheetState
@@ -1110,7 +1219,7 @@ class BottomSheetCommonTest {
11101219

11111220
assertThat(counters.measureCalls).isEqualTo(2)
11121221
assertThat(counters.maxIntrinsicHeightCalls).isEqualTo(0)
1113-
assertThat(counters.detentHeightCalls).isEqualTo(11)
1222+
assertThat(counters.detentHeightCalls).isEqualTo(9)
11141223

11151224
counters.reset()
11161225
contentHeight = 250.dp

0 commit comments

Comments
 (0)