keep the previous artwork visible and stop the neighbour flashing mid-swipe

Swipe-back now goes to the previous track (previousTrack) instead of restarting, so the previous peek stays visible past the restart threshold. The frozen strip only hands back once the live neighbour props match it, so a stale neighbour can no longer flash while the slide settles.
2026-11-10 15:11:58 +01:00 · 2026-10-04 00:30:07 +00:00
parent b4539b9ba5
commit 73add672e1
4 changed files with 83 additions and 22 deletions
@@ -25,8 +25,6 @@ import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
 import kotlin.random.Random

-internal const val PLAYBACK_RESTART_THRESHOLD_MS = 3_000L
-
 data class QueueItem(
    val id: String,
    val title: String,
@@ -747,7 +745,7 @@ class PlaybackController(

    fun previous() {
        val player = player ?: return
-        if (player.currentPositionMs > PLAYBACK_RESTART_THRESHOLD_MS) {
+        if (player.currentPositionMs > RESTART_THRESHOLD_MS) {
            scope.launch {
                startLock.withLock { player.seekTo(0) }
                refresh()
@@ -788,6 +786,26 @@ class PlaybackController(
        }
    }

+    fun previousTrack() {
+        val snapshot = snapshot ?: return
+        if (snapshot.size == 0L) return
+        val current = currentPosition() ?: return
+        if (offline) {
+            scope.launch {
+                downloads.awaitLoaded()
+                val target =
+                    downloadedAtOrBefore(snapshot, current - 1)
+                        ?: if (repeatMode == RepeatMode.All) downloadedAtOrBefore(snapshot, snapshot.size - 1) else null
+                target?.let { jumpTo(it) }
+            }
+            return
+        }
+        when {
+            current > 0 -> jumpTo(current - 1)
+            repeatMode == RepeatMode.All -> jumpTo(snapshot.size - 1)
+        }
+    }
+
    fun addToQueue(
        sourceId: Long,
        kind: QueueKind,
@@ -906,15 +924,13 @@ class PlaybackController(
    suspend fun previousItem(): QueueItem? {
        var current: QueueSnapshot? = null
        var position: Long? = null
-        var restarting = false
        startLock.withLock {
            current = snapshot
            position = currentPosition()
-            restarting = (player?.currentPositionMs ?: 0L) > PLAYBACK_RESTART_THRESHOLD_MS
        }
        val snapshot = current ?: return null
        val at = position ?: return null
-        if (at <= 0 || restarting) return null
+        if (at <= 0) return null
        return queueRepository.itemAt(snapshot, at - 1)?.toQueueItem()
    }

@@ -1513,6 +1529,7 @@ class PlaybackController(
        const val BUFFERING_INDICATOR_DELAY_MS = 1_000L
        const val QUEUE_WINDOW_RADIUS = 25L
        const val WINDOW_SHIFT_DELAY_MS = 400L
+        const val RESTART_THRESHOLD_MS = 3_000L
        const val SEED_PIN_TRIES = 1_000_000
        const val FILE_SCHEME = "file:"

@@ -77,7 +77,6 @@ import com.subtracks.data.model.Song
 import com.subtracks.data.repo.DownloadRepository
 import com.subtracks.data.repo.LibraryRepository
 import com.subtracks.data.source.StarType
-import com.subtracks.playback.PLAYBACK_RESTART_THRESHOLD_MS
 import com.subtracks.playback.PlaybackController
 import com.subtracks.playback.PlaybackState
 import com.subtracks.playback.QueueItem
@@ -153,6 +152,21 @@ internal fun rotateStrip(
        listOf(null, base.getOrNull(0), base.getOrNull(1))
    }

+internal fun handBackReady(
+    frozen: List<NowPlayingArt?>,
+    itemId: String?,
+    previousArt: NowPlayingArt?,
+    nextArt: NowPlayingArt?,
+): Boolean {
+    val centre = frozen.getOrNull(1)?.id ?: return false
+    if (centre != itemId) return false
+    return when {
+        frozen.getOrNull(2) == null -> previousArt?.id == frozen.getOrNull(0)?.id
+        frozen.getOrNull(0) == null -> nextArt?.id == frozen.getOrNull(2)?.id
+        else -> true
+    }
+}
+
@Composable
 fun NowPlayingRoute(
    onBack: () -> Unit,
@@ -211,8 +225,7 @@ fun NowPlayingRoute(
            name = item.title,
        )
    var adjacent by remember { mutableStateOf(AdjacentArt()) }
-    val previousReachable = positionMs <= PLAYBACK_RESTART_THRESHOLD_MS
-    LaunchedEffect(state.item?.id, state.layout, previousReachable) {
+    LaunchedEffect(state.item?.id, state.layout) {
        if (state.item == null) {
            adjacent = AdjacentArt()
            return@LaunchedEffect
@@ -230,8 +243,9 @@ fun NowPlayingRoute(
        title = sourceTitle.value,
        coverArt = art,
        thumbnailRef = thumbnail,
-        previousArt = if (previousReachable) adjacent.previous else null,
+        previousArt = adjacent.previous,
        nextArt = adjacent.next,
+        onSkipPrevious = controller::previousTrack,
        artwork = rememberArtworkColors(thumbnail ?: art, fallbackName = state.item?.title),
        onBack = onBack,
        onQueue = onQueue,
@@ -263,6 +277,7 @@ fun NowPlayingScreen(
    onPlayPause: () -> Unit,
    onNext: () -> Unit,
    onPrevious: () -> Unit,
+    onSkipPrevious: () -> Unit = onPrevious,
    onShuffle: () -> Unit = {},
    onRepeat: () -> Unit = {},
    onMore: () -> Unit = {},
@@ -317,18 +332,19 @@ fun NowPlayingScreen(
    fun commitSwipe(direction: Int) {
        frozenStrip = rotateStrip(strip, direction)
        swipeOffsetX += direction * coverWidthPx
-        if (direction > 0) onNext() else onPrevious()
+        if (direction > 0) onNext() else onSkipPrevious()
        animateSwipe(0f)
    }

-    // The rotated strip keeps the slide continuous while the controller catches up. Hand back once
-    // it reports the target track, or after a grace period when the action did not move there (a
-    // restart, an offline skip), so a frozen strip can never stay desynced from playback.
-    LaunchedEffect(state.item?.id, frozenStrip, dragging) {
+    // The rotated strip keeps the slide continuous while the controller catches up. It only hands
+    // back once the live props agree with the frozen slots, so a stale neighbour can never flash
+    // during the slide; the timeout covers an action that does not move (an offline skip).
+    LaunchedEffect(state.item?.id, frozenStrip, dragging, previousArt, nextArt) {
        val pending = frozenStrip ?: return@LaunchedEffect
-        if (pending.getOrNull(1)?.id == state.item?.id) {
+        if (dragging) return@LaunchedEffect
+        if (handBackReady(pending, state.item?.id, previousArt, nextArt)) {
            frozenStrip = null
-        } else if (!dragging) {
+        } else {
            delay(HAND_BACK_TIMEOUT_MS)
            if (frozenStrip === pending) frozenStrip = null
        }
@@ -422,7 +438,7 @@ fun NowPlayingScreen(
                                                }

                                                SwipeAction.Previous -> {
-                                                    onPrevious()
+                                                    onSkipPrevious()
                                                    animateSwipe(0f)
                                                }

@@ -213,7 +213,7 @@ class PlaybackControllerTest {
    }

    @Test
-    fun previousItemIsNullOncePreviousWouldRestartTheTrack() {
+    fun previousTrackMovesBackEvenOncePreviousWouldRestart() {
        seedAlbum(3, sourceId = 1)
        controller.playAlbum(1, "al1", 1)
        await {
@@ -222,8 +222,12 @@ class PlaybackControllerTest {
        }

        handle.positionMs = 3_100
+        controller.previousTrack()

-        assertNull(runBlocking { controller.previousItem() })
+        await {
+            controller.state.value.item
+                ?.id == "s1"
+        }
    }

    @Test
@@ -56,6 +56,7 @@ class NowPlayingTapsTest {
        onArtistClick: (() -> Unit)? = null,
        onNext: () -> Unit = {},
        onPrevious: () -> Unit = {},
+        onSkipPrevious: () -> Unit = onPrevious,
        previousArt: NowPlayingArt? = null,
        nextArt: NowPlayingArt? = null,
    ) {
@@ -72,6 +73,7 @@ class NowPlayingTapsTest {
                    onPlayPause = {},
                    onNext = onNext,
                    onPrevious = onPrevious,
+                    onSkipPrevious = onSkipPrevious,
                    onAlbumClick = onAlbumClick,
                    onArtistClick = onArtistClick,
                    onSeek = {},
@@ -163,12 +165,34 @@ class NowPlayingTapsTest {
    fun swipingRightOnTheCoverGoesToThePreviousTrack() {
        var next = false
        var previous = false
-        render(onNext = { next = true }, onPrevious = { previous = true })
+        var skipPrevious = false
+        render(
+            onNext = { next = true },
+            onPrevious = { previous = true },
+            onSkipPrevious = { skipPrevious = true },
+        )

        composeRule.onNodeWithTag(NOW_PLAYING_COVER_TAG).performTouchInput { swipeRight() }

-        assertTrue("a right swipe should go to the previous track", previous)
+        assertTrue("a right swipe should skip to the previous track", skipPrevious)
        assertTrue("a right swipe should not skip to the next track", !next)
+        assertTrue("a right swipe should not use the restart-on-previous button", !previous)
+    }
+
+    @Test
+    fun theStripHandsBackOnlyWhenTheLivePropsAgree() {
+        val before = adjacentArt("s0")
+        val current = adjacentArt("s1")
+        val next = adjacentArt("s2")
+        val after = adjacentArt("s3")
+        val frozenNext = listOf(current, next, null)
+        val frozenPrevious = listOf(null, before, current)
+
+        assertTrue(!handBackReady(frozenNext, itemId = "s1", previousArt = before, nextArt = after))
+        assertTrue(!handBackReady(frozenNext, itemId = "s2", previousArt = before, nextArt = after))
+        assertTrue(handBackReady(frozenNext, itemId = "s2", previousArt = current, nextArt = after))
+        assertTrue(handBackReady(frozenPrevious, itemId = "s0", previousArt = null, nextArt = current))
+        assertTrue(!handBackReady(frozenPrevious, itemId = "s1", previousArt = null, nextArt = current))
    }

    @Test