From 06cf49f4e8795f0e87f54f4fc1339023e2a9a700 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sun, 27 Sep 2026 02:19:16 +0000 Subject: [PATCH] queue view: disable drag auto-scroll; queue plays next when its anchor is removed - removing the context track the up-next block sits behind now moves the block back behind the preceding track (or to the front when the anchor was first), so the queued track plays next instead of the following context track. - disable sh.calvin.reorderable auto-scroll for every run (scrollThreshold 0). It keeps scrolling past a run's last valid target and can strand the dragged item off-screen; scroll first, then drag. --- .../subtracks/data/repo/QueueRepository.kt | 33 ++++++++----------- .../com/subtracks/ui/playback/QueueScreen.kt | 11 ++++--- .../data/repo/QueueRepositoryTest.kt | 13 ++++++++ .../playback/PlaybackControllerTest.kt | 9 ++--- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt index 73f66649..fbb3cf4d 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/QueueRepository.kt @@ -61,7 +61,7 @@ data class QueueSnapshot( ( when (val order = shuffleOrder) { null -> upNextAnchor - else -> order.indexOf(upNextAnchor).takeIf { it >= 0 }?.toLong() ?: 0L + else -> if (upNextAnchor < 0L) -1L else order.indexOf(upNextAnchor).takeIf { it >= 0 }?.toLong() ?: 0L } ).coerceAtMost(contextSize - 1L) @@ -411,33 +411,28 @@ class QueueRepository( if (order == null) { val removed = compact(removeEntry(snapshot.entries, play)) write(removed.map { it.entry }) - adjustAnchorOnRemove(play, removed.sumOf { it.length }) + adjustAnchorOnRemove(play) } else { val flat = order.getOrNull(play.toInt()) ?: return snapshot - val removed = compact(removeEntry(snapshot.entries, flat)) - write(removed.map { it.entry }) + write(compact(removeEntry(snapshot.entries, flat)).map { it.entry }) val reordered = order.toMutableList().apply { removeAt(play.toInt()) } for (index in reordered.indices) if (reordered[index] > flat) reordered[index] -= 1 setShuffle(true, reordered.toLongArray()) - adjustAnchorOnRemove(flat, removed.sumOf { it.length }) + adjustAnchorOnRemove(flat) } return snapshot() } - private suspend fun adjustAnchorOnRemove( - removedFlat: Long, - newSize: Long, - ) = cursorMutex.withLock { - val row = cursorRow() - val anchor = row.upNextAnchor - val adjusted = - when { - anchor > removedFlat -> anchor - 1 - anchor == removedFlat -> removedFlat.coerceIn(0, (newSize - 1).coerceAtLeast(0)) - else -> anchor - } - if (adjusted != anchor) dao.setCursor(row.copy(upNextAnchor = adjusted)) - } + private suspend fun adjustAnchorOnRemove(removedFlat: Long) = + cursorMutex.withLock { + val row = cursorRow() + val anchor = row.upNextAnchor + // Removing the track the block sits behind leaves the block next in line, so move the + // anchor back to the preceding track (or in front of the whole context, -1, when the + // anchor was the first track) rather than letting the following track play first. + val adjusted = if (anchor >= removedFlat) anchor - 1 else anchor + if (adjusted != anchor) dao.setCursor(row.copy(upNextAnchor = adjusted)) + } private suspend fun adjustAnchorOnMove( flatFrom: Long, diff --git a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt index 11c6ad65..482619e8 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -332,19 +332,20 @@ fun QueueScreen( // One reorder state per run of rows: sh.calvin.reorderable only targets keys registered with // the state that started the drag, so a drag can never reach a row in another run. The context // sits on both sides of the up-next block, and splitting it there stops a context drag crossing - // the block (which would move a track over the queued songs and relocate the anchor). The - // up-next state disables auto-scroll (scrollThreshold 0): its block is followed by context - // rows the scroller would otherwise keep scrolling towards with nowhere to drop. + // the block (which would move a track over the queued songs and relocate the anchor). + // scrollThreshold 0 disables auto-scroll: the library keeps scrolling past a run's last valid + // target with nowhere to drop, and the dragged item can be left off-screen. Scroll to where + // you want to drop first, then drag. val upNextReorder = rememberReorderableLazyListState(listState, scrollThreshold = 0.dp) { from, to -> onReorder(rows.rowIndexOf(from.key), rows.rowIndexOf(to.key)) } val contextBeforeReorder = - rememberReorderableLazyListState(listState) { from, to -> + rememberReorderableLazyListState(listState, scrollThreshold = 0.dp) { from, to -> onReorder(rows.rowIndexOf(from.key), rows.rowIndexOf(to.key)) } val contextAfterReorder = - rememberReorderableLazyListState(listState) { from, to -> + rememberReorderableLazyListState(listState, scrollThreshold = 0.dp) { from, to -> onReorder(rows.rowIndexOf(from.key), rows.rowIndexOf(to.key)) } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt index feddbc26..49032676 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/QueueRepositoryTest.kt @@ -465,6 +465,19 @@ class QueueRepositoryTest { assertEquals(listOf("s1", "s4", "s2", "s3"), resolveAll(repository.snapshot())) } + @Test + fun removingTheAnchorTrackPutsTheQueueFirst() = + runTest { + seedLibrary() + val snapshot = repository.snapshotAfter(listOf(repository.albumEntry(1, "al1"))) + repository.addUpNext(snapshot, currentPosition = 0, entry = repository.songEntry(1, "s4"), playNext = false) + assertEquals(listOf("s1", "s4", "s2", "s3"), resolveAll(repository.snapshot())) + + repository.removeAt(repository.snapshot(), 0) + + assertEquals(listOf("s4", "s2", "s3"), resolveAll(repository.snapshot())) + } + @Test fun removingTheLastAnchorTrackKeepsTheQueueResolvable() = runTest { diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 538148f6..5f2b52fb 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -519,7 +519,8 @@ class PlaybackControllerTest { ?.id == "x1" } - assertEquals(1L, runBlocking { queues.cursor() }) + val snapshot = runBlocking { queues.snapshot() } + assertEquals(runBlocking { queues.combinedIndexOf(snapshot, "x1") }, controller.state.value.position) } @Test @@ -979,7 +980,7 @@ class PlaybackControllerTest { } @Test - fun removingThePlayingAnchorTrackPlaysTheModelNextTrack() { + fun removingThePlayingAnchorTrackPlaysTheQueuedTrackNext() { seedAlbum(4, sourceId = 1) seedSong("x1", "al2") @@ -994,11 +995,11 @@ class PlaybackControllerTest { runBlocking { controller.removeAt(1) } await { controller.state.value.item - ?.id == "s3" + ?.id == "x1" } val snapshot = runBlocking { queues.snapshot() } - assertEquals(runBlocking { queues.combinedIndexOf(snapshot, "s3") }, controller.state.value.position) + assertEquals(runBlocking { queues.combinedIndexOf(snapshot, "x1") }, controller.state.value.position) } @Test