From 50be4e79e032c5b634c2b5b334488c7a0ca5e75e Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Sat, 26 Sep 2026 21:09:32 +0000 Subject: [PATCH] queue view: stop reloading on edits, fix drag boundaries - deleting a track no longer bumps the layout version, so the view does not reload and jump to the current track; the ViewModel already updates its rows. - layout changes (re-anchor, enqueue, consume) now reconcile positions in place instead of calling open(), which cleared the rows and scrolled back to the cursor. - section headers are separate list items and reordering maps by key, so the header no longer rides along with a dragged row (top flicker). - disable the reorderable auto-scroll (scrollThreshold = 0). The library scrolls past the section boundary with no valid target, then abandons the scroll on release; without auto-scroll a drag stays put and drops in the last reachable slot. - keep the current track when moving a queued track before it (eager dispatcher regression test). --- .../subtracks/playback/PlaybackController.kt | 1 - .../com/subtracks/ui/playback/QueueScreen.kt | 178 +++++++++++------- .../playback/PlaybackControllerTest.kt | 43 +++++ 3 files changed, 151 insertions(+), 71 deletions(-) diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 336b718f..b57b6653 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -344,7 +344,6 @@ class PlaybackController( RemovedUndo(entries, cursor, target, position, removed, snapshot.shuffleOrder, upNext, snapshot.upNextAnchor) } if (snapshot.upNextSize > 0L || updated.upNextSize > 0L) { - layoutVersion++ queueRepository.setCursor(target) loadWindow(target, autoplay = playing, startPositionMs = if (position == current) 0 else player.currentPositionMs) return@withLock 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 8f03b908..271a3308 100644 --- a/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt +++ b/app/src/main/kotlin/com/subtracks/ui/playback/QueueScreen.kt @@ -133,6 +133,30 @@ class QueueViewModel( } } + fun reconcile() { + viewModelScope.launch { + mutex.withLock { + if (!ready) return@withLock + val snapshot = queueRepository.snapshot() + if (snapshot.size == 0L) { + rows.clear() + return@withLock + } + val start = (rows.firstOrNull()?.position ?: 0L).coerceIn(0, snapshot.size - 1) + val end = (start + rows.size.coerceAtLeast(QUEUE_CHUNK) - 1).coerceAtMost(snapshot.size - 1) + val loaded = queueRepository.range(snapshot, start, end) + val reused = reusedIds() + val updated = loaded.map { rowFor(it, reused) } + if (updated.size == rows.size) { + updated.forEachIndexed { index, row -> if (rows[index] != row) rows[index] = row } + } else { + rows.clear() + rows.addAll(updated) + } + } + } + } + fun loadOlder() { val top = rows.firstOrNull()?.position ?: return if (top <= 0L) return @@ -265,7 +289,8 @@ fun QueueRoute( LaunchedEffect(queueContext) { sourceTitle.value = controller.sourceTitle(queueContext)?.takeIf { it.isNotBlank() } ?: fallbackTitle } - LaunchedEffect(playback.shuffle, playback.layout, open) { if (open) viewModel.open() } + LaunchedEffect(playback.shuffle, open) { if (open) viewModel.open() } + LaunchedEffect(playback.layout) { if (open) viewModel.reconcile() } QueueScreen( rows = viewModel.rows, ready = viewModel.ready, @@ -283,7 +308,6 @@ fun QueueRoute( onLoadOlder = viewModel::loadOlder, onLoadNewer = viewModel::loadNewer, onUndo = viewModel::undo, - onRefresh = viewModel::open, modifier = modifier, ) } @@ -308,7 +332,6 @@ fun QueueScreen( generation: Int = 0, shuffle: Boolean = false, contextTitle: String? = null, - onRefresh: () -> Unit = {}, ) { val listState = rememberLazyListState() val fill = rememberViewportFill(listState) @@ -318,25 +341,25 @@ fun QueueScreen( var dragFrom by remember { mutableStateOf(null) } val upNextReorder = - rememberReorderableLazyListState(listState) { from, to -> - onReorder(from.index, to.index) + rememberReorderableLazyListState(listState, scrollThreshold = 0.dp) { from, to -> + onReorder(rows.rowIndexOf(from.key), rows.rowIndexOf(to.key)) } val contextReorder = - rememberReorderableLazyListState(listState) { from, to -> - onReorder(from.index, to.index) + rememberReorderableLazyListState(listState, scrollThreshold = 0.dp) { from, to -> + onReorder(rows.rowIndexOf(from.key), rows.rowIndexOf(to.key)) } LaunchedEffect(listState, ready, generation) { if (!ready) return@LaunchedEffect - if (initialIndex > 0) listState.scrollToItem(initialIndex) + if (initialIndex > 0) listState.scrollToItem(rows.lazyIndexOf(initialIndex)) snapshotFlow { val info = listState.layoutInfo - val firstVisible = info.visibleItemsInfo.firstOrNull()?.index ?: -1 - val lastVisible = info.visibleItemsInfo.lastOrNull()?.index ?: -1 - firstVisible to lastVisible - }.collect { (firstVisible, lastVisible) -> - if (firstVisible in 0..LOAD_THRESHOLD) onLoadOlder() - if (lastVisible >= 0 && lastVisible >= rows.size - 1 - LOAD_THRESHOLD) onLoadNewer() + val first = info.visibleItemsInfo.firstOrNull { it.key is Long }?.let { rows.rowIndexOf(it.key) } ?: -1 + val last = info.visibleItemsInfo.lastOrNull { it.key is Long }?.let { rows.rowIndexOf(it.key) } ?: -1 + first to last + }.collect { (first, last) -> + if (first in 0..LOAD_THRESHOLD) onLoadOlder() + if (last >= 0 && last >= rows.size - 1 - LOAD_THRESHOLD) onLoadNewer() } } @@ -377,69 +400,70 @@ fun QueueScreen( modifier = Modifier.padding(padding).fillMaxSize(), contentPadding = PaddingValues(bottom = 16.dp), ) { - items(count = rows.size, key = { rows[it].id }) { index -> - val row = rows[index] + rows.forEachIndexed { index, row -> if (index == 0 || rows[index - 1].upNext != row.upNext) { - QueueSectionHeader(if (row.upNext) "Up next" else contextTitle ?: "Next up") + item(key = "header-$index") { + QueueSectionHeader(if (row.upNext) "Up next" else contextTitle ?: "Next up") + } } val enabled = row.upNext || !shuffle - ReorderableItem( - state = if (row.upNext) upNextReorder else contextReorder, - key = row.id, - enabled = enabled, - animateItemModifier = - Modifier.animateItem( - fadeInSpec = tween(150), - fadeOutSpec = tween(150), - placementSpec = tween(200), - ), - ) { isDragging -> - QueueRowItem( - row = row, - isPlaying = row.song.song.id == currentSongId, - floating = isDragging, - coverArt = coverArt, - dragHandle = - if (!enabled) { - null - } else { - Modifier.draggableHandle( - onDragStarted = { - dragId = row.id - dragFrom = row.position - }, - onDragStopped = { - val id = dragId - val from = dragFrom - if (id != null && from != null) { - val index = rows.indexOfFirst { it.id == id } - val to = if (index < 0) from else dropTarget(rows, index, from) - val left = rows.getOrNull(index - 1)?.upNext - val right = rows.getOrNull(index + 1)?.upNext - val sameSection = - (left == null || left == row.upNext) && - (right == null || right == row.upNext) - if (to != from && sameSection) { - onMove(from, to) - showUndo("Queue reordered") - } else if (to != from) { - onRefresh() + item(key = row.id) { + ReorderableItem( + state = if (row.upNext) upNextReorder else contextReorder, + key = row.id, + enabled = enabled, + animateItemModifier = + Modifier.animateItem( + fadeInSpec = tween(150), + fadeOutSpec = tween(150), + placementSpec = tween(200), + ), + ) { isDragging -> + QueueRowItem( + row = row, + isPlaying = row.song.song.id == currentSongId, + floating = isDragging, + coverArt = coverArt, + dragHandle = + if (!enabled) { + null + } else { + Modifier.draggableHandle( + onDragStarted = { + dragId = row.id + dragFrom = row.position + }, + 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("Queue reordered") + } } - } - dragId = null - dragFrom = null - }, - ) + dragId = null + dragFrom = null + }, + ) + }, + onClick = { onPlay(row.position) }, + onRemove = { + onRemove(row.position) + showUndo("Removed from queue") }, - onClick = { onPlay(row.position) }, - onRemove = { - onRemove(row.position) - showUndo("Removed from queue") - }, - ) + ) + } } } - item { Spacer(Modifier.height(fill)) } + item(key = "fill") { Spacer(Modifier.height(fill)) } } } } @@ -456,6 +480,20 @@ private fun QueueSectionHeader(label: String) { ) } +private fun List.rowIndexOf(key: Any?): Int { + val id = key as? Long ?: return -1 + return indexOfFirst { it.id == id } +} + +private fun List.lazyIndexOf(rowIndex: Int): Int { + if (rowIndex !in indices) return 0 + var headers = 0 + for (i in 0..rowIndex) { + if (i == 0 || this[i - 1].upNext != this[i].upNext) headers++ + } + return rowIndex + headers +} + internal fun dropTarget( rows: List, index: Int, diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 21fab716..9b63a348 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -690,6 +690,49 @@ class PlaybackControllerTest { eager.close() } + @Test + fun movingAQueuedTrackBeforeTheCurrentDoesNotConsumeIt() { + seedAlbum(3, sourceId = 1) + seedSong("q1", "al2") + seedSong("q2", "al2") + seedSong("q3", "al2") + val eagerHandle = FakePlayerHandle() + val inline = + object : kotlinx.coroutines.CoroutineDispatcher() { + override fun dispatch( + context: kotlin.coroutines.CoroutineContext, + block: Runnable, + ) = block.run() + } + val eager = PlaybackController(sources, queues, FakePlayerConnection(eagerHandle), dispatcher = inline) + + eager.playAlbum(1, "al1", 0) + await { + eager.state.value.item + ?.id == "s1" + } + eager.addToQueue(1, QueueKind.Song, "q1") + eager.addToQueue(1, QueueKind.Song, "q2") + eager.addToQueue(1, QueueKind.Song, "q3") + await { runBlocking { eager.upcomingItem()?.id } == "q1" } + eager.playAt(2) + await { + eager.state.value.item + ?.id == "q2" + } + + runBlocking { eager.move(3, 2) } + Thread.sleep(300) + + assertEquals( + "q2", + eager.state.value.item + ?.id, + ) + assertEquals(3, runBlocking { queues.snapshot().upNext.size }) + eager.close() + } + @Test fun nextFollowsTheReorderedQueue() { seedAlbum(3, sourceId = 1)