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 d4d09719..5ad0a80c 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/ItemContextMenu.kt @@ -80,7 +80,7 @@ sealed interface MenuTarget { ) : MenuTarget { override val title: String get() = song.title - override val subtitle: String? get() = song.artist + override val subtitle: String? get() = song.artistSubtitle() } data class Album( diff --git a/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt b/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt new file mode 100644 index 00000000..18965cad --- /dev/null +++ b/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt @@ -0,0 +1,15 @@ +package com.subtracks.ui.components + +import com.subtracks.data.model.Song + +private const val ARTIST_SEPARATOR = " \u00B7 " + +internal fun Song.artistSubtitle(): String? { + val names = artists.map { it.name }.filter { it.isNotBlank() }.distinct() + if (names.isEmpty()) return artist?.takeIf { it.isNotBlank() } + val primary = artist?.takeIf { it in names } + return buildList { + primary?.let(::add) + addAll(names.filterNot { it == primary }) + }.joinToString(ARTIST_SEPARATOR) +} diff --git a/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt b/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt index 1b99012a..feafd8d1 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt @@ -54,7 +54,7 @@ fun SongInfoDialog( text = { Column(verticalArrangement = Arrangement.spacedBy(6.dp)) { InfoRow(stringResource(R.string.resources_sort_by_title), song.title) - InfoRow(stringResource(R.string.resources_filter_artist), song.artist) + InfoRow(stringResource(R.string.resources_filter_artist), song.artistSubtitle()) InfoRow(stringResource(R.string.resources_filter_album), song.album) InfoRow(stringResource(R.string.info_duration), song.duration?.let(DateUtils::formatElapsedTime)) InfoRow( diff --git a/app/src/main/kotlin/com/subtracks/ui/library/SongsContent.kt b/app/src/main/kotlin/com/subtracks/ui/library/SongsContent.kt index 1b835107..9848e289 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/SongsContent.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/SongsContent.kt @@ -61,6 +61,7 @@ import com.subtracks.data.model.DownloadStatus import com.subtracks.data.model.Song import com.subtracks.data.model.SongDownload import com.subtracks.ui.components.CoverArt +import com.subtracks.ui.components.artistSubtitle private val TRACK_COLUMN_WIDTH = 24.dp private val TIME_COLUMN_WIDTH = 36.dp @@ -146,7 +147,7 @@ fun SongRow( }, supportingContent = { Text( - text = song.artist.orEmpty().ifEmpty { "\u00A0" }, + text = song.artistSubtitle().orEmpty().ifEmpty { "\u00A0" }, color = if (isPlaying) MaterialTheme.colorScheme.primary else MaterialTheme.colorScheme.onSurfaceVariant, maxLines = 1, overflow = TextOverflow.Ellipsis, diff --git a/app/src/test/kotlin/com/subtracks/ui/components/SongArtistDisplayTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/SongArtistDisplayTest.kt new file mode 100644 index 00000000..443e57dc --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/ui/components/SongArtistDisplayTest.kt @@ -0,0 +1,67 @@ +package com.subtracks.ui.components + +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.data.model.ArtistRef +import com.subtracks.data.model.Song +import com.subtracks.ui.library.SongRow +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 SongArtistDisplayTest { + @get:Rule + val composeRule = createAndroidComposeRule() + + private val song = + Song( + sourceId = 1, + id = "s1", + albumId = "al1", + artistId = "ar2", + title = "Song", + album = "Album", + artist = "Two", + duration = 100, + track = 1, + disc = 1, + starred = null, + artists = listOf(ArtistRef("ar1", "One"), ArtistRef("ar2", "Two"), ArtistRef("ar3", "Three")), + ) + + @Test + fun aSongRowListsThePluralArtistsWithTheSingularArtistFirst() { + composeRule.setContent { + SubtracksTheme { + SongRow(song = song) + } + } + + composeRule.onNodeWithText("Two · One · Three").assertIsDisplayed() + } + + @Test + fun theInfoDialogListsThePluralArtistsWithTheSingularArtistFirst() { + composeRule.setContent { + SubtracksTheme { + SongInfoDialog( + song = song, + download = null, + localFile = null, + streamEncoding = null, + onDismiss = {}, + genres = emptyList(), + ) + } + } + + composeRule.onNodeWithText("Two · One · Three").assertIsDisplayed() + } +} diff --git a/app/src/test/kotlin/com/subtracks/ui/library/SongArtistSubtitleTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/SongArtistSubtitleTest.kt new file mode 100644 index 00000000..31d3c34c --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/ui/library/SongArtistSubtitleTest.kt @@ -0,0 +1,52 @@ +package com.subtracks.ui.library + +import com.subtracks.data.model.ArtistRef +import com.subtracks.data.model.Song +import com.subtracks.ui.components.artistSubtitle +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +class SongArtistSubtitleTest { + @Test + fun pluralArtistsAreJoinedWithDots() { + assertEquals("One · Two", song(artist = null, artists = listOf(ArtistRef("1", "One"), ArtistRef("2", "Two"))).artistSubtitle()) + } + + @Test + fun singularArtistIsListedFirstWhenItIsAlsoPlural() { + assertEquals( + "Two · One · Three", + song(artist = "Two", artists = listOf(ArtistRef("1", "One"), ArtistRef("2", "Two"), ArtistRef("3", "Three"))) + .artistSubtitle(), + ) + } + + @Test + fun singularArtistIsUsedWhenNoPluralArtistsExist() { + assertEquals("One", song(artist = "One", artists = emptyList()).artistSubtitle()) + } + + @Test + fun noArtistGivesNoSubtitle() { + assertNull(song(artist = null, artists = emptyList()).artistSubtitle()) + } + + private fun song( + artist: String?, + artists: List, + ) = Song( + sourceId = 1, + id = "s1", + albumId = "al1", + artistId = "ar1", + title = "Song", + album = "Album", + artist = artist, + duration = 100, + track = 1, + disc = 1, + starred = null, + artists = artists, + ) +}