diff --git a/composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/shell/AppShell.kt b/composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/shell/AppShell.kt index 484fe48b..b8f3d388 100644 --- a/composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/shell/AppShell.kt +++ b/composeApp/src/commonMain/kotlin/org/spotube/spotube/modules/shell/AppShell.kt @@ -32,17 +32,23 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.navigationBars import androidx.compose.foundation.layout.padding import androidx.compose.runtime.Composable +import androidx.compose.animation.core.spring +import androidx.compose.animation.core.Spring import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.key +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue import androidx.compose.runtime.staticCompositionLocalOf import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.graphicsLayer +import androidx.compose.ui.layout.layout import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.layout.onSizeChanged import androidx.compose.ui.unit.dp @@ -299,6 +305,15 @@ internal fun CompactPlayerOverlay( onAlternativeSource: () -> Unit, onExpandLyrics: () -> Unit, floatingPlayer: @Composable (Modifier) -> Unit = { AppFloatingPlayer(modifier = it) }, + expandedPlayer: @Composable (modifier: Modifier, onCollapse: () -> Unit) -> Unit = { modifier, onCollapse -> + AppExpandedPlayer( + modifier = modifier, + onCollapse = onCollapse, + onQueue = onQueue, + onAlternativeSource = onAlternativeSource, + onExpandLyrics = onExpandLyrics + ) + }, bottomBar: @Composable (Modifier) -> Unit = { AppBottombar(navigator = navigator, navigationState = navigationState, modifier = it) }, @@ -325,16 +340,39 @@ internal fun CompactPlayerOverlay( val sheetState = key(peekHeight) { rememberBottomSheetState( initialDetent = partialDetent, - detents = listOf(partialDetent, SheetDetent.FullyExpanded) + detents = listOf(partialDetent, SheetDetent.FullyExpanded), + // A spring carries the fling velocity into the settle, so releasing mid-drag + // glides to the nearest detent instead of stalling and then easing like a tween. + animationSpec = spring( + dampingRatio = Spring.DampingRatioNoBouncy, + stiffness = Spring.StiffnessMediumLow) ) } - val progress = if (!sheetState.offset.isFinite() || - (sheetState.isIdle && sheetState.currentDetent == partialDetent) - ) 0f else runCatching { - sheetState.progress(partialDetent, SheetDetent.FullyExpanded) - }.getOrDefault(0f).takeIf { it.isFinite() } ?: 0f - val uiState = viewModel.compactSheetUiState(progress.coerceIn(0f, 1f)) + // Read only inside draw/derived scopes: the sheet offset changes every drag frame, and + // reading it in composition would recompose the whole player on each one. + val progress by remember(sheetState, partialDetent) { + derivedStateOf { + val value = if (!sheetState.offset.isFinite() || + (sheetState.isIdle && sheetState.currentDetent == partialDetent) + ) 0f else runCatching { + sheetState.progress(partialDetent, SheetDetent.FullyExpanded) + }.getOrDefault(0f).takeIf { it.isFinite() } ?: 0f + value.coerceIn(0f, 1f) + } + } + // Keyed on the sheet state: it is recreated when the peek height changes, and a derived + // value left reading the previous sheet's progress would never update again. + val expandedPlayerVisible by remember(sheetState) { + derivedStateOf { viewModel.compactSheetUiState(progress).showExpandedPlayer } + } + val floatingPlayerVisible by remember(sheetState) { + derivedStateOf { viewModel.compactSheetUiState(progress).floatingPlayerAlpha > 0.01f } + } + // Once shown, keep the expanded player composed: composing it (view models, lyrics, cover + // art) on the first drag frame of every gesture is the hitch the user feels as lag. + var expandedPlayerComposed by remember { mutableStateOf(false) } + if (expandedPlayerVisible) expandedPlayerComposed = true Box(modifier = Modifier.fillMaxSize()) { content() @@ -345,44 +383,52 @@ internal fun CompactPlayerOverlay( ) { Sheet(modifier = Modifier.fillMaxSize()) { Box(modifier = Modifier.fillMaxSize()) { - if (uiState.showExpandedPlayer) { - AppExpandedPlayer( - modifier = Modifier + if (expandedPlayerComposed) { + expandedPlayer( + Modifier .fillMaxSize() - .graphicsLayer { alpha = uiState.expandedPlayerAlpha }, - onCollapse = { - scope.launch { sheetState.animateTo(partialDetent) } - }, - onQueue = onQueue, - onAlternativeSource = onAlternativeSource, - onExpandLyrics = onExpandLyrics + .graphicsLayer { + alpha = viewModel.compactSheetUiState(progress).expandedPlayerAlpha + }, + { scope.launch { sheetState.animateTo(partialDetent) } } ) } - if (uiState.floatingPlayerAlpha > 0.01f) { - Column(modifier = Modifier.fillMaxWidth()) { - floatingPlayer( - Modifier - .graphicsLayer { - alpha = uiState.floatingPlayerAlpha - }) - } + // Always composed: building it (view model, cover art, player state) only once + // progress drops below the fade-out point stalls the collapse mid-drag. + Column(modifier = Modifier.fillMaxWidth()) { + floatingPlayer( + Modifier + .zeroSizeUnless(floatingPlayerVisible) + .graphicsLayer { + alpha = viewModel.floatingPlayerAlpha(progress) + }) } } } } - if (uiState.floatingPlayerAlpha > 0.01f) { - bottomBar( - Modifier - .fillMaxWidth() - .align(Alignment.BottomCenter) - .onSizeChanged { size -> - viewModel.onBottomBarMeasured(with(density) { size.height.toDp() }) - } - .graphicsLayer { - alpha = uiState.floatingPlayerAlpha - }) - } + bottomBar( + Modifier + .fillMaxWidth() + .align(Alignment.BottomCenter) + .zeroSizeUnless(floatingPlayerVisible) + .onSizeChanged { size -> + viewModel.onBottomBarMeasured(with(density) { size.height.toDp() }) + } + .graphicsLayer { + alpha = viewModel.floatingPlayerAlpha(progress) + }) } } + +/** + * Keeps the content composed but takes it out of layout, drawing and hit testing while it is + * fully faded out, so it neither steals touches from what is above it nor costs a re-composition + * when it comes back. + */ +private fun Modifier.zeroSizeUnless(visible: Boolean): Modifier = + if (visible) this else layout { measurable, constraints -> + measurable.measure(constraints) + layout(0, 0) {} + } diff --git a/composeApp/src/jvmTest/kotlin/org/spotube/spotube/modules/shell/CompactPlayerOverlayDragTest.kt b/composeApp/src/jvmTest/kotlin/org/spotube/spotube/modules/shell/CompactPlayerOverlayDragTest.kt new file mode 100644 index 00000000..d11f6e89 --- /dev/null +++ b/composeApp/src/jvmTest/kotlin/org/spotube/spotube/modules/shell/CompactPlayerOverlayDragTest.kt @@ -0,0 +1,113 @@ +package org.spotube.spotube.modules.shell + +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.remember +import androidx.compose.ui.ExperimentalComposeUiApi +import androidx.compose.ui.ImageComposeScene +import androidx.compose.ui.Modifier +import androidx.compose.ui.geometry.Offset +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.layout.onGloballyPositioned +import androidx.compose.ui.input.pointer.PointerEventType +import androidx.compose.ui.input.pointer.PointerType +import androidx.compose.ui.unit.dp +import androidx.compose.ui.use +import org.spotube.spotube.core.navigation.Navigator +import org.spotube.spotube.core.navigation.Routes +import org.spotube.spotube.core.navigation.TOP_LEVEL_ROUTES +import org.spotube.spotube.core.navigation.rememberNavigationState +import org.spotube.spotube.core.ui.theming.SpotubeTheme +import org.spotube.spotube.modules.settings.Theme +import org.spotube.spotube.modules.settings.UserSettings +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +@OptIn(ExperimentalComposeUiApi::class) +class CompactPlayerOverlayDragTest { + @Test + fun draggingTheSheetUpShowsTheExpandedPlayer() { + var expandedComposed = false + var floatingCompositions = 0 + var floatingPlaced = false + val width = 400 + val height = 800 + ImageComposeScene(width, height) { + SpotubeTheme(UserSettings(theme = Theme.DARK)) { + val navigation = rememberNavigationState(Routes.Home, TOP_LEVEL_ROUTES) + val navigator = remember(navigation) { Navigator(navigation) } + CompactPlayerOverlay( + navigator = navigator, + navigationState = navigation, + viewModel = remember { AppShellViewModel() }, + onQueue = {}, onAlternativeSource = {}, onExpandLyrics = {}, + floatingPlayer = { modifier -> + DisposableEffect(Unit) { + floatingCompositions++ + onDispose {} + } + Box( + modifier.fillMaxWidth().height(64.dp).background(Color.Gray) + .onGloballyPositioned { floatingPlaced = true }) + }, + expandedPlayer = { modifier, _ -> + DisposableEffect(Unit) { + expandedComposed = true + onDispose { expandedComposed = false } + } + Box(modifier.fillMaxSize().background(Color.Red)) + }, + bottomBar = { modifier -> + Box(modifier.fillMaxWidth().height(56.dp).background(Color.Blue)) + } + ) { Box(Modifier.fillMaxSize()) } + } + }.use { scene -> + var t = 0L + fun frame() { scene.render(t); t += 16_000_000L } + repeat(10) { frame() } + assertTrue(floatingPlaced, "floating player should show while collapsed") + assertFalse(expandedComposed, "expanded player should not be composed while collapsed") + + // grab the floating player (just above the bottom bar) and drag to the top + val startY = height - 120f + 20f + val start = Offset(width / 2f, startY) + scene.sendPointerEvent(PointerEventType.Press, start, type = PointerType.Touch, timeMillis = t / 1_000_000L) + repeat(5) { frame() } + var y = startY + repeat(30) { + y -= (startY - 40f) / 30f + scene.sendPointerEvent(PointerEventType.Move, Offset(width / 2f, y), type = PointerType.Touch, timeMillis = t / 1_000_000L) + frame() + } + scene.sendPointerEvent(PointerEventType.Release, Offset(width / 2f, y), type = PointerType.Touch, timeMillis = t / 1_000_000L) + repeat(120) { frame() } + + assertTrue(expandedComposed, "expanded player should be composed after dragging up") + // out of layout while the expanded player covers it, but never re-composed + floatingPlaced = false + repeat(5) { frame() } + assertFalse(floatingPlaced, "floating player should be out of layout when fully expanded") + + // drag back down: the floating player must return without being composed again + val downStart = Offset(width / 2f, 60f) + scene.sendPointerEvent(PointerEventType.Press, downStart, type = PointerType.Touch, timeMillis = t / 1_000_000L) + repeat(5) { frame() } + var dy = downStart.y + repeat(30) { + dy += (height - 160f) / 30f + scene.sendPointerEvent(PointerEventType.Move, Offset(width / 2f, dy), type = PointerType.Touch, timeMillis = t / 1_000_000L) + frame() + } + scene.sendPointerEvent(PointerEventType.Release, Offset(width / 2f, dy), type = PointerType.Touch, timeMillis = t / 1_000_000L) + repeat(120) { frame() } + assertTrue(floatingPlaced, "floating player should be back after collapsing") + assertTrue(floatingCompositions == 1, "floating player was composed $floatingCompositions times, expected 1") + } + } +}