mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-10 15:11:58 +01:00
now playing: stop clipping the cover shadow
The cover strip clipped to the 24dp content inset. A landscape cover fills that inset, so its drop shadow was cut off on the left and right and only the top and bottom survived, which reads as a stray band hugging the artwork. A tile that fills the strip's height had the same problem vertically. Drop the clip and space the tiles by TILE_GAP instead. The shadow then renders whole on all four sides while the swipe target and the artwork keep the size and the content margins they had, and the spacing leaves a gap between the tiles while dragging between tracks. It is also what keeps the neighbours off-screen now that the strip is unclipped. Refs: #173
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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++
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user