diff --git a/src/components/lyrics/LyricsBand.tsx b/src/components/lyrics/LyricsBand.tsx index 3315d3c..c6c341d 100644 --- a/src/components/lyrics/LyricsBand.tsx +++ b/src/components/lyrics/LyricsBand.tsx @@ -7,14 +7,17 @@ // static scroll; loading/not-found/error states get a centered message. import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; -import { Pressable, ScrollView, View, type LayoutChangeEvent } from 'react-native'; +import { ActivityIndicator, Pressable, ScrollView, View, type LayoutChangeEvent } from 'react-native'; +import { Ionicons } from '@expo/vector-icons'; import { Text } from '@/components/Text'; +import { radius, spacing } from '@/theme'; import { useColors } from '@/theme/themed'; import { SCROLL_PRESS_DELAY, useRipple } from '@/theme/ripple'; import { useSmoothPlaybackTime } from '@/audio/useSmoothPlaybackTime'; import { useLyricsStore } from '@/stores/lyricsStore'; import { useLyricsSettingsStore } from '@/stores/lyricsSettingsStore'; import { + getLyricsEmptyStatePresentation, getLyricsLineSeekTimeSeconds, getSyncedLyricsDisplayLines, getSyncedLyricsGapProgress, @@ -164,19 +167,7 @@ export function LyricsBand({ centerOn(focusIndex, true); }, [centerOn, focusIndex]); - const message = !hasSynced - ? result?.status === 'transient_error' - ? 'Lyrics lookup ran into a problem. A retry may work.' - : result?.status === 'not_found' - ? result.reason === 'online-disabled' - ? 'Online lyrics lookup is off.' - : result.reason === 'provider-unavailable' - ? "Lyrics providers didn't respond in time." - : 'No lyrics found for this track.' - : isLoading - ? 'Finding lyrics…' - : 'Lyrics are ready when a track is playing.' - : null; + const emptyState = getLyricsEmptyStatePresentation({ result, isLoading }); // --- plain (unsynced) hit --- if (result?.status === 'hit' && !hasSynced) { @@ -197,10 +188,52 @@ export function LyricsBand({ // --- loading / not-found / error --- if (!hasSynced) { return ( - + - {message} + {emptyState.message} + {emptyState.retryable ? ( + void loadForTrack(track, { force: true })} + accessibilityRole="button" + accessibilityLabel="Retry lyrics lookup" + accessibilityState={{ disabled: isLoading, busy: isLoading }} + style={({ pressed }) => ({ + minHeight: 40, + flexDirection: 'row', + alignItems: 'center', + justifyContent: 'center', + gap: spacing.sm, + paddingHorizontal: spacing.lg, + paddingVertical: spacing.sm, + borderRadius: radius.pill, + borderWidth: 1, + borderColor: colors.glassBorder, + backgroundColor: colors.accentGlow, + overflow: 'hidden', + opacity: isLoading ? 0.65 : pressed ? 0.82 : 1, + })} + > + {isLoading ? ( + + ) : ( + + )} + + {isLoading ? 'Retrying…' : 'Retry'} + + + ) : null} ); } diff --git a/src/lyrics/presentation.test.mts b/src/lyrics/presentation.test.mts index 116d2bf..0c36cba 100644 --- a/src/lyrics/presentation.test.mts +++ b/src/lyrics/presentation.test.mts @@ -5,6 +5,7 @@ import { findActiveSyncedLineIndex, getActiveSyncedLyricsLine, getCompensatedLyricsTime, + getLyricsEmptyStatePresentation, getLyricsLineSeekTimeSeconds, getLyricsMetaChipText, getPreferredLyricsTranslation, @@ -116,3 +117,27 @@ test('meta chip reflects source, sync, and cache state', () => { 'Not Found' ); }); + +test('lyrics empty states offer retry only when another lookup can help', () => { + const transientError = { status: 'transient_error' as const, message: 'network failed' }; + const unavailable = { status: 'not_found' as const, reason: 'provider-unavailable' as const }; + const providerMiss = { status: 'not_found' as const, reason: 'provider-not-found' as const }; + const embeddedMiss = { status: 'not_found' as const, reason: 'embedded-missing' as const }; + const onlineDisabled = { status: 'not_found' as const, reason: 'online-disabled' as const }; + + assert.equal(getLyricsEmptyStatePresentation({ result: transientError, isLoading: false }).retryable, true); + assert.equal(getLyricsEmptyStatePresentation({ result: unavailable, isLoading: false }).retryable, true); + assert.equal(getLyricsEmptyStatePresentation({ result: providerMiss, isLoading: false }).retryable, true); + assert.equal(getLyricsEmptyStatePresentation({ result: embeddedMiss, isLoading: false }).retryable, true); + assert.equal(getLyricsEmptyStatePresentation({ result: onlineDisabled, isLoading: false }).retryable, false); + assert.equal(getLyricsEmptyStatePresentation({ result: hit('xlrcdb', 'xlrc', false), isLoading: false }).retryable, false); +}); + +test('lyrics retry keeps its action while showing loading feedback', () => { + const state = getLyricsEmptyStatePresentation({ + result: { status: 'not_found', reason: 'provider-not-found' }, + isLoading: true, + }); + + assert.deepEqual(state, { message: 'Finding lyrics…', retryable: true }); +}); diff --git a/src/lyrics/presentation.ts b/src/lyrics/presentation.ts index fd8fb7a..14aa07a 100644 --- a/src/lyrics/presentation.ts +++ b/src/lyrics/presentation.ts @@ -8,6 +8,7 @@ import type { LyricsFormat, LyricsLine, + LyricsLookupResult, LyricsPayload, LyricsSource, LyricsTranslation, @@ -449,3 +450,35 @@ export function getLyricsMetaChipText(options: { } return 'Ready'; } + +export interface LyricsEmptyStatePresentation { + message: string; + /** Keep the action visible while a retry is in flight so the layout stays stable. */ + retryable: boolean; +} + +/** Copy and retry eligibility for the shared phone/tablet lyrics empty state. */ +export function getLyricsEmptyStatePresentation(options: { + result: LyricsLookupResult | null; + isLoading: boolean; +}): LyricsEmptyStatePresentation { + const { result, isLoading } = options; + const retryable = + result?.status === 'transient_error' || + (result?.status === 'not_found' && result.reason !== 'online-disabled'); + + if (isLoading) return { message: 'Finding lyrics…', retryable }; + if (result?.status === 'transient_error') { + return { message: 'Lyrics lookup ran into a problem. A retry may work.', retryable }; + } + if (result?.status === 'not_found') { + if (result.reason === 'online-disabled') { + return { message: 'Online lyrics lookup is off.', retryable }; + } + if (result.reason === 'provider-unavailable') { + return { message: "Lyrics providers didn't respond in time.", retryable }; + } + return { message: 'No lyrics found for this track.', retryable }; + } + return { message: 'Lyrics are ready when a track is playing.', retryable }; +} diff --git a/src/lyrics/resolver.test.mts b/src/lyrics/resolver.test.mts index 4fe904f..a9a8e31 100644 --- a/src/lyrics/resolver.test.mts +++ b/src/lyrics/resolver.test.mts @@ -166,3 +166,38 @@ test('local misses fall through to XLRCDB then LRCLIB and cache definitive misse assert.equal(calls.lrclib, 1); assert.equal(calls.notFound, 1); }); + +test('force refresh bypasses a cached miss and returns a fresh provider hit', async () => { + const fresh = payload('xlrcdb', 'Fresh provider lyrics'); + const { calls, deps } = dependencies({ + getCache: async () => { + calls.cache += 1; + return { + status: 'not_found', + source: 'xlrcdb', + provider: 'xlrcdb', + format: null, + plainLyrics: null, + syncedLyrics: null, + syncedLines: [], + }; + }, + lookupXlrcdb: async (_query, forceRefresh) => { + calls.xlrcdb += 1; + assert.equal(forceRefresh, true); + return { status: 'hit', lyrics: fresh }; + }, + }); + + const result = await resolveLyricsWithDependencies( + QUERY, + { forceRefresh: true, onlineEnabled: true }, + deps + ); + + assert.equal(result.status === 'hit' ? result.lyrics.plainLyrics : '', 'Fresh provider lyrics'); + assert.equal(result.status === 'hit' ? result.cached : true, false); + assert.equal(calls.cache, 1); + assert.equal(calls.xlrcdb, 1); + assert.equal(calls.lrclib, 0); +}); diff --git a/src/stores/lyricsStore.ts b/src/stores/lyricsStore.ts index 7d83e41..29775f8 100644 --- a/src/stores/lyricsStore.ts +++ b/src/stores/lyricsStore.ts @@ -49,7 +49,10 @@ export const useLyricsStore = create((set, get) => ({ const force = Boolean(options.force); const existing = get().byPath[path]; - if (!force && existing && (existing.result || existing.loading)) return; + // A forced retry may replace a completed result, but it should never start + // a second request while this track already has one in progress. + if (existing?.loading) return; + if (!force && existing?.result) return; const requestId = ++requestSeq; requestIds.set(path, requestId);