From 5ea4c893033fe79d0cad7b44b6abed290fd1ba0f Mon Sep 17 00:00:00 2001 From: Boof2015 <75185879+Boof2015@users.noreply.github.com> Date: Sat, 25 Jul 2026 20:46:32 -0400 Subject: [PATCH] fix queue bugs --- src/components/SwipeableRow.tsx | 12 ++- src/components/queue/QueueTray.tsx | 23 ++++-- src/components/queue/queueActions.test.mts | 87 ++++++++++++++++++++-- src/components/swipeableRowState.ts | 7 ++ 4 files changed, 109 insertions(+), 20 deletions(-) diff --git a/src/components/SwipeableRow.tsx b/src/components/SwipeableRow.tsx index 5dea81e..1fe32a3 100644 --- a/src/components/SwipeableRow.tsx +++ b/src/components/SwipeableRow.tsx @@ -21,16 +21,14 @@ import Animated, { import { useColors } from '@/theme/themed'; import { motion } from '@/theme/motion'; import { playHaptic } from '@/lib/haptics'; -import { swipeLaneOpacity } from '@/components/swipeableRowState'; +import { + SWIPE_ACTIVE_OFFSET_X, + SWIPE_FAIL_OFFSET_Y, + swipeLaneOpacity, +} from '@/components/swipeableRowState'; type IconName = keyof typeof Ionicons.glyphMap; -const SWIPE_ACTIVE_OFFSET_X = 10; -// Scroll-slop-sized: at 30 every vertical drag starting on a row had to travel -// 30px before the pan failed and the surrounding scrollable could win. Keep -// this tighter than the horizontal activation threshold so vertical intent -// yields immediately, especially inside the queue's BottomSheet scrollable. -const SWIPE_FAIL_OFFSET_Y = 6; // A fixed reveal distance avoids an onLayout -> setState -> gesture rebuild for // every recycled list row. It is also more predictable on wide tablet rows than // using half of the full row width. diff --git a/src/components/queue/QueueTray.tsx b/src/components/queue/QueueTray.tsx index ae6cf6e..7a8955a 100644 --- a/src/components/queue/QueueTray.tsx +++ b/src/components/queue/QueueTray.tsx @@ -39,6 +39,10 @@ import type { Track as RntpTrack } from 'react-native-track-player'; import { Text } from '@/components/Text'; import { AstraLogo } from '@/components/AstraLogo'; import { SwipeableRow } from '@/components/SwipeableRow'; +import { + SWIPE_ACTIVE_OFFSET_X, + SWIPE_FAIL_OFFSET_Y, +} from '@/components/swipeableRowState'; import { radius, spacing, @@ -621,11 +625,13 @@ export const QueueTray = memo(function QueueTray({ onClose, embedded = false }: const playNext = useCallback( (key: string) => { - const localIndex = entriesRef.current.findIndex((entry) => entry.key === key); + const snapshot = entriesRef.current; + const localIndex = snapshot.findIndex((entry) => entry.key === key); if (localIndex < 0) return; - const nextEntries = moveQueueEntry(entriesRef.current, localIndex, 0); + const absoluteIndex = snapshot[localIndex].absoluteIndex; + const nextEntries = moveQueueEntry(snapshot, localIndex, 0); setOptimisticEntries(nextEntries); - runAndRefresh(requeueToTop(entriesRef.current[localIndex].absoluteIndex, { + runAndRefresh(requeueToTop(absoluteIndex, { virtualPosition: virtualMode, })); }, @@ -634,14 +640,15 @@ export const QueueTray = memo(function QueueTray({ onClose, embedded = false }: const remove = useCallback( (key: string) => { - const localIndex = entriesRef.current.findIndex((entry) => entry.key === key); + const snapshot = entriesRef.current; + const localIndex = snapshot.findIndex((entry) => entry.key === key); if (localIndex < 0) return; - const action = removeQueueEntryAt(entriesRef.current, localIndex, baseOffsetRef.current); + const action = removeQueueEntryAt(snapshot, localIndex, baseOffsetRef.current); if (!action) return; setOptimisticEntries(action.nextEntries); runAndRefresh(removeFromQueue( - entriesRef.current[localIndex].absoluteIndex, + action.absoluteIndex, { updateMirror: false, virtualPosition: virtualMode }, )); }, @@ -963,6 +970,10 @@ export const QueueTray = memo(function QueueTray({ onClose, embedded = false }: enablePanDownToClose enableContentPanningGesture={!editMode} enableHandlePanningGesture + // Mirror SwipeableRow's axis lock so the sheet yields to an intentional + // horizontal row swipe while vertical movement still scrolls immediately. + activeOffsetY={[-SWIPE_FAIL_OFFSET_Y, SWIPE_FAIL_OFFSET_Y]} + failOffsetX={[-SWIPE_ACTIVE_OFFSET_X, SWIPE_ACTIVE_OFFSET_X]} onChange={onSheetChange} onClose={onClose} backdropComponent={renderBackdrop} diff --git a/src/components/queue/queueActions.test.mts b/src/components/queue/queueActions.test.mts index 14e2615..4171781 100644 --- a/src/components/queue/queueActions.test.mts +++ b/src/components/queue/queueActions.test.mts @@ -16,6 +16,14 @@ function entryKeys(entries: readonly KeyedQueueEntry[]): string[] { return entries.map((entry) => entry.key); } +interface PositionedQueueEntry extends KeyedQueueEntry { + absoluteIndex: number; +} + +function positioned(keys: string[], baseOffset: number): PositionedQueueEntry[] { + return keys.map((key, index) => ({ key, absoluteIndex: baseOffset + index })); +} + test('resolves each drag from the latest keyed queue order', () => { let upcoming = keyed(['A', 'B', 'C', 'D']); @@ -29,16 +37,81 @@ test('resolves each drag from the latest keyed queue order', () => { assert.deepEqual(entryKeys(upcoming), ['A', 'B', 'C', 'D']); }); -test('resolves swipe remove before the optimistic mirror mutation', () => { - const upcoming = keyed(['A', 'B', 'C']); - const action = removeQueueEntryAt(upcoming, 1, 1); +test('resolves first, middle, and last swipe removals from the pre-mutation order', () => { + const scenarios = [ + { localIndex: 0, absoluteIndex: 7, remaining: ['B', 'C'] }, + { localIndex: 1, absoluteIndex: 8, remaining: ['A', 'C'] }, + { localIndex: 2, absoluteIndex: 9, remaining: ['A', 'B'] }, + ]; + + for (const scenario of scenarios) { + const action = removeQueueEntryAt(keyed(['A', 'B', 'C']), scenario.localIndex, 7); + if (!action) throw new Error('expected a remove action'); + + assert.equal(action.absoluteIndex, scenario.absoluteIndex); + assert.deepEqual(entryKeys(action.nextEntries), scenario.remaining); + } +}); + +test('keeps the swiped remove index after the optimistic ref changes to the next order', () => { + let entriesRef = positioned(['A', 'B', 'C'], 1); + const localIndex = entriesRef.findIndex((entry) => entry.key === 'B'); + const action = removeQueueEntryAt(entriesRef, localIndex, 1); if (!action) throw new Error('expected a remove action'); - assert.equal(action.absoluteIndex, 2); - assert.deepEqual(entryKeys(action.nextEntries), ['A', 'C']); + entriesRef = action.nextEntries; - const mirrorAfterNativeCompletionWithSkippedUpdate = action.nextEntries; - assert.deepEqual(entryKeys(mirrorAfterNativeCompletionWithSkippedUpdate), ['A', 'C']); + assert.equal(action.absoluteIndex, 2); + assert.deepEqual(entryKeys(entriesRef), ['A', 'C']); + // Looking up localIndex after the optimistic update would incorrectly select C. + assert.equal(entriesRef[localIndex].absoluteIndex, 3); +}); + +test('removes duplicate tracks by unique row key and preserves the other occurrence', () => { + const upcoming = keyed(['same-track:0', 'same-track:1', 'tail']); + const action = removeQueueEntryAt(upcoming, 1, 20); + + if (!action) throw new Error('expected a remove action'); + assert.equal(action.absoluteIndex, 21); + assert.deepEqual(entryKeys(action.nextEntries), ['same-track:0', 'tail']); +}); + +test('uses the loaded virtual-page offset for swipe removal', () => { + const action = removeQueueEntryAt(keyed(['V500', 'V501', 'V502']), 2, 500); + + if (!action) throw new Error('expected a remove action'); + assert.equal(action.absoluteIndex, 502); + assert.deepEqual(entryKeys(action.nextEntries), ['V500', 'V501']); +}); + +test('re-resolves consecutive removals against each optimistic order', () => { + let upcoming = keyed(['A', 'B', 'C', 'D']); + + const first = removeQueueEntryAt(upcoming, 1, 10); + if (!first) throw new Error('expected the first remove action'); + upcoming = first.nextEntries; + + const second = removeQueueEntryAt(upcoming, 1, 10); + if (!second) throw new Error('expected the second remove action'); + upcoming = second.nextEntries; + + assert.equal(first.absoluteIndex, 11); + assert.equal(second.absoluteIndex, 11); + assert.deepEqual(entryKeys(upcoming), ['A', 'D']); +}); + +test('captures play-next absolute index before optimistic reordering', () => { + let entriesRef = positioned(['A', 'B', 'C'], 40); + const snapshot = entriesRef; + const localIndex = snapshot.findIndex((entry) => entry.key === 'C'); + const absoluteIndex = snapshot[localIndex].absoluteIndex; + + entriesRef = moveQueueEntry(snapshot, localIndex, 0); + + assert.equal(absoluteIndex, 42); + assert.deepEqual(entryKeys(entriesRef), ['C', 'A', 'B']); + // Looking up localIndex after the optimistic update would incorrectly select B. + assert.equal(entriesRef[localIndex].absoluteIndex, 41); }); test('captures selected absolute indices before optimistic batch mutation', () => { diff --git a/src/components/swipeableRowState.ts b/src/components/swipeableRowState.ts index d343dac..3384288 100644 --- a/src/components/swipeableRowState.ts +++ b/src/components/swipeableRowState.ts @@ -1,5 +1,12 @@ export type SwipeLaneSide = 'left' | 'right'; +export const SWIPE_ACTIVE_OFFSET_X = 10; +// Scroll-slop-sized: at 30 every vertical drag starting on a row had to travel +// 30px before the pan failed and the surrounding scrollable could win. Keep +// this tighter than the horizontal activation threshold so vertical intent +// yields immediately. +export const SWIPE_FAIL_OFFSET_Y = 6; + /** * Action lanes sit underneath an opaque row and should only exist visually * while that row is moving toward them. Keeping them opaque at rest lets a