diff --git a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt index 0522c751..8fbf4fb2 100644 --- a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt +++ b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt @@ -1,7 +1,8 @@ package com.subtracks.ui import android.net.Uri -import androidx.activity.compose.BackHandler +import androidx.activity.OnBackPressedCallback +import androidx.activity.compose.LocalOnBackPressedDispatcherOwner import androidx.compose.animation.AnimatedVisibility import androidx.compose.animation.core.animate import androidx.compose.animation.core.animateDpAsState @@ -32,6 +33,7 @@ import androidx.compose.foundation.layout.navigationBars import androidx.compose.foundation.layout.statusBars import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.SideEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableFloatStateOf import androidx.compose.runtime.mutableStateOf @@ -72,6 +74,7 @@ import com.subtracks.ui.settings.SettingsRoute import com.subtracks.ui.theme.ArtworkTheme import com.subtracks.ui.theme.rememberArtworkColors import kotlinx.coroutines.Job +import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn @@ -157,6 +160,30 @@ private fun MainNavigation() { if (!open) nowPlayingOpen = false } } + // The NavHost registers its back callback from a LaunchedEffect, so a BackHandler composed + // before it can lose the back to the NavHost (which would pop the route under the overlay). + // Register ours from a LaunchedEffect after the NavHost so it lands later and wins. + val backDispatcher = LocalOnBackPressedDispatcherOwner.current?.onBackPressedDispatcher + val overlayBack = + remember { + object : OnBackPressedCallback(false) { + override fun handleOnBackPressed() { + when { + showingQueue -> showingQueue = false + nowPlayingOpen -> settleNowPlaying(false) + } + } + } + } + SideEffect { overlayBack.isEnabled = showingQueue || nowPlayingOpen } + LaunchedEffect(backDispatcher, overlayBack) { + backDispatcher?.addCallback(overlayBack) + try { + awaitCancellation() + } finally { + overlayBack.remove() + } + } LaunchedEffect(Unit) { playbackController.connect() } LaunchedEffect(Unit) { if (nowPlayingOpen) nowPlayingProgress = 1f } @@ -370,9 +397,4 @@ private fun MainNavigation() { } } } - - BackHandler(enabled = showingQueue) { showingQueue = false } - BackHandler(enabled = nowPlayingOpen && !showingQueue) { - settleNowPlaying(false) - } } diff --git a/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt index 185b59af..2333224a 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt @@ -128,7 +128,7 @@ fun AlbumDetailScreen( val disc = song.disc ?: 1L val discLabel = discLabels[disc] if (disc != lastDisc && (multiDisc || discLabel != null)) { - item(key = "disc:$disc") { DiscHeader(discLabel ?: "Disc $disc") } + item(key = "disc:$disc") { DiscHeader(discLabel ?: "Disc $disc", first = lastDisc == null) } } lastDisc = disc item(key = song.id) { @@ -146,10 +146,13 @@ fun AlbumDetailScreen( } @Composable -private fun DiscHeader(text: String) { +private fun DiscHeader( + text: String, + first: Boolean, +) { Row( verticalAlignment = Alignment.CenterVertically, - modifier = Modifier.padding(start = 16.dp, end = 16.dp, top = 28.dp, bottom = 4.dp), + modifier = Modifier.padding(start = 16.dp, end = 16.dp, top = if (first) 0.dp else 40.dp, bottom = 4.dp), ) { Icon( imageVector = Icons.Outlined.FiberSmartRecord, diff --git a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt index 7ed86992..7f00f9e5 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt @@ -653,7 +653,7 @@ private val TAB_BAR_CONTENT_HEIGHT = 52.dp private val TAB_ICON_SIZE = 24.dp private val TAB_VERTICAL_PADDING = 6.dp private val TOUCH_TARGET = 48.dp -private const val LIST_OPTIONS_WEIGHT = 0.15f +private const val LIST_OPTIONS_WEIGHT = 0.11f private val LIST_OPTIONS_BAR_WIDTHS = listOf(1f, 0.62f, 0.34f) @Composable diff --git a/app/src/test/kotlin/com/subtracks/ui/NowPlayingBackTest.kt b/app/src/test/kotlin/com/subtracks/ui/NowPlayingBackTest.kt index cb70ee08..21a1d7dd 100644 --- a/app/src/test/kotlin/com/subtracks/ui/NowPlayingBackTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/NowPlayingBackTest.kt @@ -1,12 +1,21 @@ package com.subtracks.ui import androidx.activity.ComponentActivity -import androidx.activity.compose.BackHandler +import androidx.activity.OnBackPressedCallback +import androidx.activity.compose.LocalOnBackPressedDispatcherOwner +import androidx.compose.animation.core.tween +import androidx.compose.animation.fadeIn +import androidx.compose.animation.fadeOut +import androidx.compose.animation.slideInHorizontally +import androidx.compose.animation.slideOutHorizontally import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.material3.Text +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.SideEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule @@ -17,6 +26,7 @@ import androidx.navigation.compose.composable import androidx.navigation.compose.rememberNavController import androidx.test.ext.junit.runners.AndroidJUnit4 import com.github.takahirom.roborazzi.RobolectricDeviceQualifiers +import kotlinx.coroutines.awaitCancellation import org.junit.Assert.assertFalse import org.junit.Rule import org.junit.Test @@ -38,8 +48,37 @@ class NowPlayingBackTest { composeRule.setContent { val nav = rememberNavController() navController = nav + val dispatcher = LocalOnBackPressedDispatcherOwner.current!!.onBackPressedDispatcher + val overlayBackCallback = + remember { + object : OnBackPressedCallback(false) { + override fun handleOnBackPressed() { + overlayOpen = false + } + } + } + SideEffect { overlayBackCallback.isEnabled = overlayOpen } Box(Modifier.fillMaxSize()) { - NavHost(navController = nav, startDestination = "a") { + NavHost( + navController = nav, + startDestination = "a", + enterTransition = { + slideInHorizontally(initialOffsetX = { it / 4 }, animationSpec = tween(260)) + + fadeIn(tween(260)) + }, + exitTransition = { + slideOutHorizontally(targetOffsetX = { -it / 4 }, animationSpec = tween(260)) + + fadeOut(tween(260)) + }, + popEnterTransition = { + slideInHorizontally(initialOffsetX = { -it / 4 }, animationSpec = tween(260)) + + fadeIn(tween(260)) + }, + popExitTransition = { + slideOutHorizontally(targetOffsetX = { it / 4 }, animationSpec = tween(260)) + + fadeOut(tween(260)) + }, + ) { composable("a") { Text("A") } composable("b") { Text("B") } } @@ -47,10 +86,15 @@ class NowPlayingBackTest { Text("OVERLAY", modifier = Modifier.fillMaxSize()) } } - // Same ordering as MainNavigation: the overlay's back handler is registered after the - // NavHost, so it must consume back and leave the destination in place. - BackHandler(enabled = overlayOpen) { - overlayOpen = false + // The NavHost registers its back callback from a LaunchedEffect, so the overlay must + // register its own from a LaunchedEffect after it to land later in the dispatcher. + LaunchedEffect(dispatcher, overlayBackCallback) { + dispatcher.addCallback(overlayBackCallback) + try { + awaitCancellation() + } finally { + overlayBackCallback.remove() + } } }