From 6dea45a6344b0033768a842cddb93d3410182270 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 4 Aug 2026 08:43:36 -0400 Subject: [PATCH] =?UTF-8?q?feat(player):=20album=20art=20is=20the=20queue'?= =?UTF-8?q?s=20grab=20surface=20=E2=80=94=20#2395?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grip icon took a column out of every queue row, competing with the title for space — worst on Android, where the row is narrowest and the icon plus its 12dp gap cost roughly 36dp. Operator pre-approved dropping the icon and making the album art the drag surface; that's what this does. ## Android: the gesture change is the load-bearing part Moved the drag from the grip onto the thumbnail AND switched detectDragGestures → detectDragGesturesAfterLongPress. That second half is not cosmetic. The grip was a small target, so a plain drag detector on it never competed with anything; a 48dp thumbnail is a large chunk of every row, and with a plain detector any vertical pan starting on artwork would be swallowed as a reorder instead of scrolling the queue. The list would have felt broken exactly where it's easiest to touch. Long-press-then-drag separates the three gestures: pan scrolls, long-press reorders, tap still plays (the detector doesn't consume a plain tap, so it reaches the row's clickable). Dropping the grip also removed its contentDescription ("Reorder track"), which was the ONLY thing telling a screen reader this list could be reordered — and a long-press drag isn't operable with TalkBack regardless. Added "Move up"/"Move down" custom accessibility actions on the row, the Android counterpart to the web row's ArrowUp/ArrowDown. Without them this change would have quietly removed reordering for anyone not using touch. ## Web: the grip was never the drag surface `use:draggable` is on the row, not the handle, so dragging already worked from anywhere — the grip's only unique jobs were being the visual cue and the keyboard target. It now sits OVER the art, costing zero horizontal space, and keeps both jobs. Deliberately still VISIBLE at rest, just quiet, with the scrim appearing only on hover/focus. Overlaying already solved the space complaint, so hiding it buys nothing and would cost the only cue that the queue is reorderable — on touch especially, which has no hover. ## Scope walked back Also considered the web PlaylistTrackRow, which carries an identical grip. Left alone: it has no album art, so the approved direction doesn't apply, and its handle is already the smallest of the three at 14px. Forcing consistency would have meant inventing a third treatment for a surface nobody complained about. (Android has no playlist reorder at all — that parity gap is pre-existing and out of scope here.) Co-Authored-By: Claude Opus 5 (1M context) --- .../minstrel/player/ui/QueueScreen.kt | 112 ++++++++++++------ web/src/lib/components/QueueTrackRow.svelte | 49 +++++--- web/src/lib/components/QueueTrackRow.test.ts | 26 ++++ 3 files changed, 133 insertions(+), 54 deletions(-) diff --git a/android/app/src/main/java/com/fabledsword/minstrel/player/ui/QueueScreen.kt b/android/app/src/main/java/com/fabledsword/minstrel/player/ui/QueueScreen.kt index 2f2b5f91..490fad42 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/player/ui/QueueScreen.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/player/ui/QueueScreen.kt @@ -3,7 +3,7 @@ package com.fabledsword.minstrel.player.ui import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.background import androidx.compose.foundation.clickable -import androidx.compose.foundation.gestures.detectDragGestures +import androidx.compose.foundation.gestures.detectDragGesturesAfterLongPress import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -38,11 +38,15 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.composed import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.layout.onSizeChanged +import androidx.compose.ui.semantics.CustomAccessibilityAction +import androidx.compose.ui.semantics.customActions +import androidx.compose.ui.semantics.semantics import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp @@ -52,7 +56,6 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavHostController import com.composables.icons.lucide.ArrowDown import com.composables.icons.lucide.ArrowLeft -import com.composables.icons.lucide.GripVertical import com.composables.icons.lucide.Lucide import com.composables.icons.lucide.Music import com.composables.icons.lucide.Trash2 @@ -231,18 +234,40 @@ private fun QueueRow( .graphicsLayer { translationY = dragOffsetY } .background(highlight) .clickable(onClick = onClick) + // Replaces the capability the grip icon carried. Its + // contentDescription ("Reorder track") was the ONLY thing telling a + // screen reader this list could be reordered, and a long-press drag + // is not operable with TalkBack at all. These custom actions are the + // Android counterpart to the web row's ArrowUp/ArrowDown keys — + // without them, dropping the grip would have quietly removed + // reordering for anyone not using touch. + .semantics { + customActions = listOf( + CustomAccessibilityAction("Move up") { + if (index > 0) { onMove(index, index - 1); true } else false + }, + CustomAccessibilityAction("Move down") { + if (index < queueSize - 1) { onMove(index, index + 1); true } else false + }, + ) + } .padding(horizontal = 16.dp, vertical = 12.dp), verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(12.dp), ) { - DragHandle( - index = index, - queueSize = queueSize, - rowHeightPx = rowHeightPx, - onOffsetChange = { dragOffsetY = it }, - onMove = onMove, + // The album art IS the grab surface (#2395). The grip icon it replaces + // cost ~36dp of every row's width — icon plus its 12dp gap — on the + // narrowest surface in the app, competing with the title for space. + QueueRowThumbnail( + track = track, + dragModifier = Modifier.queueReorderDrag( + index = index, + queueSize = queueSize, + rowHeightPx = rowHeightPx, + onOffsetChange = { dragOffsetY = it }, + onMove = onMove, + ), ) - QueueRowThumbnail(track = track) if (isCurrent) { Icon( Lucide.Volume2, @@ -265,51 +290,60 @@ private fun QueueRow( } } -@Composable -private fun DragHandle( +/** + * Reorder-drag behaviour for a queue row, applied to whatever element is the + * grab surface — the album art, since #2395 removed the grip icon. + * + * Uses **detectDragGesturesAfterLongPress**, not detectDragGestures, and that + * is the load-bearing detail. The grip was a small target, so a plain drag + * gesture on it never competed with anything. A 48dp thumbnail is a large + * chunk of every row, and with a plain drag detector any vertical pan starting + * on artwork would be swallowed as a row-reorder instead of scrolling the + * queue — the list would feel broken precisely where it's easiest to touch. + * Long-press-then-drag separates the two: pan scrolls, long-press reorders, + * tap still plays (the detector doesn't consume a plain tap, so it falls + * through to the row's clickable). + */ +private fun Modifier.queueReorderDrag( index: Int, queueSize: Int, rowHeightPx: Int, onOffsetChange: (Float) -> Unit, onMove: (Int, Int) -> Unit, -) { +): Modifier = composed { // Mirrors the web queue: the row follows the finger during a drag, then on // release we translate the accumulated offset into a row delta and reorder. var offset by remember { mutableFloatStateOf(0f) } - Icon( - Lucide.GripVertical, - contentDescription = "Reorder track", - tint = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.pointerInput(index, queueSize, rowHeightPx) { - detectDragGestures( - onDrag = { change, dragAmount -> - change.consume() - offset += dragAmount.y - onOffsetChange(offset) - }, - onDragEnd = { - val delta = if (rowHeightPx > 0) (offset / rowHeightPx).roundToInt() else 0 - val target = (index + delta).coerceIn(0, queueSize - 1) - if (target != index) onMove(index, target) - offset = 0f - onOffsetChange(0f) - }, - onDragCancel = { - offset = 0f - onOffsetChange(0f) - }, - ) - }, - ) + pointerInput(index, queueSize, rowHeightPx) { + detectDragGesturesAfterLongPress( + onDrag = { change, dragAmount -> + change.consume() + offset += dragAmount.y + onOffsetChange(offset) + }, + onDragEnd = { + val delta = if (rowHeightPx > 0) (offset / rowHeightPx).roundToInt() else 0 + val target = (index + delta).coerceIn(0, queueSize - 1) + if (target != index) onMove(index, target) + offset = 0f + onOffsetChange(0f) + }, + onDragCancel = { + offset = 0f + onOffsetChange(0f) + }, + ) + } } @Composable -private fun QueueRowThumbnail(track: TrackRef) { +private fun QueueRowThumbnail(track: TrackRef, dragModifier: Modifier = Modifier) { Box( modifier = Modifier .size(48.dp) .clip(RoundedCornerShape(4.dp)) - .background(MaterialTheme.colorScheme.surfaceVariant), + .background(MaterialTheme.colorScheme.surfaceVariant) + .then(dragModifier), contentAlignment = Alignment.Center, ) { ServerImage( diff --git a/web/src/lib/components/QueueTrackRow.svelte b/web/src/lib/components/QueueTrackRow.svelte index 1c40c17c..e14a245c 100644 --- a/web/src/lib/components/QueueTrackRow.svelte +++ b/web/src/lib/components/QueueTrackRow.svelte @@ -57,22 +57,41 @@ class="flex items-center gap-2 border-b border-border px-3 py-2 h-16 {isCurrent ? 'border-l-2 border-l-accent bg-surface-hover' : ''}" > - + +
+ ((e.currentTarget as HTMLImageElement).src = FALLBACK_COVER)} + class="h-10 w-10 rounded object-cover" + /> + +