From 07fb07762c45e0b8260c4b7946881df5d9eae311 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 27 Sep 2026 02:15:43 +0000 Subject: [PATCH] keep the artist targets narrow and clear of the controls Both targets were too greedy. They now hug the text instead of spanning the row, so a tap beside the line does nothing, and the album detail target stops short of the controls instead of reaching down to them. Album detail: the line plus its 4dp lead-in and 8dp below (32dp), with the original spacing kept, so the layout is exactly where it was. Now playing keeps the title+artist block (58dp) but is no longer full width. Both are shorter than the 48dp recommendation on purpose, to stay clear of the slider and the play/shuffle buttons. Tests assert the measured geometry: no full-width target, a 12dp gap above the controls, and the now playing block staying at its own 58dp. --- .../ui/components/HeroDetailScaffold.kt | 11 +++--- .../subtracks/ui/playback/NowPlayingScreen.kt | 1 - .../ui/library/AlbumDetailTapsTest.kt | 35 +++++++++++++++++-- .../ui/playback/NowPlayingTapsTest.kt | 23 ++++++++++++ 4 files changed, 61 insertions(+), 9 deletions(-) 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 309fc358..7cbe6e24 100644 --- a/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt +++ b/app/src/main/kotlin/com/subtracks/ui/components/HeroDetailScaffold.kt @@ -246,14 +246,13 @@ fun HeroHeader( ) 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. + // The line plus the 4dp lead-in above and 8dp below: shorter than the 48dp + // recommendation, deliberately, so it stays 12dp clear of the controls rather + // than reaching down to them. Hugs the text instead of the row. Box( modifier = Modifier - .fillMaxWidth() - .heightIn(min = 48.dp) + .heightIn(min = 32.dp) .clickable(onClickLabel = "Open artist") { onSubtitleClick?.invoke() }, contentAlignment = Alignment.TopCenter, ) { @@ -267,7 +266,7 @@ fun HeroHeader( Spacer(Modifier.height(4.dp)) HeroSubtitle(comment) } - if (!subtitleIsTarget) Spacer(Modifier.height(20.dp)) + if (!subtitleIsTarget) Spacer(Modifier.height(20.dp)) else Spacer(Modifier.height(12.dp)) Row( modifier = controlsModifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically, diff --git a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt index 83541725..3a792810 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt @@ -293,7 +293,6 @@ fun NowPlayingScreen( verticalArrangement = Arrangement.spacedBy(6.dp), modifier = Modifier - .fillMaxWidth() .padding(top = 12.dp) .height(titleHeight + 6.dp + subtitleHeight) .clickable( diff --git a/app/src/test/kotlin/com/subtracks/ui/library/AlbumDetailTapsTest.kt b/app/src/test/kotlin/com/subtracks/ui/library/AlbumDetailTapsTest.kt index 9fea2113..1b5fe07a 100644 --- a/app/src/test/kotlin/com/subtracks/ui/library/AlbumDetailTapsTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/library/AlbumDetailTapsTest.kt @@ -7,7 +7,9 @@ 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.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.onRoot import androidx.compose.ui.test.performClick import androidx.compose.ui.unit.dp import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -68,10 +70,39 @@ class AlbumDetailTapsTest { } @Test - fun theArtistTargetIsAtLeastTheMinimumTouchSize() { + fun theArtistTargetCoversTheLineAndItsGaps() { render(onArtistClick = {}) - composeRule.onNodeWithText("Radiohead \u00B7 2000").assertHeightIsAtLeast(48.dp) + composeRule.onNodeWithText("Radiohead \u00B7 2000").assertHeightIsAtLeast(32.dp) + } + + @Test + fun theArtistTargetHugsTheTextRatherThanTheRow() { + render(onArtistClick = {}) + + val target = + composeRule + .onNodeWithText("Radiohead \u00B7 2000") + .fetchSemanticsNode() + .boundsInRoot.width + val screen = + composeRule + .onRoot() + .fetchSemanticsNode() + .boundsInRoot.width + + assertTrue("the target should not span the row", target < screen) + } + + @Test + fun theArtistTargetKeepsClearOfTheControls() { + render(onArtistClick = {}) + + val target = composeRule.onNodeWithText("Radiohead \u00B7 2000").fetchSemanticsNode().boundsInRoot + val controls = composeRule.onNodeWithContentDescription("Download").fetchSemanticsNode().boundsInRoot + val gap = controls.top - target.bottom + + assertTrue("expected a clear gap above the controls, was ${gap}px", gap >= with(composeRule.density) { 12.dp.roundToPx() }) } @Test diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt index c9998ee1..5b2ed230 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt @@ -2,11 +2,13 @@ package com.subtracks.ui.playback import androidx.activity.ComponentActivity import androidx.compose.ui.test.assertHeightIsAtLeast +import androidx.compose.ui.test.assertHeightIsEqualTo import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.assertWidthIsAtLeast import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.onRoot import androidx.compose.ui.test.performClick import androidx.compose.ui.unit.dp import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -92,10 +94,31 @@ class NowPlayingTapsTest { fun theArtistTargetIsAtLeastTheMinimumTouchSize() { render(onArtistClick = {}) + // The block is the title line (32dp) plus the gap (6dp) plus the artist line + // (20dp); it must not absorb the slider's own top padding. composeRule.onNodeWithText("Radiohead").assertHeightIsAtLeast(48.dp) + composeRule.onNodeWithText("Radiohead").assertHeightIsEqualTo(58.dp) composeRule.onNodeWithText("Radiohead").assertWidthIsAtLeast(48.dp) } + @Test + fun theArtistTargetHugsTheTextRatherThanTheRow() { + render(onArtistClick = {}) + + val target = + composeRule + .onNodeWithText("Radiohead") + .fetchSemanticsNode() + .boundsInRoot.width + val screen = + composeRule + .onRoot() + .fetchSemanticsNode() + .boundsInRoot.width + + assertTrue("the target should not span the row", target < screen) + } + @Test fun theTapTargetsStayInertWithoutADestination() { render()