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 ad3b3396..1dd94730 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -50,6 +50,7 @@ import androidx.compose.material3.TopAppBar import androidx.compose.material3.TopAppBarDefaults import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableFloatStateOf @@ -152,6 +153,7 @@ private const val ARTIST_NAME_MAX_LINES = 6 private const val THEME_TRANSITION_MS = 100 private const val SCRIM_FADE_MS = 180 internal const val ARTIST_NAME_SCRIM_TAG = "artistNameScrim" +internal const val ARTIST_BIO_MORE_TAG = "artistBioMore" @Composable fun ArtistDetailRoute( @@ -168,6 +170,10 @@ fun ArtistDetailRoute( val artist by viewModel.artist.collectAsStateWithLifecycle(initialValue = null) val artistName by viewModel.artistName.collectAsStateWithLifecycle(initialValue = null) val albums by viewModel.albums.collectAsStateWithLifecycle(initialValue = null) + val artistBio by viewModel.artistBio.collectAsStateWithLifecycle(initialValue = null) + val bioLoading by viewModel.bioLoading.collectAsStateWithLifecycle() + val bioExpanded by viewModel.bioExpanded.collectAsStateWithLifecycle() + LaunchedEffect(artistId) { viewModel.refreshBio() } val art by viewModel.art.collectAsStateWithLifecycle() val artThumbnail by viewModel.artThumbnail.collectAsStateWithLifecycle() val downloadStatus by viewModel.downloadStatus.collectAsStateWithLifecycle() @@ -202,6 +208,9 @@ fun ArtistDetailRoute( artistName = artistName.orEmpty(), albums = albums ?: ArtistAlbums(), albumsLoaded = albums != null, + bio = artistBio?.biography, + bioLoading = bioLoading && artistBio == null, + bioExpanded = bioExpanded, art = art, artThumbnail = artThumbnail, artwork = rememberArtworkColors(shortcutArt ?: artThumbnail ?: art, THEME_TRANSITION_MS, fallbackName = artistName), @@ -213,6 +222,7 @@ fun ArtistDetailRoute( onMore = { artist?.let { contextMenuHost?.show(MenuTarget.Artist(it, artThumbnail ?: art, downloadStatus), actions) } }, starred = artist?.starred != null, onToggleStar = artist?.let { a -> { setStar(StarType.Artist, a.id, a.starred == null) } }, + onExpandBio = viewModel::expandBio, onShuffle = viewModel::shuffle, ) @@ -234,6 +244,9 @@ fun ArtistDetailScreen( artistName: String = "", albums: ArtistAlbums = ArtistAlbums(), albumsLoaded: Boolean = true, + bio: String? = null, + bioLoading: Boolean = false, + bioExpanded: Boolean = false, art: CoverArtRef?, artThumbnail: CoverArtRef? = null, artwork: ArtworkColors? = null, @@ -245,6 +258,7 @@ fun ArtistDetailScreen( onMore: () -> Unit = {}, starred: Boolean = false, onToggleStar: (() -> Unit)? = null, + onExpandBio: () -> Unit = {}, onShuffle: () -> Unit = {}, ) { val listState = rememberLazyGridState() @@ -266,12 +280,14 @@ fun ArtistDetailScreen( ) val albumCountText = pluralStringResource(R.plurals.resources_album_count, albumCount.toInt(), albumCount) val metaText = if (albumsLoaded) stringResource(R.string.artist_meta, artistType, albumCountText) else "" - // No server biography is mirrored yet, so the page ships with the empty state. When the bio is - // wired, set bioIsEmpty and bioText from it and use bioCollapsed for the two-line preview; the - // collapsed line ends with a bold More that should expand the block. - val bioCollapsed = true - val bioIsEmpty = true - val bioText = stringResource(R.string.artist_bio_empty) + val hasBio = !bio.isNullOrBlank() + val bioCollapsed = hasBio && !bioExpanded + val bioText = + when { + hasBio -> bio.orEmpty() + bioLoading -> stringResource(R.string.artist_bio_loading) + else -> stringResource(R.string.artist_bio_empty) + } var titleHeightPx by remember { mutableFloatStateOf(0f) } val titleHeight = if (titleHeightPx > 0f) with(density) { titleHeightPx.toDp() } else lineHeight val titleMaxLines = @@ -412,19 +428,20 @@ fun ArtistDetailScreen( ArtistBio( text = bioText, color = - if (bioIsEmpty) { - MaterialTheme.colorScheme.outline - } else { + if (hasBio) { MaterialTheme.colorScheme.onSurfaceVariant + } else { + MaterialTheme.colorScheme.outline }, collapsed = bioCollapsed, artist = artist, starred = starred, onToggleStar = onToggleStar, onMore = onMore, + onExpand = onExpandBio, modifier = Modifier.padding( - bottom = if (!bioCollapsed && !bioIsEmpty) 8.dp else 0.dp, + bottom = if (hasBio && !bioCollapsed) 8.dp else 0.dp, ), ) } @@ -509,7 +526,7 @@ fun ArtistDetailScreen( } // The expanded layout narrows only the lines that overlap the action buttons; later lines use the -// full width. Collapsed keeps two lines and appends a bold More, not yet wired to expand. +// full width. @Composable private fun ArtistBio( text: String, @@ -519,6 +536,7 @@ private fun ArtistBio( starred: Boolean, onToggleStar: (() -> Unit)?, onMore: () -> Unit, + onExpand: () -> Unit, modifier: Modifier = Modifier, ) { val measurer = rememberTextMeasurer() @@ -533,25 +551,35 @@ private fun ArtistBio( measurer.measure(AnnotatedString("X"), style, maxLines = 1).size.height }.coerceAtLeast(1) val reserveLines = ceil(with(density) { BIO_ACTION_HEIGHT.toPx() } / lineHeightPx).toInt().coerceAtLeast(1) - val lines = + val bioLines = remember(text, fullWidth, reserve, reserveLines, collapsed, moreText) { if (collapsed) { splitBioCollapsed(measurer, text, style, fullWidth, reserve, moreText) } else { - splitBio(measurer, text, style, fullWidth, reserve, reserveLines).map { - AnnotatedString(it.ifEmpty { " " }) - } + BioLines( + splitBio(measurer, text, style, fullWidth, reserve, reserveLines).map { + AnnotatedString(it.ifEmpty { " " }) + }, + more = false, + ) } } Box { Column(Modifier.fillMaxWidth()) { - lines.forEach { line -> + bioLines.lines.forEachIndexed { index, line -> + val more = bioLines.more && index == bioLines.lines.lastIndex Text( text = line, style = style, color = color, maxLines = 1, overflow = TextOverflow.Ellipsis, + modifier = + if (more) { + Modifier.testTag(ARTIST_BIO_MORE_TAG).clickable(onClick = onExpand) + } else { + Modifier + }, ) } } @@ -597,7 +625,12 @@ internal fun splitBio( return lines } -/** Collapses text to two lines around the actions, ending with a bold More. */ +internal data class BioLines( + val lines: List, + val more: Boolean, +) + +/** Collapses text to two lines, appending a bold More only when it does not fit. */ internal fun splitBioCollapsed( measurer: TextMeasurer, text: String, @@ -605,14 +638,14 @@ internal fun splitBioCollapsed( fullWidth: Int, reserve: Int, moreText: String, -): List { +): BioLines { val words = text .replace("\n\n", " ") .replace('\n', ' ') .split(' ') .filter { it.isNotEmpty() } - if (words.isEmpty()) return listOf(AnnotatedString("")) + if (words.isEmpty()) return BioLines(listOf(AnnotatedString("")), more = false) val available = (fullWidth - reserve).coerceAtLeast(1) var first = "" var index = 0 @@ -625,31 +658,51 @@ internal fun splitBioCollapsed( break } } - if (index >= words.size) return listOf(AnnotatedString(first)) + if (index >= words.size) return BioLines(listOf(AnnotatedString(first)), more = false) + + val secondStart = index + var second = "" + var secondIndex = secondStart + while (secondIndex < words.size) { + val candidate = if (second.isEmpty()) words[secondIndex] else "$second ${words[secondIndex]}" + if (measurer.measure(AnnotatedString(candidate), style, maxLines = 1).size.width <= available) { + second = candidate + secondIndex++ + } else { + break + } + } + if (secondIndex >= words.size) { + return BioLines(listOf(AnnotatedString(first), AnnotatedString(second)), more = false) + } + val more = buildAnnotatedString { append("… ") withStyle(SpanStyle(fontWeight = FontWeight.Bold)) { append(moreText) } } - val moreWidth = measurer.measure(more, style, maxLines = 1).size.width + val moreWidth = + measurer.measure(more, style, maxLines = 1).size.width + + measurer.measure(AnnotatedString(" "), style, maxLines = 1).size.width val secondAvailable = (available - moreWidth).coerceAtLeast(1) - var second = "" - while (index < words.size) { - val candidate = if (second.isEmpty()) words[index] else "$second ${words[index]}" + var truncated = "" + var truncatedIndex = secondStart + while (truncatedIndex < words.size) { + val candidate = if (truncated.isEmpty()) words[truncatedIndex] else "$truncated ${words[truncatedIndex]}" if (measurer.measure(AnnotatedString(candidate), style, maxLines = 1).size.width <= secondAvailable) { - second = candidate - index++ + truncated = candidate + truncatedIndex++ } else { break } } val secondLine = buildAnnotatedString { - if (second.isNotEmpty()) append("$second ") + if (truncated.isNotEmpty()) append("$truncated ") append("… ") withStyle(SpanStyle(fontWeight = FontWeight.Bold)) { append(moreText) } } - return listOf(AnnotatedString(first), secondLine) + return BioLines(listOf(AnnotatedString(first), secondLine), more = true) } @Composable 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 faedd579..636db0a3 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailViewModel.kt @@ -4,6 +4,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.subtracks.data.model.Artist import com.subtracks.data.model.ArtistAlbums +import com.subtracks.data.model.ArtistBio import com.subtracks.data.model.BulkDownloadAction import com.subtracks.data.model.CoverArtRef import com.subtracks.data.model.DownloadList @@ -13,6 +14,7 @@ import com.subtracks.data.repo.LibraryRepository import com.subtracks.data.repo.SourceRepository import com.subtracks.playback.PlaybackController import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow @@ -66,6 +68,42 @@ class ArtistDetailViewModel( .flatMapLatest { downloadRepository.statuses(it, DownloadList.Album) } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyMap()) + val artistBio: Flow = sourceId.flatMapLatest { libraryRepository.artistBio(it, artistId) } + + private val _bioLoading = MutableStateFlow(true) + val bioLoading: StateFlow = _bioLoading + + private val _bioExpanded = MutableStateFlow(false) + val bioExpanded: StateFlow = _bioExpanded + + private var bioRefresh: Job? = null + + fun refreshBio() { + if (bioRefresh?.isActive == true) return + bioRefresh = + viewModelScope.launch { + if (sourceRepository.offline.value) { + _bioLoading.value = false + return@launch + } + val id = sourceRepository.activeSourceIdOnce() + if (id == null) { + _bioLoading.value = false + return@launch + } + _bioLoading.value = true + try { + libraryRepository.refreshArtistBio(id, artistId) + } finally { + _bioLoading.value = false + } + } + } + + fun expandBio() { + _bioExpanded.value = true + } + suspend fun downloadedBytes( list: DownloadList, refId: String, diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 5c7449b0..06d4ce00 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -38,6 +38,7 @@ Albums Appears on No artist bio available. + Loading artist bio… More %1$s · %2$s Genre diff --git a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt index 1036cb9f..67088ee3 100644 --- a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt @@ -303,6 +303,25 @@ class AppScreenshotTest { @Test fun artistDetail() { + renderArtistDetail(bioExpanded = false) + awaitText("Kid A") + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodesWithTag(ARTIST_NAME_SCRIM_TAG).fetchSemanticsNodes().isNotEmpty() + } + composeRule.onRoot().captureRoboImage("src/test/screenshots/artist_detail.png") + } + + @Test + fun artistDetailExpandedBio() { + renderArtistDetail(bioExpanded = true) + awaitText("Kid A") + composeRule.waitUntil(timeoutMillis = 5_000) { + composeRule.onAllNodesWithTag(ARTIST_NAME_SCRIM_TAG).fetchSemanticsNodes().isNotEmpty() + } + composeRule.onRoot().captureRoboImage("src/test/screenshots/artist_detail_bio_expanded.png") + } + + private fun renderArtistDetail(bioExpanded: Boolean) { composeRule.setContent { SubtracksTheme { ArtistDetailScreen( @@ -311,6 +330,11 @@ class AppScreenshotTest { ArtistAlbums( albumCredits = Fixtures.albums.filter { it.artistId == Fixtures.artists.first().id }, ), + bio = + "Radiohead are an English rock band formed in Abingdon, Oxfordshire, in 1985. " + + "The band consists of Thom Yorke, Jonny Greenwood, Colin Greenwood, Ed O'Brien, " + + "and Philip Selway.", + bioExpanded = bioExpanded, art = CoverArtRef("art-ar-radiohead", "test:art-ar-radiohead"), artwork = artworkColorsFromSeed(Color.rgb(120, 80, 200)), coverArt = { id, _ -> id?.let { CoverArtRef(it, "test:$it") } }, @@ -320,11 +344,6 @@ class AppScreenshotTest { ) } } - awaitText("Kid A") - composeRule.waitUntil(timeoutMillis = 5_000) { - composeRule.onAllNodesWithTag(ARTIST_NAME_SCRIM_TAG).fetchSemanticsNodes().isNotEmpty() - } - composeRule.onRoot().captureRoboImage("src/test/screenshots/artist_detail.png") } @Test diff --git a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt index 6c73e01a..14d72ecd 100644 --- a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt @@ -1,6 +1,10 @@ package com.subtracks.ui.library import androidx.activity.ComponentActivity +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.hasScrollAction import androidx.compose.ui.test.hasText @@ -8,13 +12,20 @@ import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule import androidx.compose.ui.test.onAllNodesWithContentDescription import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithContentDescription +import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performScrollTo import androidx.compose.ui.test.performScrollToNode +import androidx.compose.ui.text.TextStyle +import androidx.compose.ui.text.rememberTextMeasurer +import androidx.compose.ui.unit.sp import androidx.test.ext.junit.runners.AndroidJUnit4 import com.subtracks.data.model.Album import com.subtracks.data.model.Artist import com.subtracks.data.model.ArtistAlbums import com.subtracks.ui.theme.SubtracksTheme +import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith @@ -31,18 +42,25 @@ class ArtistDetailScreenTest { artistName: String, albums: ArtistAlbums = ArtistAlbums(), albumsLoaded: Boolean = true, + bio: String? = null, + bioLoading: Boolean = false, ) { composeRule.setContent { + var expanded by remember { mutableStateOf(false) } SubtracksTheme { ArtistDetailScreen( artist = artist, artistName = artistName, albums = albums, albumsLoaded = albumsLoaded, + bio = bio, + bioLoading = bioLoading, + bioExpanded = expanded, art = null, coverArt = { _, _ -> null }, onBack = {}, onAlbumClick = {}, + onExpandBio = { expanded = true }, ) } } @@ -108,6 +126,98 @@ class ArtistDetailScreenTest { composeRule.onNodeWithText("Album artist · 0 albums").assertDoesNotExist() } + @Test + fun aStoredBioShowsImmediatelyEvenWhileItRefreshes() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + bio = "A stored biography.", + bioLoading = true, + ) + + composeRule.onNodeWithText("A stored biography.").assertExists() + composeRule.onNodeWithText("Loading artist bio…").assertDoesNotExist() + } + + @Test + fun aMissingBioShowsTheLoadingStateFirst() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + bioLoading = true, + ) + + composeRule.onNodeWithText("Loading artist bio…").assertExists() + composeRule.onNodeWithText("No artist bio available.").assertDoesNotExist() + } + + @Test + fun aMissingBioShowsTheEmptyStateAfterTheRefresh() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + ) + + composeRule.onNodeWithText("No artist bio available.").assertExists() + composeRule.onNodeWithText("Loading artist bio…").assertDoesNotExist() + } + + @Test + fun aShortBioDoesNotOfferMore() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + bio = "A short biography.", + ) + + composeRule.onNodeWithTag(ARTIST_BIO_MORE_TAG).assertDoesNotExist() + } + + @Test + fun aLongBioExpandsWhenMoreIsTapped() { + render( + artist = Artist(sourceId = 1, id = "ar-1", name = "Radiohead", albumCount = 1, starred = null), + artistName = "Radiohead", + bio = + "Radiohead are an English rock band formed in Abingdon, Oxfordshire, in 1985. " + + "The band consists of Thom Yorke, Jonny Greenwood, Colin Greenwood, Ed O'Brien, " + + "and Philip Selway.", + ) + + composeRule.onNodeWithTag(ARTIST_BIO_MORE_TAG).performScrollTo().assertExists() + + composeRule.onNodeWithTag(ARTIST_BIO_MORE_TAG).performClick() + + composeRule.onNodeWithTag(ARTIST_BIO_MORE_TAG).assertDoesNotExist() + } + + @Test + fun theCollapsedSecondLineContinuesWhereTheFirstLineEnds() { + val bio = + "Radiohead are an English rock band formed in Abingdon, Oxfordshire, in 1985. " + + "The band consists of Thom Yorke, Jonny Greenwood, Colin Greenwood, Ed O'Brien, " + + "and Philip Selway." + var result: BioLines? = null + composeRule.setContent { + val measurer = rememberTextMeasurer() + result = splitBioCollapsed(measurer, bio, TextStyle(fontSize = 14.sp), 400, 0, "More") + } + composeRule.waitForIdle() + + val lines = requireNotNull(result) + assertTrue(lines.more) + val firstWords = + lines.lines[0] + .text + .split(' ') + .filter { it.isNotEmpty() } + val next = bio.split(' ').filter { it.isNotEmpty() }[firstWords.size] + assertTrue( + "expected the second line to continue with $next, was ${lines.lines[1].text}", + lines.lines[1].text.startsWith(next), + ) + } + private fun album( id: String, name: String,