From e76976dfdf3f29855b1897de10818241873bd440 Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Wed, 23 Sep 2026 06:27:04 +0000 Subject: [PATCH] fix drag drop target, serialize queue edits, guard empty ranges --- .../com/subtracks/ui/playback/QueueScreen.kt | 74 +++++++++++-------- .../playback/PlaybackControllerTest.kt | 43 +++++++++++ .../ui/playback/QueueViewModelTest.kt | 16 ++++ 3 files changed, 102 insertions(+), 31 deletions(-) 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 a9a6869a..8fda4a2e 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -104,8 +104,8 @@ class QueueViewModel( val start = cursor val end = (start + QUEUE_CHUNK - 1).coerceAtMost(size - 1) rows.addAll(queueRepository.range(snapshot, start, end).map(::newRow)) - first = rows.first().position - last = rows.last().position + first = rows.firstOrNull()?.position ?: 0 + last = rows.lastOrNull()?.position ?: -1 } ready = true } @@ -178,30 +178,29 @@ class QueueViewModel( fun remove(position: Long) { viewModelScope.launch { - val index = rows.indexOfFirst { it.position == position } - if (index >= 0) { - rows.removeAt(index) - for (i in index until rows.size) { - val row = rows[i] - rows[i] = row.copy(position = row.position - 1) - } - } - val next = (rows.lastOrNull()?.position ?: (first - 1)) + 1 - playbackController.removeAt(position) mutex.withLock { - if (rows.isEmpty()) { - size = 0 - first = 0 - last = -1 - return@withLock + val index = rows.indexOfFirst { it.position == position } + if (index >= 0) { + rows.removeAt(index) + for (i in index until rows.size) { + val row = rows[i] + rows[i] = row.copy(position = row.position - 1) + } } + playbackController.removeAt(position) val snapshot = queueRepository.snapshot() size = snapshot.size - if (next <= size - 1) { - queueRepository.range(snapshot, next, next).firstOrNull()?.let { rows.add(newRow(it)) } + if (rows.isEmpty()) { + reload() + return@withLock } first = rows.first().position last = rows.last().position + val next = last + 1 + if (next <= size - 1) { + queueRepository.range(snapshot, next, next).firstOrNull()?.let { rows.add(newRow(it)) } + last = rows.last().position + } } } } @@ -234,8 +233,8 @@ class QueueViewModel( } rows.clear() rows.addAll(updated) - first = rows.first().position - last = rows.last().position + first = rows.firstOrNull()?.position ?: 0 + last = rows.lastOrNull()?.position ?: -1 } } @@ -286,16 +285,12 @@ fun QueueScreen( val listState = rememberLazyListState() val snackbarHostState = remember { SnackbarHostState() } val scope = rememberCoroutineScope() + var dragId by remember { mutableStateOf(null) } var dragFrom by remember { mutableStateOf(null) } - var dragTo by remember { mutableStateOf(null) } val reorderState = rememberReorderableLazyListState(listState) { from, to -> - val fromPosition = rows.getOrNull(from.index)?.position - val toPosition = rows.getOrNull(to.index)?.position onReorder(from.index, to.index) - dragFrom = fromPosition - dragTo = toPosition } LaunchedEffect(listState, ready) { @@ -368,16 +363,22 @@ fun QueueScreen( dragHandle = Modifier.draggableHandle( onDragStarted = { + dragId = row.id dragFrom = row.position - dragTo = row.position }, onDragStopped = { + val id = dragId val from = dragFrom - val to = dragTo - if (from != null && to != null && from != to) { - onMove(from, to) - showUndo("Queue reordered") + if (id != null && from != null) { + val index = rows.indexOfFirst { it.id == id } + val to = if (index < 0) from else dropTarget(rows, index, from) + if (to != from) { + onMove(from, to) + showUndo("Queue reordered") + } } + dragId = null + dragFrom = null }, ), onClick = { onPlay(row.position) }, @@ -394,6 +395,17 @@ fun QueueScreen( } } +private fun dropTarget( + rows: List, + index: Int, + from: Long, +): Long { + val next = rows.getOrNull(index + 1) + if (next != null) return next.position - if (next.position > from) 1 else 0 + val before = rows.getOrNull(index - 1)?.position ?: return from + return (before - if (before > from) 1 else 0) + 1 +} + @Composable private fun QueueRowItem( row: QueueRow, diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 5d7e683c..58abbdb5 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -437,6 +437,49 @@ class PlaybackControllerTest { ) } + @Test + fun movingTracksFarFromThePlayingOneDoesNotReloadTheWindow() { + seedAlbum(100, sourceId = 1) + + controller.playAlbum(1, "al1", 50) + await { + controller.state.value.item + ?.id == "s51" + } + handle.operations.clear() + + runBlocking { controller.move(80, 90) } + + assertEquals( + "s51", + controller.state.value.item + ?.id, + ) + assertEquals(50L, controller.state.value.position) + assertFalse(handle.operations.any { it.startsWith("setWindow") }) + } + + @Test + fun movingATrackAcrossTheWindowKeepsPlaying() { + seedAlbum(100, sourceId = 1) + + controller.playAlbum(1, "al1", 50) + await { + controller.state.value.item + ?.id == "s51" + } + handle.operations.clear() + + runBlocking { controller.move(10, 90) } + + assertEquals( + "s51", + controller.state.value.item + ?.id, + ) + assertFalse(handle.operations.any { it.startsWith("setWindow") }) + } + @Test fun undoRestoresARemovedTrack() { seedAlbum(5, sourceId = 1) diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt index 8debd903..729b43e2 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -82,6 +82,22 @@ class QueueViewModelTest { assertEquals((40L..199L).toList(), viewModel.rows.map { it.position }) } + @Test + fun openingAtTheEndLoadsASingleRowAndExtendsOlder() { + runBlocking { seedSongs(200) } + controller.playSongs(1, 199) + await { controller.state.value.position == 199L } + + viewModel.open() + await { viewModel.ready } + + assertEquals(listOf(199L), viewModel.rows.map { it.position }) + + viewModel.loadOlder() + await { viewModel.rows.firstOrNull()?.position == 139L } + assertEquals((139L..199L).toList(), viewModel.rows.map { it.position }) + } + @Test fun reorderingMovesTheRowLocally() { runBlocking { seedSongs(200) }