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 2e1e91fd..9c3ed189 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -113,6 +113,8 @@ class QueueViewModel( var generation by mutableIntStateOf(0) private set + private var reorderPending = false + private var nextId = 0L private val mutex = Mutex() @@ -249,12 +251,19 @@ class QueueViewModel( fun move( from: Long, to: Long, - ) { - if (from == to) return + ): Boolean { + if (from == to) return false + if (reorderPending) return false + reorderPending = true viewModelScope.launch { - playbackController.move(from, to) - mutex.withLock { reconcileLocked() } + try { + playbackController.move(from, to) + mutex.withLock { reconcileLocked() } + } finally { + reorderPending = false + } } + return true } fun remove(position: Long) { @@ -331,7 +340,7 @@ fun QueueScreen( onPlay: (Long) -> Unit, onRemove: (Long) -> Unit, onReorder: (Int, Int) -> Unit, - onMove: (Long, Long) -> Unit, + onMove: (Long, Long) -> Boolean, onLoadOlder: () -> Unit, onLoadNewer: () -> Unit, onUndo: () -> Unit, @@ -347,7 +356,6 @@ fun QueueScreen( val snackbarHostState = remember { SnackbarHostState() } val scope = rememberCoroutineScope() var dragId by remember { mutableStateOf(null) } - var dragFrom by remember { mutableStateOf(null) } val undoLabel = stringResource(R.string.undo) val queueReordered = stringResource(R.string.queue_reordered) val queueRemoved = stringResource(R.string.queue_removed) @@ -470,28 +478,14 @@ fun QueueScreen( null } else { Modifier.draggableHandle( - onDragStarted = { - dragId = row.id - dragFrom = row.position - }, + onDragStarted = { dragId = row.id }, onDragStopped = { - val id = dragId - val from = dragFrom - if (id != null && from != null) { - val draggedIndex = rows.indexOfFirst { it.id == id } - val to = - if (draggedIndex < 0) { - from - } else { - dropTarget(rows, draggedIndex, from) - } - if (to != from) { - onMove(from, to) - showUndo(queueReordered) + dragId?.let { id -> + rows.dropMoveFor(id)?.let { move -> + if (onMove(move.from, move.to)) showUndo(queueReordered) } } dragId = null - dragFrom = null }, ) }, @@ -563,6 +557,19 @@ internal fun dropTarget( return (before - if (before > from) 1 else 0) + 1 } +internal data class QueueMove( + val from: Long, + val to: Long, +) + +internal fun List.dropMoveFor(draggedId: Long): QueueMove? { + val index = indexOfFirst { it.id == draggedId } + if (index < 0) return null + val from = this[index].position + val to = dropTarget(this, index, from) + return if (to == from) null else QueueMove(from, to) +} + @Suppress("ModifierParameter") @Composable private fun QueueRowItem( diff --git a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt index b1e972cb..658ed8d9 100644 --- a/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/AppScreenshotTest.kt @@ -506,7 +506,7 @@ class AppScreenshotTest { onPlay = {}, onRemove = {}, onReorder = { _, _ -> }, - onMove = { _, _ -> }, + onMove = { _, _ -> true }, onLoadOlder = {}, onLoadNewer = {}, onUndo = {}, 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 8ab83c3f..6b1b6df1 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -325,6 +325,42 @@ class QueueViewModelTest { assertEquals((220L..399L).toList(), rowsSnapshot().map { it.position }) } + @Test + fun ignoringASecondMoveWhileTheFirstIsStillCommitting() { + runBlocking { seedSongs(10) } + controller.playAlbum(1, "al1", 0) + await { controller.state.value.position == 0L } + + viewModel.open() + await { viewModel.ready } + val before = rowsSnapshot().map { it.song.song.id } + val from = rowsSnapshot().first().position + + runOnMain { + assertTrue(viewModel.move(from, from + 3)) + assertFalse(viewModel.move(from, from + 1)) + } + + val expected = before.toMutableList().also { it.add(3, it.removeAt(0)) } + await { rowsSnapshot().map { it.song.song.id } == expected } + assertEquals(expected, rowsSnapshot().map { it.song.song.id }) + } + + @Test + fun dropMoveTakesTheDraggedRowsCurrentPosition() { + val rows = + listOf( + row(10).copy(id = 2L), + row(11).copy(id = 3L), + row(9).copy(id = 1L), + ) + + val move = rows.dropMoveFor(1L)!! + + assertEquals(9L, move.from) + assertEquals(11L, move.to) + } + @Test fun removingATrackUnderTheBlockReloadsRows() { runBlocking { seedSongs(10) }