address review: signal transient dismissal with an event flow, not an int

An Int counter was already rejected for the now-playing launch signal; use
a broadcast Flow on the shared ContextMenuHost and collect it, so the
dismissal is an event rather than an ever-increasing number.
This commit is contained in:
austinried
2026-10-04 00:56:53 +00:00
parent 1b83a5ff93
commit af6979459b
10 changed files with 43 additions and 30 deletions
@@ -402,13 +402,13 @@ private fun MainNavigation() {
onOpenLicenses = { navController.navigate(Routes.LICENSES) }, onOpenLicenses = { navController.navigate(Routes.LICENSES) },
onEditServer = { id -> navController.navigate(Routes.editServer(id)) }, onEditServer = { id -> navController.navigate(Routes.editServer(id)) },
onBack = { navController.popBackStack() }, onBack = { navController.popBackStack() },
dismissRequests = contextMenuHost.dismissRequests, dismissals = contextMenuHost.dismissals,
) )
} }
composable(Routes.DOWNLOADS) { composable(Routes.DOWNLOADS) {
DownloadsRoute( DownloadsRoute(
onBack = { navController.popBackStack() }, onBack = { navController.popBackStack() },
dismissRequests = contextMenuHost.dismissRequests, dismissals = contextMenuHost.dismissals,
) )
} }
composable(Routes.SEARCH) { composable(Routes.SEARCH) {
@@ -607,7 +607,7 @@ private fun MainNavigation() {
AddSourceRoute( AddSourceRoute(
onSaved = { navController.popBackStack() }, onSaved = { navController.popBackStack() },
onBack = { navController.popBackStack() }, onBack = { navController.popBackStack() },
dismissRequests = contextMenuHost.dismissRequests, dismissals = contextMenuHost.dismissals,
) )
} }
composable( composable(
@@ -618,7 +618,7 @@ private fun MainNavigation() {
onSaved = { navController.popBackStack() }, onSaved = { navController.popBackStack() },
onBack = { navController.popBackStack() }, onBack = { navController.popBackStack() },
sourceId = entry.arguments?.getLong("sourceId"), sourceId = entry.arguments?.getLong("sourceId"),
dismissRequests = contextMenuHost.dismissRequests, dismissals = contextMenuHost.dismissals,
) )
} }
} }
@@ -34,7 +34,6 @@ import androidx.compose.material3.rememberModalBottomSheetState
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableIntStateOf
import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.rememberCoroutineScope
@@ -57,6 +56,9 @@ import com.subtracks.data.model.QueueKind
import com.subtracks.data.model.SongDownload import com.subtracks.data.model.SongDownload
import com.subtracks.data.source.StarType import com.subtracks.data.source.StarType
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.asSharedFlow
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import com.subtracks.data.model.Album as AlbumModel import com.subtracks.data.model.Album as AlbumModel
import com.subtracks.data.model.Artist as ArtistModel import com.subtracks.data.model.Artist as ArtistModel
@@ -149,10 +151,11 @@ class ContextMenuHost {
var actions by mutableStateOf(ItemActions()) var actions by mutableStateOf(ItemActions())
private set private set
// Bumped when an external event (opening now playing) should close menu-like UI the host does private val transientDismissals = MutableSharedFlow<Unit>(extraBufferCapacity = 1)
// Fires when an external event (opening now playing) should close menu-like UI the host does
// not own, such as the library's list options sheet. // not own, such as the library's list options sheet.
var dismissRequests by mutableIntStateOf(0) val dismissals: Flow<Unit> = transientDismissals.asSharedFlow()
private set
fun show( fun show(
target: MenuTarget, target: MenuTarget,
@@ -169,17 +172,17 @@ class ContextMenuHost {
fun dismissTransients() { fun dismissTransients() {
dismiss() dismiss()
dismissRequests++ transientDismissals.tryEmit(Unit)
} }
} }
@Composable @Composable
internal fun DismissOnRequest( internal fun DismissOnRequest(
dismissRequests: Int, dismissals: Flow<Unit>,
onDismiss: () -> Unit, onDismiss: () -> Unit,
) { ) {
LaunchedEffect(dismissRequests) { LaunchedEffect(dismissals) {
if (dismissRequests > 0) onDismiss() dismissals.collect { onDismiss() }
} }
} }
@@ -51,6 +51,8 @@ import com.subtracks.data.model.DownloadedSong
import com.subtracks.ui.components.DeleteDownloadsDialog import com.subtracks.ui.components.DeleteDownloadsDialog
import com.subtracks.ui.components.DismissOnRequest import com.subtracks.ui.components.DismissOnRequest
import com.subtracks.ui.components.EmptyState import com.subtracks.ui.components.EmptyState
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.emptyFlow
import org.koin.compose.viewmodel.koinViewModel import org.koin.compose.viewmodel.koinViewModel
sealed interface DownloadDeleteTarget { sealed interface DownloadDeleteTarget {
@@ -71,12 +73,12 @@ sealed interface DownloadDeleteTarget {
@Composable @Composable
fun DownloadsRoute( fun DownloadsRoute(
onBack: () -> Unit, onBack: () -> Unit,
dismissRequests: Int = 0, dismissals: Flow<Unit> = emptyFlow(),
viewModel: DownloadsViewModel = koinViewModel(), viewModel: DownloadsViewModel = koinViewModel(),
) { ) {
val tree by viewModel.tree.collectAsStateWithLifecycle() val tree by viewModel.tree.collectAsStateWithLifecycle()
var pending by remember { mutableStateOf<DownloadDeleteTarget?>(null) } var pending by remember { mutableStateOf<DownloadDeleteTarget?>(null) }
DismissOnRequest(dismissRequests) { pending = null } DismissOnRequest(dismissals) { pending = null }
DownloadsScreen( DownloadsScreen(
tree = tree, tree = tree,
loadEncoding = viewModel::encoding, loadEncoding = viewModel::encoding,
@@ -46,6 +46,7 @@ import com.subtracks.ui.components.ItemActions
import com.subtracks.ui.components.MenuTarget import com.subtracks.ui.components.MenuTarget
import com.subtracks.ui.theme.ArtworkColors import com.subtracks.ui.theme.ArtworkColors
import com.subtracks.ui.theme.rememberArtworkColors import com.subtracks.ui.theme.rememberArtworkColors
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import org.koin.compose.koinInject import org.koin.compose.koinInject
import org.koin.compose.viewmodel.koinViewModel import org.koin.compose.viewmodel.koinViewModel
@@ -74,7 +75,7 @@ fun AlbumDetailRoute(
CloseWhenDownloadsGone(downloadStatus.downloaded, offline, onBack) CloseWhenDownloadsGone(downloadStatus.downloaded, offline, onBack)
val scope = rememberCoroutineScope() val scope = rememberCoroutineScope()
var pendingDelete by remember { mutableStateOf<Long?>(null) } var pendingDelete by remember { mutableStateOf<Long?>(null) }
DismissOnRequest(contextMenuHost?.dismissRequests ?: 0) { pendingDelete = null } DismissOnRequest(contextMenuHost?.dismissals ?: emptyFlow()) { pendingDelete = null }
val playback by playbackController.state.collectAsStateWithLifecycle() val playback by playbackController.state.collectAsStateWithLifecycle()
val context = playback.context val context = playback.context
val shortcutArt = remember(coverArtId) { coverArtId?.let { viewModel.coverArt(it, true) } } val shortcutArt = remember(coverArtId) { coverArtId?.let { viewModel.coverArt(it, true) } }
@@ -105,6 +105,7 @@ import com.subtracks.ui.theme.baseArtworkColors
import com.subtracks.ui.theme.heroBarColor import com.subtracks.ui.theme.heroBarColor
import com.subtracks.ui.theme.rememberArtworkColors import com.subtracks.ui.theme.rememberArtworkColors
import com.subtracks.ui.theme.rememberOverlaidNameBusy import com.subtracks.ui.theme.rememberOverlaidNameBusy
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import org.koin.compose.koinInject import org.koin.compose.koinInject
import org.koin.compose.viewmodel.koinViewModel import org.koin.compose.viewmodel.koinViewModel
@@ -160,7 +161,7 @@ fun ArtistDetailRoute(
val albumDownloads by viewModel.albumDownloads.collectAsStateWithLifecycle() val albumDownloads by viewModel.albumDownloads.collectAsStateWithLifecycle()
val scope = rememberCoroutineScope() val scope = rememberCoroutineScope()
var pendingDelete by remember { mutableStateOf<PendingDownloadDelete?>(null) } var pendingDelete by remember { mutableStateOf<PendingDownloadDelete?>(null) }
DismissOnRequest(contextMenuHost?.dismissRequests ?: 0) { pendingDelete = null } DismissOnRequest(contextMenuHost?.dismissals ?: emptyFlow()) { pendingDelete = null }
val shortcutArt = remember(coverArtId) { coverArtId?.let { viewModel.coverArt(it, true) } } val shortcutArt = remember(coverArtId) { coverArtId?.let { viewModel.coverArt(it, true) } }
val actions = val actions =
ItemActions( ItemActions(
@@ -134,6 +134,7 @@ import com.subtracks.ui.theme.librarySurfaceColor
import com.subtracks.ui.theme.playerSurfaceColor import com.subtracks.ui.theme.playerSurfaceColor
import com.subtracks.ui.theme.rememberArtworkColors import com.subtracks.ui.theme.rememberArtworkColors
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import org.koin.compose.koinInject import org.koin.compose.koinInject
import org.koin.compose.viewmodel.koinViewModel import org.koin.compose.viewmodel.koinViewModel
@@ -270,7 +271,7 @@ fun LibraryRoute(
val downloads = downloadsState?.value ?: emptyMap() val downloads = downloadsState?.value ?: emptyMap()
val scope = rememberCoroutineScope() val scope = rememberCoroutineScope()
var pendingDelete by remember { mutableStateOf<PendingDownloadDelete?>(null) } var pendingDelete by remember { mutableStateOf<PendingDownloadDelete?>(null) }
DismissOnRequest(contextMenuHost?.dismissRequests ?: 0) { pendingDelete = null } DismissOnRequest(contextMenuHost?.dismissals ?: emptyFlow()) { pendingDelete = null }
val itemActions = val itemActions =
ItemActions( ItemActions(
playNext = { playbackController.playNext(it.sourceId, it.kind, it.refId) }, playNext = { playbackController.playNext(it.sourceId, it.kind, it.refId) },
@@ -341,7 +342,7 @@ fun LibraryRoute(
albumDownloads = downloads.takeIf { listTab == LibraryListTab.Albums }.orEmpty(), albumDownloads = downloads.takeIf { listTab == LibraryListTab.Albums }.orEmpty(),
artistDownloads = downloads.takeIf { listTab == LibraryListTab.Artists }.orEmpty(), artistDownloads = downloads.takeIf { listTab == LibraryListTab.Artists }.orEmpty(),
playlistDownloads = downloads.takeIf { listTab == LibraryListTab.Playlists }.orEmpty(), playlistDownloads = downloads.takeIf { listTab == LibraryListTab.Playlists }.orEmpty(),
dismissRequests = contextMenuHost?.dismissRequests ?: 0, dismissals = contextMenuHost?.dismissals ?: emptyFlow(),
homeContent = { topInset, bottomInset -> homeContent = { topInset, bottomInset ->
HomeRoute( HomeRoute(
onAlbumClick = onAlbumClick, onAlbumClick = onAlbumClick,
@@ -406,7 +407,7 @@ fun LibraryScreen(
albumDownloads: Map<String, ListDownloadStatus> = emptyMap(), albumDownloads: Map<String, ListDownloadStatus> = emptyMap(),
artistDownloads: Map<String, ListDownloadStatus> = emptyMap(), artistDownloads: Map<String, ListDownloadStatus> = emptyMap(),
playlistDownloads: Map<String, ListDownloadStatus> = emptyMap(), playlistDownloads: Map<String, ListDownloadStatus> = emptyMap(),
dismissRequests: Int = 0, dismissals: Flow<Unit> = emptyFlow(),
homeContent: @Composable (topInset: Dp, bottomInset: Dp) -> Unit = { _, _ -> }, homeContent: @Composable (topInset: Dp, bottomInset: Dp) -> Unit = { _, _ -> },
modifier: Modifier = Modifier, modifier: Modifier = Modifier,
) { ) {
@@ -419,7 +420,7 @@ fun LibraryScreen(
var searchActive by rememberSaveable { mutableStateOf(false) } var searchActive by rememberSaveable { mutableStateOf(false) }
val searchFocus = remember { FocusRequester() } val searchFocus = remember { FocusRequester() }
DismissOnRequest(dismissRequests) { showOptions = false } DismissOnRequest(dismissals) { showOptions = false }
LaunchedEffect(pagerState) { LaunchedEffect(pagerState) {
snapshotFlow { pagerState.settledPage }.collect { page -> snapshotFlow { pagerState.settledPage }.collect { page ->
@@ -35,6 +35,7 @@ import com.subtracks.ui.components.ItemActions
import com.subtracks.ui.components.MenuTarget import com.subtracks.ui.components.MenuTarget
import com.subtracks.ui.theme.ArtworkColors import com.subtracks.ui.theme.ArtworkColors
import com.subtracks.ui.theme.rememberArtworkColors import com.subtracks.ui.theme.rememberArtworkColors
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import org.koin.compose.koinInject import org.koin.compose.koinInject
import org.koin.compose.viewmodel.koinViewModel import org.koin.compose.viewmodel.koinViewModel
@@ -61,7 +62,7 @@ fun PlaylistDetailRoute(
CloseWhenDownloadsGone(downloadStatus.downloaded, offline, onBack) CloseWhenDownloadsGone(downloadStatus.downloaded, offline, onBack)
val scope = rememberCoroutineScope() val scope = rememberCoroutineScope()
var pendingDelete by remember { mutableStateOf<Long?>(null) } var pendingDelete by remember { mutableStateOf<Long?>(null) }
DismissOnRequest(contextMenuHost?.dismissRequests ?: 0) { pendingDelete = null } DismissOnRequest(contextMenuHost?.dismissals ?: emptyFlow()) { pendingDelete = null }
val playback by playbackController.state.collectAsStateWithLifecycle() val playback by playbackController.state.collectAsStateWithLifecycle()
val context = playback.context val context = playback.context
val requestDownloadAction: (BulkDownloadAction) -> Unit = { action -> val requestDownloadAction: (BulkDownloadAction) -> Unit = { action ->
@@ -48,6 +48,8 @@ import androidx.compose.ui.unit.dp
import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.compose.collectAsStateWithLifecycle
import com.subtracks.R import com.subtracks.R
import com.subtracks.ui.components.DismissOnRequest import com.subtracks.ui.components.DismissOnRequest
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.emptyFlow
import org.koin.compose.viewmodel.koinViewModel import org.koin.compose.viewmodel.koinViewModel
import org.koin.core.parameter.parametersOf import org.koin.core.parameter.parametersOf
@@ -56,7 +58,7 @@ fun AddSourceRoute(
onSaved: () -> Unit, onSaved: () -> Unit,
onBack: (() -> Unit)?, onBack: (() -> Unit)?,
sourceId: Long? = null, sourceId: Long? = null,
dismissRequests: Int = 0, dismissals: Flow<Unit> = emptyFlow(),
viewModel: AddSourceViewModel = koinViewModel(key = sourceId?.toString() ?: "new") { parametersOf(sourceId) }, viewModel: AddSourceViewModel = koinViewModel(key = sourceId?.toString() ?: "new") { parametersOf(sourceId) },
) { ) {
val state by viewModel.state.collectAsStateWithLifecycle() val state by viewModel.state.collectAsStateWithLifecycle()
@@ -71,7 +73,7 @@ fun AddSourceRoute(
onSave = { viewModel.save(onSaved) }, onSave = { viewModel.save(onSaved) },
onDelete = { viewModel.delete(onSaved) }, onDelete = { viewModel.delete(onSaved) },
onBack = onBack, onBack = onBack,
dismissRequests = dismissRequests, dismissals = dismissals,
) )
} }
@@ -88,13 +90,13 @@ fun AddSourceScreen(
onSave: () -> Unit, onSave: () -> Unit,
onDelete: () -> Unit, onDelete: () -> Unit,
onBack: (() -> Unit)?, onBack: (() -> Unit)?,
dismissRequests: Int = 0, dismissals: Flow<Unit> = emptyFlow(),
modifier: Modifier = Modifier, modifier: Modifier = Modifier,
) { ) {
val nameFocus = remember { FocusRequester() } val nameFocus = remember { FocusRequester() }
val addressFocus = remember { FocusRequester() } val addressFocus = remember { FocusRequester() }
var confirmingDelete by remember { mutableStateOf(false) } var confirmingDelete by remember { mutableStateOf(false) }
DismissOnRequest(dismissRequests) { confirmingDelete = false } DismissOnRequest(dismissals) { confirmingDelete = false }
LaunchedEffect(state.nameError, state.addressError) { LaunchedEffect(state.nameError, state.addressError) {
when { when {
state.nameError -> nameFocus.requestFocus() state.nameError -> nameFocus.requestFocus()
@@ -58,6 +58,8 @@ import com.subtracks.data.model.Source
import com.subtracks.data.prefs.StreamQuality import com.subtracks.data.prefs.StreamQuality
import com.subtracks.ui.components.DismissOnRequest import com.subtracks.ui.components.DismissOnRequest
import com.subtracks.ui.components.rememberViewportFill import com.subtracks.ui.components.rememberViewportFill
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.emptyFlow
import org.koin.compose.viewmodel.koinViewModel import org.koin.compose.viewmodel.koinViewModel
private val bitrateOptions = listOf(0, 24, 32, 64, 96, 128, 192, 256, 320) private val bitrateOptions = listOf(0, 24, 32, 64, 96, 128, 192, 256, 320)
@@ -72,7 +74,7 @@ fun SettingsRoute(
onOpenLicenses: () -> Unit, onOpenLicenses: () -> Unit,
onEditServer: (Long) -> Unit, onEditServer: (Long) -> Unit,
onBack: () -> Unit, onBack: () -> Unit,
dismissRequests: Int = 0, dismissals: Flow<Unit> = emptyFlow(),
viewModel: SettingsViewModel = koinViewModel(), viewModel: SettingsViewModel = koinViewModel(),
) { ) {
val sources by viewModel.sources.collectAsStateWithLifecycle() val sources by viewModel.sources.collectAsStateWithLifecycle()
@@ -107,7 +109,7 @@ fun SettingsRoute(
onOpenDownloads = onOpenDownloads, onOpenDownloads = onOpenDownloads,
onOpenLicenses = onOpenLicenses, onOpenLicenses = onOpenLicenses,
onBack = onBack, onBack = onBack,
dismissRequests = dismissRequests, dismissals = dismissals,
) )
} }
@@ -136,11 +138,11 @@ fun SettingsScreen(
onOpenDownloads: () -> Unit, onOpenDownloads: () -> Unit,
onOpenLicenses: () -> Unit, onOpenLicenses: () -> Unit,
onBack: () -> Unit, onBack: () -> Unit,
dismissRequests: Int = 0, dismissals: Flow<Unit> = emptyFlow(),
modifier: Modifier = Modifier, modifier: Modifier = Modifier,
) { ) {
var dialog by remember { mutableStateOf<SettingsDialog?>(null) } var dialog by remember { mutableStateOf<SettingsDialog?>(null) }
DismissOnRequest(dismissRequests) { dialog = null } DismissOnRequest(dismissals) { dialog = null }
val listState = rememberLazyListState() val listState = rememberLazyListState()
val fill = rememberViewportFill(listState) val fill = rememberViewportFill(listState)
@@ -119,7 +119,7 @@ class LibraryOptionsTest {
onOpenSettings = {}, onOpenSettings = {},
sortOptions = sortOptionsFor(LibraryTab.Albums), sortOptions = sortOptionsFor(LibraryTab.Albums),
starredSupported = true, starredSupported = true,
dismissRequests = host.dismissRequests, dismissals = host.dismissals,
) )
} }
} }