diff --git a/package.json b/package.json index ea6b9ea..7416437 100644 --- a/package.json +++ b/package.json @@ -81,6 +81,7 @@ "test:settings-search": "node --experimental-strip-types --test src/components/search/settingsSearchRoutes.test.mts", "test:now-playing-layout": "node --experimental-strip-types --experimental-specifier-resolution=node --test src/components/player/nowPlayingLayout.test.mts src/components/player/nowPlayingPreferences.test.mts src/components/player/nowPlayingDismiss.test.mts src/playback/playbackTargetPresentation.test.mts", "test:memory-lifecycle": "node --experimental-strip-types --experimental-specifier-resolution=node --test src/components/delayedPresence.test.mts scripts/android-memory-profile.test.mjs", + "test:ui-navigation": "node --experimental-strip-types --experimental-specifier-resolution=node --test src/stores/playerPresence.test.mts src/navigation/tabsAnchor.test.mts src/navigation/libraryDetailBack.test.mts", "test:haptics": "node --experimental-strip-types --experimental-specifier-resolution=node --test src/lib/haptics.test.mts", "test:home-greeting": "node --experimental-strip-types --test src/home/homeGreeting.test.mts", "test:session": "node --experimental-strip-types --test src/session/sessionState.test.mts src/session/playbackMaterialization.test.mts", diff --git a/scripts/release/android-release.test.mjs b/scripts/release/android-release.test.mjs index 46d6fbe..f031d3b 100644 --- a/scripts/release/android-release.test.mjs +++ b/scripts/release/android-release.test.mjs @@ -1,18 +1,52 @@ import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; import test from 'node:test'; +import { fileURLToPath } from 'node:url'; import { getReleaseIdentity } from './android-release.mjs'; +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const readJson = (relativePath) => + JSON.parse(readFileSync(path.join(ROOT, relativePath), 'utf8')); + +// Read the identity from the same tracked sources the implementation uses rather +// than freezing a literal. A hardcoded version turns every release bump into a +// spurious failure — and because run-release-tests.mjs exits on the first +// failure, this suite running first meant one stale string hid every later suite. +const { version: versionName } = readJson('package.json'); +const { androidVersionCode: versionCode } = readJson('release.json'); + test('builds stable artifact names from the tracked release identity', () => { assert.deepEqual(getReleaseIdentity('github'), { - artifactFileName: 'Astra-0.1.0-1-GitHub-arm-universal.apk', + artifactFileName: `Astra-${versionName}-${versionCode}-GitHub-arm-universal.apk`, distribution: 'github', distributionLabel: 'GitHub', packageId: 'io.github.boof2015.astra', - versionCode: 1, - versionName: '0.1.0', + versionCode, + versionName, }); - assert.equal(getReleaseIdentity('google-play').artifactFileName, 'Astra-0.1.0-1-GooglePlay.aab'); + assert.equal( + getReleaseIdentity('google-play').artifactFileName, + `Astra-${versionName}-${versionCode}-GooglePlay.aab` + ); +}); + +test('artifact names carry the real version and code, not a placeholder', () => { + // Guards the composition itself: deriving both sides above would still pass if + // the template dropped a field, so assert the values actually appear. + const { artifactFileName } = getReleaseIdentity('github'); + assert.match(artifactFileName, /^Astra-\d+\.\d+\.\d+/u); + assert.ok( + artifactFileName.includes(`-${versionName}-${versionCode}-`), + `expected ${artifactFileName} to carry version ${versionName} and code ${versionCode}` + ); +}); + +test('distribution channels differ in package format', () => { + assert.ok(getReleaseIdentity('github').artifactFileName.endsWith('.apk')); + assert.ok(getReleaseIdentity('google-play').artifactFileName.endsWith('.aab')); + assert.equal(getReleaseIdentity('google-play').distributionLabel, 'Google Play'); }); test('rejects unknown distribution channels', () => { diff --git a/scripts/run-release-tests.mjs b/scripts/run-release-tests.mjs index ce6080a..1b611c0 100644 --- a/scripts/run-release-tests.mjs +++ b/scripts/run-release-tests.mjs @@ -19,6 +19,7 @@ const TEST_SCRIPTS = [ 'test:settings-search', 'test:now-playing-layout', 'test:memory-lifecycle', + 'test:ui-navigation', 'test:haptics', 'test:home-greeting', 'test:session', diff --git a/src/app/(tabs)/_layout.tsx b/src/app/(tabs)/_layout.tsx index a2272f2..ebefffe 100644 --- a/src/app/(tabs)/_layout.tsx +++ b/src/app/(tabs)/_layout.tsx @@ -5,6 +5,7 @@ import { TAB_TRANSITION_SETTLE_MS, TAB_TRANSITION_SPEC, } from '@/navigation/tabTransition'; +import { popToTop } from '@/navigation/stackActions'; import { useColors } from '@/theme/themed'; export default function TabsLayout() { @@ -46,10 +47,21 @@ export default function TabsLayout() { target: item.key, canPreventDefault: true, }); - if (!item.focused && !event.defaultPrevented) { - lastSwitchAt.current = now; - navigation.navigate(item.name); + if (event.defaultPrevented) return; + + if (item.focused) { + // Re-tapping the active tab resets its nested stack. This is the + // one-tap escape from a deep library chain (artist → album → + // another artist), which is why back itself only pops one level. + const nested = state.routes[state.index]?.state; + if (nested?.key && (nested.index ?? 0) > 0) { + navigation.dispatch({ ...popToTop(), target: nested.key }); + } + return; } + + lastSwitchAt.current = now; + navigation.navigate(item.name); }; return ; diff --git a/src/app/(tabs)/library/_layout.tsx b/src/app/(tabs)/library/_layout.tsx index ca79269..7e6c891 100644 --- a/src/app/(tabs)/library/_layout.tsx +++ b/src/app/(tabs)/library/_layout.tsx @@ -1,6 +1,16 @@ import { Stack } from 'expo-router'; import { useColors } from '@/theme/themed'; +/** + * Anchor the library stack at its list. Without this, navigating straight to a + * detail route (from Home, quick search, or the now-playing overlay) builds a + * stack of just `[album]` with nothing beneath it — so popping one level had + * nowhere to go and back escaped the tab entirely. + */ +export const unstable_settings = { + initialRouteName: 'index', +}; + /** * Nested stack inside the Library tab so album/artist detail screens keep the * tab bar + mini-player visible. diff --git a/src/app/(tabs)/library/album/[key].tsx b/src/app/(tabs)/library/album/[key].tsx index 5db4d92..792b233 100644 --- a/src/app/(tabs)/library/album/[key].tsx +++ b/src/app/(tabs)/library/album/[key].tsx @@ -40,7 +40,7 @@ export default function AlbumScreen() { const { key } = useLocalSearchParams<{ key: string }>(); const { items: tracks, summary: album, totalCount, loadMore } = useNativeAlbumDetail(key); const currentPath = usePlayerStore((s) => s.currentTrack?.path); - const handleBack = useLibraryDetailBack(); + const { goBack, backLabel } = useLibraryDetailBack(); const insets = useSafeAreaInsets(); const { scrollY, heroFaded, collapsed, onScroll, scrollEventThrottle, expandedHeight, onHeroBlockLayout } = useDetailCollapse(); @@ -154,7 +154,8 @@ export default function AlbumScreen() { } disabled={tracks.length === 0} - onBack={handleBack} + onBack={goBack} + backLabel={backLabel} onPlay={() => playFrom(0)} onShuffle={() => void playLibraryQuery({ kind: 'album', albumKey: key }, { shuffle: true, diff --git a/src/app/(tabs)/library/artist/[name].tsx b/src/app/(tabs)/library/artist/[name].tsx index 0d5c9d6..cfc051c 100644 --- a/src/app/(tabs)/library/artist/[name].tsx +++ b/src/app/(tabs)/library/artist/[name].tsx @@ -71,7 +71,7 @@ export default function ArtistScreen() { name: string; credit?: string; }>(); - const handleBack = useLibraryDetailBack(); + const { goBack, backLabel } = useLibraryDetailBack(); const insets = useSafeAreaInsets(); const { scrollY, heroFaded, collapsed, onScroll, scrollEventThrottle, expandedHeight, onHeroBlockLayout } = useDetailCollapse(); @@ -246,7 +246,8 @@ export default function ArtistScreen() { } disabled={disabled} - onBack={handleBack} + onBack={goBack} + backLabel={backLabel} onPlay={playArtist} onShuffle={shuffleArtist} scrollY={scrollY} diff --git a/src/app/(tabs)/library/index.tsx b/src/app/(tabs)/library/index.tsx index 2e62ab5..26d711a 100644 --- a/src/app/(tabs)/library/index.tsx +++ b/src/app/(tabs)/library/index.tsx @@ -1,5 +1,5 @@ import { - useEffect, + useCallback, useMemo, useState } from 'react'; @@ -11,7 +11,7 @@ import { } from 'react-native'; import { Ionicons } from '@expo/vector-icons'; import { FlashList } from '@shopify/flash-list'; -import { useRouter } from 'expo-router'; +import { useFocusEffect, useRouter } from 'expo-router'; import { Screen } from '@/components/Screen'; import { Text } from '@/components/Text'; import { ViewModeSwitcher } from '@/components/library/ViewModeSwitcher'; @@ -165,14 +165,20 @@ export default function LibraryScreen() { setPlaylistPickerOpen(false); }; - useEffect(() => { - if (!selectMode) return; - const sub = BackHandler.addEventListener('hardwareBackPress', () => { - exitSelection(); - return true; - }); - return () => sub.remove(); - }, [selectMode]); + // Focus-gated, not a plain effect: the tabs layout keeps this screen mounted + // while blurred (`detachInactiveScreens={false}` + `freezeOnBlur: false`), so + // an ungated handler swallowed one back press anywhere in the app — any other + // tab, any settings screen — whenever selection happened to be active. + useFocusEffect( + useCallback(() => { + if (!selectMode) return undefined; + const sub = BackHandler.addEventListener('hardwareBackPress', () => { + exitSelection(); + return true; + }); + return () => sub.remove(); + }, [selectMode]) + ); const selectedDbTracks = () => sortedTracks.filter((track) => selectedIds.has(track.id)); diff --git a/src/app/(tabs)/library/playlist/[id].tsx b/src/app/(tabs)/library/playlist/[id].tsx index 58fab1f..6fc6335 100644 --- a/src/app/(tabs)/library/playlist/[id].tsx +++ b/src/app/(tabs)/library/playlist/[id].tsx @@ -89,7 +89,7 @@ export default function PlaylistScreen() { const colors = useColors(); const router = useRouter(); const { id } = useLocalSearchParams<{ id: string }>(); - const handleBack = useLibraryDetailBack(); + const { goBack, backLabel } = useLibraryDetailBack(); const isFavorites = id === 'favorites'; const playlistId = isFavorites ? null : Number(id); @@ -219,7 +219,7 @@ export default function PlaylistScreen() { void (async () => { try { await deletePlaylist(target.id); - handleBack(); + goBack(); } catch (err) { Alert.alert('Delete failed', errorMessage(err)); } @@ -340,7 +340,8 @@ export default function PlaylistScreen() { ) : null } disabled={playable.length === 0} - onBack={handleBack} + onBack={goBack} + backLabel={backLabel} onMore={() => setOptionsOpen(true)} onPlay={() => startPlayback(0)} onShuffle={startShuffle} diff --git a/src/app/desktop-remote.tsx b/src/app/desktop-remote.tsx index 408e87d..1fb25d7 100644 --- a/src/app/desktop-remote.tsx +++ b/src/app/desktop-remote.tsx @@ -18,6 +18,7 @@ import { import { Image } from 'expo-image'; import { Ionicons } from '@expo/vector-icons'; import { useLocalSearchParams, useRouter } from 'expo-router'; +import { useReturnToTabs } from '@/navigation/returnToTabs'; import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { AstraLogo } from '@/components/AstraLogo'; import { Screen } from '@/components/Screen'; @@ -225,6 +226,7 @@ export default function DesktopRemoteScreen() { const ripple = useRipple(); const colors = useColors(); const router = useRouter(); + const returnToTabs = useReturnToTabs(); const pairingParams = useLocalSearchParams<{ pair?: string; baseUrl?: string; @@ -620,7 +622,7 @@ export default function DesktopRemoteScreen() { // screen so it slides in above wherever the user came from. usePlayerUiStore.getState().openPlayer(); if (router.canGoBack()) router.back(); - else router.replace('/'); + else returnToTabs('/'); }} > diff --git a/src/app/eq/import.tsx b/src/app/eq/import.tsx index e613d53..39141af 100644 --- a/src/app/eq/import.tsx +++ b/src/app/eq/import.tsx @@ -1,7 +1,8 @@ import { useMemo } from 'react'; import { Pressable, StyleSheet, View } from 'react-native'; import { Ionicons } from '@expo/vector-icons'; -import { useLocalSearchParams, useRouter } from 'expo-router'; +import { useLocalSearchParams } from 'expo-router'; +import { useReturnToTabs } from '@/navigation/returnToTabs'; import { Screen } from '@/components/Screen'; import { Text } from '@/components/Text'; import { EQPresetPreviewSheet } from '@/components/eq/EQPresetPreviewSheet'; @@ -17,7 +18,7 @@ export default function EQPresetImportScreen() { const styles = useStyles(); const ripple = useRipple(); const colors = useColors(); - const router = useRouter(); + const returnToTabs = useReturnToTabs(); const importPreset = useEQStore((state) => state.importPreset); const { data } = useLocalSearchParams<{ data?: string }>(); @@ -30,7 +31,9 @@ export default function EQPresetImportScreen() { } }, [data]); - const goToEq = () => router.replace('/eq' as never); + // `/eq` is a tab, and this screen is a root-stack sibling of `(tabs)`, so a + // bare replace here mints a second copy of the whole tab tree. + const goToEq = () => returnToTabs('/eq' as never); if (!preset) { return ( diff --git a/src/app/eq/scan.tsx b/src/app/eq/scan.tsx index 75397ce..50ff7b4 100644 --- a/src/app/eq/scan.tsx +++ b/src/app/eq/scan.tsx @@ -11,6 +11,7 @@ import { } from 'expo-camera'; import { Ionicons } from '@expo/vector-icons'; import { useRouter } from 'expo-router'; +import { useReturnToTabs } from '@/navigation/returnToTabs'; import { Screen } from '@/components/Screen'; import { Text } from '@/components/Text'; import { EQPresetPreviewSheet } from '@/components/eq/EQPresetPreviewSheet'; @@ -30,6 +31,7 @@ export default function EQPresetScanScreen() { const ripple = useRipple(); const colors = useColors(); const router = useRouter(); + const returnToTabs = useReturnToTabs(); const importPreset = useEQStore((state) => state.importPreset); const [permission, requestPermission] = useCameraPermissions(); const [pendingPreset, setPendingPreset] = useState(null); @@ -109,7 +111,8 @@ export default function EQPresetScanScreen() { onConfirm={() => { importPreset(pendingPreset); setPendingPreset(null); - router.replace('/eq' as never); + // `/eq` is a tab; see useReturnToTabs. + returnToTabs('/eq' as never); }} onClose={() => setPendingPreset(null)} /> diff --git a/src/app/notification.click.tsx b/src/app/notification.click.tsx index 1ba35de..50b0c46 100644 --- a/src/app/notification.click.tsx +++ b/src/app/notification.click.tsx @@ -1,28 +1,31 @@ import { useEffect } from 'react'; import { View } from 'react-native'; -import { useRouter } from 'expo-router'; import { resolveNotificationClick } from '@/audio/notificationIntent'; +import { useReturnToTabs } from '@/navigation/returnToTabs'; import { createThemedStyles } from '@/theme/themed'; export default function NotificationClickRoute() { const styles = useStyles(); - const router = useRouter(); + const returnToTabs = useReturnToTabs(); + // This route is a root-stack sibling of `(tabs)`, and it is entered often + // (media-notification and widget taps). A bare replace toward a tab route + // therefore left a duplicate `(tabs)` behind on every single tap. useEffect(() => { let cancelled = false; resolveNotificationClick() .then((href) => { - if (!cancelled) router.replace(href); + if (!cancelled) returnToTabs(href); }) .catch(() => { - if (!cancelled) router.replace('/'); + if (!cancelled) returnToTabs('/'); }); return () => { cancelled = true; }; - }, [router]); + }, [returnToTabs]); return ; } diff --git a/src/components/MiniPlayer.tsx b/src/components/MiniPlayer.tsx index 75f9588..c19667a 100644 --- a/src/components/MiniPlayer.tsx +++ b/src/components/MiniPlayer.tsx @@ -16,7 +16,7 @@ import Animated, { useSharedValue, withTiming, } from 'react-native-reanimated'; -import { usePlayerUiStore } from '@/stores/playerUiStore'; +import { usePlayerOnScreen, usePlayerUiStore } from '@/stores/playerUiStore'; import { Text } from './Text'; import { AstraLogo } from './AstraLogo'; import { SpectrumCurve } from './SpectrumCurve'; @@ -118,7 +118,7 @@ export function MiniPlayer() { const styles = useStyles(); const colors = useColors(); const ripple = useRipple(); - const playerOpen = usePlayerUiStore((s) => s.playerOpen); + const playerOpen = usePlayerOnScreen(); const selectedTarget = usePlaybackTargetStore((s) => s.target); const track = usePlayerStore((s) => s.currentTrack); const playbackState = usePlayerStore((s) => s.playbackState); diff --git a/src/components/PlaybackTargetPicker.tsx b/src/components/PlaybackTargetPicker.tsx index 5432502..f1b5a9b 100644 --- a/src/components/PlaybackTargetPicker.tsx +++ b/src/components/PlaybackTargetPicker.tsx @@ -14,6 +14,7 @@ import { useRipple } from '@/theme/ripple'; import { useDesktopRemoteStore } from '@/stores/desktopRemoteStore'; import { usePlaybackTargetStore, type PlaybackTarget } from '@/stores/playbackTargetStore'; import { usePlayerStore } from '@/stores/playerStore'; +import { usePlayerUiStore } from '@/stores/playerUiStore'; import { desktopConnectionLabel, hostFromBaseUrl, @@ -52,6 +53,10 @@ export function PlaybackTargetPicker({ visible, onClose }: PlaybackTargetPickerP const pairDesktop = () => { onClose(); + // This picker is usually open above the full-screen now-playing overlay, + // which would cover the pushed screen completely — the tap looked like a + // no-op. Close the player so the route is actually visible. + usePlayerUiStore.getState().closePlayer(); router.push('/desktop-remote' as never); }; diff --git a/src/components/library/CollapsingDetail.tsx b/src/components/library/CollapsingDetail.tsx index 3e5e202..76dcbdf 100644 --- a/src/components/library/CollapsingDetail.tsx +++ b/src/components/library/CollapsingDetail.tsx @@ -126,6 +126,7 @@ export function CollapsingHeader({ heroExtra, disabled, onBack, + backLabel, onMore, onPlay, onShuffle, @@ -145,6 +146,8 @@ export function CollapsingHeader({ heroExtra?: ReactNode; disabled?: boolean; onBack: () => void; + /** Names the screen `onBack` returns to; must track the real action. */ + backLabel: string; onMore?: () => void; onPlay: () => void; onShuffle: () => void; @@ -282,8 +285,17 @@ export function CollapsingHeader({ > - - Library + + {backLabel} (initialStep); const groupingMode = useSettingsStore((s) => s.artistGroupingMode); const isFavorite = usePlaylistStore((s) => s.favoritePaths.has(track.path)); const toggleFavorite = usePlaylistStore((s) => s.toggleFavorite); const artistName = resolveNavigationArtist(track, groupingMode); + const albumHref = `/library/album/${encodeURIComponent(track.album_identity_key)}`; + const artistHref = `/library/artist/${encodeURIComponent(artistName)}`; const closeAndRun = (run: () => void) => { onClose(); @@ -79,24 +86,28 @@ function TrackActionsSheetInner({ label: 'View album', icon: 'albums-outline', onPress: () => - closeAndRun(() => - router.push({ - pathname: '/library/album/[key]', - params: { key: track.album_identity_key }, - }) - ), + closeAndRun(() => { + // Already here (this sheet also opens from the album screen itself): + // pushing would stack a duplicate of the current screen. + if (pathname === albumHref) return; + returnToTabs( + { pathname: '/library/album/[key]', params: { key: track.album_identity_key } }, + 'push' + ); + }), }, { key: 'view-artist', label: 'View artist', icon: 'person-outline', onPress: () => - closeAndRun(() => - router.push({ - pathname: '/library/artist/[name]', - params: { name: artistName }, - }) - ), + closeAndRun(() => { + if (pathname === artistHref) return; + returnToTabs( + { pathname: '/library/artist/[name]', params: { name: artistName } }, + 'push' + ); + }), }, { key: 'favorite', diff --git a/src/components/player/NowPlayingHost.tsx b/src/components/player/NowPlayingHost.tsx index 63f15ba..4415352 100644 --- a/src/components/player/NowPlayingHost.tsx +++ b/src/components/player/NowPlayingHost.tsx @@ -1,24 +1,20 @@ import { NowPlayingOverlay } from '@/components/player/NowPlayingOverlay'; -import { useDelayedUnmountPresence } from '@/components/delayedPresence'; -import { NOW_PLAYING_CLOSE_UNMOUNT_MS } from '@/components/renderPresenceTiming'; -import { useAppForeground } from '@/lib/useAppForeground'; -import { usePlayerUiStore } from '@/stores/playerUiStore'; +import { usePlayerMounted } from '@/stores/playerUiStore'; /** - * Presence gate for the heavyweight now-playing tree. It stays alive just past - * the 200 ms close animation, but never remains hidden indefinitely. Android - * backgrounding drops it immediately so TextureViews and decoded art release. + * Presence gate for the heavyweight now-playing tree. The store's `closing` + * phase is the linger that lets the exit animation finish, so this is a plain + * mirror of the phase — no second timer and no foreground gate. + * + * Backgrounding deliberately does NOT unmount here any more. Dropping the whole + * tree tore down the shared value mid-close, which stranded the phase and made + * the player permanently unopenable. The overlay releases its own TextureViews + * and decoded art instead, by folding `useAppForeground` into the `active` / + * `paused` props of the scope surfaces (the same thing MiniPlayer does). */ export function NowPlayingHost() { - const playerOpen = usePlayerUiStore((s) => s.playerOpen); - const foreground = useAppForeground(); + const mounted = usePlayerMounted(); - const renderOverlay = useDelayedUnmountPresence( - playerOpen, - NOW_PLAYING_CLOSE_UNMOUNT_MS, - !foreground - ); - - if (!renderOverlay) return null; + if (!mounted) return null; return ; } diff --git a/src/components/player/NowPlayingOverlay.tsx b/src/components/player/NowPlayingOverlay.tsx index bd33a59..d97991f 100644 --- a/src/components/player/NowPlayingOverlay.tsx +++ b/src/components/player/NowPlayingOverlay.tsx @@ -42,6 +42,11 @@ import { PlayerStateIcon } from '@/components/player/PlayerStateIcon'; import { CachedLyricPeek } from '@/components/player/CachedLyricPeek'; import { resolveNowPlayingDismissSpring } from '@/components/player/nowPlayingDismiss'; import { useDelayedUnmountPresence } from '@/components/delayedPresence'; +import { + NOW_PLAYING_CLOSE_COMMIT_MS, + NOW_PLAYING_OPEN_SETTLE_MS, +} from '@/components/renderPresenceTiming'; +import { useAppForeground } from '@/lib/useAppForeground'; import { SleepTimerControls } from '@/components/player/SleepTimerControls'; import { AppSheet, AppSheetTitle } from '@/components/sheets/AppSheet'; import { @@ -63,6 +68,7 @@ import { NOW_PLAYING_WAVEFORM_TOUCH_PADDING, NOW_PLAYING_WIDE_PANE_GAP, } from '@/components/player/nowPlayingLayout'; +import { useReturnToTabs } from '@/navigation/returnToTabs'; import { resolveNavigationArtist, splitCollaborators } from '@/library/artistGrouping'; import { buildArtistNameTokens } from '@/shared/library/artistCredits'; import { @@ -76,6 +82,7 @@ import { useQueueStore } from '@/stores/queueStore'; import { usePlaylistStore } from '@/stores/playlistStore'; import { usePlaybackTargetStore } from '@/stores/playbackTargetStore'; import { usePlayerUiStore } from '@/stores/playerUiStore'; +import { isPlayerOnScreen } from '@/stores/playerPresence'; import { useSettingsStore, type ScopeMode } from '@/stores/settingsStore'; import { useSleepTimerStore } from '@/stores/sleepTimerStore'; import type { DbTrack } from '@/types/library'; @@ -125,10 +132,19 @@ export function NowPlayingOverlay() { const colors = useColors(); const ripple = useRipple(); const router = useRouter(); + const returnToTabs = useReturnToTabs(); const insets = useSafeAreaInsets(); const { width: windowWidth, height: windowHeight } = useWindowDimensions(); const reduceMotion = useReducedMotion(); - const playerOpen = usePlayerUiStore((s) => s.playerOpen); + const phase = usePlayerUiStore((s) => s.phase); + const openRequest = usePlayerUiStore((s) => s.openRequest); + const exitAnimated = usePlayerUiStore((s) => s.exitAnimated); + const playerOpen = isPlayerOnScreen(phase); + // Heavy scope/spectrum surfaces release their native backing in the + // background. The overlay itself stays mounted — unmounting it mid-close used + // to tear down the exit animation and strand the phase. + const foreground = useAppForeground(); + const surfacesLive = playerOpen && foreground; const [queueOpen, setQueueOpen] = useState(false); // Stable identity: QueueTray is memo'd, so a fresh arrow here would defeat it. const closeQueue = useCallback(() => setQueueOpen(false), []); @@ -181,13 +197,19 @@ export function NowPlayingOverlay() { }); const isDesktopTarget = activePresentation.target === 'desktop'; const effectiveScopeStageVisible = !isDesktopTarget && scopeStageVisible; + // Backgrounding drops these two subtrees, which is what actually releases the + // scope TextureViews and the decoded artwork. The overlay shell around them + // deliberately stays mounted instead — dropping the whole tree used to tear + // down the close animation mid-flight and strand the player unopenable. const renderScopeSurfaces = useDelayedUnmountPresence( effectiveScopeStageVisible, - motion.snap.duration + motion.snap.duration, + !foreground ); const renderArtworkFace = useDelayedUnmountPresence( railStyle || !effectiveScopeStageVisible, - motion.snap.duration + motion.snap.duration, + !foreground ); const activeTrack = desktopSnapshot?.currentTrack ?? null; const transitionTrackKey = isDesktopTarget ? activeTrack?.id ?? '' : track?.id ?? ''; @@ -295,23 +317,26 @@ export function NowPlayingOverlay() { .getState() .setScopeMode(scopeMode === 'spectrum' ? 'scope' : 'spectrum'); + // The player is an overlay, so it can be open over a root-stack sibling of + // `(tabs)` (e.g. desktop-remote). Going straight to a library route from there + // diverges at the root stack and mints a second copy of the whole tab tree. const navigateToArtist = (targetArtist = artistName, credit = false) => { if (!targetArtist) return; // Slide the overlay away while the library detail loads underneath. dismissSheet(); - router.navigate({ - pathname: '/library/artist/[name]', - params: { name: targetArtist, ...(credit ? { credit: '1' } : {}) }, - }); + returnToTabs( + { + pathname: '/library/artist/[name]', + params: { name: targetArtist, ...(credit ? { credit: '1' } : {}) }, + }, + 'push' + ); }; const navigateToAlbum = () => { if (!albumKey) return; dismissSheet(); - router.navigate({ - pathname: '/library/album/[key]', - params: { key: albumKey }, - }); + returnToTabs({ pathname: '/library/album/[key]', params: { key: albumKey } }, 'push'); }; const menuItems: NowPlayingMenuItem[] = []; @@ -400,12 +425,20 @@ export function NowPlayingOverlay() { useEffect(() => { stageProgress.value = withTiming(effectiveScopeStageVisible ? 1 : 0, motion.snap); }, [effectiveScopeStageVisible, stageProgress]); - // Closing is a store toggle, not navigation. Reset the inner layers so a - // reopen starts from the plain player (parity with the old per-open mount). - const dismiss = () => { + const commitClosed = () => usePlayerUiStore.getState().commitClosed(); + /** + * Enter the closing phase and drop the inner layers. Split out from + * `dismissSheet` so the pan gesture can commit the phase without handing the + * offset back to a fresh animation — it is already driving `translateY`. + * Clearing the layers here rather than at the end also unpins the menu card, + * which renders outside the translating content. + */ + const beginDismiss = () => { setMenuOpen(false); setQueueOpen(false); - usePlayerUiStore.getState().closePlayer(); + // `true`: this path drives the sheet away itself, so the effect below must + // not overwrite the offset with a competing generic slide-out. + usePlayerUiStore.getState().closePlayer(true); }; const finishCloseMenu = () => setMenuOpen(false); @@ -422,7 +455,14 @@ export function NowPlayingOverlay() { }); } + // Closing is a store transition, not navigation. The phase moves to `closing` + // BEFORE the animation starts — a spring that gets cancelled never reports + // completion, and depending on that callback is what used to leave the player + // flagged open with the sheet parked off-screen, unopenable for the rest of + // the session. The completion callback now only commits the release early; + // a fallback timer commits it otherwise. const dismissSheet = (velocity = 0) => { + beginDismiss(); translateY.value = withSpring( windowHeight, { @@ -432,7 +472,7 @@ export function NowPlayingOverlay() { overshootClamping: true, }, (finished) => { - if (finished) runOnJS(dismiss)(); + if (finished) runOnJS(commitClosed)(); } ); }; @@ -450,6 +490,9 @@ export function NowPlayingOverlay() { e.velocityY, windowHeight - translateY.value ); + // Same commit-first contract as dismissSheet, with the release spring's + // velocity-matched shaping preserved. + runOnJS(beginDismiss)(); translateY.value = withSpring( windowHeight, { @@ -460,7 +503,7 @@ export function NowPlayingOverlay() { energyThreshold: 1e-4, }, (finished) => { - if (finished) runOnJS(dismiss)(); + if (finished) runOnJS(commitClosed)(); } ); } else { @@ -468,29 +511,74 @@ export function NowPlayingOverlay() { } }); - // Open animation (close normally animates via dismissSheet's spring first; - // the else branch covers direct closePlayer calls and keeps the resting - // offset pinned to the current window height across rotation). NOTE: this - // effect must stay BELOW every direct `translateY.value` write — the react - // compiler forbids mutations after an effect that depends on the value. + // Enter animation. Keyed on `openRequest` as well as the phase, so asking for + // a player that already believes it is open still re-runs the slide-in — that + // is the recovery path for a sheet stranded off-screen by an interrupted + // close. `windowHeight` is deliberately NOT a dependency: a dimension change + // (rotation, or an RN Modal like the output picker) would re-run this effect + // and cancel an in-flight exit spring. NOTE: this effect must stay BELOW + // every direct `translateY.value` write — the react compiler forbids + // mutations after an effect that depends on the value. useEffect(() => { - if (playerOpen) { - translateY.value = withTiming(0, { duration: 240 }); - } else { - translateY.value = withTiming(windowHeight, { duration: 200 }); + if (phase === 'closing') { + // The gesture and button paths own the exit animation, including its + // velocity-matched spring shaping. Only animate here when `closing` + // arrived from a direct closePlayer() with no animation attached. + if (!exitAnimated) { + translateY.value = withTiming(windowHeight, { duration: 200 }); + } + return; } - }, [playerOpen, windowHeight, translateY]); + translateY.value = withTiming(0, { duration: 240 }); + // eslint-disable-next-line react-hooks/exhaustive-deps -- windowHeight excluded on purpose (see above) + }, [phase, openRequest, exitAnimated, translateY]); + + // `closing` → `closed`, and `opening` → `open`. Both are timers rather than + // animation callbacks, so a cancelled animation can never strand the phase. + // The store guards each transition, and this cleanup cancels a pending commit + // if the user reopens mid-close. The store actions are read through getState + // so this effect's identity never changes between playback ticks — a restarted + // timer would mean the commit never lands. + useEffect(() => { + if (phase === 'closing') { + const timer = setTimeout( + () => usePlayerUiStore.getState().commitClosed(), + NOW_PLAYING_CLOSE_COMMIT_MS + ); + return () => clearTimeout(timer); + } + if (phase === 'opening') { + const timer = setTimeout( + () => usePlayerUiStore.getState().settleOpen(), + NOW_PLAYING_OPEN_SETTLE_MS + ); + return () => clearTimeout(timer); + } + return undefined; + }, [phase, openRequest]); // Hardware back, innermost layer first: menu → queue tray → player. Registered // only while open, so it sits above the focused screen's own handlers (LIFO) // — e.g. the library-detail back interceptor underneath. useEffect(() => { - if (!playerOpen) return; + if (!playerOpen) return undefined; const sub = BackHandler.addEventListener('hardwareBackPress', () => { if (menuOpen) { closeMenu(); return true; } + if (targetPickerOpen) { + setTargetPickerOpen(false); + return true; + } + if (sleepTimerOpen) { + setSleepTimerOpen(false); + return true; + } + if (playlistActionTrack) { + setPlaylistActionTrack(null); + return true; + } if (queueOpen) { setQueueOpen(false); return true; @@ -499,7 +587,16 @@ export function NowPlayingOverlay() { return true; }); return () => sub.remove(); - }); + // eslint-disable-next-line react-hooks/exhaustive-deps -- closeMenu/dismissSheet are re-created every render; re-subscribing on each would thrash the LIFO chain. windowHeight is listed because dismissSheet captures it. + }, [ + playerOpen, + menuOpen, + targetPickerOpen, + sleepTimerOpen, + playlistActionTrack, + queueOpen, + windowHeight, + ]); const contentStyle = useAnimatedStyle(() => ({ transform: [{ translateY: translateY.value }], @@ -636,7 +733,7 @@ export function NowPlayingOverlay() { {lyricsMode && track ? ( )} @@ -1017,7 +1114,7 @@ export function NowPlayingOverlay() { width={layout.scopeWidth} height={layout.scopeHeight} mode={scopeMode} - paused={!playerOpen || queueOpen || !effectiveScopeStageVisible} + paused={!surfacesLive || queueOpen || !effectiveScopeStageVisible} revealed={effectiveScopeStageVisible} onSwap={swapScopeMode} /> @@ -1039,7 +1136,7 @@ export function NowPlayingOverlay() { width={layout.scopeWidth} height={layout.scopeHeight} mode={scopeMode} - paused={!playerOpen || queueOpen || !effectiveScopeStageVisible} + paused={!surfacesLive || queueOpen || !effectiveScopeStageVisible} revealed={effectiveScopeStageVisible} onSwap={swapScopeMode} /> @@ -1059,7 +1156,7 @@ export function NowPlayingOverlay() { {lyricPeekEnabled ? (