Skip to content

Commit

Permalink
Fix ANR when showing bottom sheets
Browse files Browse the repository at this point in the history
Our `internalShow` method could end up in an infinite loop sometimes (yay, recursion!). We now use the official `show` API which *should* work. Unfortunately, our `show` calls all get cancelled but Swipeable still settles in the closest state. This seems to always be the expanded/half-expanded state. We look at the state's targetValue in our try's `finally` block to figure out if the state will settle in an expanded state and call the `onSheetShown` callback based on that.

Fixes #660
  • Loading branch information
jossiwolf committed Aug 25, 2021
1 parent 62f693b commit 3c6b2d7
Showing 1 changed file with 13 additions and 16 deletions.
Expand Up @@ -45,6 +45,7 @@ import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.drop
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.launch
import kotlin.coroutines.EmptyCoroutineContext

/**
* Hosts a [BottomSheetNavigator.Destination]'s [NavBackStackEntry] and its
Expand Down Expand Up @@ -93,8 +94,18 @@ internal fun SheetContentHost(
// want to show the sheet, and hide it when this composable leaves the composition
DisposableEffect(backStackEntry) {
scope.launch {
sheetState.internalShow()
currentOnSheetShown(backStackEntry)
// Our show call can get cancelled in which case Swipeable will move to the closest
// anchor
try {
sheetState.show()
} finally {
// If the target state is a visible state, it's fairly safe to assume that
// Swipeable will end up settling in that state
if (sheetState.targetValue == ModalBottomSheetValue.Expanded ||
sheetState.targetValue == ModalBottomSheetValue.HalfExpanded) {
currentOnSheetShown(backStackEntry)
}
}
}
onDispose {
scope.launch {
Expand Down Expand Up @@ -127,20 +138,6 @@ private fun EmptySheet() {
Box(Modifier.height(1.dp))
}

// There is a race condition in ModalBottomSheetLayout that happens when the sheet content
// changes due to the anchors being re-calculated. This causes an animation to run for a
// short time. Re-running the animation when it is cancelled works around this.
// b/181593642
@OptIn(ExperimentalMaterialApi::class)
private suspend fun ModalBottomSheetState.internalShow() {
try {
show()
} catch (animationCancelled: CancellationException) {
currentCoroutineContext().ensureActive()
internalShow()
}
}

// We have the same issue when we are hiding the sheet, but snapTo works better
@OptIn(ExperimentalMaterialApi::class)
private suspend fun ModalBottomSheetState.internalHide() {
Expand Down

0 comments on commit 3c6b2d7

Please sign in to comment.