From 5dd6e8dab9946f64d2cf0775ee4b6dfe6fd67852 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Mon, 19 Oct 2026 04:44:26 +0000 Subject: [PATCH] ui: lower the artist actions and drop section counts Move the star and more buttons from the meta line onto the first section header so they sit below the hanging shuffle button again, and remove the per-section counts now that the meta line carries the total. --- .../ui/library/ArtistDetailScreen.kt | 89 ++++++++++++------- app/src/main/res/values/strings.xml | 4 +- .../ui/library/ArtistDetailScreenTest.kt | 9 +- 3 files changed, 63 insertions(+), 39 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 910f88a2..346f0c4a 100644 --- a/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/library/ArtistDetailScreen.kt @@ -254,6 +254,8 @@ 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 "" + val actionsInHeader = albums.albumCredits.isNotEmpty() || albums.appearances.isNotEmpty() + val actions: @Composable () -> Unit = { ArtistActions(artist, starred, onToggleStar, onMore) } var titleHeightPx by remember { mutableFloatStateOf(0f) } val titleHeight = if (titleHeightPx > 0f) with(density) { titleHeightPx.toDp() } else lineHeight val titleMaxLines = @@ -386,7 +388,7 @@ fun ArtistDetailScreen( modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 8.dp, top = TITLE_INSET), ) { Row( - modifier = Modifier.fillMaxWidth().defaultMinSize(minHeight = 48.dp), + modifier = Modifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically, ) { Text( @@ -395,36 +397,14 @@ fun ArtistDetailScreen( color = MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.weight(1f), ) - if (artist != null) { - IconButton( - onClick = { onToggleStar?.invoke() }, - enabled = onToggleStar != null, - ) { - Icon( - imageVector = if (starred) Icons.Rounded.Star else Icons.Rounded.StarBorder, - contentDescription = - stringResource( - if (starred) R.string.actions_unstar else R.string.actions_star, - ), - tint = - if (starred) { - MaterialTheme.colorScheme.primary - } else { - MaterialTheme.colorScheme.onSurfaceVariant - }, - ) - } - IconButton(onClick = onMore) { - Icon(Icons.Rounded.MoreHoriz, contentDescription = stringResource(R.string.more_options)) - } - } + if (!actionsInHeader) actions() } } } } if (albums.albumCredits.isNotEmpty()) { item(span = { GridItemSpan(maxLineSpan) }) { - ArtistSectionHeader(stringResource(R.string.artist_section_albums, albums.albumCredits.size)) + ArtistSectionHeader(stringResource(R.string.artist_section_albums), trailing = actions) } itemsIndexed(albums.albumCredits, key = { _, album -> album.id }) { index, album -> ArtistAlbumCard( @@ -442,7 +422,10 @@ fun ArtistDetailScreen( } if (albums.appearances.isNotEmpty()) { item(span = { GridItemSpan(maxLineSpan) }) { - ArtistSectionHeader(stringResource(R.string.artist_section_appearances, albums.appearances.size)) + ArtistSectionHeader( + stringResource(R.string.artist_section_appearances), + trailing = if (albums.albumCredits.isEmpty()) actions else null, + ) } itemsIndexed(albums.appearances, key = { _, album -> album.id }) { index, album -> ArtistAlbumCard( @@ -501,13 +484,53 @@ fun ArtistDetailScreen( } @Composable -private fun ArtistSectionHeader(text: String) { - Text( - text = text, - style = MaterialTheme.typography.titleMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.fillMaxWidth().padding(start = 16.dp, end = 16.dp), - ) +private fun ArtistSectionHeader( + text: String, + trailing: (@Composable () -> Unit)? = null, +) { + Row( + modifier = Modifier.fillMaxWidth().defaultMinSize(minHeight = 48.dp).padding(start = 16.dp, end = 8.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = text, + style = MaterialTheme.typography.titleMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.weight(1f), + ) + trailing?.invoke() + } +} + +@Composable +private fun ArtistActions( + artist: Artist?, + starred: Boolean, + onToggleStar: (() -> Unit)?, + onMore: () -> Unit, +) { + if (artist == null) return + IconButton( + onClick = { onToggleStar?.invoke() }, + enabled = onToggleStar != null, + ) { + Icon( + imageVector = if (starred) Icons.Rounded.Star else Icons.Rounded.StarBorder, + contentDescription = + stringResource( + if (starred) R.string.actions_unstar else R.string.actions_star, + ), + tint = + if (starred) { + MaterialTheme.colorScheme.primary + } else { + MaterialTheme.colorScheme.onSurfaceVariant + }, + ) + } + IconButton(onClick = onMore) { + Icon(Icons.Rounded.MoreHoriz, contentDescription = stringResource(R.string.more_options)) + } } @Composable diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index d1f5080b..b87ff283 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -35,8 +35,8 @@ Artist Artist Album artist - Albums (%1$d) - Appears on (%1$d) + Albums + Appears on %1$s · %2$s Genre Starred 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 a208f3ca..7d9ed8c4 100644 --- a/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/library/ArtistDetailScreenTest.kt @@ -70,6 +70,7 @@ class ArtistDetailScreenTest { ) composeRule.onNodeWithText("Album artist · 1 album").assertExists() + composeRule.onNode(hasScrollAction()).performScrollToNode(hasText("Albums")) composeRule.onNodeWithContentDescription("More options").assertExists() } @@ -87,12 +88,12 @@ class ArtistDetailScreenTest { composeRule.onNodeWithText("Album artist · 2 albums").assertExists() - composeRule.onNode(hasScrollAction()).performScrollToNode(hasText("Albums (1)")) - composeRule.onNodeWithText("Albums (1)").assertExists() + composeRule.onNode(hasScrollAction()).performScrollToNode(hasText("Albums")) + composeRule.onNodeWithText("Albums").assertExists() composeRule.onNodeWithText("Kid A").assertExists() - composeRule.onNode(hasScrollAction()).performScrollToNode(hasText("Appears on (1)")) - composeRule.onNodeWithText("Appears on (1)").assertExists() + composeRule.onNode(hasScrollAction()).performScrollToNode(hasText("Appears on")) + composeRule.onNodeWithText("Appears on").assertExists() composeRule.onNodeWithText("Guest Album").assertExists() }