feat(playlists): system playlists default to shuffle on tile play
Closes Fable #412 (For You force-refresh on play) and #413 (shuffle-on-play default for system playlists). Web (PlaylistCard.svelte + player/store.svelte.ts): - Drop the refreshForYou() call from the play handler. The daily 03:00 user-local snapshot is what plays now. Stops burning server compute on every press and stops swapping the playlist out from under the user. - Generalize the kebab affordance to render for any system playlist (was Discover-only). Adds "Refresh For You" as an explicit replacement so users can still force a regen when they want one. - Extend playQueue(tracks, startIndex, { shuffle? }) to Fisher-Yates the queue when shuffle:true. PlaylistCard passes shuffle:true for any non-null system_variant. Flutter (player_provider.dart + playlists/widgets/playlist_card.dart): - playTracks now accepts shuffle:bool. When true, picks a random starting index and enables AudioServiceShuffleMode.all after setQueueFromTracks. PlaylistCard passes shuffle:playlist.isSystem. User playlists keep linear order. Detail-screen play buttons are unchanged for now (follow-up if user requests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,5 @@
|
||||
import 'dart:math' show Random;
|
||||
|
||||
import 'package:audio_service/audio_service.dart';
|
||||
import 'package:flutter_riverpod/flutter_riverpod.dart';
|
||||
|
||||
@@ -65,7 +67,18 @@ class PlayerActions {
|
||||
}
|
||||
final Ref _ref;
|
||||
|
||||
Future<void> playTracks(List<TrackRef> tracks, {int initialIndex = 0}) async {
|
||||
Future<void> playTracks(
|
||||
List<TrackRef> tracks, {
|
||||
int initialIndex = 0,
|
||||
bool shuffle = false,
|
||||
}) async {
|
||||
// shuffle=true means "play this pool randomly, starting at a
|
||||
// random track." Used for system playlists so the first track of
|
||||
// a re-play within the day is different from the prior one.
|
||||
var startAt = initialIndex;
|
||||
if (shuffle && tracks.length > 1) {
|
||||
startAt = Random().nextInt(tracks.length);
|
||||
}
|
||||
final url = await _ref.read(serverUrlProvider.future);
|
||||
final token = await _ref.read(secureStorageProvider).read(key: 'session_token');
|
||||
final cache = _ref.read(albumCoverCacheProvider);
|
||||
@@ -78,7 +91,10 @@ class PlayerActions {
|
||||
audioCacheManager: audioCache,
|
||||
likeBridge: _buildLikeBridge(),
|
||||
);
|
||||
await h.setQueueFromTracks(tracks, initialIndex: initialIndex);
|
||||
await h.setQueueFromTracks(tracks, initialIndex: startAt);
|
||||
if (shuffle) {
|
||||
await h.setShuffleMode(AudioServiceShuffleMode.all);
|
||||
}
|
||||
await h.play();
|
||||
}
|
||||
|
||||
|
||||
@@ -120,6 +120,13 @@ class PlaylistCard extends ConsumerWidget {
|
||||
));
|
||||
}
|
||||
if (refs.isEmpty) return;
|
||||
await ref.read(playerActionsProvider).playTracks(refs, initialIndex: 0);
|
||||
// System playlists default to shuffle so replaying within a day
|
||||
// doesn't deliver the identical experience. User playlists keep
|
||||
// stored order.
|
||||
await ref.read(playerActionsProvider).playTracks(
|
||||
refs,
|
||||
initialIndex: 0,
|
||||
shuffle: playlist.isSystem,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,6 +14,11 @@
|
||||
|
||||
const isOwn = $derived(user.value?.id === playlist.user_id);
|
||||
const isDiscover = $derived(playlist.system_variant === 'discover');
|
||||
const isForYou = $derived(playlist.system_variant === 'for_you');
|
||||
const isSystem = $derived(playlist.system_variant != null);
|
||||
const refreshLabel = $derived(
|
||||
isDiscover ? 'Refresh Discover' : isForYou ? 'Refresh For You' : 'Refresh',
|
||||
);
|
||||
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
@@ -40,38 +45,34 @@
|
||||
if (starting) return;
|
||||
starting = true;
|
||||
try {
|
||||
let detailID = playlist.id;
|
||||
// For-You: refresh first so click means "give me a fresh mix
|
||||
// and play it." Atomic replace creates a new playlist row, so
|
||||
// the response's playlist_id is what we follow up with.
|
||||
if (playlist.system_variant === 'for_you') {
|
||||
const refreshed = await refreshForYou();
|
||||
if (!refreshed.playlist_id) {
|
||||
// Empty library — nothing to play.
|
||||
return;
|
||||
}
|
||||
detailID = refreshed.playlist_id;
|
||||
// Invalidate caches so the home + detail views see the new
|
||||
// playlist on their next read.
|
||||
await queryClient.invalidateQueries({ queryKey: qk.playlists() });
|
||||
}
|
||||
const detail = await getPlaylist(detailID);
|
||||
const detail = await getPlaylist(playlist.id);
|
||||
const refs = toTrackRefs(detail.tracks);
|
||||
if (refs.length > 0) {
|
||||
playQueue(refs, 0);
|
||||
// System playlists default to shuffle so replaying within a
|
||||
// day feels different. User playlists keep stored order.
|
||||
// Explicit refresh moved to the kebab menu — playing no
|
||||
// longer rebuilds the snapshot.
|
||||
playQueue(refs, 0, { shuffle: playlist.system_variant != null });
|
||||
}
|
||||
} finally {
|
||||
starting = false;
|
||||
}
|
||||
}
|
||||
|
||||
async function onRefreshDiscover(e: MouseEvent) {
|
||||
async function onRefresh(e: MouseEvent) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
menuOpen = false;
|
||||
try {
|
||||
await refreshDiscover();
|
||||
pushToast('Discover refreshed.');
|
||||
if (isDiscover) {
|
||||
await refreshDiscover();
|
||||
pushToast('Discover refreshed.');
|
||||
} else if (isForYou) {
|
||||
await refreshForYou();
|
||||
pushToast('For You refreshed.');
|
||||
} else {
|
||||
return;
|
||||
}
|
||||
await queryClient.invalidateQueries({ queryKey: qk.playlist(playlist.id) });
|
||||
await queryClient.invalidateQueries({ queryKey: qk.playlists() });
|
||||
} catch (err: unknown) {
|
||||
@@ -83,7 +84,7 @@
|
||||
<svelte:window onclick={() => { menuOpen = false; }} />
|
||||
|
||||
<div class="card relative">
|
||||
{#if isDiscover}
|
||||
{#if isSystem}
|
||||
<div class="kebab-wrap absolute right-1 top-1 z-10">
|
||||
<button
|
||||
type="button"
|
||||
@@ -108,11 +109,11 @@
|
||||
<button
|
||||
type="button"
|
||||
role="menuitem"
|
||||
onclick={onRefreshDiscover}
|
||||
onclick={onRefresh}
|
||||
class="flex w-full items-center gap-2 rounded px-2 py-1.5 text-sm text-text-primary hover:bg-surface-hover"
|
||||
>
|
||||
<RefreshCcw size={14} strokeWidth={1} />
|
||||
Refresh Discover
|
||||
{refreshLabel}
|
||||
</button>
|
||||
</div>
|
||||
{/if}
|
||||
|
||||
@@ -143,7 +143,7 @@ describe('PlaylistCard', () => {
|
||||
expect(playQueue).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('shows kebab button only on Discover playlists', () => {
|
||||
test('shows kebab button on Discover playlists', () => {
|
||||
const discoverPlaylist: Playlist = {
|
||||
...base,
|
||||
kind: 'system',
|
||||
@@ -154,7 +154,7 @@ describe('PlaylistCard', () => {
|
||||
expect(screen.getByLabelText(/playlist actions/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('does not show kebab button on for_you playlists', () => {
|
||||
test('shows kebab button on For You playlists', () => {
|
||||
const forYouPlaylist: Playlist = {
|
||||
...base,
|
||||
kind: 'system',
|
||||
@@ -162,7 +162,7 @@ describe('PlaylistCard', () => {
|
||||
name: 'For You'
|
||||
};
|
||||
render(PlaylistCard, { props: { playlist: forYouPlaylist } });
|
||||
expect(screen.queryByLabelText(/playlist actions/i)).not.toBeInTheDocument();
|
||||
expect(screen.getByLabelText(/playlist actions/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('does not show kebab button on user playlists', () => {
|
||||
@@ -170,7 +170,7 @@ describe('PlaylistCard', () => {
|
||||
expect(screen.queryByLabelText(/playlist actions/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('Refresh Discover item appears in kebab menu when opened', async () => {
|
||||
test('Refresh Discover item appears in Discover kebab', async () => {
|
||||
const discoverPlaylist: Playlist = {
|
||||
...base,
|
||||
kind: 'system',
|
||||
@@ -183,7 +183,7 @@ describe('PlaylistCard', () => {
|
||||
expect(screen.getByRole('menuitem', { name: /refresh discover/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('Refresh Discover item is absent from for_you card kebab', () => {
|
||||
test('Refresh For You item appears in For You kebab', async () => {
|
||||
const forYouPlaylist: Playlist = {
|
||||
...base,
|
||||
kind: 'system',
|
||||
@@ -191,7 +191,9 @@ describe('PlaylistCard', () => {
|
||||
name: 'For You'
|
||||
};
|
||||
render(PlaylistCard, { props: { playlist: forYouPlaylist } });
|
||||
expect(screen.queryByRole('menuitem', { name: /refresh discover/i })).not.toBeInTheDocument();
|
||||
const kebabBtn = screen.getByLabelText(/playlist actions/i);
|
||||
await fireEvent.click(kebabBtn);
|
||||
expect(screen.getByRole('menuitem', { name: /refresh for you/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('clicking Refresh Discover calls refreshDiscover', async () => {
|
||||
@@ -210,7 +212,23 @@ describe('PlaylistCard', () => {
|
||||
await waitFor(() => expect(refreshDiscover).toHaveBeenCalled());
|
||||
});
|
||||
|
||||
test('For-You play button calls refreshForYou before getPlaylist', async () => {
|
||||
test('clicking Refresh For You calls refreshForYou', async () => {
|
||||
const { refreshForYou } = await import('$lib/api/playlists');
|
||||
const forYouPlaylist: Playlist = {
|
||||
...base,
|
||||
kind: 'system',
|
||||
system_variant: 'for_you',
|
||||
name: 'For You'
|
||||
};
|
||||
render(PlaylistCard, { props: { playlist: forYouPlaylist } });
|
||||
const kebabBtn = screen.getByLabelText(/playlist actions/i);
|
||||
await fireEvent.click(kebabBtn);
|
||||
const refreshItem = screen.getByRole('menuitem', { name: /refresh for you/i });
|
||||
await fireEvent.click(refreshItem);
|
||||
await waitFor(() => expect(refreshForYou).toHaveBeenCalled());
|
||||
});
|
||||
|
||||
test('For-You play button does NOT call refreshForYou (uses existing snapshot)', async () => {
|
||||
const { refreshForYou, getPlaylist } = await import('$lib/api/playlists');
|
||||
const { playQueue } = await import('$lib/player/store.svelte');
|
||||
const forYouPlaylist: Playlist = {
|
||||
@@ -220,70 +238,50 @@ describe('PlaylistCard', () => {
|
||||
name: 'For You',
|
||||
track_count: 25
|
||||
};
|
||||
// getPlaylist should return a detail with the NEW id
|
||||
(getPlaylist as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
|
||||
id: 'p-new',
|
||||
user_id: 'u-self',
|
||||
owner_username: 'me',
|
||||
name: 'For You',
|
||||
description: '',
|
||||
is_public: false,
|
||||
kind: 'system',
|
||||
system_variant: 'for_you',
|
||||
seed_artist_id: null,
|
||||
cover_url: '',
|
||||
track_count: 1,
|
||||
duration_sec: 60,
|
||||
created_at: '2026-01-01T00:00:00Z',
|
||||
updated_at: '2026-01-01T00:00:00Z',
|
||||
tracks: [
|
||||
{
|
||||
position: 0,
|
||||
track_id: 't-1',
|
||||
album_id: 'al-1',
|
||||
artist_id: 'ar-1',
|
||||
title: 'Test Track',
|
||||
artist_name: 'Test Artist',
|
||||
album_title: 'Test Album',
|
||||
duration_sec: 60,
|
||||
stream_url: '/s/t-1',
|
||||
added_at: '2026-01-01T00:00:00Z'
|
||||
}
|
||||
]
|
||||
} as PlaylistDetail);
|
||||
|
||||
render(PlaylistCard, { props: { playlist: forYouPlaylist } });
|
||||
const playButton = screen.getByLabelText(/Play For You/i);
|
||||
await fireEvent.click(playButton);
|
||||
await new Promise(resolve => setTimeout(resolve, 10));
|
||||
|
||||
expect(refreshForYou).toHaveBeenCalled();
|
||||
expect(getPlaylist).toHaveBeenCalledWith('p-new');
|
||||
// Play uses the existing playlist snapshot — no refresh on press.
|
||||
expect(refreshForYou).not.toHaveBeenCalled();
|
||||
expect(getPlaylist).toHaveBeenCalledWith('p-1');
|
||||
expect(playQueue).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('For-You play with empty-library response does not call playQueue', async () => {
|
||||
const { refreshForYou } = await import('$lib/api/playlists');
|
||||
test('For-You play passes shuffle:true to playQueue', async () => {
|
||||
const { playQueue } = await import('$lib/player/store.svelte');
|
||||
(refreshForYou as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
|
||||
playlist_id: null,
|
||||
track_count: 0,
|
||||
track_ids: []
|
||||
});
|
||||
const forYouPlaylist: Playlist = {
|
||||
...base,
|
||||
kind: 'system',
|
||||
system_variant: 'for_you',
|
||||
name: 'For You',
|
||||
track_count: 1
|
||||
name: 'For You'
|
||||
};
|
||||
render(PlaylistCard, { props: { playlist: forYouPlaylist } });
|
||||
const playButton = screen.getByLabelText(/Play For You/i);
|
||||
await fireEvent.click(playButton);
|
||||
await new Promise(resolve => setTimeout(resolve, 10));
|
||||
|
||||
expect(refreshForYou).toHaveBeenCalled();
|
||||
expect(playQueue).not.toHaveBeenCalled();
|
||||
expect(playQueue).toHaveBeenCalledWith(
|
||||
expect.any(Array),
|
||||
0,
|
||||
expect.objectContaining({ shuffle: true })
|
||||
);
|
||||
});
|
||||
|
||||
test('user-playlist play passes shuffle:false to playQueue', async () => {
|
||||
const { playQueue } = await import('$lib/player/store.svelte');
|
||||
render(PlaylistCard, { props: { playlist: base } });
|
||||
const playButton = screen.getByLabelText(/Play Saturday morning/i);
|
||||
await fireEvent.click(playButton);
|
||||
await new Promise(resolve => setTimeout(resolve, 10));
|
||||
|
||||
expect(playQueue).toHaveBeenCalledWith(
|
||||
expect.any(Array),
|
||||
0,
|
||||
expect.objectContaining({ shuffle: false })
|
||||
);
|
||||
});
|
||||
|
||||
test('non-For-You play button does NOT call refreshForYou', async () => {
|
||||
|
||||
@@ -62,15 +62,34 @@ export function registerAudioEl(el: HTMLAudioElement | null): void {
|
||||
_audioEl = el;
|
||||
}
|
||||
|
||||
export function playQueue(tracks: TrackRef[], startIndex = 0): void {
|
||||
export function playQueue(
|
||||
tracks: TrackRef[],
|
||||
startIndex = 0,
|
||||
opts: { shuffle?: boolean } = {},
|
||||
): void {
|
||||
_radioSeedId = null; // M4c: non-radio enqueue clears the radio refresh state
|
||||
_queue = tracks;
|
||||
if (tracks.length === 0) {
|
||||
if (opts.shuffle && tracks.length > 1) {
|
||||
// Fisher-Yates over the whole list. startIndex is ignored — the
|
||||
// caller is asking for "random play from this pool," so the first
|
||||
// track should also be random, not the one at startIndex.
|
||||
const arr = tracks.slice();
|
||||
for (let j = arr.length - 1; j > 0; j--) {
|
||||
const r = Math.floor(_rng() * (j + 1));
|
||||
[arr[j], arr[r]] = [arr[r], arr[j]];
|
||||
}
|
||||
_queue = arr;
|
||||
_index = 0;
|
||||
_state = 'idle';
|
||||
} else {
|
||||
_index = Math.max(0, Math.min(startIndex, tracks.length - 1));
|
||||
_shuffle = true;
|
||||
_state = 'loading';
|
||||
} else {
|
||||
_queue = tracks;
|
||||
if (tracks.length === 0) {
|
||||
_index = 0;
|
||||
_state = 'idle';
|
||||
} else {
|
||||
_index = Math.max(0, Math.min(startIndex, tracks.length - 1));
|
||||
_state = 'loading';
|
||||
}
|
||||
}
|
||||
_position = 0;
|
||||
_duration = 0;
|
||||
|
||||
Reference in New Issue
Block a user