From af6979459bbef45501ddcee08d447eed92169665 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 4 Oct 2026 00:12:40 +0000 Subject: [PATCH] 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. --- .../main/kotlin/com/subtracks/ui/AppRoot.kt | 8 ++++---- .../ui/components/ItemContextMenu.kt | 19 +++++++++++-------- .../subtracks/ui/downloads/DownloadsScreen.kt | 6 ++++-- .../subtracks/ui/library/AlbumDetailScreen.kt | 3 ++- .../ui/library/ArtistDetailScreen.kt | 3 ++- .../com/subtracks/ui/library/LibraryScreen.kt | 9 +++++---- .../ui/library/PlaylistDetailScreen.kt | 3 ++- .../subtracks/ui/settings/AddSourceScreen.kt | 10 ++++++---- .../subtracks/ui/settings/SettingsScreen.kt | 10 ++++++---- .../com/subtracks/ui/LibraryOptionsTest.kt | 2 +- 10 files changed, 43 insertions(+), 30 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt index 5f21e0f5..cd32bb00 100644 --- a/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt +++ b/app/src/main/kotlin/com/subtracks/ui/AppRoot.kt @@ -402,13 +402,13 @@ private fun MainNavigation() { onOpenLicenses = { navController.navigate(Routes.LICENSES) }, onEditServer = { id -> navController.navigate(Routes.editServer(id)) }, onBack = { navController.popBackStack() }, - dismissRequests = contextMenuHost.dismissRequests, + dismissals = contextMenuHost.dismissals, ) } composable(Routes.DOWNLOADS) { DownloadsRoute( onBack = { navController.popBackStack() }, - dismissRequests = contextMenuHost.dismissRequests, + dismissals = contextMenuHost.dismissals, ) } composable(Routes.SEARCH) { @@ -607,7 +607,7 @@ private fun MainNavigation() { AddSourceRoute( onSaved = { navController.popBackStack() }, onBack = { navController.popBackStack() }, - dismissRequests = contextMenuHost.dismissRequests, + dismissals = contextMenuHost.dismissals, ) } composable( @@ -618,7 +618,7 @@ private fun MainNavigation() { onSaved = { navController.popBackStack() }, onBack = { navController.popBackStack() }, sourceId = entry.arguments?.getLong("sourceId"), - dismissRequests = contextMenuHost.dismissRequests, + dismissals = contextMenuHost.dismissals, ) } } diff --git a/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt b/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt index 89d4c9bc..ecde6f85 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt @@ -34,7 +34,6 @@ import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember 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.source.StarType import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.launch import com.subtracks.data.model.Album as AlbumModel import com.subtracks.data.model.Artist as ArtistModel @@ -149,10 +151,11 @@ class ContextMenuHost { var actions by mutableStateOf(ItemActions()) private set - // Bumped when an external event (opening now playing) should close menu-like UI the host does + private val transientDismissals = MutableSharedFlow(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. - var dismissRequests by mutableIntStateOf(0) - private set + val dismissals: Flow = transientDismissals.asSharedFlow() fun show( target: MenuTarget, @@ -169,17 +172,17 @@ class ContextMenuHost { fun dismissTransients() { dismiss() - dismissRequests++ + transientDismissals.tryEmit(Unit) } } @Composable internal fun DismissOnRequest( - dismissRequests: Int, + dismissals: Flow, onDismiss: () -> Unit, ) { - LaunchedEffect(dismissRequests) { - if (dismissRequests > 0) onDismiss() + LaunchedEffect(dismissals) { + dismissals.collect { onDismiss() } } } diff --git a/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsScreen.kt b/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsScreen.kt index 521978ec..34a75e7e 100644 --- a/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsScreen.kt @@ -51,6 +51,8 @@ import com.subtracks.data.model.DownloadedSong import com.subtracks.ui.components.DeleteDownloadsDialog import com.subtracks.ui.components.DismissOnRequest import com.subtracks.ui.components.EmptyState +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow import org.koin.compose.viewmodel.koinViewModel sealed interface DownloadDeleteTarget { @@ -71,12 +73,12 @@ sealed interface DownloadDeleteTarget { @Composable fun DownloadsRoute( onBack: () -> Unit, - dismissRequests: Int = 0, + dismissals: Flow = emptyFlow(), viewModel: DownloadsViewModel = koinViewModel(), ) { val tree by viewModel.tree.collectAsStateWithLifecycle() var pending by remember { mutableStateOf(null) } - DismissOnRequest(dismissRequests) { pending = null } + DismissOnRequest(dismissals) { pending = null } DownloadsScreen( tree = tree, loadEncoding = viewModel::encoding, 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 8ca94b52..2293941f 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt @@ -46,6 +46,7 @@ import com.subtracks.ui.components.ItemActions import com.subtracks.ui.components.MenuTarget import com.subtracks.ui.theme.ArtworkColors import com.subtracks.ui.theme.rememberArtworkColors +import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.launch import org.koin.compose.koinInject import org.koin.compose.viewmodel.koinViewModel @@ -74,7 +75,7 @@ fun AlbumDetailRoute( CloseWhenDownloadsGone(downloadStatus.downloaded, offline, onBack) val scope = rememberCoroutineScope() var pendingDelete by remember { mutableStateOf(null) } - DismissOnRequest(contextMenuHost?.dismissRequests ?: 0) { pendingDelete = null } + DismissOnRequest(contextMenuHost?.dismissals ?: emptyFlow()) { pendingDelete = null } val playback by playbackController.state.collectAsStateWithLifecycle() val context = playback.context val shortcutArt = remember(coverArtId) { coverArtId?.let { viewModel.coverArt(it, true) } } diff --git a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt index 0114d0dc..b4faac06 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -105,6 +105,7 @@ import com.subtracks.ui.theme.baseArtworkColors import com.subtracks.ui.theme.heroBarColor import com.subtracks.ui.theme.rememberArtworkColors import com.subtracks.ui.theme.rememberOverlaidNameBusy +import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.launch import org.koin.compose.koinInject import org.koin.compose.viewmodel.koinViewModel @@ -160,7 +161,7 @@ fun ArtistDetailRoute( val albumDownloads by viewModel.albumDownloads.collectAsStateWithLifecycle() val scope = rememberCoroutineScope() var pendingDelete by remember { mutableStateOf(null) } - DismissOnRequest(contextMenuHost?.dismissRequests ?: 0) { pendingDelete = null } + DismissOnRequest(contextMenuHost?.dismissals ?: emptyFlow()) { pendingDelete = null } val shortcutArt = remember(coverArtId) { coverArtId?.let { viewModel.coverArt(it, true) } } val actions = ItemActions( 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 07e5af19..2e53ffda 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/LibraryScreen.kt @@ -134,6 +134,7 @@ import com.subtracks.ui.theme.librarySurfaceColor import com.subtracks.ui.theme.playerSurfaceColor import com.subtracks.ui.theme.rememberArtworkColors import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.launch import org.koin.compose.koinInject import org.koin.compose.viewmodel.koinViewModel @@ -270,7 +271,7 @@ fun LibraryRoute( val downloads = downloadsState?.value ?: emptyMap() val scope = rememberCoroutineScope() var pendingDelete by remember { mutableStateOf(null) } - DismissOnRequest(contextMenuHost?.dismissRequests ?: 0) { pendingDelete = null } + DismissOnRequest(contextMenuHost?.dismissals ?: emptyFlow()) { pendingDelete = null } val itemActions = ItemActions( playNext = { playbackController.playNext(it.sourceId, it.kind, it.refId) }, @@ -341,7 +342,7 @@ fun LibraryRoute( albumDownloads = downloads.takeIf { listTab == LibraryListTab.Albums }.orEmpty(), artistDownloads = downloads.takeIf { listTab == LibraryListTab.Artists }.orEmpty(), playlistDownloads = downloads.takeIf { listTab == LibraryListTab.Playlists }.orEmpty(), - dismissRequests = contextMenuHost?.dismissRequests ?: 0, + dismissals = contextMenuHost?.dismissals ?: emptyFlow(), homeContent = { topInset, bottomInset -> HomeRoute( onAlbumClick = onAlbumClick, @@ -406,7 +407,7 @@ fun LibraryScreen( albumDownloads: Map = emptyMap(), artistDownloads: Map = emptyMap(), playlistDownloads: Map = emptyMap(), - dismissRequests: Int = 0, + dismissals: Flow = emptyFlow(), homeContent: @Composable (topInset: Dp, bottomInset: Dp) -> Unit = { _, _ -> }, modifier: Modifier = Modifier, ) { @@ -419,7 +420,7 @@ fun LibraryScreen( var searchActive by rememberSaveable { mutableStateOf(false) } val searchFocus = remember { FocusRequester() } - DismissOnRequest(dismissRequests) { showOptions = false } + DismissOnRequest(dismissals) { showOptions = false } LaunchedEffect(pagerState) { snapshotFlow { pagerState.settledPage }.collect { page -> diff --git a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt index 1525a34d..32df8bae 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt @@ -35,6 +35,7 @@ import com.subtracks.ui.components.ItemActions import com.subtracks.ui.components.MenuTarget import com.subtracks.ui.theme.ArtworkColors import com.subtracks.ui.theme.rememberArtworkColors +import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.launch import org.koin.compose.koinInject import org.koin.compose.viewmodel.koinViewModel @@ -61,7 +62,7 @@ fun PlaylistDetailRoute( CloseWhenDownloadsGone(downloadStatus.downloaded, offline, onBack) val scope = rememberCoroutineScope() var pendingDelete by remember { mutableStateOf(null) } - DismissOnRequest(contextMenuHost?.dismissRequests ?: 0) { pendingDelete = null } + DismissOnRequest(contextMenuHost?.dismissals ?: emptyFlow()) { pendingDelete = null } val playback by playbackController.state.collectAsStateWithLifecycle() val context = playback.context val requestDownloadAction: (BulkDownloadAction) -> Unit = { action -> diff --git a/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceScreen.kt b/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceScreen.kt index 8b383c09..d3036db4 100644 --- a/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/settings/AddSourceScreen.kt @@ -48,6 +48,8 @@ import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.subtracks.R 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.core.parameter.parametersOf @@ -56,7 +58,7 @@ fun AddSourceRoute( onSaved: () -> Unit, onBack: (() -> Unit)?, sourceId: Long? = null, - dismissRequests: Int = 0, + dismissals: Flow = emptyFlow(), viewModel: AddSourceViewModel = koinViewModel(key = sourceId?.toString() ?: "new") { parametersOf(sourceId) }, ) { val state by viewModel.state.collectAsStateWithLifecycle() @@ -71,7 +73,7 @@ fun AddSourceRoute( onSave = { viewModel.save(onSaved) }, onDelete = { viewModel.delete(onSaved) }, onBack = onBack, - dismissRequests = dismissRequests, + dismissals = dismissals, ) } @@ -88,13 +90,13 @@ fun AddSourceScreen( onSave: () -> Unit, onDelete: () -> Unit, onBack: (() -> Unit)?, - dismissRequests: Int = 0, + dismissals: Flow = emptyFlow(), modifier: Modifier = Modifier, ) { val nameFocus = remember { FocusRequester() } val addressFocus = remember { FocusRequester() } var confirmingDelete by remember { mutableStateOf(false) } - DismissOnRequest(dismissRequests) { confirmingDelete = false } + DismissOnRequest(dismissals) { confirmingDelete = false } LaunchedEffect(state.nameError, state.addressError) { when { state.nameError -> nameFocus.requestFocus() diff --git a/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt b/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt index 3abd7ba8..2c3b40e1 100644 --- a/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt @@ -58,6 +58,8 @@ import com.subtracks.data.model.Source import com.subtracks.data.prefs.StreamQuality import com.subtracks.ui.components.DismissOnRequest import com.subtracks.ui.components.rememberViewportFill +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow import org.koin.compose.viewmodel.koinViewModel private val bitrateOptions = listOf(0, 24, 32, 64, 96, 128, 192, 256, 320) @@ -72,7 +74,7 @@ fun SettingsRoute( onOpenLicenses: () -> Unit, onEditServer: (Long) -> Unit, onBack: () -> Unit, - dismissRequests: Int = 0, + dismissals: Flow = emptyFlow(), viewModel: SettingsViewModel = koinViewModel(), ) { val sources by viewModel.sources.collectAsStateWithLifecycle() @@ -107,7 +109,7 @@ fun SettingsRoute( onOpenDownloads = onOpenDownloads, onOpenLicenses = onOpenLicenses, onBack = onBack, - dismissRequests = dismissRequests, + dismissals = dismissals, ) } @@ -136,11 +138,11 @@ fun SettingsScreen( onOpenDownloads: () -> Unit, onOpenLicenses: () -> Unit, onBack: () -> Unit, - dismissRequests: Int = 0, + dismissals: Flow = emptyFlow(), modifier: Modifier = Modifier, ) { var dialog by remember { mutableStateOf(null) } - DismissOnRequest(dismissRequests) { dialog = null } + DismissOnRequest(dismissals) { dialog = null } val listState = rememberLazyListState() val fill = rememberViewportFill(listState) diff --git a/app/src/test/kotlin/com/subtracks/ui/LibraryOptionsTest.kt b/app/src/test/kotlin/com/subtracks/ui/LibraryOptionsTest.kt index eb42d76c..c26703e5 100644 --- a/app/src/test/kotlin/com/subtracks/ui/LibraryOptionsTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/LibraryOptionsTest.kt @@ -119,7 +119,7 @@ class LibraryOptionsTest { onOpenSettings = {}, sortOptions = sortOptionsFor(LibraryTab.Albums), starredSupported = true, - dismissRequests = host.dismissRequests, + dismissals = host.dismissals, ) } }