From fd2e1cfd8607532714b7fe1886e1bdb4713be604 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 4 Oct 2026 01:16:53 +0000 Subject: [PATCH] artists: show a credited track artist's name and type on their page A track artist without an artists row had no name on the artist page, so the header and the placeholder letter were blank. Derive the name from the songs that credit them, and label the page 'Album artist' when they lead an album or 'Artist' when they are only credited on tracks. The hero art keeps the letter placeholder when there is no artist image. --- .../com/subtracks/data/db/LibraryDao.kt | 10 ++++ .../subtracks/data/repo/LibraryRepository.kt | 5 ++ .../ui/library/ArtistDetailScreen.kt | 33 ++++++---- .../ui/library/ArtistDetailViewModel.kt | 8 +++ app/src/main/res/values/strings.xml | 2 + .../com/subtracks/data/db/LibraryDaoTest.kt | 14 +++++ .../ui/library/ArtistDetailScreenTest.kt | 60 +++++++++++++++++++ 7 files changed, 122 insertions(+), 10 deletions(-) create mode 100644 app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt diff --git a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt index 1b7a33e8..2e0e73bf 100644 --- a/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt +++ b/app/src/main/kotlin/com/subtracks/data/db/LibraryDao.kt @@ -942,6 +942,16 @@ interface LibraryDao { artistId: String, ): Flow + // A credited track artist may not have an artists row; its name only lives on the songs. + @Query( + "SELECT artist FROM songs WHERE sourceId = :sourceId AND artistId = :artistId " + + "AND artist IS NOT NULL AND artist != '' ORDER BY id LIMIT 1", + ) + fun artistName( + sourceId: Long, + artistId: String, + ): Flow + @Query("SELECT * FROM songs WHERE sourceId = :sourceId AND id = :songId") fun song( sourceId: Long, diff --git a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt index 694ceaa0..9d1fca58 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -348,6 +348,11 @@ class LibraryRepository( artistId: String, ): Flow = db.libraryDao().artist(sourceId, artistId) + fun artistName( + sourceId: Long, + artistId: String, + ): Flow = db.libraryDao().artistName(sourceId, artistId) + fun song( sourceId: Long, songId: String, 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 e88aa820..22ade453 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -152,6 +152,7 @@ fun ArtistDetailRoute( playbackController: PlaybackController = koinInject(), ) { val artist by viewModel.artist.collectAsStateWithLifecycle(initialValue = null) + val artistName by viewModel.artistName.collectAsStateWithLifecycle(initialValue = null) val albums by viewModel.albums.collectAsStateWithLifecycle(initialValue = emptyList()) val art by viewModel.art.collectAsStateWithLifecycle() val artThumbnail by viewModel.artThumbnail.collectAsStateWithLifecycle() @@ -184,10 +185,12 @@ fun ArtistDetailRoute( ) ArtistDetailScreen( artist = artist, + artistName = artistName.orEmpty(), + isAlbumArtist = albums.any { it.artistId == artistId }, albums = albums, art = art, artThumbnail = artThumbnail, - artwork = rememberArtworkColors(shortcutArt ?: artThumbnail ?: art, THEME_TRANSITION_MS, fallbackName = artist?.name), + artwork = rememberArtworkColors(shortcutArt ?: artThumbnail ?: art, THEME_TRANSITION_MS, fallbackName = artistName), coverArt = viewModel::coverArt, onBack = onBack, onAlbumClick = onAlbumClick, @@ -213,6 +216,8 @@ fun ArtistDetailRoute( @Composable fun ArtistDetailScreen( artist: Artist?, + artistName: String = "", + isAlbumArtist: Boolean = false, albums: List, art: CoverArtRef?, artThumbnail: CoverArtRef? = null, @@ -240,6 +245,8 @@ fun ArtistDetailScreen( val navBarBottom = with(density) { WindowInsets.navigationBars.getBottom(this).toDp() } val barHeight = statusBarTop + TopAppBarDefaults.TopAppBarExpandedHeight val albumCount = albums.size.toLong() + val displayName = artistName.ifBlank { artist?.name.orEmpty() } + val artistType = stringResource(if (isAlbumArtist) R.string.artist_type_album_artist else R.string.artist_type_artist) val imageTitleTop = ART_HEIGHT - TITLE_INSET - lineHeight val fadeStartPx = with(density) { (imageTitleTop - barHeight - FADE_LEAD).toPx() } val fadeEndPx = with(density) { (imageTitleTop + lineHeight / 2 - barHeight).toPx() } @@ -309,7 +316,7 @@ fun ArtistDetailScreen( Box(Modifier.fillMaxWidth()) { CoverArt( ref = art, - name = artist?.name.orEmpty(), + name = displayName, thumbnailRef = artThumbnail, showPlaceholder = art == null, modifier = Modifier.fillMaxWidth().height(ART_HEIGHT), @@ -331,7 +338,7 @@ fun ArtistDetailScreen( } Box(Modifier.align(Alignment.BottomStart).fillMaxWidth()) { ArtistTitle( - text = artist?.name.orEmpty(), + text = displayName, style = imageNameStyle, color = Color.White, endReserve = SHUFFLE_RESERVE, @@ -360,12 +367,18 @@ fun ArtistDetailScreen( modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 8.dp, top = 34.dp, bottom = 0.dp), verticalAlignment = Alignment.CenterVertically, ) { - Text( - text = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount), - style = MaterialTheme.typography.bodyLarge, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.weight(1f), - ) + Column(Modifier.weight(1f)) { + Text( + text = artistType, + style = MaterialTheme.typography.labelLarge, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + Text( + text = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount), + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } IconButton( onClick = { onToggleStar?.invoke() }, enabled = onToggleStar != null, @@ -452,7 +465,7 @@ fun ArtistDetailScreen( TopAppBar( title = { Text( - text = artist?.name.orEmpty(), + text = displayName, maxLines = 1, overflow = TextOverflow.Ellipsis, modifier = Modifier.graphicsLayer { alpha = barFraction }, diff --git a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt index 5edc2e10..604c1e84 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt @@ -38,6 +38,14 @@ class ArtistDetailViewModel( val artist: Flow = sourceId.flatMapLatest { libraryRepository.artist(it, artistId) } + val artistName: Flow = + sourceId.flatMapLatest { id -> + combine( + libraryRepository.artist(id, artistId), + libraryRepository.artistName(id, artistId), + ) { loaded, fromSongs -> loaded?.name?.takeIf { it.isNotBlank() } ?: fromSongs } + } + val offline: StateFlow = sourceRepository.offline val albums: Flow> = diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 939b9dc7..39301538 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -29,6 +29,8 @@ Playlists Album Artist + Artist + Album artist Genre Starred %1$s\n\nDeleting will free %2$s diff --git a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt index 1a31140f..b8ad315a 100644 --- a/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/db/LibraryDaoTest.kt @@ -558,6 +558,20 @@ class LibraryDaoTest { ) } + @Test + fun aCreditedTrackArtistsNameComesFromItsSongs() = + runTest { + val sourceId = source() + val dao = db.libraryDao() + dao.upsertSongs( + listOf( + song(sourceId, "s1", "One", starred = null).copy(artistId = "ar-guest", artist = "Guest"), + ), + ) + + assertEquals("Guest", dao.artistName(sourceId, "ar-guest").first()) + } + @Test fun downloadedFilterReportsTheDownloadedAlbumCount() = runTest { diff --git a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt new file mode 100644 index 00000000..d0cf3035 --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt @@ -0,0 +1,60 @@ +package com.subtracks.ui.library + +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithText +import androidx.compose.ui.test.onNodeWithText +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.data.model.Artist +import com.subtracks.ui.theme.SubtracksTheme +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.annotation.GraphicsMode + +@RunWith(AndroidJUnit4::class) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class ArtistDetailScreenTest { + @get:Rule + val composeRule = createAndroidComposeRule() + + private fun render( + artist: Artist?, + artistName: String, + isAlbumArtist: Boolean, + ) { + composeRule.setContent { + SubtracksTheme { + ArtistDetailScreen( + artist = artist, + artistName = artistName, + isAlbumArtist = isAlbumArtist, + albums = emptyList(), + art = null, + coverArt = { _, _ -> null }, + onBack = {}, + onAlbumClick = {}, + ) + } + } + } + + @Test + fun aCreditedTrackArtistShowsItsDerivedNameAndIsLabelledArtist() { + render(artist = null, artistName = "Guest", isAlbumArtist = false) + + composeRule.onAllNodesWithText("Guest")[0].assertExists() + composeRule.onNodeWithText("Artist").assertExists() + } + + @Test + fun anAlbumArtistIsLabelledAlbumArtist() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + isAlbumArtist = true, + ) + + composeRule.onNodeWithText("Album artist").assertExists() + } +}