Files
minstrel/web/src/lib/components/QueueTrackRow.test.ts
T
bvandeusenandClaude Opus 5 6dea45a634
test-web / test (push) Successful in 34s
android / Build + lint + test (push) Failing after 1m30s
feat(player): album art is the queue's grab surface — #2395
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>
2026-08-04 08:43:36 -04:00

124 lines
5.6 KiB
TypeScript

import { describe, it, expect, vi, beforeEach } from 'vitest';
import { render, screen, fireEvent } from '@testing-library/svelte';
import { emptyLikesMock } from '../../test-utils/mocks/likes';
import { makeTrack } from '$test-utils/fixtures/track';
// QueueTrackRow now renders a LikeButton, which reads createLikedIdsQuery.
// Stub the likes API so the row doesn't need a real QueryClient in context.
// The mock (and its emptyLikesMock import) must precede the component import
// below: importing QueueTrackRow transitively loads LikeButton → the mocked
// module, and the hoisted factory needs emptyLikesMock already initialized.
vi.mock('$lib/api/likes', () => emptyLikesMock());
import QueueTrackRow from './QueueTrackRow.svelte';
const playFromQueueIndex = vi.fn();
const removeFromQueue = vi.fn();
const moveQueueItem = vi.fn();
vi.mock('$lib/player/store.svelte', () => ({
playFromQueueIndex: (...args: unknown[]) => playFromQueueIndex(...args),
removeFromQueue: (...args: unknown[]) => removeFromQueue(...args),
moveQueueItem: (...args: unknown[]) => moveQueueItem(...args)
}));
const sampleTrack = makeTrack({
title: 'Song Title',
artist_name: 'Artist Name'
});
describe('QueueTrackRow', () => {
beforeEach(() => {
playFromQueueIndex.mockClear();
removeFromQueue.mockClear();
moveQueueItem.mockClear();
});
it('renders track title and artist', () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: false } });
expect(screen.getByText('Song Title')).toBeInTheDocument();
expect(screen.getByText(/Artist Name/)).toBeInTheDocument();
});
it('clicking the body calls playFromQueueIndex with the row index', async () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: false } });
const body = screen.getByRole('button', { name: /play song title/i });
await fireEvent.click(body);
expect(playFromQueueIndex).toHaveBeenCalledWith(3);
});
it('clicking the remove button calls removeFromQueue with the row index', async () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: false } });
const remove = screen.getByLabelText(/remove from queue/i);
await fireEvent.click(remove);
expect(removeFromQueue).toHaveBeenCalledWith(3);
});
it('current row shows the "Now playing" chip', () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: true } });
expect(screen.getByText(/now playing/i)).toBeInTheDocument();
});
it('current row body click does NOT call playFromQueueIndex', async () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: true } });
// The body button has a different aria-label when isCurrent
const body = screen.getByLabelText(/now playing/i);
await fireEvent.click(body);
expect(playFromQueueIndex).not.toHaveBeenCalled();
});
it('ArrowDown on the drag handle calls moveQueueItem with index+1', async () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: false } });
const handle = screen.getByLabelText(/reorder track/i);
await fireEvent.keyDown(handle, { key: 'ArrowDown' });
expect(moveQueueItem).toHaveBeenCalledWith(3, 4);
});
it('ArrowUp on the drag handle calls moveQueueItem with index-1', async () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: false } });
const handle = screen.getByLabelText(/reorder track/i);
await fireEvent.keyDown(handle, { key: 'ArrowUp' });
expect(moveQueueItem).toHaveBeenCalledWith(3, 2);
});
it('Enter on the drag handle prevents default but does not call moveQueueItem', async () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: false } });
const handle = screen.getByLabelText(/reorder track/i);
await fireEvent.keyDown(handle, { key: 'Enter' });
expect(moveQueueItem).not.toHaveBeenCalled();
});
it('Space on the drag handle prevents default but does not call moveQueueItem', async () => {
render(QueueTrackRow, { props: { track: sampleTrack, index: 3, isCurrent: false } });
const handle = screen.getByLabelText(/reorder track/i);
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/);
});
});