feat(library): play all and shuffle all on every track list
Four pages that list tracks — Tracks, genre, artist, and both playlist views — had no way to start the whole list, or had a broken one. One shared helper (utils/play-all.ts) now owns what "shuffle this collection" means: SetQueue's shuffleStart only picks a random first track when shuffle mode is already on, it does not turn it on, so the mode is toggled before the queue is set. Each host passes an honest queue Source (#14): anything that builds a queue names what it built it from, so "Playing from" stops lying. Two behaviour changes ride along, both flagged: smart-playlist-details' Shuffle was a live no-op (shuffleStart without enabling mode played track 1 in order) and is fixed; playlist-details' Play all drops its shuffleStart:true, so with shuffle mode already on it now starts at the first row instead of a random one — the album page's existing semantics. Verified: make ui-test (1147, incl. a case that fails when the smart-playlist fix is reverted), npx tsc --noEmit, make e2e (255, incl. new play-all and header-fit specs), make lint, make test, make bindings-check, make css-check; artist header read from screenshots at 424/320/900 (the pair wraps below the name on a phone). Closes #31
This commit is contained in:
@@ -42,10 +42,10 @@ const ACTIONS = ['Import', 'New Playlist', 'New Smart Playlist'];
|
||||
* because the number this issue is about (a button 48px wider than the
|
||||
* box holding it) is not in the accessibility tree at all.
|
||||
*/
|
||||
const headerFit = (page: import('@playwright/test').Page) =>
|
||||
page.evaluate(() => {
|
||||
const headerFit = (page: import('@playwright/test').Page, view = 'playlist-view') =>
|
||||
page.evaluate((tag) => {
|
||||
const root = document
|
||||
.querySelector('[data-testid="main-content"] playlist-view')
|
||||
.querySelector(`[data-testid="main-content"] ${tag}`)
|
||||
?.shadowRoot?.querySelector('page-header')?.shadowRoot;
|
||||
|
||||
if (!root) return null;
|
||||
@@ -76,7 +76,7 @@ const headerFit = (page: import('@playwright/test').Page) =>
|
||||
...root.querySelectorAll('#page-header-overflow wa-dropdown-item'),
|
||||
].map((i) => i.textContent?.trim() ?? ''),
|
||||
};
|
||||
});
|
||||
}, view);
|
||||
|
||||
test.describe('the page header never clips an action', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
@@ -316,3 +316,58 @@ test.describe('the page header never clips an action', () => {
|
||||
await expect.poll(async () => (await headerFit(app))?.menu).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The Tracks header carries the play-all/shuffle-all pair (#31), so
|
||||
* the promise above has to hold for it too — the same per-button
|
||||
* measurement, one view over. Its two actions are the whole of the
|
||||
* header's declared set, and the pair is what plays the list the row
|
||||
* is in, so a button rendered 20px of its 90px is a queue of nothing.
|
||||
*/
|
||||
const TRACK_ACTIONS = ['Play all', 'Shuffle all'];
|
||||
|
||||
test.describe('the Tracks header never clips an action', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.getByTestId('nav-tracks').click();
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'tracks',
|
||||
);
|
||||
});
|
||||
|
||||
test.afterEach(async ({ app }) => {
|
||||
await app.setViewportSize({ width: 1280, height: 800 });
|
||||
});
|
||||
|
||||
for (const vp of VIEWPORTS) {
|
||||
test(`every action is reachable at ${vp.name}`, async ({ app }) => {
|
||||
await app.setViewportSize({ width: vp.width, height: vp.height });
|
||||
|
||||
await expect
|
||||
.poll(async () => (await headerFit(app, 'track-list'))?.clipped)
|
||||
.toEqual([]);
|
||||
|
||||
const fit = (await headerFit(app, 'track-list'))!;
|
||||
|
||||
expect(fit.overflow).toBeLessThanOrEqual(0);
|
||||
|
||||
// Between them, buttons and menu account for both actions —
|
||||
// not "it fits" but "nothing was dropped to make it fit".
|
||||
expect([...fit.buttons, ...fit.menu].sort()).toEqual(
|
||||
[...TRACK_ACTIONS].sort(),
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* The pair's names, through the accessibility tree — a shadow query
|
||||
* measures, but it cannot say what a screen reader is offered.
|
||||
*/
|
||||
test('both actions are named controls', async ({ app }) => {
|
||||
for (const label of TRACK_ACTIONS) {
|
||||
await expect(
|
||||
app.getByRole('button', { name: label, exact: true }),
|
||||
).toBeVisible();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,220 @@
|
||||
import { test, expect, callBinding, resetEvents, waitForEvent } from '../support/fixtures.js';
|
||||
|
||||
type Page = import('@playwright/test').Page;
|
||||
|
||||
/**
|
||||
* Play-all/Shuffle-all, asserted on what the backend queued rather than
|
||||
* on playback pixels.
|
||||
*
|
||||
* `SetQueue` reports the queue through `QueueChanged`, and `GetState`
|
||||
* says exactly what it holds: the tracks in order, whether shuffle is
|
||||
* on, and the `Source` the "Playing from" link is built from. That is
|
||||
* the honest contract here — the buttons are only as good as the queue
|
||||
* they build, and the queue is only as good as the source it names.
|
||||
*/
|
||||
|
||||
interface QueueState {
|
||||
tracks: { filePath: string; title: string }[];
|
||||
currentIndex: number;
|
||||
shuffleMode: boolean;
|
||||
source: { type: string; id: number; label: string };
|
||||
}
|
||||
|
||||
const TRACKS_SOURCE = { type: 'tracks', id: 0, label: 'All Tracks' };
|
||||
|
||||
const getQueue = (app: Page) =>
|
||||
callBinding<QueueState>(app, 'queue.Queue.GetState');
|
||||
|
||||
/** The track paths a rendered track list shows, in row order. */
|
||||
function displayedPaths(app: Page, scope: string): Promise<string[]> {
|
||||
return app
|
||||
.locator(`${scope} [data-testid="track-row"]`)
|
||||
.evaluateAll((els) =>
|
||||
els.map((el) => el.getAttribute('data-file-path') ?? ''),
|
||||
);
|
||||
}
|
||||
|
||||
/** Leave shuffle in a known state. The mode persists across specs in
|
||||
* one backend process, so a test that asserts on it has to set it. */
|
||||
async function setShuffleMode(app: Page, on: boolean): Promise<void> {
|
||||
const state = await getQueue(app);
|
||||
|
||||
if (state.shuffleMode !== on) {
|
||||
await resetEvents(app);
|
||||
await callBinding(app, 'queue.Queue.ToggleShuffle');
|
||||
await waitForEvent(app, 'QueueModeChanged');
|
||||
}
|
||||
}
|
||||
|
||||
test.describe('play-all/shuffle-all on the track list', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await callBinding(app, 'queue.Queue.Clear').catch(() => {
|
||||
/* the queue is clearable on every build these specs run against */
|
||||
});
|
||||
await setShuffleMode(app, false);
|
||||
});
|
||||
|
||||
test('Tracks Play all queues the displayed list with an honest source', async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.getByTestId('nav-tracks').click();
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'tracks',
|
||||
);
|
||||
await expect(
|
||||
app.locator('track-list [data-testid="track-row"]').first(),
|
||||
).toBeVisible();
|
||||
|
||||
const paths = await displayedPaths(app, 'track-list');
|
||||
|
||||
await resetEvents(app);
|
||||
await app.getByTestId('page-action-play-all').click();
|
||||
await waitForEvent(app, 'QueueChanged');
|
||||
|
||||
const state = await getQueue(app);
|
||||
|
||||
expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
|
||||
expect(state.currentIndex).toBe(0);
|
||||
expect(state.shuffleMode).toBe(false);
|
||||
expect(state.source).toEqual(TRACKS_SOURCE);
|
||||
});
|
||||
|
||||
test('Tracks Shuffle all turns shuffle on and keeps the source', async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.getByTestId('nav-tracks').click();
|
||||
await expect(
|
||||
app.locator('track-list [data-testid="track-row"]').first(),
|
||||
).toBeVisible();
|
||||
|
||||
const paths = await displayedPaths(app, 'track-list');
|
||||
|
||||
await resetEvents(app);
|
||||
await app.getByTestId('page-action-shuffle-all').click();
|
||||
await waitForEvent(app, 'QueueChanged');
|
||||
|
||||
const state = await getQueue(app);
|
||||
|
||||
expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
|
||||
expect(state.shuffleMode).toBe(true);
|
||||
expect(state.source).toEqual(TRACKS_SOURCE);
|
||||
});
|
||||
});
|
||||
|
||||
test.describe('play-all on an embedded track list', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await callBinding(app, 'queue.Queue.Clear').catch(() => {});
|
||||
await setShuffleMode(app, false);
|
||||
});
|
||||
|
||||
test('a genre page queues the genre with its name as the source', async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.getByTestId('nav-genres').click();
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'genres',
|
||||
);
|
||||
|
||||
const first = app.locator('genres-view .genre-card').first();
|
||||
|
||||
await expect(first).toBeVisible();
|
||||
await first.click();
|
||||
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'genre-details',
|
||||
);
|
||||
await expect(
|
||||
app.locator('genre-details [data-testid="track-row"]').first(),
|
||||
).toBeVisible();
|
||||
|
||||
const genreName = (await app
|
||||
.locator('genre-details .genre-title')
|
||||
.textContent())?.trim();
|
||||
const paths = await displayedPaths(app, 'genre-details');
|
||||
|
||||
await resetEvents(app);
|
||||
await app
|
||||
.locator('genre-details [data-testid="page-action-play-all"]')
|
||||
.click();
|
||||
await waitForEvent(app, 'QueueChanged');
|
||||
|
||||
const state = await getQueue(app);
|
||||
|
||||
expect(state.tracks.map((t) => t.filePath)).toEqual(paths);
|
||||
expect(state.currentIndex).toBe(0);
|
||||
expect(state.source).toEqual({ type: 'genre', id: 0, label: genreName });
|
||||
});
|
||||
});
|
||||
|
||||
test.describe('play-all on the library artist page', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await callBinding(app, 'queue.Queue.Clear').catch(() => {});
|
||||
await setShuffleMode(app, false);
|
||||
});
|
||||
|
||||
test('an artist page queues album paths in album order with the artist source', async ({
|
||||
app,
|
||||
}) => {
|
||||
const artists = await callBinding<{ ID: number; Name: string }[]>(
|
||||
app,
|
||||
'library.Library.GetArtists',
|
||||
[0],
|
||||
);
|
||||
const first = artists[0]!;
|
||||
|
||||
await app.evaluate(
|
||||
([id, name]) => {
|
||||
document.dispatchEvent(
|
||||
new CustomEvent('navigate', {
|
||||
detail: {
|
||||
view: 'artist-details',
|
||||
artistId: id,
|
||||
artistName: name,
|
||||
},
|
||||
bubbles: true,
|
||||
composed: true,
|
||||
}),
|
||||
);
|
||||
},
|
||||
[first.ID, first.Name] as const,
|
||||
);
|
||||
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'artist-details',
|
||||
);
|
||||
await expect(app.getByTestId('artist-play-all')).toBeEnabled();
|
||||
|
||||
const albums = await callBinding<{ ID: number }[]>(
|
||||
app,
|
||||
'library.Library.GetAlbumsByArtist',
|
||||
[first.Name, 0],
|
||||
);
|
||||
const byAlbum = await callBinding<Record<string, string[]>>(
|
||||
app,
|
||||
'library.Library.GetFilePathsByAlbums',
|
||||
[albums.map((a) => a.ID), 0],
|
||||
);
|
||||
const expected: string[] = [];
|
||||
|
||||
for (const album of albums) {
|
||||
expected.push(...(byAlbum[String(album.ID)] ?? []));
|
||||
}
|
||||
|
||||
await resetEvents(app);
|
||||
await app.getByTestId('artist-play-all').click();
|
||||
await waitForEvent(app, 'QueueChanged');
|
||||
|
||||
const state = await getQueue(app);
|
||||
|
||||
expect(state.tracks.map((t) => t.filePath)).toEqual(expected);
|
||||
expect(state.source).toEqual({
|
||||
type: 'artist',
|
||||
id: first.ID,
|
||||
label: first.Name,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -137,7 +137,7 @@ test.describe('queue', () => {
|
||||
});
|
||||
|
||||
test('shuffle and repeat toggles report their state', async ({ app }) => {
|
||||
const shuffle = app.getByRole('button', { name: 'Shuffle' });
|
||||
const shuffle = app.getByRole('button', { name: 'Shuffle', exact: true });
|
||||
|
||||
await resetEvents(app);
|
||||
await shuffle.click();
|
||||
|
||||
Reference in New Issue
Block a user