diff --git a/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt b/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt index 18945502..4adc6bfe 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/SongArtist.kt @@ -9,6 +9,14 @@ internal fun Song.artistSubtitle(): String? = artistSubtitle(artist, artists) internal fun Song.artistNames(): List = artistNames(artist, artists) +internal fun Song.artistList(): String? = + artists + .map { it.name } + .filter { it.isNotBlank() } + .distinct() + .joinToString(ARTIST_SEPARATOR) + .takeIf { it.isNotEmpty() } + internal fun artistNames( artist: String?, artists: List, 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 1d8c583b..defe15fb 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/SongInfoDialog.kt @@ -46,20 +46,13 @@ fun SongInfoDialog( title = { Text(stringResource(R.string.info)) }, text = { Column(verticalArrangement = Arrangement.spacedBy(6.dp)) { - val multipleArtists = song.artistNames().size > 1 - val regularArtistSort = song.artist?.takeIf { it.isNotBlank() } ?: song.artistRefs.firstOrNull()?.name + val artist = song.artist?.takeIf { it.isNotBlank() } + val regularArtistSort = artist ?: song.artists.firstOrNull()?.name InfoRow(stringResource(R.string.resources_sort_by_title), song.title) InfoRow(stringResource(R.string.info_sort_title), song.sortName.takeIfDifferentFrom(song.title)) - InfoRow( - stringResource( - if (multipleArtists) R.string.info_artists else R.string.resources_filter_artist, - ), - song.artistSubtitle(), - ) - InfoRow( - stringResource(if (multipleArtists) R.string.info_artists_sort else R.string.info_artist_sort), - song.artistSort.takeIfDifferentFrom(regularArtistSort), - ) + InfoRow(stringResource(R.string.resources_filter_artist), artist) + InfoRow(stringResource(R.string.info_artists), song.artistList().takeIfDifferentFrom(artist)) + InfoRow(stringResource(R.string.info_artist_sort), song.artistSort.takeIfDifferentFrom(regularArtistSort)) 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/res/values/strings.xml b/app/src/main/res/values/strings.xml index ef193ebf..1aafdf47 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -198,7 +198,6 @@ Channels Close Artists - Artists sort Album artists Album artist sort Sort title diff --git a/app/src/test/kotlin/com/subtracks/ui/components/LibraryInfoDialogTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/LibraryInfoDialogTest.kt index 878f03e8..e327406e 100644 --- a/app/src/test/kotlin/com/subtracks/ui/components/LibraryInfoDialogTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/components/LibraryInfoDialogTest.kt @@ -215,7 +215,7 @@ class LibraryInfoDialogTest { } @Test - fun aSingleArtistTrackUsesSingularLabels() { + fun aSingleArtistTrackHidesTheDuplicateArtistsRow() { composeRule.setContent { SubtracksTheme { SongInfoDialog( @@ -236,7 +236,7 @@ class LibraryInfoDialogTest { } @Test - fun duplicateAndBlankArtistRefsStaySingular() { + fun duplicateAndBlankArtistRefsDoNotAddAnArtistsRow() { composeRule.setContent { SubtracksTheme { SongInfoDialog( @@ -259,7 +259,7 @@ class LibraryInfoDialogTest { } @Test - fun aMixedArtistTrackUsesPluralLabelsForBothRows() { + fun aMixedArtistTrackShowsBothTheArtistAndArtistsRows() { composeRule.setContent { SubtracksTheme { SongInfoDialog( @@ -277,10 +277,12 @@ class LibraryInfoDialogTest { } } + composeRule.onNodeWithText("Artist").assertIsDisplayed() composeRule.onNodeWithText("Artists").assertIsDisplayed() - composeRule.onNodeWithText("Artists sort").assertIsDisplayed() - composeRule.onNodeWithText("Artist").assertDoesNotExist() - composeRule.onNodeWithText("Artist sort").assertDoesNotExist() + composeRule.onNodeWithText("Artist A").assertIsDisplayed() + composeRule.onNodeWithText("Artist A · Artist B").assertIsDisplayed() + composeRule.onNodeWithText("Artist sort").assertIsDisplayed() + composeRule.onNodeWithText("Artists sort").assertDoesNotExist() composeRule.onNodeWithText("Alpha").assertIsDisplayed() } @@ -304,7 +306,7 @@ class LibraryInfoDialogTest { } @Test - fun aMixedArtistTrackWithoutASortShowsOnlyTheArtistsRow() { + fun aMixedArtistTrackWithoutASortStillShowsBothArtistRows() { composeRule.setContent { SubtracksTheme { SongInfoDialog( @@ -322,9 +324,11 @@ class LibraryInfoDialogTest { } } + composeRule.onNodeWithText("Artist").assertIsDisplayed() composeRule.onNodeWithText("Artists").assertIsDisplayed() + composeRule.onNodeWithText("Artist A").assertIsDisplayed() composeRule.onNodeWithText("Artist A · Artist B").assertIsDisplayed() - composeRule.onNodeWithText("Artists sort").assertDoesNotExist() + composeRule.onNodeWithText("Artist sort").assertDoesNotExist() } @Test diff --git a/app/src/test/kotlin/com/subtracks/ui/components/SongArtistDisplayTest.kt b/app/src/test/kotlin/com/subtracks/ui/components/SongArtistDisplayTest.kt index 443e57dc..9fe7cdcd 100644 --- a/app/src/test/kotlin/com/subtracks/ui/components/SongArtistDisplayTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/components/SongArtistDisplayTest.kt @@ -48,7 +48,7 @@ class SongArtistDisplayTest { } @Test - fun theInfoDialogListsThePluralArtistsWithTheSingularArtistFirst() { + fun theInfoDialogShowsTheSingularArtistAndThePluralArtistsSeparately() { composeRule.setContent { SubtracksTheme { SongInfoDialog( @@ -62,6 +62,7 @@ class SongArtistDisplayTest { } } - composeRule.onNodeWithText("Two · One · Three").assertIsDisplayed() + composeRule.onNodeWithText("Two").assertIsDisplayed() + composeRule.onNodeWithText("One · Two · Three").assertIsDisplayed() } }