mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 15:29:15 +01:00
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.
This commit is contained in:
@@ -942,6 +942,16 @@ interface LibraryDao {
|
|||||||
artistId: String,
|
artistId: String,
|
||||||
): Flow<Artist?>
|
): Flow<Artist?>
|
||||||
|
|
||||||
|
// 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<String?>
|
||||||
|
|
||||||
@Query("SELECT * FROM songs WHERE sourceId = :sourceId AND id = :songId")
|
@Query("SELECT * FROM songs WHERE sourceId = :sourceId AND id = :songId")
|
||||||
fun song(
|
fun song(
|
||||||
sourceId: Long,
|
sourceId: Long,
|
||||||
|
|||||||
@@ -348,6 +348,11 @@ class LibraryRepository(
|
|||||||
artistId: String,
|
artistId: String,
|
||||||
): Flow<Artist?> = db.libraryDao().artist(sourceId, artistId)
|
): Flow<Artist?> = db.libraryDao().artist(sourceId, artistId)
|
||||||
|
|
||||||
|
fun artistName(
|
||||||
|
sourceId: Long,
|
||||||
|
artistId: String,
|
||||||
|
): Flow<String?> = db.libraryDao().artistName(sourceId, artistId)
|
||||||
|
|
||||||
fun song(
|
fun song(
|
||||||
sourceId: Long,
|
sourceId: Long,
|
||||||
songId: String,
|
songId: String,
|
||||||
|
|||||||
@@ -152,6 +152,7 @@ fun ArtistDetailRoute(
|
|||||||
playbackController: PlaybackController = koinInject(),
|
playbackController: PlaybackController = koinInject(),
|
||||||
) {
|
) {
|
||||||
val artist by viewModel.artist.collectAsStateWithLifecycle(initialValue = null)
|
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 albums by viewModel.albums.collectAsStateWithLifecycle(initialValue = emptyList())
|
||||||
val art by viewModel.art.collectAsStateWithLifecycle()
|
val art by viewModel.art.collectAsStateWithLifecycle()
|
||||||
val artThumbnail by viewModel.artThumbnail.collectAsStateWithLifecycle()
|
val artThumbnail by viewModel.artThumbnail.collectAsStateWithLifecycle()
|
||||||
@@ -184,10 +185,12 @@ fun ArtistDetailRoute(
|
|||||||
)
|
)
|
||||||
ArtistDetailScreen(
|
ArtistDetailScreen(
|
||||||
artist = artist,
|
artist = artist,
|
||||||
|
artistName = artistName.orEmpty(),
|
||||||
|
isAlbumArtist = albums.any { it.artistId == artistId },
|
||||||
albums = albums,
|
albums = albums,
|
||||||
art = art,
|
art = art,
|
||||||
artThumbnail = artThumbnail,
|
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,
|
coverArt = viewModel::coverArt,
|
||||||
onBack = onBack,
|
onBack = onBack,
|
||||||
onAlbumClick = onAlbumClick,
|
onAlbumClick = onAlbumClick,
|
||||||
@@ -213,6 +216,8 @@ fun ArtistDetailRoute(
|
|||||||
@Composable
|
@Composable
|
||||||
fun ArtistDetailScreen(
|
fun ArtistDetailScreen(
|
||||||
artist: Artist?,
|
artist: Artist?,
|
||||||
|
artistName: String = "",
|
||||||
|
isAlbumArtist: Boolean = false,
|
||||||
albums: List<Album>,
|
albums: List<Album>,
|
||||||
art: CoverArtRef?,
|
art: CoverArtRef?,
|
||||||
artThumbnail: CoverArtRef? = null,
|
artThumbnail: CoverArtRef? = null,
|
||||||
@@ -240,6 +245,8 @@ fun ArtistDetailScreen(
|
|||||||
val navBarBottom = with(density) { WindowInsets.navigationBars.getBottom(this).toDp() }
|
val navBarBottom = with(density) { WindowInsets.navigationBars.getBottom(this).toDp() }
|
||||||
val barHeight = statusBarTop + TopAppBarDefaults.TopAppBarExpandedHeight
|
val barHeight = statusBarTop + TopAppBarDefaults.TopAppBarExpandedHeight
|
||||||
val albumCount = albums.size.toLong()
|
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 imageTitleTop = ART_HEIGHT - TITLE_INSET - lineHeight
|
||||||
val fadeStartPx = with(density) { (imageTitleTop - barHeight - FADE_LEAD).toPx() }
|
val fadeStartPx = with(density) { (imageTitleTop - barHeight - FADE_LEAD).toPx() }
|
||||||
val fadeEndPx = with(density) { (imageTitleTop + lineHeight / 2 - barHeight).toPx() }
|
val fadeEndPx = with(density) { (imageTitleTop + lineHeight / 2 - barHeight).toPx() }
|
||||||
@@ -309,7 +316,7 @@ fun ArtistDetailScreen(
|
|||||||
Box(Modifier.fillMaxWidth()) {
|
Box(Modifier.fillMaxWidth()) {
|
||||||
CoverArt(
|
CoverArt(
|
||||||
ref = art,
|
ref = art,
|
||||||
name = artist?.name.orEmpty(),
|
name = displayName,
|
||||||
thumbnailRef = artThumbnail,
|
thumbnailRef = artThumbnail,
|
||||||
showPlaceholder = art == null,
|
showPlaceholder = art == null,
|
||||||
modifier = Modifier.fillMaxWidth().height(ART_HEIGHT),
|
modifier = Modifier.fillMaxWidth().height(ART_HEIGHT),
|
||||||
@@ -331,7 +338,7 @@ fun ArtistDetailScreen(
|
|||||||
}
|
}
|
||||||
Box(Modifier.align(Alignment.BottomStart).fillMaxWidth()) {
|
Box(Modifier.align(Alignment.BottomStart).fillMaxWidth()) {
|
||||||
ArtistTitle(
|
ArtistTitle(
|
||||||
text = artist?.name.orEmpty(),
|
text = displayName,
|
||||||
style = imageNameStyle,
|
style = imageNameStyle,
|
||||||
color = Color.White,
|
color = Color.White,
|
||||||
endReserve = SHUFFLE_RESERVE,
|
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),
|
modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 8.dp, top = 34.dp, bottom = 0.dp),
|
||||||
verticalAlignment = Alignment.CenterVertically,
|
verticalAlignment = Alignment.CenterVertically,
|
||||||
) {
|
) {
|
||||||
|
Column(Modifier.weight(1f)) {
|
||||||
|
Text(
|
||||||
|
text = artistType,
|
||||||
|
style = MaterialTheme.typography.labelLarge,
|
||||||
|
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||||
|
)
|
||||||
Text(
|
Text(
|
||||||
text = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount),
|
text = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount),
|
||||||
style = MaterialTheme.typography.bodyLarge,
|
style = MaterialTheme.typography.bodyLarge,
|
||||||
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
color = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||||
modifier = Modifier.weight(1f),
|
|
||||||
)
|
)
|
||||||
|
}
|
||||||
IconButton(
|
IconButton(
|
||||||
onClick = { onToggleStar?.invoke() },
|
onClick = { onToggleStar?.invoke() },
|
||||||
enabled = onToggleStar != null,
|
enabled = onToggleStar != null,
|
||||||
@@ -452,7 +465,7 @@ fun ArtistDetailScreen(
|
|||||||
TopAppBar(
|
TopAppBar(
|
||||||
title = {
|
title = {
|
||||||
Text(
|
Text(
|
||||||
text = artist?.name.orEmpty(),
|
text = displayName,
|
||||||
maxLines = 1,
|
maxLines = 1,
|
||||||
overflow = TextOverflow.Ellipsis,
|
overflow = TextOverflow.Ellipsis,
|
||||||
modifier = Modifier.graphicsLayer { alpha = barFraction },
|
modifier = Modifier.graphicsLayer { alpha = barFraction },
|
||||||
|
|||||||
@@ -38,6 +38,14 @@ class ArtistDetailViewModel(
|
|||||||
|
|
||||||
val artist: Flow<Artist?> = sourceId.flatMapLatest { libraryRepository.artist(it, artistId) }
|
val artist: Flow<Artist?> = sourceId.flatMapLatest { libraryRepository.artist(it, artistId) }
|
||||||
|
|
||||||
|
val artistName: Flow<String?> =
|
||||||
|
sourceId.flatMapLatest { id ->
|
||||||
|
combine(
|
||||||
|
libraryRepository.artist(id, artistId),
|
||||||
|
libraryRepository.artistName(id, artistId),
|
||||||
|
) { loaded, fromSongs -> loaded?.name?.takeIf { it.isNotBlank() } ?: fromSongs }
|
||||||
|
}
|
||||||
|
|
||||||
val offline: StateFlow<Boolean> = sourceRepository.offline
|
val offline: StateFlow<Boolean> = sourceRepository.offline
|
||||||
|
|
||||||
val albums: Flow<List<Album>> =
|
val albums: Flow<List<Album>> =
|
||||||
|
|||||||
@@ -29,6 +29,8 @@
|
|||||||
<string name="resources_playlist_name">Playlists</string>
|
<string name="resources_playlist_name">Playlists</string>
|
||||||
<string name="resources_filter_album">Album</string>
|
<string name="resources_filter_album">Album</string>
|
||||||
<string name="resources_filter_artist">Artist</string>
|
<string name="resources_filter_artist">Artist</string>
|
||||||
|
<string name="artist_type_artist">Artist</string>
|
||||||
|
<string name="artist_type_album_artist">Album artist</string>
|
||||||
<string name="resources_filter_genre">Genre</string>
|
<string name="resources_filter_genre">Genre</string>
|
||||||
<string name="resources_filter_starred">Starred</string>
|
<string name="resources_filter_starred">Starred</string>
|
||||||
<string name="resources_song_list_delete_all_content">%1$s\n\nDeleting will free %2$s</string>
|
<string name="resources_song_list_delete_all_content">%1$s\n\nDeleting will free %2$s</string>
|
||||||
|
|||||||
@@ -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
|
@Test
|
||||||
fun downloadedFilterReportsTheDownloadedAlbumCount() =
|
fun downloadedFilterReportsTheDownloadedAlbumCount() =
|
||||||
runTest {
|
runTest {
|
||||||
|
|||||||
@@ -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<ComponentActivity>()
|
||||||
|
|
||||||
|
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()
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user