mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 15:29:15 +01:00
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.
This commit is contained in:
@@ -246,14 +246,13 @@ fun HeroHeader(
|
|||||||
)
|
)
|
||||||
val subtitleIsTarget = onSubtitleClick != null && subtitle.isNotEmpty() && comment.isNullOrBlank()
|
val subtitleIsTarget = onSubtitleClick != null && subtitle.isNotEmpty() && comment.isNullOrBlank()
|
||||||
if (subtitleIsTarget) {
|
if (subtitleIsTarget) {
|
||||||
// The target spans the 4dp lead-in and the 20dp gap that would sit below the
|
// The line plus the 4dp lead-in above and 8dp below: shorter than the 48dp
|
||||||
// line, so the text stays put and the row clears 48dp; without those the
|
// recommendation, deliberately, so it stays 12dp clear of the controls rather
|
||||||
// subtitle alone is a single 20dp line.
|
// than reaching down to them. Hugs the text instead of the row.
|
||||||
Box(
|
Box(
|
||||||
modifier =
|
modifier =
|
||||||
Modifier
|
Modifier
|
||||||
.fillMaxWidth()
|
.heightIn(min = 32.dp)
|
||||||
.heightIn(min = 48.dp)
|
|
||||||
.clickable(onClickLabel = "Open artist") { onSubtitleClick?.invoke() },
|
.clickable(onClickLabel = "Open artist") { onSubtitleClick?.invoke() },
|
||||||
contentAlignment = Alignment.TopCenter,
|
contentAlignment = Alignment.TopCenter,
|
||||||
) {
|
) {
|
||||||
@@ -267,7 +266,7 @@ fun HeroHeader(
|
|||||||
Spacer(Modifier.height(4.dp))
|
Spacer(Modifier.height(4.dp))
|
||||||
HeroSubtitle(comment)
|
HeroSubtitle(comment)
|
||||||
}
|
}
|
||||||
if (!subtitleIsTarget) Spacer(Modifier.height(20.dp))
|
if (!subtitleIsTarget) Spacer(Modifier.height(20.dp)) else Spacer(Modifier.height(12.dp))
|
||||||
Row(
|
Row(
|
||||||
modifier = controlsModifier.fillMaxWidth(),
|
modifier = controlsModifier.fillMaxWidth(),
|
||||||
verticalAlignment = Alignment.CenterVertically,
|
verticalAlignment = Alignment.CenterVertically,
|
||||||
|
|||||||
@@ -293,7 +293,6 @@ fun NowPlayingScreen(
|
|||||||
verticalArrangement = Arrangement.spacedBy(6.dp),
|
verticalArrangement = Arrangement.spacedBy(6.dp),
|
||||||
modifier =
|
modifier =
|
||||||
Modifier
|
Modifier
|
||||||
.fillMaxWidth()
|
|
||||||
.padding(top = 12.dp)
|
.padding(top = 12.dp)
|
||||||
.height(titleHeight + 6.dp + subtitleHeight)
|
.height(titleHeight + 6.dp + subtitleHeight)
|
||||||
.clickable(
|
.clickable(
|
||||||
|
|||||||
@@ -7,7 +7,9 @@ import androidx.compose.ui.test.filter
|
|||||||
import androidx.compose.ui.test.hasClickAction
|
import androidx.compose.ui.test.hasClickAction
|
||||||
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
||||||
import androidx.compose.ui.test.onAllNodesWithText
|
import androidx.compose.ui.test.onAllNodesWithText
|
||||||
|
import androidx.compose.ui.test.onNodeWithContentDescription
|
||||||
import androidx.compose.ui.test.onNodeWithText
|
import androidx.compose.ui.test.onNodeWithText
|
||||||
|
import androidx.compose.ui.test.onRoot
|
||||||
import androidx.compose.ui.test.performClick
|
import androidx.compose.ui.test.performClick
|
||||||
import androidx.compose.ui.unit.dp
|
import androidx.compose.ui.unit.dp
|
||||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||||
@@ -68,10 +70,39 @@ class AlbumDetailTapsTest {
|
|||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun theArtistTargetIsAtLeastTheMinimumTouchSize() {
|
fun theArtistTargetCoversTheLineAndItsGaps() {
|
||||||
render(onArtistClick = {})
|
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
|
@Test
|
||||||
|
|||||||
@@ -2,11 +2,13 @@ package com.subtracks.ui.playback
|
|||||||
|
|
||||||
import androidx.activity.ComponentActivity
|
import androidx.activity.ComponentActivity
|
||||||
import androidx.compose.ui.test.assertHeightIsAtLeast
|
import androidx.compose.ui.test.assertHeightIsAtLeast
|
||||||
|
import androidx.compose.ui.test.assertHeightIsEqualTo
|
||||||
import androidx.compose.ui.test.assertIsNotEnabled
|
import androidx.compose.ui.test.assertIsNotEnabled
|
||||||
import androidx.compose.ui.test.assertWidthIsAtLeast
|
import androidx.compose.ui.test.assertWidthIsAtLeast
|
||||||
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
|
||||||
import androidx.compose.ui.test.onNodeWithTag
|
import androidx.compose.ui.test.onNodeWithTag
|
||||||
import androidx.compose.ui.test.onNodeWithText
|
import androidx.compose.ui.test.onNodeWithText
|
||||||
|
import androidx.compose.ui.test.onRoot
|
||||||
import androidx.compose.ui.test.performClick
|
import androidx.compose.ui.test.performClick
|
||||||
import androidx.compose.ui.unit.dp
|
import androidx.compose.ui.unit.dp
|
||||||
import androidx.test.ext.junit.runners.AndroidJUnit4
|
import androidx.test.ext.junit.runners.AndroidJUnit4
|
||||||
@@ -92,10 +94,31 @@ class NowPlayingTapsTest {
|
|||||||
fun theArtistTargetIsAtLeastTheMinimumTouchSize() {
|
fun theArtistTargetIsAtLeastTheMinimumTouchSize() {
|
||||||
render(onArtistClick = {})
|
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").assertHeightIsAtLeast(48.dp)
|
||||||
|
composeRule.onNodeWithText("Radiohead").assertHeightIsEqualTo(58.dp)
|
||||||
composeRule.onNodeWithText("Radiohead").assertWidthIsAtLeast(48.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
|
@Test
|
||||||
fun theTapTargetsStayInertWithoutADestination() {
|
fun theTapTargetsStayInertWithoutADestination() {
|
||||||
render()
|
render()
|
||||||
|
|||||||
Reference in New Issue
Block a user