From dac56a7fcb6145fc34fa35e08bd37823a20ae6b4 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 28 Sep 2026 06:25:18 +0000 Subject: [PATCH] group the downloads tree by the album's artist The tree read the track artist off each song, so a downloaded album split into one group per guest performer and every song without a track artist landed in "Unknown artist". It reads the album's artist now, which is also the artist the node's delete resolves through, so the two agree. The badges are white glyphs with a dark outline and a progress ring with a visible track, after a look at the rendered result; larger and clearer than the first pass, which the art swallowed. --- .../com/subtracks/data/db/DownloadDao.kt | 4 +- .../ui/components/DownloadIndicator.kt | 37 +++++++++++-------- .../data/repo/DownloadRepositoryTest.kt | 35 ++++++++++++++++++ .../com/subtracks/ui/AppScreenshotTest.kt | 32 ++++++++++++++++ 4 files changed, 90 insertions(+), 18 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 22623d26..3f407e55 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/DownloadDao.kt @@ -99,11 +99,11 @@ interface DownloadDao { @Query( "SELECT d.songId AS songId, s.title AS title, s.albumId AS albumId, al.name AS albumName, " + - "s.artistId AS artistId, ar.name AS artistName, d.status AS status, d.total AS size " + + "al.artistId AS artistId, ar.name AS artistName, d.status AS status, d.total AS size " + "FROM song_downloads d " + "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 = s.sourceId AND ar.id = s.artistId " + + "LEFT JOIN artists ar ON ar.sourceId = al.sourceId AND ar.id = al.artistId " + "WHERE d.sourceId = :sourceId", ) fun downloadedSongs(sourceId: Long): Flow> 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 4db5a484..650fe58b 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/DownloadIndicator.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/DownloadIndicator.kt @@ -12,6 +12,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.blur import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.semantics.contentDescription import androidx.compose.ui.semantics.semantics import androidx.compose.ui.unit.dp @@ -38,36 +39,42 @@ fun ListDownloadIndicator( val current = status ?: return val kind = current.indicator() if (kind == DownloadIndicator.Hidden) return - Box(modifier = modifier.size(20.dp), contentAlignment = Alignment.Center) { + Box(modifier = modifier.size(26.dp), contentAlignment = Alignment.Center) { when (kind) { DownloadIndicator.Complete -> { Icon( imageVector = Icons.Rounded.DownloadDone, contentDescription = null, - tint = ICON_SHADOW, - modifier = Modifier.size(18.dp).blur(3.dp).offset(y = 1.dp), + tint = Color.Black.copy(alpha = 0.5f), + modifier = Modifier.size(22.dp).blur(4.dp).offset(y = 1.dp), + ) + Icon( + imageVector = Icons.Rounded.DownloadDone, + contentDescription = null, + tint = Color.Black.copy(alpha = 0.85f), + modifier = + Modifier + .size(22.dp) + .graphicsLayer { + scaleX = 1.18f + scaleY = 1.18f + }.blur(1.dp), ) Icon( imageVector = Icons.Rounded.DownloadDone, contentDescription = DOWNLOADED_INDICATOR_TAG, tint = Color.White, - modifier = Modifier.size(18.dp), + modifier = Modifier.size(22.dp), ) } DownloadIndicator.InProgress -> { - val progress = { current.downloaded.toFloat() / current.total } CircularProgressIndicator( - progress = progress, - color = ICON_SHADOW, - strokeWidth = 2.dp, - modifier = Modifier.size(16.dp).blur(3.dp).offset(y = 1.dp), - ) - CircularProgressIndicator( - progress = progress, + progress = { current.downloaded.toFloat() / current.total }, color = Color.White, - strokeWidth = 2.dp, - modifier = Modifier.size(16.dp).semantics { contentDescription = DOWNLOADING_INDICATOR_TAG }, + trackColor = Color.Black.copy(alpha = 0.45f), + strokeWidth = 2.5.dp, + modifier = Modifier.size(20.dp).semantics { contentDescription = DOWNLOADING_INDICATOR_TAG }, ) } @@ -77,5 +84,3 @@ fun ListDownloadIndicator( } } } - -private val ICON_SHADOW = Color.Black.copy(alpha = 0.45f) 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 8995e69f..6669e1e5 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt @@ -816,6 +816,41 @@ class DownloadRepositoryTest { assertFalse(file(1, "s2").exists()) } + @Test + fun theTreeGroupsAnAlbumUnderItsAlbumArtistNotEachTrackArtist() = + runTest { + seedLibrary() + runBlocking { + db.libraryDao().upsertSongs( + listOf( + Song( + sourceId = 1, + id = "s3", + albumId = "al1", + artistId = "ar-guest", + title = "Song 3", + album = "Album", + artist = "Guest", + duration = 100, + track = 3, + disc = 1, + starred = null, + genre = null, + ), + ), + ) + db.libraryDao().upsertArtists( + listOf(Artist(sourceId = 1, id = "ar-guest", name = "Guest", albumCount = 0, starred = null, coverArt = null)), + ) + db.downloadDao().upsert(SongDownload(1, "s1", DownloadStatus.Completed)) + db.downloadDao().upsert(SongDownload(1, "s3", DownloadStatus.Completed)) + } + + val songs = runBlocking { repository.downloadedSongs(1).first() } + + assertEquals(setOf("ar1" to "Artist"), songs.map { it.artistId to it.artistName }.toSet()) + } + private fun requestedSongIds(): List = engine.requests.map { it.second.path.substringAfterLast('/') } private fun seedSongs(tracks: IntRange) { diff --git a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt index 48033aa7..69334c8f 100644 --- a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt @@ -32,6 +32,7 @@ import com.github.takahirom.roborazzi.captureRoboImage import com.subtracks.data.model.Album import com.subtracks.data.model.Artist import com.subtracks.data.model.CoverArtRef +import com.subtracks.data.model.ListDownloadStatus import com.subtracks.data.model.Playlist import com.subtracks.data.model.PlaylistSongItem import com.subtracks.data.model.Song @@ -125,6 +126,37 @@ class AppScreenshotTest { composeRule.onRoot().captureRoboImage("src/test/screenshots/library_scrolled_under_header.png") } + @Test + fun libraryAlbumsWithDownloads() { + val ids = Fixtures.albums.take(3).map { it.id } + composeRule.setContent { + SubtracksTheme { + LibraryScreen( + selectedTab = LibraryTab.Albums, + onTabSelected = {}, + albums = remember { flowOf(PagingData.from(Fixtures.albums)) }, + artists = remember { flowOf(PagingData.empty()) }, + playlists = remember { flowOf(PagingData.empty()) }, + coverArt = { id, _ -> id?.let { CoverArtRef(it, "test:$it") } }, + onAlbumClick = {}, + onArtistClick = {}, + onPlaylistClick = {}, + onSync = {}, + onOpenSettings = {}, + artwork = artworkColorsFromSeed(Color.rgb(120, 80, 200)), + albumDownloads = + mapOf( + ids[0] to ListDownloadStatus(total = 10, downloaded = 10), + ids[1] to ListDownloadStatus(total = 10, downloaded = 4, downloading = 1), + ids[2] to ListDownloadStatus(total = 10, downloaded = 4), + ), + ) + } + } + awaitTag(ALBUM_COVER_TAG) + composeRule.onRoot().captureRoboImage("src/test/screenshots/library_albums_downloads.png") + } + @Test fun libraryArtists() { setLibraryContent(LibraryTab.Artists)