fix back popping the route under now playing; disc header spacing; dial back the fab glyph

- register the now playing and queue back callback from a LaunchedEffect instead of BackHandler: the NavHost registers its own callback from a LaunchedEffect, so a BackHandler composed before it loses the back and pops the route underneath
- NowPlayingBackTest registers the overlay handler before the NavHost (the losing order) and asserts back closes the overlay without popping the route
- more top padding on disc headers, except the first which sits flush
- LIST_OPTIONS_WEIGHT 0.15 -> 0.11
This commit is contained in:
austinried
2026-10-04 00:29:58 +00:00
parent d43dd51b15
commit fd37c3ad86
4 changed files with 85 additions and 16 deletions
@@ -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)
}
}
@@ -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,
@@ -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
@@ -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()
}
}
}