From 889bdb452b1766715ada1db13eeb97c1001d654e Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 19 Oct 2026 09:22:34 +0000 Subject: [PATCH] ui: show and expand the artist biography Load the biography when the artist page is entered. A stored biography is rendered immediately while it refreshes in the background; otherwise the page starts with the loading message and falls back to the empty state. Collapse a biography to two lines and append a bold More only when the text does not fit. Tapping that line expands the biography, after which the first lines keep clear of the star and overflow buttons and the rest use the full width. --- .../ui/library/ArtistDetailScreen.kt | 109 ++++++++++++----- .../ui/library/ArtistDetailViewModel.kt | 38 ++++++ app/src/main/res/values/strings.xml | 1 + .../com/subtracks/ui/AppScreenshotTest.kt | 29 ++++- .../ui/library/ArtistDetailScreenTest.kt | 110 ++++++++++++++++++ 5 files changed, 254 insertions(+), 33 deletions(-) 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,