address the review of the tap targets

The "stays inert" test was tautological: the render helper always passed non-null no-op
lambdas, so its assertion held for any implementation. It now passes nullable callbacks
and asserts the targets are disabled, which fails if the gate is removed.

Accessibility: both taps carry onClickLabel ("Open album"/"Open artist") and the artist
box spans the full row, and the now playing cover passes showPlaceholder = coverArt ==
null so the always-present placeholder letter is not merged into the tap target's label
(the same flash the artist detail cells had).

Left as-is: the artist target is still one line tall (~20dp), and the album detail
subtitle tap covers the year too. Both are layout/scope trade-offs.
This commit is contained in:
austinried
2026-10-04 00:30:00 +00:00
parent b55e5cc5e8
commit 7b1696da5e
2 changed files with 15 additions and 10 deletions
@@ -260,7 +260,10 @@ fun NowPlayingScreen(
.weight(1f) .weight(1f)
.fillMaxWidth() .fillMaxWidth()
.padding(top = 2.dp) .padding(top = 2.dp)
.clickable(enabled = onAlbumClick != null) { onAlbumClick?.invoke() } .clickable(
enabled = onAlbumClick != null,
onClickLabel = "Open album",
) { onAlbumClick?.invoke() }
.testTag(NOW_PLAYING_COVER_TAG), .testTag(NOW_PLAYING_COVER_TAG),
contentAlignment = Alignment.Center, contentAlignment = Alignment.Center,
) { ) {
@@ -268,6 +271,7 @@ fun NowPlayingScreen(
ref = coverArt, ref = coverArt,
name = state.item?.title.orEmpty(), name = state.item?.title.orEmpty(),
thumbnailRef = thumbnailRef, thumbnailRef = thumbnailRef,
showPlaceholder = coverArt == null,
square = false, square = false,
elevation = 3.dp, elevation = 3.dp,
modifier = Modifier.fillMaxSize(), modifier = Modifier.fillMaxSize(),
@@ -303,8 +307,12 @@ fun NowPlayingScreen(
Box( Box(
modifier = modifier =
Modifier Modifier
.fillMaxWidth()
.height(subtitleHeight) .height(subtitleHeight)
.clickable(enabled = onArtistClick != null) { onArtistClick?.invoke() }, .clickable(
enabled = onArtistClick != null,
onClickLabel = "Open artist",
) { onArtistClick?.invoke() },
contentAlignment = Alignment.Center, contentAlignment = Alignment.Center,
) { ) {
Text( Text(
@@ -1,6 +1,7 @@
package com.subtracks.ui.playback package com.subtracks.ui.playback
import androidx.activity.ComponentActivity import androidx.activity.ComponentActivity
import androidx.compose.ui.test.assertIsNotEnabled
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
@@ -11,7 +12,6 @@ import com.subtracks.playback.PlaybackState
import com.subtracks.playback.QueueItem import com.subtracks.playback.QueueItem
import com.subtracks.ui.theme.SubtracksTheme import com.subtracks.ui.theme.SubtracksTheme
import com.subtracks.ui.theme.artworkColorsFromSeed import com.subtracks.ui.theme.artworkColorsFromSeed
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue import org.junit.Assert.assertTrue
import org.junit.Rule import org.junit.Rule
import org.junit.Test import org.junit.Test
@@ -41,8 +41,8 @@ class NowPlayingTapsTest {
) )
private fun render( private fun render(
onAlbumClick: () -> Unit = {}, onAlbumClick: (() -> Unit)? = null,
onArtistClick: () -> Unit = {}, onArtistClick: (() -> Unit)? = null,
) { ) {
composeRule.setContent { composeRule.setContent {
SubtracksTheme { SubtracksTheme {
@@ -87,12 +87,9 @@ class NowPlayingTapsTest {
@Test @Test
fun theTapTargetsStayInertWithoutADestination() { fun theTapTargetsStayInertWithoutADestination() {
var opened = false
render() render()
composeRule.onNodeWithTag(NOW_PLAYING_COVER_TAG).performClick() composeRule.onNodeWithTag(NOW_PLAYING_COVER_TAG).assertIsNotEnabled()
composeRule.onNodeWithText("Radiohead").performClick() composeRule.onNodeWithText("Radiohead").assertIsNotEnabled()
assertFalse(opened)
} }
} }