feat(player): album art is the queue's grab surface — #2395
test-web / test (push) Successful in 34s
android / Build + lint + test (push) Failing after 1m30s

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) <noreply@anthropic.com>
This commit is contained in:
2026-08-04 08:43:36 -04:00
co-authored by Claude Opus 5
parent e1e591b520
commit 6dea45a634
3 changed files with 133 additions and 54 deletions
@@ -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(
+34 -15
View File
@@ -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' : ''}"
>
<button
type="button"
aria-label="Reorder track (use arrow keys)"
aria-keyshortcuts="ArrowUp ArrowDown"
onkeydown={handleHandleKeydown}
class="cursor-grab text-text-secondary hover:text-text-primary flex-shrink-0"
>
<GripVertical size={16} />
</button>
<!--
The album art is the grab surface (#2395). The grip used to occupy its own
column in every row; it now sits OVER the art, so it costs no horizontal
space at all. `use:draggable` is on the row (above), so dragging already
worked from anywhere — the grip's real jobs are being the visual cue and
the keyboard target, and both survive here.
<img
src={coverUrl(track.album_id)}
alt=""
onerror={(e) => ((e.currentTarget as HTMLImageElement).src = FALLBACK_COVER)}
class="h-10 w-10 flex-shrink-0 rounded object-cover"
/>
It stays VISIBLE at rest, just quiet — it is the only thing that says this
list can be reordered at all, so hiding it until hover would trade the
operator's space complaint for a discoverability one (rule #24), and would
leave nothing for touch, which has no hover. The scrim only appears on
hover/focus so the artwork stays legible the rest of the time; the drop
shadow is what keeps the glyph readable over pale covers without one.
-->
<div class="relative h-10 w-10 flex-shrink-0">
<img
src={coverUrl(track.album_id)}
alt=""
onerror={(e) => ((e.currentTarget as HTMLImageElement).src = FALLBACK_COVER)}
class="h-10 w-10 rounded object-cover"
/>
<button
type="button"
aria-label="Reorder track (use arrow keys)"
aria-keyshortcuts="ArrowUp ArrowDown"
onkeydown={handleHandleKeydown}
class="group absolute inset-0 flex cursor-grab items-center justify-center rounded
text-white/70 transition hover:bg-black/45 hover:text-white
focus-visible:bg-black/45 focus-visible:text-white
focus-visible:outline focus-visible:outline-2 focus-visible:outline-accent"
style="filter: drop-shadow(0 1px 1px rgb(0 0 0 / 0.9))"
>
<GripVertical size={16} />
</button>
</div>
<button
type="button"
@@ -94,4 +94,30 @@ describe('QueueTrackRow', () => {
await fireEvent.keyDown(handle, { key: ' ' });
expect(moveQueueItem).not.toHaveBeenCalled();
});
// --- handle placement (#2395) ---
it('the reorder handle overlays the album art instead of taking its own column', () => {
const { container } = render(QueueTrackRow, {
props: { track: sampleTrack, index: 3, isCurrent: false }
});
const handle = screen.getByLabelText(/reorder track/i);
const art = container.querySelector('img');
expect(art).not.toBeNull();
// Sharing a parent is what "overlaid" means structurally. If someone moves
// the grip back into its own flex slot, this fails — which is the point:
// that slot cost horizontal space in every row and is why #2395 exists.
expect(handle.parentElement).toBe(art!.parentElement);
});
it('the handle is visible at rest, not hover-revealed', () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: false } });
const handle = screen.getByLabelText(/reorder track/i);
// Overlaying already solved the space complaint, so there is nothing to buy
// by hiding it — and hiding it would cost the only cue that the queue can
// be reordered, on touch especially, where there is no hover at all.
// Asserting the absence of `opacity-0` is stylistic and a bit brittle, but
// it is the only handle jsdom gives us on a decision worth protecting.
expect(handle.className).not.toMatch(/\bopacity-0\b/);
});
});