From b4f69affc912c70b64f3af8415122dc9c644c5e8 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 28 Sep 2026 06:01:23 +0000 Subject: [PATCH] put the mixed menu back to two rows, and only badge real progress The split bar read badly; a partly downloaded list goes back to stacked Download and Delete items. The badge showed progress for any list with a downloaded song in it, which made a mostly-downloaded album look like it was fetching. It now shows only when that list is complete or has a download actually in flight; a partly downloaded idle list shows nothing. The album and playlist detail screens' more-menus had no download entries at all, leaving delete reachable only via the hero's long press. They now take the same status-driven items as everywhere else. --- .../ui/components/DownloadIndicator.kt | 5 +- .../ui/components/ItemContextMenu.kt | 52 ++----------------- .../subtracks/ui/library/AlbumDetailScreen.kt | 25 ++++++--- .../ui/library/PlaylistDetailScreen.kt | 20 ++++--- .../ui/components/DownloadIndicatorTest.kt | 9 +++- 5 files changed, 44 insertions(+), 67 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/ui/components/DownloadIndicator.kt b/app/src/main/kotlin/com/subtracks/ui/components/DownloadIndicator.kt index 8e194f8e..0ff225b0 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/DownloadIndicator.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/DownloadIndicator.kt @@ -24,9 +24,10 @@ enum class DownloadIndicator { Hidden, InProgress, Complete } fun ListDownloadStatus.indicator(): DownloadIndicator = when { - total == 0L || (downloaded == 0L && downloading == 0L) -> DownloadIndicator.Hidden + total == 0L -> DownloadIndicator.Hidden complete -> DownloadIndicator.Complete - else -> DownloadIndicator.InProgress + downloading > 0 -> DownloadIndicator.InProgress + else -> DownloadIndicator.Hidden } @Composable 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 ad3d13f9..df04357d 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt @@ -1,7 +1,6 @@ package com.subtracks.ui.components import androidx.compose.foundation.clickable -import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer @@ -32,7 +31,6 @@ import androidx.compose.material3.LocalContentColor import androidx.compose.material3.MaterialTheme import androidx.compose.material3.ModalBottomSheet import androidx.compose.material3.Text -import androidx.compose.material3.VerticalDivider import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue @@ -353,55 +351,15 @@ private fun BulkDownloadItem( dismiss: (() -> Unit) -> Unit, ) { val bulk = actions.bulkDownload ?: return - val offered = (target.listDownload() ?: ListDownloadStatus()).actions() - if (offered.size > 1) { - SplitDownloadItem(offered, target, bulk, dismiss) - return - } - offered.singleOrNull()?.let { action -> - MenuItem(bulkIcon(action), bulkLabel(action)) { dismiss { bulk(target, action) } } - } -} - -@Composable -private fun SplitDownloadItem( - offered: List, - target: MenuTarget, - bulk: (MenuTarget, BulkDownloadAction) -> Unit, - dismiss: (() -> Unit) -> Unit, -) { - Row(Modifier.fillMaxWidth().height(56.dp)) { - offered.forEachIndexed { index, action -> - if (index > 0) { - VerticalDivider(color = MaterialTheme.colorScheme.outlineVariant) - } - Row( - modifier = Modifier.weight(1f).clickable { dismiss { bulk(target, action) } }, - horizontalArrangement = Arrangement.Center, - verticalAlignment = Alignment.CenterVertically, - ) { - Icon(bulkIcon(action), contentDescription = null) - Spacer(Modifier.width(8.dp)) - Text(bulkLabel(action)) - } + for (action in (target.listDownload() ?: ListDownloadStatus()).actions()) { + when (action) { + BulkDownloadAction.Delete -> MenuItem(Icons.Rounded.Delete, "Delete downloads") { dismiss { bulk(target, action) } } + BulkDownloadAction.Cancel -> MenuItem(Icons.Rounded.Cancel, "Cancel downloads") { dismiss { bulk(target, action) } } + BulkDownloadAction.Download -> MenuItem(Icons.Rounded.Download, "Download") { dismiss { bulk(target, action) } } } } } -private fun bulkIcon(action: BulkDownloadAction): ImageVector = - when (action) { - BulkDownloadAction.Download -> Icons.Rounded.Download - BulkDownloadAction.Cancel -> Icons.Rounded.Cancel - BulkDownloadAction.Delete -> Icons.Rounded.Delete - } - -private fun bulkLabel(action: BulkDownloadAction): String = - when (action) { - BulkDownloadAction.Download -> "Download" - BulkDownloadAction.Cancel -> "Cancel downloads" - BulkDownloadAction.Delete -> "Delete downloads" - } - @Composable private fun StarItem( current: Long?, 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 ce080356..7ea89ab5 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt @@ -70,6 +70,13 @@ fun AlbumDetailRoute( val playback by playbackController.state.collectAsStateWithLifecycle() val context = playback.context val shortcutArt = remember(coverArtId) { coverArtId?.let { viewModel.coverArt(it, true) } } + val requestDownloadAction: (BulkDownloadAction) -> Unit = { action -> + if (action == BulkDownloadAction.Delete) { + scope.launch { pendingDelete = viewModel.downloadedBytes() } + } else { + viewModel.onDownloadAction(action) + } + } val actions = ItemActions( playSong = { song -> songs.indexOfFirst { it.id == song.id }.takeIf { it >= 0 }?.let(viewModel::play) }, @@ -80,6 +87,7 @@ fun AlbumDetailRoute( download = viewModel::download, cancelDownload = viewModel::cancelDownload, deleteDownload = viewModel::deleteDownload, + bulkDownload = { _, action -> requestDownloadAction(action) }, setStar = setStar, viewArtist = onViewArtist, ) @@ -91,19 +99,20 @@ fun AlbumDetailRoute( artwork = rememberArtworkColors(shortcutArt ?: viewModel.coverArt(album?.coverArt, true), THEME_TRANSITION_MS), downloads = downloads, downloadStatus = downloadStatus, - onDownloadAction = { action -> - if (action == BulkDownloadAction.Delete) { - scope.launch { pendingDelete = viewModel.downloadedBytes() } - } else { - viewModel.onDownloadAction(action) - } - }, + onDownloadAction = requestDownloadAction, onBack = onBack, onSongClick = viewModel::play, onSongLongClick = { contextMenuHost?.show(it, actions) }, onShuffle = viewModel::shuffle, onPlay = viewModel::playAll, - onMore = { album?.let { contextMenuHost?.show(MenuTarget.Album(it, viewModel.coverArt(it.coverArt, true)), actions) } }, + onMore = { + album?.let { + contextMenuHost?.show( + MenuTarget.Album(it, viewModel.coverArt(it.coverArt, true), downloadStatus), + actions, + ) + } + }, onArtistClick = album ?.takeIf { !it.albumArtist.isNullOrBlank() } 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 89aeb2e1..6d730123 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/PlaylistDetailScreen.kt @@ -55,6 +55,13 @@ fun PlaylistDetailRoute( var pendingDelete by remember { mutableStateOf(null) } val playback by playbackController.state.collectAsStateWithLifecycle() val context = playback.context + val requestDownloadAction: (BulkDownloadAction) -> Unit = { action -> + if (action == BulkDownloadAction.Delete) { + scope.launch { pendingDelete = viewModel.downloadedBytes() } + } else { + viewModel.onDownloadAction(action) + } + } val actions = ItemActions( playPlaylist = { viewModel.playAll() }, @@ -64,6 +71,7 @@ fun PlaylistDetailRoute( download = viewModel::download, cancelDownload = viewModel::cancelDownload, deleteDownload = viewModel::deleteDownload, + bulkDownload = { _, action -> requestDownloadAction(action) }, setStar = setStar, viewAlbum = onViewAlbum, viewArtist = onViewArtist, @@ -75,19 +83,15 @@ fun PlaylistDetailRoute( artwork = rememberArtworkColors(viewModel.coverArt(playlist?.coverArt, true), THEME_TRANSITION_MS), downloads = downloads, downloadStatus = downloadStatus, - onDownloadAction = { action -> - if (action == BulkDownloadAction.Delete) { - scope.launch { pendingDelete = viewModel.downloadedBytes() } - } else { - viewModel.onDownloadAction(action) - } - }, + onDownloadAction = requestDownloadAction, onBack = onBack, onSongClick = viewModel::play, onSongLongClick = { contextMenuHost?.show(it, actions) }, onShuffle = viewModel::shuffle, onPlay = viewModel::playAll, - onMore = { playlist?.let { contextMenuHost?.show(MenuTarget.Playlist(it, viewModel.coverArt(it.coverArt, true)), actions) } }, + onMore = { + playlist?.let { contextMenuHost?.show(MenuTarget.Playlist(it, viewModel.coverArt(it.coverArt, true), downloadStatus), actions) } + }, playingSongId = playback.item?.id.takeIf { context?.kind == QueueKind.Playlist && context.refId == playlistId }, ) diff --git a/app/src/test/kotlin/com/subtracks/ui/components/DownloadIndicatorTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/DownloadIndicatorTest.kt index 4bd3ef20..aac8ef66 100644 --- a/app/src/test/kotlin/com/subtracks/ui/components/DownloadIndicatorTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/components/DownloadIndicatorTest.kt @@ -21,8 +21,13 @@ class DownloadIndicatorTest { } @Test - fun aPartlyDownloadedListShowsProgress() { - assertEquals(DownloadIndicator.InProgress, ListDownloadStatus(total = 4, downloaded = 2).indicator()) + fun aPartlyDownloadedIdleListShowsNothing() { + assertEquals(DownloadIndicator.Hidden, ListDownloadStatus(total = 4, downloaded = 2).indicator()) + } + + @Test + fun aPartlyDownloadedListWithSomethingInFlightShowsProgress() { + assertEquals(DownloadIndicator.InProgress, ListDownloadStatus(total = 4, downloaded = 2, downloading = 1).indicator()) } @Test