diff --git a/flutter_client/lib/cache/cache_first.dart b/flutter_client/lib/cache/cache_first.dart index bc724b03..3c6c15d9 100644 --- a/flutter_client/lib/cache/cache_first.dart +++ b/flutter_client/lib/cache/cache_first.dart @@ -5,6 +5,12 @@ // - empty + online → fetch via REST, populate drift, await re-emission // - empty + offline → yield mapped empty result (UI shows empty state) // +// With `alwaysRefresh: true`, also kicks off a one-shot REST refresh +// in the background after the first non-empty emission. Use for +// aggregate lists (playlists, etc.) where the server may have rows +// the local sync didn't pick up — yields cache immediately, refreshes +// silently, and drift watch() re-emits with whatever new rows landed. +// // The pattern lets every read provider trust drift as the source of // truth. SyncController keeps drift fresh in the background; widget // rebuilds happen automatically as drift writes propagate via watch(). @@ -28,10 +34,25 @@ Stream cacheFirst({ required Future Function() fetchAndPopulate, required T Function(List) toResult, required Future Function() isOnline, + bool alwaysRefresh = false, }) async* { + // Tracks whether we've already kicked off a stale-while-revalidate + // refresh for this stream subscription, so we don't fire one on every + // drift re-emission (otherwise the populate cycles forever). + var revalidated = false; + await for (final rows in driftStream) { if (rows.isNotEmpty) { yield toResult(rows); + // Stale-while-revalidate: yield cache immediately, then kick off + // a REST refresh in the background. Drift watch() picks up the + // resulting writes and re-emits via this same stream loop. + // Useful for aggregate lists (e.g. playlists) where the server + // may have rows the local sync didn't pick up. + if (alwaysRefresh && !revalidated && await isOnline()) { + revalidated = true; + unawaited(_safeFetch(fetchAndPopulate)); + } continue; } if (await isOnline()) { @@ -47,3 +68,11 @@ Stream cacheFirst({ } } } + +Future _safeFetch(Future Function() fn) async { + try { + await fn(); + } catch (_) { + // Background revalidate — swallow; UI already showed cached state. + } +} diff --git a/flutter_client/lib/playlists/playlists_provider.dart b/flutter_client/lib/playlists/playlists_provider.dart index 56b9175c..bef6f556 100644 --- a/flutter_client/lib/playlists/playlists_provider.dart +++ b/flutter_client/lib/playlists/playlists_provider.dart @@ -61,6 +61,11 @@ final playlistsListProvider = return PlaylistsList(owned: owned, public: pub); }, isOnline: () async => (await ref.read(connectivityProvider.future)), + // Aggregate list — server may add system playlists (For-You / + // Discover) that the per-user delta sync doesn't always emit. + // Stale-while-revalidate ensures the home tile row reflects the + // current server state on every screen visit. + alwaysRefresh: true, ); }); diff --git a/web/src/lib/components/PlayerBar.test.ts b/web/src/lib/components/PlayerBar.test.ts index 14f4ac00..2e8441c6 100644 --- a/web/src/lib/components/PlayerBar.test.ts +++ b/web/src/lib/components/PlayerBar.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/svelte'; +import { render, screen, fireEvent, within } from '@testing-library/svelte'; import type { TrackRef } from '$lib/api/types'; import { emptyLikesMock } from '../../test-utils/mocks/likes'; import { emptyQuarantineMock } from '../../test-utils/mocks/quarantine'; @@ -66,6 +66,15 @@ function track(): TrackRef { }); } +// PlayerBar renders both a compact (mobile, md:hidden) and desktop +// (hidden md:flex) block. jsdom doesn't apply CSS media queries, so +// both DOM trees are present in tests. Scope every query to the +// compact block — that's the user-facing change in #358; desktop +// behavior is covered structurally by the same store + handlers. +function compact() { + return within(screen.getByTestId('player-bar-compact')); +} + beforeEach(() => { state.queue = [track()]; state.current = state.queue[0]; @@ -85,9 +94,11 @@ afterEach(() => vi.clearAllMocks()); describe('PlayerBar', () => { test('renders title, linked artist, linked cover', () => { render(PlayerBar); - expect(screen.getByText('So What')).toBeInTheDocument(); - expect(screen.getByRole('link', { name: /Miles Davis/ })).toHaveAttribute('href', '/artists/md'); - const cover = screen.getByRole('link', { name: /open album/i }); + expect(compact().getByText('So What')).toBeInTheDocument(); + // The artist label sits under the title in compact (no link wrapper); + // look for it as plain text. + expect(compact().getByText('Miles Davis')).toBeInTheDocument(); + const cover = compact().getByRole('link', { name: /open album/i }); expect(cover).toHaveAttribute('href', '/albums/xyz'); }); @@ -95,14 +106,14 @@ describe('PlayerBar', () => { render(PlayerBar); // Anchored regex — "Player options" (the new overflow ⋮) also matches // /play|pause/i otherwise. - await fireEvent.click(screen.getByRole('button', { name: /^(play|pause)$/i })); + await fireEvent.click(compact().getByRole('button', { name: /^(play|pause)$/i })); expect(togglePlay).toHaveBeenCalledTimes(1); }); test('skip-next click calls skipNext; skip-prev click calls skipPrev', async () => { render(PlayerBar); - await fireEvent.click(screen.getByRole('button', { name: /next/i })); - await fireEvent.click(screen.getByRole('button', { name: /previous/i })); + await fireEvent.click(compact().getByRole('button', { name: /next/i })); + await fireEvent.click(compact().getByRole('button', { name: /previous/i })); expect(skipNext).toHaveBeenCalledTimes(1); expect(skipPrev).toHaveBeenCalledTimes(1); }); @@ -111,7 +122,7 @@ describe('PlayerBar', () => { state.index = 0; state.position = 1; render(PlayerBar); - expect(screen.getByRole('button', { name: /previous/i })).toBeDisabled(); + expect(compact().getByRole('button', { name: /previous/i })).toBeDisabled(); }); test('skip-next disabled at end with repeat=off', () => { @@ -119,19 +130,19 @@ describe('PlayerBar', () => { state.index = 0; state.repeat = 'off'; render(PlayerBar); - expect(screen.getByRole('button', { name: /next/i })).toBeDisabled(); + expect(compact().getByRole('button', { name: /next/i })).toBeDisabled(); }); test('seek slider input calls seekTo(value)', async () => { render(PlayerBar); - const slider = screen.getByLabelText(/seek/i) as HTMLInputElement; + const slider = compact().getByLabelText(/seek/i) as HTMLInputElement; await fireEvent.input(slider, { target: { value: '120' } }); expect(seekTo).toHaveBeenCalledWith(120); }); test('volume slider input calls setVolume(value)', async () => { render(PlayerBar); - const slider = screen.getByLabelText(/volume/i) as HTMLInputElement; + const slider = compact().getByLabelText(/volume/i) as HTMLInputElement; await fireEvent.input(slider, { target: { value: '0.3' } }); expect(setVolume).toHaveBeenCalledWith(0.3); }); @@ -139,28 +150,28 @@ describe('PlayerBar', () => { test('shuffle button click calls toggleShuffle; active class reflects shuffle', () => { state.shuffle = true; render(PlayerBar); - const btn = screen.getByRole('button', { name: /shuffle/i }); + const btn = compact().getByRole('button', { name: /shuffle/i }); expect(btn.getAttribute('aria-pressed')).toBe('true'); }); test('repeat button aria-label reflects current mode', () => { state.repeat = 'one'; render(PlayerBar); - expect(screen.getByRole('button', { name: /repeat one/i })).toBeInTheDocument(); + expect(compact().getByRole('button', { name: /repeat one/i })).toBeInTheDocument(); }); test('loading state shows spinner in place of play icon', () => { state.state = 'loading'; render(PlayerBar); - expect(screen.getByTestId('play-spinner')).toBeInTheDocument(); + expect(compact().getByTestId('play-spinner')).toBeInTheDocument(); }); test('error state renders retry card; retry click calls playQueue(queue, index)', async () => { state.state = 'error'; state.error = 'Playback failed.'; render(PlayerBar); - expect(screen.getByText(/playback failed/i)).toBeInTheDocument(); - await fireEvent.click(screen.getByRole('button', { name: /try again/i })); + expect(compact().getByText(/playback failed/i)).toBeInTheDocument(); + await fireEvent.click(compact().getByRole('button', { name: /try again/i })); expect(playQueue).toHaveBeenCalledWith(state.queue, state.index); }); @@ -168,21 +179,21 @@ describe('PlayerBar', () => { state.position = 65; state.duration = 245; render(PlayerBar); - expect(screen.getByText('1:05')).toBeInTheDocument(); - expect(screen.getByText('4:05')).toBeInTheDocument(); + expect(compact().getByText('1:05')).toBeInTheDocument(); + expect(compact().getByText('4:05')).toBeInTheDocument(); }); test('renders the TrackMenu kebab button when a track is current', () => { render(PlayerBar); expect( - screen.getByRole('button', { name: /track actions for/i }) + compact().getByRole('button', { name: /track actions for/i }) ).toBeInTheDocument(); }); describe('queue toggle button', () => { test('clicking the queue toggle calls toggleQueueDrawer', async () => { render(PlayerBar); - const btn = screen.getByLabelText(/(open|close) queue/i); + const btn = compact().getByLabelText(/(open|close) queue/i); await fireEvent.click(btn); expect(toggleQueueDrawer).toHaveBeenCalled(); }); @@ -190,12 +201,18 @@ describe('PlayerBar', () => { test('aria-pressed reflects queueDrawerOpen state', () => { state.queueDrawerOpen = true; render(PlayerBar); - const btn = screen.getByLabelText(/close queue/i); + const btn = compact().getByLabelText(/close queue/i); expect(btn).toHaveAttribute('aria-pressed', 'true'); }); }); - describe('"Up next" line', () => { + describe('"Up next" line (desktop only)', () => { + // The "Next · …" copy was dropped from compact per #358; only the + // desktop block surfaces it. Scope queries to desktop. + function desktop() { + return within(screen.getByTestId('player-bar-desktop')); + } + test('renders the next track title when index + 1 < queue.length', () => { state.queue = [ { id: 'a', title: 'Track A', artist_name: 'Art A' } as TrackRef, @@ -204,7 +221,7 @@ describe('PlayerBar', () => { state.index = 0; state.current = state.queue[0]; render(PlayerBar); - expect(screen.getByText(/track b/i)).toBeInTheDocument(); + expect(desktop().getByText(/track b/i)).toBeInTheDocument(); }); test('omits the line on the last track in queue', () => { @@ -214,7 +231,7 @@ describe('PlayerBar', () => { state.index = 0; state.current = state.queue[0]; render(PlayerBar); - expect(screen.queryByText(/^Next ·/i)).not.toBeInTheDocument(); + expect(desktop().queryByText(/^Next ·/i)).not.toBeInTheDocument(); }); test('omits the line when queue is empty', () => {