From 7b1696da5e6fa027892de6c57e5ffbfe6b016bd7 Mon Sep 17 00:00:00 2001
From: austinried <4966622+austinried@users.noreply.github.com>
Date: Sat, 26 Sep 2026 21:41:36 +0000
Subject: [PATCH] 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.
---
 .../com/subtracks/ui/playback/NowPlayingScreen.kt   | 12 ++++++++++--
 .../com/subtracks/ui/playback/NowPlayingTapsTest.kt | 13 +++++--------
 2 files changed, 15 insertions(+), 10 deletions(-)

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 1c5029d2..9655844c 100644
--- a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt
+++ b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt
@@ -260,7 +260,10 @@ fun NowPlayingScreen(
                                     .weight(1f)
                                     .fillMaxWidth()
                                     .padding(top = 2.dp)
-                                    .clickable(enabled = onAlbumClick != null) { onAlbumClick?.invoke() }
+                                    .clickable(
+                                        enabled = onAlbumClick != null,
+                                        onClickLabel = "Open album",
+                                    ) { onAlbumClick?.invoke() }
                                     .testTag(NOW_PLAYING_COVER_TAG),
                             contentAlignment = Alignment.Center,
                         ) {
@@ -268,6 +271,7 @@ fun NowPlayingScreen(
                                 ref = coverArt,
                                 name = state.item?.title.orEmpty(),
                                 thumbnailRef = thumbnailRef,
+                                showPlaceholder = coverArt == null,
                                 square = false,
                                 elevation = 3.dp,
                                 modifier = Modifier.fillMaxSize(),
@@ -303,8 +307,12 @@ fun NowPlayingScreen(
                             Box(
                                 modifier =
                                     Modifier
+                                        .fillMaxWidth()
                                         .height(subtitleHeight)
-                                        .clickable(enabled = onArtistClick != null) { onArtistClick?.invoke() },
+                                        .clickable(
+                                            enabled = onArtistClick != null,
+                                            onClickLabel = "Open artist",
+                                        ) { onArtistClick?.invoke() },
                                 contentAlignment = Alignment.Center,
                             ) {
                                 Text(
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 f55f3f39..d7714969 100644
--- a/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt
+++ b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt
@@ -1,6 +1,7 @@
 package com.subtracks.ui.playback
 
 import androidx.activity.ComponentActivity
+import androidx.compose.ui.test.assertIsNotEnabled
 import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule
 import androidx.compose.ui.test.onNodeWithTag
 import androidx.compose.ui.test.onNodeWithText
@@ -11,7 +12,6 @@ import com.subtracks.playback.PlaybackState
 import com.subtracks.playback.QueueItem
 import com.subtracks.ui.theme.SubtracksTheme
 import com.subtracks.ui.theme.artworkColorsFromSeed
-import org.junit.Assert.assertFalse
 import org.junit.Assert.assertTrue
 import org.junit.Rule
 import org.junit.Test
@@ -41,8 +41,8 @@ class NowPlayingTapsTest {
         )
 
     private fun render(
-        onAlbumClick: () -> Unit = {},
-        onArtistClick: () -> Unit = {},
+        onAlbumClick: (() -> Unit)? = null,
+        onArtistClick: (() -> Unit)? = null,
     ) {
         composeRule.setContent {
             SubtracksTheme {
@@ -87,12 +87,9 @@ class NowPlayingTapsTest {
 
     @Test
     fun theTapTargetsStayInertWithoutADestination() {
-        var opened = false
         render()
 
-        composeRule.onNodeWithTag(NOW_PLAYING_COVER_TAG).performClick()
-        composeRule.onNodeWithText("Radiohead").performClick()
-
-        assertFalse(opened)
+        composeRule.onNodeWithTag(NOW_PLAYING_COVER_TAG).assertIsNotEnabled()
+        composeRule.onNodeWithText("Radiohead").assertIsNotEnabled()
     }
 }