From 17a41fd5796faaebb9058daca4ac150871bd9717 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 27 Sep 2026 01:57:52 +0000 Subject: [PATCH] give the album detail artist line a full touch target The subtitle was a bare bodyMedium line: 20dp tall and only as wide as the text. It now sits in a full-width target spanning the 4dp lead-in and the 20dp gap below it, so the row clears 48dp while the text stays where it was. The album title is deliberately not part of it, unlike now playing: it can wrap to two lines, so tapping it should not jump to the artist. The controls row moves down 4dp, the only layout delta. AlbumDetailTapsTest covers the tap, the 48dp minimum (fails on a 20dp node) and that the title carries no click action. --- .../ui/components/HeroDetailScaffold.kt | 55 +++++++----- .../ui/library/AlbumDetailTapsTest.kt | 84 +++++++++++++++++++ 2 files changed, 120 insertions(+), 19 deletions(-) create mode 100644 app/src/test/kotlin/com/subtracks/ui/library/AlbumDetailTapsTest.kt diff --git a/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt b/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt index e32a33fc..309fc358 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt @@ -14,6 +14,7 @@ import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.navigationBars import androidx.compose.foundation.layout.offset import androidx.compose.foundation.layout.padding @@ -187,6 +188,22 @@ fun HeroDetailScaffold( } } +@Composable +private fun HeroSubtitle( + text: String, + modifier: Modifier = Modifier, +) { + Text( + text = text, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + textAlign = TextAlign.Center, + maxLines = 2, + overflow = TextOverflow.Ellipsis, + modifier = modifier, + ) +} + @Composable fun HeroHeader( art: CoverArtRef?, @@ -227,30 +244,30 @@ fun HeroHeader( maxLines = 2, overflow = TextOverflow.Ellipsis, ) - if (subtitle.isNotEmpty()) { + val subtitleIsTarget = onSubtitleClick != null && subtitle.isNotEmpty() && comment.isNullOrBlank() + if (subtitleIsTarget) { + // The target spans the 4dp lead-in and the 20dp gap that would sit below the + // line, so the text stays put and the row clears 48dp; without those the + // subtitle alone is a single 20dp line. + Box( + modifier = + Modifier + .fillMaxWidth() + .heightIn(min = 48.dp) + .clickable(onClickLabel = "Open artist") { onSubtitleClick?.invoke() }, + contentAlignment = Alignment.TopCenter, + ) { + HeroSubtitle(subtitle, Modifier.padding(top = 4.dp)) + } + } else if (subtitle.isNotEmpty()) { Spacer(Modifier.height(4.dp)) - Text( - text = subtitle, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - textAlign = TextAlign.Center, - maxLines = 2, - overflow = TextOverflow.Ellipsis, - modifier = if (onSubtitleClick != null) Modifier.clickable { onSubtitleClick() } else Modifier, - ) + HeroSubtitle(subtitle) } if (!comment.isNullOrBlank()) { Spacer(Modifier.height(4.dp)) - Text( - text = comment, - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - textAlign = TextAlign.Center, - maxLines = 2, - overflow = TextOverflow.Ellipsis, - ) + HeroSubtitle(comment) } - Spacer(Modifier.height(20.dp)) + if (!subtitleIsTarget) Spacer(Modifier.height(20.dp)) Row( modifier = controlsModifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically, diff --git a/app/src/test/kotlin/com/subtracks/ui/library/AlbumDetailTapsTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/AlbumDetailTapsTest.kt new file mode 100644 index 00000000..9fea2113 --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/ui/library/AlbumDetailTapsTest.kt @@ -0,0 +1,84 @@ +package com.subtracks.ui.library + +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.assertCountEquals +import androidx.compose.ui.test.assertHeightIsAtLeast +import androidx.compose.ui.test.filter +import androidx.compose.ui.test.hasClickAction +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithText +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.unit.dp +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.data.model.Album +import com.subtracks.data.model.CoverArtRef +import com.subtracks.ui.theme.SubtracksTheme +import org.junit.Assert.assertTrue +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 AlbumDetailTapsTest { + @get:Rule + val composeRule = createAndroidComposeRule() + + private val album = + Album( + sourceId = 1, + id = "al-kid-a", + artistId = "ar-radiohead", + name = "Kid A", + albumArtist = "Radiohead", + created = 0, + coverArt = "art-al-kid-a", + genre = null, + year = 2000, + starred = null, + songCount = 1, + ) + + private fun render(onArtistClick: (() -> Unit)? = null) { + composeRule.setContent { + SubtracksTheme { + AlbumDetailScreen( + album = album, + songs = emptyList(), + coverArt = { id, _ -> id?.let { CoverArtRef(it, "test:$it") } }, + artwork = null, + onBack = {}, + onSongClick = {}, + onArtistClick = onArtistClick, + ) + } + } + } + + @Test + fun tappingTheArtistOpensTheArtist() { + var opened = false + render(onArtistClick = { opened = true }) + + composeRule.onNodeWithText("Radiohead \u00B7 2000").performClick() + + assertTrue("the artist name should open the artist", opened) + } + + @Test + fun theArtistTargetIsAtLeastTheMinimumTouchSize() { + render(onArtistClick = {}) + + composeRule.onNodeWithText("Radiohead \u00B7 2000").assertHeightIsAtLeast(48.dp) + } + + @Test + fun theAlbumTitleIsNotATapTarget() { + render(onArtistClick = {}) + + // The app bar repeats the album name, so match the hero title by having no click. + composeRule.onAllNodesWithText("Kid A").filter(hasClickAction().not()).assertCountEquals(1) + } +}