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 267a18e8..290bcf41 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/NowPlayingScreen.kt @@ -58,7 +58,6 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.draw.clipToBounds import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.layout.onSizeChanged @@ -111,6 +110,10 @@ private const val SWIPE_SETTLE_MS = 180 private const val SWIPE_EDGE_RESISTANCE = 0.3f private const val HAND_BACK_TIMEOUT_MS = 750L +// The strip is unclipped, so this spacing is what keeps the neighbours off-screen, and it has to +// clear the content inset plus a tile's shadow. It doubles as the gap between tiles while dragging. +private val TILE_GAP = 40.dp + data class NowPlayingArt( val id: String?, val ref: CoverArtRef?, @@ -329,6 +332,7 @@ fun NowPlayingScreen( val strip = frozenStrip ?: liveStrip val canGoNext = strip.getOrNull(2) != null val canGoPrevious = strip.getOrNull(0) != null + val stridePx = coverWidthPx + with(LocalDensity.current) { TILE_GAP.toPx() } fun animateSwipe(target: Float) { settleJob?.cancel() @@ -340,7 +344,7 @@ fun NowPlayingScreen( fun commitSwipe(direction: Int) { frozenStrip = rotateStrip(strip, direction) - swipeOffsetX += direction * coverWidthPx + swipeOffsetX += direction * stridePx if (direction > 0) onNext() else onSkipPrevious() animateSwipe(0f) } @@ -410,13 +414,14 @@ fun NowPlayingScreen( .padding(horizontal = 24.dp), horizontalAlignment = Alignment.CenterHorizontally, ) { + // Deliberately unclipped: a tile that fills the width would lose its drop + // shadow. TILE_GAP keeps the neighbours off-screen instead. Box( modifier = Modifier .weight(1f) .fillMaxWidth() .padding(top = 2.dp) - .clipToBounds() .onSizeChanged { coverWidthPx = it.width.toFloat() } .draggable( orientation = Orientation.Horizontal, @@ -428,7 +433,7 @@ fun NowPlayingScreen( delta < 0 && !canGoNext -> delta * SWIPE_EDGE_RESISTANCE else -> delta } - swipeOffsetX = (swipeOffsetX + resisted).coerceIn(-coverWidthPx, coverWidthPx) + swipeOffsetX = (swipeOffsetX + resisted).coerceIn(-stridePx, stridePx) }, onDragStarted = { dragging = true @@ -484,7 +489,7 @@ fun NowPlayingScreen( Modifier .fillMaxSize() .graphicsLayer { - translationX = (index - 1) * coverWidthPx + swipeOffsetX + translationX = (index - 1) * stridePx + swipeOffsetX }.then( when (index) { 0 -> Modifier.testTag(NOW_PLAYING_PREVIOUS_TAG) diff --git a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt index 658ed8d9..c6635480 100644 --- a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt @@ -444,38 +444,31 @@ class AppScreenshotTest { } @Test - fun nowPlaying() { - composeRule.setContent { - SubtracksTheme { - NowPlayingScreen( - state = Fixtures.playbackState(), - positionMs = 62_000, - title = "Kid A", - coverArt = CoverArtRef("art-al-kid-a", "test:art-al-kid-a"), - artwork = artworkColorsFromSeed(Color.rgb(120, 80, 200)), - onBack = {}, - onQueue = {}, - onPlayPause = {}, - onNext = {}, - onPrevious = {}, - onSeek = {}, - modifier = Modifier.fillMaxSize(), - ) - } - } - awaitText("Everything In Its Right Place") - composeRule.onRoot().captureRoboImage("src/test/screenshots/now_playing.png") - } + fun nowPlaying() = nowPlayingShot(Fixtures.playbackState(), "art-al-kid-a", "now_playing.png") @Test - fun nowPlayingModes() { + fun nowPlayingModes() = + nowPlayingShot( + Fixtures.playbackState().copy(shuffle = true, repeat = RepeatMode.One), + "art-al-kid-a", + "now_playing_modes.png", + ) + + @Test + fun nowPlayingNonSquare() = nowPlayingShot(Fixtures.playbackState(), "art-nonsquare", "now_playing_nonsquare.png") + + private fun nowPlayingShot( + state: PlaybackState, + coverArtId: String, + file: String, + ) { composeRule.setContent { SubtracksTheme { NowPlayingScreen( - state = Fixtures.playbackState().copy(shuffle = true, repeat = RepeatMode.One), + state = state, positionMs = 62_000, title = "Kid A", - coverArt = CoverArtRef("art-al-kid-a", "test:art-al-kid-a"), + coverArt = CoverArtRef(coverArtId, "test:$coverArtId"), artwork = artworkColorsFromSeed(Color.rgb(120, 80, 200)), onBack = {}, onQueue = {}, @@ -488,7 +481,7 @@ class AppScreenshotTest { } } awaitText("Everything In Its Right Place") - composeRule.onRoot().captureRoboImage("src/test/screenshots/now_playing_modes.png") + composeRule.onRoot().captureRoboImage("src/test/screenshots/$file") } @Test @@ -725,6 +718,7 @@ private object Fixtures { fun artEngine(): FakeImageLoaderEngine { val builder = FakeImageLoaderEngine.Builder() + builder.intercept("art-nonsquare", art("art-nonsquare", ArtKind.Album, width = 640, height = 256)) albums.mapNotNull { it.coverArt }.forEach { builder.intercept(it, art(it, ArtKind.Album)) } playlists.mapNotNull { it.coverArt }.forEach { builder.intercept(it, art(it, ArtKind.Playlist)) } artists.mapNotNull { it.coverArt }.forEach { builder.intercept(it, art(it, ArtKind.Artist)) } @@ -734,9 +728,14 @@ private object Fixtures { private fun art( key: String, kind: ArtKind, + width: Int = 256, + height: Int = 256, ): Image { - val size = 256 - val f = size.toFloat() + val w = width.toFloat() + val h = height.toFloat() + val cx = w / 2f + val cy = h / 2f + val span = maxOf(w, h) val random = Random(key.hashCode()) val base = random.nextFloat() * 360f @@ -752,21 +751,21 @@ private object Fixtures { listOf(color(0f), color(170f + random.nextFloat() * 30f)) } - val bitmap = Bitmap.createBitmap(size, size, Bitmap.Config.ARGB_8888) + val bitmap = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888) val canvas = Canvas(bitmap) val paint = Paint(Paint.ANTI_ALIAS_FLAG) canvas.drawColor(palette[0]) when (kind) { ArtKind.Album -> { - val stripe = f / 9f + val stripe = w / 9f canvas.save() - canvas.rotate(-30f, f / 2f, f / 2f) - var x = -f + canvas.rotate(-30f, cx, cy) + var x = -span var index = 1 - while (x < f * 2f) { + while (x < span * 2f) { paint.color = palette[index % palette.size] - canvas.drawRect(x, -f, x + stripe * 0.6f, f * 2f, paint) + canvas.drawRect(x, -span, x + stripe * 0.6f, span * 2f, paint) x += stripe index++ } @@ -774,12 +773,12 @@ private object Fixtures { } ArtKind.Playlist -> { - val step = f / 4f + val step = w / 4f var row = 1 var y = step / 2f - while (y < f) { + while (y < h) { var x = step / 2f - while (x < f) { + while (x < w) { paint.color = palette[row % palette.size] canvas.drawCircle(x, y, step * 0.3f, paint) x += step @@ -791,12 +790,12 @@ private object Fixtures { ArtKind.Artist -> { paint.style = Paint.Style.STROKE - paint.strokeWidth = f / 13f - var radius = f * 0.5f + paint.strokeWidth = span / 13f + var radius = span * 0.5f var index = 1 while (radius > 0f) { paint.color = palette[index % palette.size] - canvas.drawCircle(f / 2f, f / 2f, radius, paint) + canvas.drawCircle(cx, cy, radius, paint) radius -= paint.strokeWidth index++ } 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 6c92da7c..10dfafdd 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/NowPlayingTapsTest.kt @@ -4,6 +4,7 @@ import androidx.activity.ComponentActivity import androidx.compose.ui.geometry.Offset import androidx.compose.ui.test.assertHeightIsAtLeast import androidx.compose.ui.test.assertHeightIsEqualTo +import androidx.compose.ui.test.assertIsNotDisplayed import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.assertWidthIsAtLeast import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule @@ -285,6 +286,29 @@ class NowPlayingTapsTest { composeRule.onNodeWithTag(NOW_PLAYING_NEXT_TAG, useUnmergedTree = true).assertDoesNotExist() } + @Test + fun theSwipeTargetStaysInsideTheContentMargins() { + render() + + val target = composeRule.onNodeWithTag(NOW_PLAYING_COVER_TAG).fetchSemanticsNode().boundsInRoot + val screen = composeRule.onRoot().fetchSemanticsNode().boundsInRoot + + assertEquals( + "the swipe target must not reach into the screen margins", + screen.width - with(composeRule.density) { 48.dp.toPx() }, + target.width, + ) + } + + @Test + fun theNeighboursStayHiddenAtRest() { + render(previousArt = adjacentArt("s-previous"), nextArt = adjacentArt("s-next")) + + // TILE_GAP is what keeps them off-screen now that the strip is unclipped. + composeRule.onNodeWithTag(NOW_PLAYING_PREVIOUS_TAG, useUnmergedTree = true).assertIsNotDisplayed() + composeRule.onNodeWithTag(NOW_PLAYING_NEXT_TAG, useUnmergedTree = true).assertIsNotDisplayed() + } + @Test fun aShortSlowDragOnTheCoverDoesNotSkip() { var next = false