From 7161121922a59bf986cd4880d5cc351d9c884188 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 18 Oct 2026 06:46:40 +0000 Subject: [PATCH] Polish downloads, settings, and detail titles Use right/down chevrons in the download manager and order downloaded tracks by disc and track number, matching the album screen. Move sync concurrency to the end of the network settings, let the artist hero name wrap to six lines, and leave the album hero title unbounded. --- .../com/subtracks/data/db/DownloadDao.kt | 2 +- .../ui/components/HeroDetailScaffold.kt | 5 +- .../subtracks/ui/downloads/DownloadsScreen.kt | 6 +- .../ui/downloads/DownloadsViewModel.kt | 2 +- .../subtracks/ui/library/AlbumDetailScreen.kt | 1 + .../ui/library/ArtistDetailScreen.kt | 55 ++++++++++++++----- .../subtracks/ui/settings/SettingsScreen.kt | 16 +++--- .../data/repo/DownloadRepositoryTest.kt | 47 ++++++++++++++++ .../subtracks/ui/library/HangingTitleTest.kt | 15 +++++ 9 files changed, 121 insertions(+), 28 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt b/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt index 7b475ce4..7af9fcdf 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt @@ -174,7 +174,7 @@ interface DownloadDao { "JOIN songs s ON s.sourceId = d.sourceId AND s.id = d.songId " + "LEFT JOIN albums al ON al.sourceId = s.sourceId AND al.id = s.albumId " + "LEFT JOIN artists ar ON ar.sourceId = al.sourceId AND ar.id = al.artistId " + - "WHERE d.sourceId = :sourceId", + "WHERE d.sourceId = :sourceId ORDER BY s.disc, s.track, s.id", ) fun downloadedSongs(sourceId: Long): Flow> diff --git a/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt b/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt index cd327d71..c835b6f8 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt @@ -257,6 +257,7 @@ fun HeroHeader( thumbnailRef: CoverArtRef? = null, comment: String? = null, onSubtitleClick: (() -> Unit)? = null, + nameMaxLines: Int = 2, ) { Column( modifier = @@ -278,8 +279,8 @@ fun HeroHeader( text = name, style = MaterialTheme.typography.headlineMedium, textAlign = TextAlign.Center, - maxLines = 2, - overflow = TextOverflow.Ellipsis, + maxLines = nameMaxLines, + overflow = if (nameMaxLines == Int.MAX_VALUE) TextOverflow.Clip else TextOverflow.Ellipsis, ) val subtitleIsTarget = onSubtitleClick != null && subtitle.isNotEmpty() && comment.isNullOrBlank() if (subtitleIsTarget) { 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 34a75e7e..511fe58b 100644 --- a/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsScreen.kt @@ -13,10 +13,10 @@ import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.items import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.rounded.ArrowBack +import androidx.compose.material.icons.automirrored.rounded.KeyboardArrowRight import androidx.compose.material.icons.rounded.Close import androidx.compose.material.icons.rounded.Delete -import androidx.compose.material.icons.rounded.ExpandLess -import androidx.compose.material.icons.rounded.ExpandMore +import androidx.compose.material.icons.rounded.KeyboardArrowDown import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -274,7 +274,7 @@ private fun NodeRow( leadingContent = { IconButton(onClick = onToggle) { Icon( - imageVector = if (expanded) Icons.Rounded.ExpandLess else Icons.Rounded.ExpandMore, + imageVector = if (expanded) Icons.Rounded.KeyboardArrowDown else Icons.AutoMirrored.Rounded.KeyboardArrowRight, contentDescription = if (expanded) stringResource(R.string.collapse) else stringResource(R.string.expand), ) } diff --git a/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsViewModel.kt index f823f266..04328630 100644 --- a/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/downloads/DownloadsViewModel.kt @@ -117,7 +117,7 @@ internal fun buildTree( name = albumKey.second.ifBlank { unknownAlbum }, bytes = albumSongs.sumOf { it.size }, sortName = albumSongs.first().albumSort.ifBlank { albumKey.second }, - songs = albumSongs.sortedBy { it.songSort.lowercase() }, + songs = albumSongs, ) }.sortedBy { it.sortName.lowercase() }, ) 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 99e7860b..80829033 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/AlbumDetailScreen.kt @@ -235,6 +235,7 @@ fun AlbumDetailScreen( onMore = onMore, topInset = topInset, controlsModifier = controlsModifier, + nameMaxLines = Int.MAX_VALUE, ) }, content = { rowModifier -> 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 133ea20d..fffb24cc 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -52,6 +52,7 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableFloatStateOf +import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope @@ -136,6 +137,7 @@ internal fun estimateScrollPx( } private val FADE_LEAD = 24.dp +private const val ARTIST_NAME_MAX_LINES = 6 private const val THEME_TRANSITION_MS = 100 private const val SCRIM_FADE_MS = 180 internal const val ARTIST_NAME_SCRIM_TAG = "artistNameScrim" @@ -255,7 +257,8 @@ fun ArtistDetailScreen( val albumCount = albums.size.toLong() val displayName = artistName.ifBlank { artist?.name.orEmpty() } val artistType = stringResource(if (isAlbumArtist == true) R.string.artist_type_album_artist else R.string.artist_type_artist) - val imageTitleTop = ART_HEIGHT - TITLE_INSET - lineHeight + var titleLineCount by remember { mutableIntStateOf(1) } + val imageTitleTop = ART_HEIGHT - TITLE_INSET - lineHeight * titleLineCount val fadeStartPx = with(density) { (imageTitleTop - barHeight - FADE_LEAD).toPx() } val fadeEndPx = with(density) { (imageTitleTop + lineHeight / 2 - barHeight).toPx() } val barFraction by remember(fadeStartPx, fadeEndPx) { @@ -351,6 +354,7 @@ fun ArtistDetailScreen( color = Color.White, endReserve = SHUFFLE_RESERVE, modifier = Modifier.align(Alignment.BottomStart).fillMaxWidth().padding(TITLE_INSET), + onLineCountChanged = { titleLineCount = it }, ) FilledIconButton( onClick = onShuffle, @@ -518,13 +522,19 @@ private fun ArtistTitle( color: Color, endReserve: Dp, modifier: Modifier = Modifier, + maxLines: Int = ARTIST_NAME_MAX_LINES, + onLineCountChanged: (Int) -> Unit = {}, ) { val measurer = rememberTextMeasurer() val density = LocalDensity.current BoxWithConstraints(modifier) { val fullWidth = with(density) { maxWidth.roundToPx() } val reserve = with(density) { endReserve.roundToPx() } - val lines = remember(text, style, fullWidth, reserve) { splitTitle(measurer, text, style, fullWidth, reserve) } + val lines = + remember(text, style, fullWidth, reserve, maxLines) { + splitTitle(measurer, text, style, fullWidth, reserve, maxLines) + } + LaunchedEffect(lines.size) { onLineCountChanged(lines.size) } Column { lines.forEachIndexed { index, line -> Text( @@ -549,6 +559,7 @@ internal fun splitTitle( style: TextStyle, fullWidth: Int, reserve: Int, + maxLines: Int = ARTIST_NAME_MAX_LINES, ): List { if (text.isBlank()) return listOf(text) if (measurer.measure(AnnotatedString(text), style, maxLines = 1).size.width <= fullWidth - reserve) { @@ -556,18 +567,36 @@ internal fun splitTitle( } val words = text.split(' ').filter { it.isNotEmpty() } if (words.size <= 1) return listOf(text) - var count = 0 - var acc = "" - for (i in words.indices) { - val candidate = if (acc.isEmpty()) words[i] else "$acc ${words[i]}" - if (measurer.measure(AnnotatedString(candidate), style, maxLines = 1).size.width <= fullWidth) { - acc = candidate - count = i + 1 - } else { + val lines = mutableListOf() + var index = 0 + while (index < words.size && lines.size < maxLines) { + val remaining = words.subList(index, words.size).joinToString(" ") + if (lines.size == maxLines - 1 || + measurer.measure(AnnotatedString(remaining), style, maxLines = 1).size.width <= fullWidth - reserve + ) { + lines.add(remaining) break } + var end = index + var acc = "" + while (end < words.size) { + val candidate = if (acc.isEmpty()) words[end] else "$acc ${words[end]}" + if (measurer.measure(AnnotatedString(candidate), style, maxLines = 1).size.width <= fullWidth) { + acc = candidate + end++ + } else { + break + } + } + if (end == index) { + acc = words[index] + end = index + 1 + } else if (end >= words.size && end - index > 1) { + end -= 1 + acc = words.subList(index, end).joinToString(" ") + } + lines.add(acc) + index = end } - if (count >= words.size) count = words.size - 1 - if (count < 1) count = 1 - return listOf(words.take(count).joinToString(" "), words.drop(count).joinToString(" ")) + return lines } 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 04607211..65e9c21e 100644 --- a/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/settings/SettingsScreen.kt @@ -324,14 +324,6 @@ fun SettingsScreen( colors = ListItemDefaults.colors(containerColor = Color.Transparent), ) } - item { - ListItem( - headlineContent = { Text(stringResource(R.string.sync_concurrency)) }, - supportingContent = { Text(concurrencyLabel(LocalResources.current, syncConcurrency)) }, - modifier = Modifier.clickable { dialog = SettingsDialog.SyncConcurrency }, - colors = ListItemDefaults.colors(containerColor = Color.Transparent), - ) - } item { ListItem( headlineContent = { Text(stringResource(R.string.settings_sync_auto_title)) }, @@ -348,6 +340,14 @@ fun SettingsScreen( colors = ListItemDefaults.colors(containerColor = Color.Transparent), ) } + item { + ListItem( + headlineContent = { Text(stringResource(R.string.sync_concurrency)) }, + supportingContent = { Text(concurrencyLabel(LocalResources.current, syncConcurrency)) }, + modifier = Modifier.clickable { dialog = SettingsDialog.SyncConcurrency }, + colors = ListItemDefaults.colors(containerColor = Color.Transparent), + ) + } item { SectionHeader(stringResource(R.string.settings_music_name)) } item { ListItem( diff --git a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt index 2b6ee0bd..ee077b97 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt @@ -975,6 +975,53 @@ class DownloadRepositoryTest { assertFalse(file(1, "s2").exists()) } + @Test + fun downloadedSongsFollowTheAlbumDiscAndTrackOrder() = + runTest { + seedLibrary() + runBlocking { + db.libraryDao().upsertSongs( + listOf( + Song( + sourceId = 1, + id = "s0", + albumId = "al1", + artistId = "ar1", + title = "Alpha", + album = "Album", + artist = "Artist", + duration = 100, + track = 3, + disc = 1, + starred = null, + genre = null, + ), + Song( + sourceId = 1, + id = "s3", + albumId = "al1", + artistId = "ar1", + title = "Zulu", + album = "Album", + artist = "Artist", + duration = 100, + track = 1, + disc = 2, + starred = null, + genre = null, + ), + ), + ) + listOf("s0", "s1", "s2", "s3").forEach { + db.downloadDao().upsert(SongDownload(1, it, DownloadStatus.Completed)) + } + } + + val songs = runBlocking { repository.downloadedSongs(1).first() } + + assertEquals(listOf("s1", "s2", "s0", "s3"), songs.map { it.songId }) + } + @Test fun theTreeGroupsAnAlbumUnderItsAlbumArtistNotEachTrackArtist() = runTest { diff --git a/app/src/test/kotlin/com/subtracks/ui/library/HangingTitleTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/HangingTitleTest.kt index c26fa925..7fd7088b 100644 --- a/app/src/test/kotlin/com/subtracks/ui/library/HangingTitleTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/library/HangingTitleTest.kt @@ -49,6 +49,21 @@ class HangingTitleTest { assertEquals(listOf("Boards"), splitTitle(measurer!!, "Boards", style, fullWidth, reserve)) } + @Test + fun aLongNameWrapsToAtMostSixLines() { + setUpMeasurements() + + val text = (1..200).joinToString(" ") { "word$it" } + val lines = splitTitle(measurer!!, text, style, fullWidth, reserve) + + assertEquals(6, lines.size) + lines.dropLast(1).forEach { line -> + val width = measurer!!.measure(AnnotatedString(line), style, maxLines = 1).size.width + assertTrue("every line before the last must fit the full width", width <= fullWidth) + } + assertTrue(lines.last().isNotEmpty()) + } + @Test fun theLastLineStopsBeforeTheReservedWidth() { setUpMeasurements()