diff --git a/HANDOFF.md b/HANDOFF.md index d8a45b3..26d830f 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -17,17 +17,29 @@ npm run lint && npm run typecheck && npm test cd android && ./gradlew :audio-tags:testDebugUnitTest ``` -All four must be green — currently **292 tests / 20 suites**, working tree clean. +All four must be green — currently **335 tests / 24 suites**, working tree clean. Do not build on red. -## Do NOT branch from `main` +`npm test` needs watchman to be usable. Where it is not, `npx jest --watchman=false` +is the same run. -`main` contains **3 files** (AGENTS.md and two docs). It is not the project. All -225 files live on `fix/performance-ux-stats`, and `main` is an ancestor of it. -Two previous sessions were told "the user merged to main, branch from there" — -it was not true either time, and branching from `main` would discard everything. -Stay on `fix/performance-ux-stats` unless `git ls-tree -r --name-only main | wc -l` -says otherwise. +## Check what `main` holds before branching from it + +This section used to say `main` contained three files and must never be branched +from. **That is no longer true**: PR #2 merged the project into `main` on +2026-08-02, and `main` is now the whole app. + +It is still worth checking rather than assuming, in either direction. As of +2026-08-02 the newest work sits *ahead* of `main` on `fix/ux-round-2` — four +commits including `listenCycle.ts`, migration 0003 and the removal of +`app/player.tsx` — so a branch taken from `main` on that date would have +silently dropped them. `fix/final-polish` was therefore taken from +`fix/ux-round-2`, which is `main` plus those four. + +```bash +git log --oneline main..HEAD # what would be lost by branching from main +git diff --stat main..HEAD | tail -1 +``` ## State diff --git a/app/(tabs)/_layout.tsx b/app/(tabs)/_layout.tsx index 50441c2..8241e69 100644 --- a/app/(tabs)/_layout.tsx +++ b/app/(tabs)/_layout.tsx @@ -5,11 +5,12 @@ import { Tabs } from 'expo-router'; import { BottomTabBar } from 'expo-router/build/react-navigation/bottom-tabs'; import type { BottomTabBarProps } from 'expo-router/build/react-navigation/bottom-tabs'; import { BarChart3, Disc3, ListMusic, SlidersHorizontal } from 'lucide-react-native'; +import { useCallback, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; -import { View } from 'react-native'; +import { type LayoutChangeEvent, View } from 'react-native'; import { Toaster } from '@/components/ui/Toaster'; -import { MiniPlayer } from '@/features/player/components/MiniPlayer'; +import { setPlayerTabBarHeight } from '@/features/player/playerLayerLayout'; import * as perf from '@/services/perf'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { useTheme } from '@/theme/useTheme'; @@ -23,8 +24,14 @@ import { useTheme } from '@/theme/useTheme'; */ function TabBarWithPlayer(props: BottomTabBarProps) { useLifecycleTrace('TabBar'); + const onLayout = useCallback((event: LayoutChangeEvent) => { + setPlayerTabBarHeight(event.nativeEvent.layout.height); + }, []); + + useEffect(() => () => setPlayerTabBarHeight(0), []); + return ( - + {/* Toasts stack directly on top of the transport, which is why they live here rather than at the root. Positioning them from the root would mean @@ -35,7 +42,6 @@ function TabBarWithPlayer(props: BottomTabBarProps) { Stacking solves it with neither. */} - ); diff --git a/app/_layout.tsx b/app/_layout.tsx index 10b4db4..c341692 100644 --- a/app/_layout.tsx +++ b/app/_layout.tsx @@ -9,6 +9,7 @@ import { useEffect } from 'react'; import { GestureHandlerRootView } from 'react-native-gesture-handler'; import { useDatabase } from '@/db/useDatabase'; +import { PlayerLayer } from '@/features/player/PlayerLayer'; import { startListenRecording } from '@/features/player/listenRecorder'; import { initI18n } from '@/i18n'; import { APP_FONTS } from '@/theme/fonts'; @@ -56,14 +57,17 @@ export default function RootLayout() { // one, and gesture-handler throws rather than silently ignoring gestures. - - - {/* Now Playing is somewhere you go from a track and dismiss, not a - destination you switch to — so it presents rather than pushes. */} - - - - + + {/* + No queue route. It was a modal here and it never appeared: PlayerLayer + mounts Now Playing outside the navigator, and an opaque full-screen + overlay covers whatever the navigator puts under it. The queue is a + root-level sheet now — see `QueueOverlay`. + */} + + + + ); } diff --git a/app/player.tsx b/app/player.tsx deleted file mode 100644 index 4110895..0000000 --- a/app/player.tsx +++ /dev/null @@ -1,5 +0,0 @@ -import { PlayerScreen } from '@/features/player/PlayerScreen'; - -export default function Player() { - return ; -} diff --git a/app/queue.tsx b/app/queue.tsx deleted file mode 100644 index cc595aa..0000000 --- a/app/queue.tsx +++ /dev/null @@ -1,5 +0,0 @@ -import { QueueScreen } from '@/features/player/QueueScreen'; - -export default function Queue() { - return ; -} diff --git a/assets.d.ts b/assets.d.ts new file mode 100644 index 0000000..4a1cae0 --- /dev/null +++ b/assets.d.ts @@ -0,0 +1,20 @@ +/** + * Image imports. + * + * Neither `expo/types` nor React Native 0.86 declares these — `expo/types` + * covers CSS and nothing else, and the image declarations that used to come + * from `@types/react-native` went away with that package. So importing a PNG + * is a type error until something says what one is. + * + * On native, Metro replaces the import with an asset-registry id, which is a + * number. That is what `expo-asset`'s `Asset.fromModule` takes. + */ +declare module '*.png' { + const asset: number; + export default asset; +} + +declare module '*.jpg' { + const asset: number; + export default asset; +} diff --git a/assets/images/notification-artwork.png b/assets/images/notification-artwork.png new file mode 100644 index 0000000..50ffe47 Binary files /dev/null and b/assets/images/notification-artwork.png differ diff --git a/docs/01-TECH-STACK.md b/docs/01-TECH-STACK.md index a0133ac..27abd2e 100644 --- a/docs/01-TECH-STACK.md +++ b/docs/01-TECH-STACK.md @@ -25,7 +25,7 @@ These are the four choices that shape everything else. Read §2 before locking t |---|---|---| | Framework | **Expo SDK 57** (RN 0.86, React 19.2) | SDK 56 (RN 0.85) is the conservative pick. SDK 55+ is New-Architecture-only. | | Language | **TypeScript**, `strict: true` | No `any` in committed code. | -| Routing | **expo-router** (file-based) | Tabs + a modal route for Now Playing. | +| Routing | **expo-router** (file-based) | Tabs and detail routes; Now Playing is a root overlay. | | Audio | **`expo-audio`** | Background playback, media notification, lock-screen controls, playlists. Media3/ExoPlayer on Android → native FLAC, ALAC, Opus, Vorbis, WAV support. | | Styling | **NativeWind** (Tailwind for RN) | Requested. Dark mode via `dark:` variant. | | Database | **`expo-sqlite`** + **Drizzle ORM** | Typed schema, `drizzle-kit` migrations, `useLiveQuery` for reactive lists. | @@ -127,7 +127,6 @@ app/ # expo-router routes ONLY — thin, no logic playlists.tsx stats.tsx settings.tsx - player.tsx # Now Playing (modal) playlist/[id].tsx album/[id].tsx artist/[id].tsx @@ -140,6 +139,7 @@ src/ stats/ # StatCard, TopList, PeriodPicker, WrappedCard features/ # feature-scoped hooks + orchestration library/ player/ playlists/ stats/ settings/ scanner/ + player/PlayerLayer.tsx # root Now Playing overlay, outside routes db/ client.ts # openDatabaseSync + pragmas schema.ts # Drizzle schema diff --git a/docs/adr/006-manual-add-is-first-class.md b/docs/adr/006-manual-add-is-first-class.md index c41e19b..fb84bd1 100644 --- a/docs/adr/006-manual-add-is-first-class.md +++ b/docs/adr/006-manual-add-is-first-class.md @@ -1,5 +1,8 @@ # 006 — Manual adding is a first-class entry point +> **Partly superseded by ADR 010.** Folder import remains a first-class path, +> but the automatic launch sweep was removed. Every scan is now user-initiated. + ## Context The original Phase 2 plan had automatic MediaStore scanning as the way music @@ -24,8 +27,8 @@ Two equal entry points into one pipeline. action inside the empty state. It opens the system folder picker, records the chosen tree URI in `scan_folders`, and runs the same scan. -The automatic MediaStore sweep still runs in the background without the user -asking, because for the common case it costs nothing and needs no interaction. +The library scan is explicit as well. The user starts it from the permanent +Scan action; the common case stays one action without hidden work at launch. Both go through `enumerateLibrary` then `enrichLibrary`, write through the same queries, and report the same `ScanProgress`. There is no second code path to diff --git a/docs/adr/007-saf-folders-go-through-mediastore.md b/docs/adr/007-saf-folders-go-through-mediastore.md index 91540c2..295e138 100644 --- a/docs/adr/007-saf-folders-go-through-mediastore.md +++ b/docs/adr/007-saf-folders-go-through-mediastore.md @@ -24,16 +24,18 @@ handles it with no second code path. The second also fixes a separate problem the tree walk would not have touched: a file copied over USB is frequently not in MediaStore for minutes, because nothing has told the scanner it exists. That is not a "manual add" case at all -— it is the ordinary automatic scan appearing to lose files. +— it is the ordinary library scan appearing to lose files. ## Decision **Trigger a media scan; do not walk the tree.** `requestMediaScan(paths)` wraps `MediaScannerConnection.scanFile()` and -resolves once the scanner has visited every path. `addFolder` calls it for the -picked folder, then runs the normal two-stage scan. `tracks.file_uri` stays a -single kind of URI throughout the app. +resolves once the scanner has visited every path. `importFolder` calls it for +the picked folder, then runs the normal two-stage scan. `tracks.file_uri` stays +a single kind of URI throughout the app. The library first shows a clear +confirmation, then a full-screen cancellable progress state until both stages +finish. The same call also backs the manual **rescan** affordance, so a user who has just copied files in can pull to refresh and see them without restarting the @@ -53,14 +55,14 @@ scan reads MediaStore, not the folder list. They exist so a future rescan can re-index the same folders, and so Library settings can show what was added. Converting a SAF tree URI to filesystem paths is not always possible on modern -Android. Where it fails, `addFolder` still runs the normal sweep — the user +Android. Where it fails, `importFolder` still runs the normal sweep — the user gets whatever MediaStore already knows, rather than an error for something they cannot act on. `requestMediaScan` no longer resolves strictly on the last callback. The callback is not guaranteed to fire once per path — a path that does not exist, or a directory the provider declines to walk, can be dropped — and a dropped -one left the promise unsettled forever. Since `addFolder` awaits it *before* +one left the promise unsettled forever. Since `importFolder` awaits it *before* starting the scan, that was a frozen screen with no error state to show. It now settles on whatever has arrived after ten seconds. diff --git a/docs/adr/008-permission-is-asked-not-assumed.md b/docs/adr/008-permission-is-asked-not-assumed.md index 8bc2a97..42f9cd4 100644 --- a/docs/adr/008-permission-is-asked-not-assumed.md +++ b/docs/adr/008-permission-is-asked-not-assumed.md @@ -39,7 +39,7 @@ is a button that silently does nothing. `permissionErrorFor` maps the answer to the error code the screen renders and is unit tested, per the rule that logic belongs in `src/services/` rather than in a hook body. -`addFolder` asks **before** opening the picker. Asking afterwards means a user +`pickFolder` asks **before** opening the picker. Asking afterwards means a user who declines has chosen a folder for nothing. ## Consequences diff --git a/docs/adr/009-expo-audio-and-our-own-queue.md b/docs/adr/009-expo-audio-and-our-own-queue.md index 4a8bbfe..277678b 100644 --- a/docs/adr/009-expo-audio-and-our-own-queue.md +++ b/docs/adr/009-expo-audio-and-our-own-queue.md @@ -45,8 +45,8 @@ repeat-one meeting an explicit skip — is a unit test that runs on a laptop. repeat-one repeats; a track the user skips advances. Same mode, different input, and treating them identically makes the button look broken. -**The three-second rule lives in the engine, not the queue.** Pressing previous -more than three seconds in restarts the current track. That needs the playback +**The ten-second rule lives in the engine, not the queue.** Pressing previous +at or beyond ten seconds restarts the current track. That needs the playback position, which the queue does not have and should not. ## Consequences diff --git a/docs/adr/014-queue-is-a-root-sheet-not-a-route.md b/docs/adr/014-queue-is-a-root-sheet-not-a-route.md new file mode 100644 index 0000000..6ff16c0 --- /dev/null +++ b/docs/adr/014-queue-is-a-root-sheet-not-a-route.md @@ -0,0 +1,70 @@ +# 014 — The queue is a root-level sheet, not a route + +## Context + +The queue was `app/queue.tsx`, declared in the root `Stack` with +`presentation: 'modal'` and opened with `router.navigate('/queue')`. Pressing +the queue button did open it. Nothing was ever visible. + +The cause is not in `QueueScreen`. `PlayerLayer` mounts Now Playing **outside +the navigator** — an absolutely positioned, full-screen, opaque surface over +every route, which is what lets the mini player and the expanded player share +one gesture progress value and what keeps playback controls above every screen. +An overlay at that level covers whatever the navigator puts underneath it. The +queue was rendering correctly, one layer down, behind the player that opened +it. + +That is worth stating in its general form, because it is not a fact about the +queue: **any route pushed while Now Playing is open disappears the same way.** +The overlay is a second, higher stacking context that expo-router does not know +about. + +There is a second, independent reason a route cannot work here. The overlay +carries a `translateY` transform and clips its contents, and a transformed +ancestor creates a containing block — so even a surface that won the z-order +would be positioned and clipped relative to the overlay rather than the window. + +## Decision + +**The queue is a sibling of Now Playing at the root, owned by `PlayerLayer`, +one layer above it.** `QueueOverlay` renders it; `PlayerLayer` holds the open +state; `PlayerScreen` receives `onOpenQueue` as a prop. The route and its +`Stack.Screen` entry are deleted. + +This is the same treatment Now Playing itself already gets, and for the same +reason: it is a surface belonging to the player, not a destination in the app's +navigation. It also matches how it behaves — the queue is opened from Now +Playing and dismissed back to it, never navigated *through*. + +The alternative considered was collapsing Now Playing before navigating, so the +modal had nothing above it. It works, and it is wrong: dismissing the queue +would then return to the tab the user came from rather than to the player they +opened it from, and the queue would visibly close the player to open itself. + +## Consequences + +`QueueScreen` takes an `onClose` prop instead of calling `router.back()`. It is +mounted only while open — it carries a FlashList over the whole queue, and the +player should not pay for that while nobody is looking at it. + +The sheet animates with Reanimated's `SlideInDown`/`SlideOutDown` layout +animations rather than a shared value, which keeps it clear of the React +Compiler's immutability rule about shared values captured by hooks — the reason +`playerExpansion` is a module-level `makeMutable` and the reason gestures in +this feature are built inline. + +Deep-linking to the queue is gone. Nothing linked to it, `mufify://queue` was +never documented, and a queue is transient state rather than an addressable +place. + +**Anything else that needs to appear over Now Playing has to go here too.** A +future "add to playlist" sheet opened from the player cannot be a route. That +is the standing cost of a root-mounted player overlay, and it is a cost this +project already accepted deliberately — see `docs/player.md` on why playback +outlives every screen. + +## References + +- `src/features/player/PlayerLayer.tsx` — the root layer and its stacking order. +- `src/features/player/components/QueueOverlay.tsx` — the sheet. +- `docs/components.md` — the component tree. diff --git a/docs/adr/015-no-favourite-button-in-the-media-notification.md b/docs/adr/015-no-favourite-button-in-the-media-notification.md new file mode 100644 index 0000000..c8b3164 --- /dev/null +++ b/docs/adr/015-no-favourite-button-in-the-media-notification.md @@ -0,0 +1,85 @@ +# 015 — No favourite button in the media notification, for now + +## Context + +The brief asks for a working favourite button in Android's system media +notification: pressing it should write `track_stats.is_favorite` and the icon +should reflect the real state. On Android this is a MediaSession custom action — +`PlaybackStateCompat.CustomAction` on the old API, a `CommandButton` with a +`SessionCommand` in the custom layout on Media3. + +The app cannot reach either. `expo-audio` owns the MediaSession, and it does not +expose one. + +What it does expose, in full: `setActiveForLockScreen(active, metadata, +options)`, `updateLockScreenMetadata(metadata)`, and +`clearLockScreenControls()`. `AudioMetadata` is `{ title, artist, albumTitle, +artworkUrl }`. `AudioLockScreenOptions` is `{ showSeekForward, showSeekBackward, +isLiveStream }`. There is no custom-action field, no command callback, and no +event a JS listener could receive a button press on. + +Inside the library, `AudioControlsService` builds the notification and calls +`session.setCustomLayout(...)` with a fixed list — seek back, play/pause, seek +forward. `mediaSession` is a private field of that service. On API 29, which is +the device this was reported on, the notification is assembled from explicit +`NotificationCompat.Action`s in the same private method. + +Four routes were considered. + +**A local Expo module, as `modules/audio-focus` already does.** This is the +project's established way to add a native capability without breaking the audio +boundary, and it does not work here. A second module can bind to +`AudioControlsService` — it exports a binder — but a custom action has to be +added by the session's *owner*. A `MediaController` connected from outside can +send commands, not publish buttons. Reaching `mediaSession` through the binder +means reflection into a private Kotlin field in the notification path of a music +player, which is not something to ship. + +**A second MediaSession of our own.** Two sessions means Android picks one for +the notification and the media buttons, and the loser's controls silently do +nothing. Strictly worse than no button. + +**Patching `expo-audio`.** Roughly a hundred lines of Kotlin across +`AudioControlsService` and `AudioMediaSessionCallback`, plus a new event to JS, +re-applied on every install and silently broken by the next SDK bump. In a +project whose `docs/adr/009` records that the engine may have to be replaced +outright, a private fork of it is the wrong direction. + +**Replacing the engine.** `react-native-audio-pro` and RNTP v5 are already named +in `docs/01-TECH-STACK.md` §2.1 as the fallbacks, and both support custom +actions. This is a real answer to a much larger question than one button. + +## Decision + +**Ship the notification without a favourite button, and say so.** + +The rest of the metadata is delivered: title, artist, album, and artwork — with +the app's own music-note placeholder unpacked to a file and handed over when a +track has no cover, so the notification and the app show one mark rather than +two that nearly match. + +The button is deferred to whenever the engine question is reopened. It is +recorded here rather than in a backlog because the next person to try it will +otherwise spend the same afternoon discovering that `AudioLockScreenOptions` has +three fields. + +## Consequences + +Favouriting stays a thing you do in the app. Every other transport control the +notification offers works, and the seek buttons that were already configured are +untouched. + +If `expo-audio` grows a custom-action API this becomes a small change on our +side, because the metadata push already goes through one place — +`AudioEngine.bindLockScreen`. + +This is the second capability the engine choice has cost, after the ones ADR 009 +already weighed. A third should probably force the swap rather than another ADR. + +## References + +- `docs/adr/009-expo-audio-and-our-own-queue.md` — why this engine, and the + named fallbacks. +- `src/services/audio/AudioEngine.ts` — `bindLockScreen`, the one place + metadata is pushed. +- `src/services/audio/notificationArtwork.ts` — the placeholder. diff --git a/docs/components.md b/docs/components.md index 12aa94f..9b2e23a 100644 --- a/docs/components.md +++ b/docs/components.md @@ -51,17 +51,16 @@ it is repeated here because that is the thing a reader needs before touching it. | Component | What it is for | |---|---| | `LibraryScreen` | Owns the *library*: scanning, searching, which of the three views is showing. | -| `LibraryTracks` | Owns *tracks*: selection, the sheets, playing. Split from the screen at the 300-line limit; the boundary is by subject, and it is reused verbatim by `CollectionDetailScreen`. | +| `LibraryTracks` | Owns *tracks*: the sheets and playing. Split from the screen at the 300-line limit; the boundary is by subject, and it is reused verbatim by `CollectionDetailScreen`. | | `CollectionDetailScreen` | One artist or one album. Reuses `LibraryTracks` so a track has the same verbs everywhere. | -| `LibraryHeader` | Count, select toggle, folder picker, Scan. The count is always `tracks.length` — never a second query. | +| `LibraryHeader` | Count, folder picker, Scan. The count is always `tracks.length` — never a second query. | | `TrackList` | The FlashList. `drawDistance` is 1200 rather than the 250 default; at 64px rows the default is under four rows of buffer and a fling outruns it, which is what left blank rows behind the finger. | -| `LibraryRow` | One row plus its swipe gesture. Memoized on primitives only — it takes no selection object, so a render elsewhere cannot reach it. | +| `LibraryRow` | One row plus its swipe gesture. Memoized on primitives only, so a render elsewhere cannot reach it. | | `TrackRow` | Artwork, title, artist, duration. Compared by value, because a live query hands back fresh objects on every re-run. | | `CollectionCard` / `CollectionGrid` | An artist or album as a square card, and the two-column grid of them. | | `CollectionHeader` | Cover, name and size for a detail screen. | | `SearchField` | Debounced input. The field stays instant; only the query waits. | | `ScanBanner` | Progress above the list, never instead of it — the user can scroll throughout. Hides the counter until a total is known rather than showing "0 / 0". | -| `SelectionBar` | What you can do with a selection. At the bottom, where the thumb already is. | | `TrackActionSheet` | Long-press actions. Its doc says which two items from the brief are deliberately absent, and why. | | `TrackInfoSheet` | Every technical field. Absent values render as an em dash rather than disappearing. | | `TrackListSkeleton` | Named wrapper over `SkeletonRows`, so row geometry stays in one place next to `TrackRow`. | @@ -72,14 +71,16 @@ it is repeated here because that is the thing a reader needs before touching it. | Component | What it is for | |---|---| -| `PlayerScreen` | Now Playing. A modal route: somewhere you go from something and dismiss. | -| `ArtworkCarousel` | Three slots — previous, current, next — all mounted, so a neighbour slides in already decoded. Commits on distance **or** velocity. Rubber-bands at the ends of the queue. | +| `PlayerLayer` | The root stacking order, and the only place that decides it: children, then the transport strip at `z-20`, then Now Playing at `z-30`, then the queue at `z-40`. Owns the one expansion value and publishes the strip's measured height for every scrollable screen to pad by. | +| `NowPlayingOverlay` / `PlayerScreen` | The overlay and its content. The mini player and full screen share one Reanimated expansion value; no route transition sits between them. | +| `QueueOverlay` | The queue as a root-level sheet above Now Playing. It was a route and could not be seen from under the overlay — `docs/adr/014`. Mounted only while open. | +| `ArtworkCarousel` | Three slots — previous, current, next — all mounted, so a neighbour slides in already decoded. Commits on distance **or** velocity. Rubber-bands at the ends of the queue. Takes the height flex leaves it and sizes the cover from both axes, because a width-derived square overflowed the column. | | `MiniPlayer` | The persistent transport strip. Subscribes to phase and track only, never position; horizontal swipe changes tracks and vertical swipe opens Now Playing. | | `MiniProgress` | The progress hairline, and the only thing in the tab bar that hears about position. Width lives in a shared value; React renders it once. | | `Scrubber` | The seek bar. The drag runs entirely in a worklet; React hears about it once, on release. | | `SpecStrip` | The signature element — one monospaced line of a file's technical truth. | | `FavoriteButton` | The heart. It is also the only writer of `is_favorite`, which the `favorites` shuffle weights on. | -| `QueueScreen` / `QueueRow` | What is playing and what follows. Subscribes to the engine's queue rather than its playback state, so it does not re-render at 2 Hz. | +| `QueueScreen` / `QueueRow` | What is playing and what follows. Subscribes to the engine's queue rather than its playback state, so it does not re-render at 2 Hz. Dismissed through an `onClose` prop; it is not a route. | --- @@ -91,7 +92,6 @@ it is repeated here because that is the thing a reader needs before touching it. | `PlaylistDetailScreen` / `PlaylistDetailHeader` | One playlist, laid out the way a streaming app does because that arrangement is already in everyone's hands. | | `PlaylistMosaic` | A playlist's cover: the first four album covers in a 2×2 grid. Each count is a deliberate case, not a degradation — one cover fills the square rather than repeating four times. | | `ReorderableEntry` | Drag by a handle, not by long-press: long-press is the action sheet everywhere else, and a list where holding sometimes does either is a list nobody trusts. | -| `AddTracksSheet` | Pick tracks *from* the playlist. Mounted only while open, because it runs the full library query. | | `AddToPlaylistSheet` | Pick a playlist for some tracks, or make one. | | `NamePlaylistDialog` | A `Modal`, not `Alert.prompt`, which is iOS-only and silently does nothing on Android. | diff --git a/docs/database.md b/docs/database.md index f94cee5..f8fddb4 100644 --- a/docs/database.md +++ b/docs/database.md @@ -63,6 +63,12 @@ removing "the track" would take both copies. `is_missing = 1`. Deleting it would take playlist entries and play history with it, and an SD card that is merely unmounted would look like a library wipe. +**Liked songs are virtual.** `track_stats.is_favorite` is the source of truth; +there is no reserved row in `playlists`. `favorite_at` is set when a track is +liked and cleared when it is unliked, so the virtual Liked Songs list can sort +newest first without changing playlist data. Existing favourite rows predate +this column and have a null timestamp, so they sort after newly liked tracks. + ### Spec strip columns `container`, `codec`, `bitrate_kbps`, `sample_rate_hz`, `bit_depth`, @@ -106,8 +112,9 @@ recorder and the stats screens both go through it, so they cannot disagree. - A **skip** is `ms_played < duration_ms * 0.2` — abandoning it in the first fifth. - Anything else is **partial**: it happened, but it counts as neither. -- Seeking backwards does not create a second event. That is a recorder concern - and lands with playback in Phase 3. +- A rewind can start a second listen only after the current listen has earned a + play and the position jumps back to the first quarter. See ADR 011 and + `docs/stats.md`; ordinary seeks remain part of the same listen. ### The thresholds overlap — this is unresolved diff --git a/docs/performance.md b/docs/performance.md index 327d424..a8033b2 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -321,3 +321,93 @@ rather than being a defect, but it is a real gap. This was the last thing in the project never to have been run. Note it is still a *functional* smoke test — no release-build numbers were taken, so the cold-start caveat below stands unchanged. + +--- + +## Device verification, 2026-08-02 — the final polish branch + +Pixel_7 AVD, API 35, arm64, debug build over Metro, 531 tracks. The Mi 9T could +not be driven at all this session: `adb shell input` is still refused +(`SecurityException: INJECT_EVENTS`), and its notification shade was stuck open +with `mCurrentFocus=StatusBar`, which no shell command would collapse. Waking +it with `svc power stayon usb` worked; nothing else did. **Everything below is +the emulator.** + +### The listen-counting matrix + +A 25-second WAV pushed to `/sdcard/Music`, so the thresholds are round numbers: +play at 12,500 ms, skip below 5,000 ms. + +| # | Scenario | Expected | Actual | | +|---|---|---|---|---| +| a | played start to finish | 1 event, `play` | 1 event, `play`, completed | ✅ | +| b | looped, repeat on | 1 event per pass | 7 passes → 7 events, all `play`, started 27–30 s apart | ✅ | +| c | past the play mark, then rewound to 0 | 2 events | 2 events, `play` + `play` | ✅ | +| d | heard briefly, skipped forward, finished | 1 event, only heard audio counted | **not run on device** | ⚠️ | +| e | scrubbed back and forth | no extra events | **not run on device** | ⚠️ | + +The gaps in (b) are the load-bearing number: 27–30 s between consecutive +`started_at_utc` values, each carrying ~27 s of `ms_played`. A double-written +event would sit milliseconds from its twin. None did. + +(c) was driven with the **previous** button, which restarts the track at or +past ten seconds — the same rewind-to-zero a scrubber drag produces, through +the same code path. + +**(d) and (e) need a forward seek and there is no way to perform one from +automation.** The scrubber is a Reanimated pan; `input swipe` does not activate +it and neither does a hand-built `input motionevent` sequence, which is the +same class of problem the reorder handle had. `cmd media_session dispatch +fast-forward` and `KEYCODE_MEDIA_FAST_FORWARD` both left the position where it +was. The only forward-seek control left is the notification's +10 s button, +which needs the shade. + +Both are covered by `src/services/audio/listenRecording.test.ts`, which drives +the real engine through the real `AudioEngine.seekTo`. That suite is 16/16 and +three of its cases fail against the previous behaviour. **The matrix is 5/5 in +the harness and 3/5 on hardware, and the three that ran on hardware are the +three the harness cannot fully model** — real audio timing, a real 500 ms +status stream, and a real SQLite write per event. + +### Notification and MediaSession + +| Check | Result | +|---|---| +| Metadata while playing | `description=loop-test-25s`, artwork drawn, artist shown | +| `dumpsys media_session` accuracy | **now correct** — `state=PLAYING(3)` with the right title | +| External pause (media button) | `state=PAUSED(2)`, and the app's own phase followed | +| Resume | `state=PLAYING(3)` | +| Headphone unplug | **not verified** — `ACTION_AUDIO_BECOMING_NOISY` is a protected broadcast | +| Audio-focus loss | **not verified** — `adb emu gsm call` left `mCallState=0`, and the focus stack was empty | + +The second row is worth its own note. `docs/player.md` has carried a warning +that `dumpsys media_session` "lies about this app", reporting `state=NONE` with +a stale title, and put it down to the session being rebuilt on every track +change. It was — that is exactly what `setActiveForLockScreen` does to an +already-active player, and it is the same window that let the notification show +"playing" for stopped audio. Claiming the session once and updating metadata in +place fixed the notification and made the dump truthful in the same change. + +### The UI reports + +| Item | Result | +|---|---| +| Mini player drawing over Now Playing | ✅ gone — the overlay is opaque and complete | +| Transport row under the navigation bar | ✅ clear of it, with space below | +| Artwork the wrong shape | ✅ square, measured 790 × 789 px on a 1080-wide screen | +| Queue opens but is invisible | ✅ visible — "Sıra", 529 remaining, current track marked | +| Liked Songs card reads 0 | ✅ "2 liste" and "Beğenilenler — 4 parça", matching `track_stats` | +| Empty state above a full list | ✅ gone | +| Back closes the player surfaces | ✅ closes the queue, then the overlay | + +**The opening animation is not on this list.** It is a real change — velocity +handoff from both gestures, a softer spring, an opacity ramp that finishes at +40% of the travel — and none of that can be judged from a screenshot. It needs +a hand on the phone. + +### What the emulator cost + +Two SystemUI ANRs and a restart mid-session, and taps that silently do nothing +often enough that every step had to be confirmed from `MUFIFY_PERF` in the +Metro log rather than assumed. The `library.play.handler` measure is the only +reliable signal that a row press landed. diff --git a/docs/player.md b/docs/player.md index 7630d83..5ce9bf8 100644 --- a/docs/player.md +++ b/docs/player.md @@ -46,8 +46,8 @@ Everything easy to get wrong lives there and is unit tested: repeats, a track the user *skips* advances. Same mode, different input. Treating them the same makes the skip button look broken. -The three-second rule — previous restarts the track rather than going back, -once you are far enough in — lives in the engine instead, because it needs the +The ten-second rule — previous restarts the track rather than going back, +at or beyond 10 seconds — lives in the engine instead, because it needs the playback position and the queue does not have one. ## Two Android requirements that are not optional @@ -66,6 +66,40 @@ anyone wants. buttons is worse than no notification, and Android will happily keep showing one for a player that has gone away. +### Claim the session once, then update it + +`setActiveForLockScreen` is called on the **first** track only. Every track +after it goes through `updateLockScreenMetadata`. + +The difference is not cosmetic. Inside expo-audio, calling +`setActiveForLockScreen` on a player that is already active does not refresh +anything — it releases the MediaSession and builds a new one on the main queue. +Between those two there is a window with no live session, and the notification's +play/pause icon is drawn from `session.player.isPlaying` at the moment it is +posted. A state change landing in that window is drawn against a released +session and never corrected, which is how the notification came to show +"playing" for audio that had stopped. + +It also explains the entry below about `dumpsys media_session` reporting +`state=NONE` with a stale title: the dump was catching the swap, and the swap +happened on every single track change. + +`updateLockScreenMetadata` changes metadata on the live session and re-posts the +notification, with no release and no window. + +### What the notification shows + +Title, artist, album, and artwork. A track with no cover gets the app's own +music-note placeholder — `src/services/audio/notificationArtwork.ts` unpacks it +from the bundle to a `file://` path, because the service loads artwork through +`java.net.URL(...).openConnection()`, which knows nothing about `asset://` or a +Metro URL. + +**There is no favourite button, and there cannot be one through this engine.** +`AudioLockScreenOptions` has three fields and none of them is a custom action; +the MediaSession is a private field of expo-audio's own service. The options +and the reasoning are in `docs/adr/015`. + ## States `PlaybackState.phase` is `idle | loading | playing | paused | error`. diff --git a/docs/scanner.md b/docs/scanner.md index 3a08a2b..e783cd0 100644 --- a/docs/scanner.md +++ b/docs/scanner.md @@ -3,7 +3,7 @@ How music gets into the library. Two ways in, one pipeline. > **Phase 2 is closed.** The module, pipeline, queries and UI are written and -> unit tested; the automatic sweep, manual add, incremental rescan, artwork +> unit tested; the user-initiated scan, folder import, incremental rescan, artwork > extraction, tag reading and directory recursion are all verified on hardware. > > One item is carried forward rather than closed: frame timing over a large @@ -15,8 +15,9 @@ How music gets into the library. Two ways in, one pipeline. ## Two entry points, deliberately -**Automatic.** A MediaStore sweep runs in the background. It costs nothing and -covers the common case: music the system already indexed. +**Library scan.** The user starts a MediaStore sweep from the library. It covers +the common case: music the system already indexed, without starting hidden work +at launch. Both paths need the audio permission first, and the app **asks** for it rather than assuming it — see `docs/adr/008-permission-is-asked-not-assumed.md`. @@ -24,9 +25,9 @@ Without the grant a MediaStore query does not fail, it returns nothing, so a scan without permission looks exactly like a device with no music on it. That distinction is the whole reason the request exists. -**Manual — `Add music`.** Opens the system folder picker (SAF) and scans what -was chosen. This is **not** a fallback for when the automatic scan -disappoints. MediaStore does not index: +**Folder import — `Add music`.** Opens the system folder picker (SAF), warns +before starting, then scans what was chosen. This is **not** a fallback for +when the library scan finds nothing. MediaStore does not index: - files the media scanner has not seen yet — a fresh `adb push`, a just-copied album diff --git a/docs/stats.md b/docs/stats.md index 72b3342..646cb28 100644 --- a/docs/stats.md +++ b/docs/stats.md @@ -3,8 +3,11 @@ Everything is computed on-device from the user's own history. No network, no account, no export unless the user asks for one. -> Phase status: the counting rule and period keys are implemented and tested -> (Phase 1). Event recording is wired; rollups and the screens land in Phase 7. +> Phase status: counting, event recording, incremental rollups and the stats +> screen are implemented. The repeat-listen behaviour is pinned by +> `src/services/audio/listenRecording.test.ts` against the real engine, and +> partly confirmed on hardware — see "The repeat-listen device check, +> corrected" below for what a device session can and cannot settle. --- @@ -121,6 +124,18 @@ The thresholds are pinned by `repeatListen.test.ts`. This decides whether a listen is counted once or twice, so a quiet change to either condition silently rewrites the user's history. +### Where the rule meets the engine + +`repeatListen.ts` and `listenCycle.ts` are both pure and both have had good +tests for some time. Every miscount actually reported has lived in neither: +it lived in `AudioEngine.onStatus`, where a listen is opened, banked and +reopened against a stream of status ticks, and which nothing covered. + +`src/services/audio/listenRecording.test.ts` now replays scripted tick streams +through the real engine with `expo-audio` behind a fake player. It is the only +place the wiring is checked, and it is where the duration defect below was +found. A device session cannot be re-run; that suite can. + --- ## Period keys @@ -164,6 +179,41 @@ trigger a rebuild, not a renumber. --- +## Which duration the rule is applied to + +Every threshold in this document is a fraction of the track's duration, so the +counting rule is only as good as the number it divides. There are two of them +and they disagree. + +The **scanner's** duration comes from MediaStore. It is null more often than +anyone expects: a file copied onto the device and indexed before its metadata +was read has a row with no duration at all, and it stays that way until stage +two of a scan reaches it. The **engine's** duration comes from the open file +and is authoritative. + +`classifyListen(msPlayed, 0)` returns `partial` — the first line is +`if (durationMs <= 0 || msPlayed <= 0) return 'partial'`. So a track with no +stored duration recorded a `play_event` for every listen, moved neither +counter, and disappeared from the play counts entirely. Listening to it ten +times produced ten honest-looking rows and a play count of zero. + +`PlaybackState.durationMs` already preferred the engine's figure. The listen +handed to the recorder did not — it carried `track.durationMs` straight off the +row. **`FinishedListen.durationMs` is now the engine's, falling back to the +scanner's only when the file never opened**, and the same preference decides +`isRewindToRestart`, so a repeat is detected on the same number the outcome is +judged against. + +This is the third report of "the loop count is wrong" and the first one with a +mechanism that explains a partial failure rather than a total one: tracks with +a good stored duration counted correctly all along, which is why the defect +survived two rounds of device verification that happened to use them. + +Pinned by `src/services/audio/listenRecording.test.ts` — three cases that fail +against the old behaviour. + +--- + ## Recording `recordListen()` in `src/db/queries/playEvents.ts` does one insert into @@ -224,6 +274,25 @@ Verified on device as well as in tests: after playing ten tracks, the rollups reproduced exactly the ten most recent `play_events` — 10 plays and 365,560 ms, consistent across all three period types. +### The repeat-listen device check, corrected + +An earlier version of this file said the repeat-listen behaviour had been +"verified on device". It had — twice — and both reports were true about what +they measured and wrong about what they implied, which is that the feature was +correct. Every track used in those sessions had a good stored duration, and +those counted correctly the whole time. The failure needs a track whose +MediaStore duration is missing, which is the state a freshly copied file is in. + +Re-run 2026-08-02 against the real engine, with the matrix written down before +it was run rather than after: a, b and c pass on hardware — a loop produces one +event per pass, 27–30 s apart, with no duplicates — and d and e could not be +driven on device at all, because seeking forward needs the scrubber's pan and +no `adb input` sequence activates it. Full record in `docs/performance.md`. + +**Prefer the harness over another device session.** A device session cannot be +re-run and so cannot catch the next regression; that is how this defect survived +two of them. + ### Events recorded before rollups existed are not backfilled Rollups began being written partway through development, so events older than diff --git a/jest.config.js b/jest.config.js index bd66324..879e64b 100644 --- a/jest.config.js +++ b/jest.config.js @@ -9,6 +9,12 @@ module.exports = { // `.claude/worktrees/*` holds full checkouts of other branches; without // this, every suite there is collected a second time. testPathIgnorePatterns: ['/node_modules/', '/\\.claude/'], + // Ignoring the *tests* there is not enough. Each checkout also carries a copy + // of `modules/audio-focus/package.json`, and four packages claiming the name + // `audio-focus` make the Haste map ambiguous — `jest.mock('audio-focus')` + // then fails to resolve at all, and every run printed a duplicate-name + // warning nobody could act on. + modulePathIgnorePatterns: ['/\\.claude/'], // AGENTS.md: real coverage on services/, not on components. collectCoverageFrom: ['src/services/**/*.ts', 'src/utils/**/*.ts'], }; diff --git a/package.json b/package.json index 935bc7f..b9fb9a1 100644 --- a/package.json +++ b/package.json @@ -20,6 +20,7 @@ "@shopify/flash-list": "2.0.2", "drizzle-orm": "^0.45.2", "expo": "~57.0.9", + "expo-asset": "~57.0.8", "expo-audio": "~57.0.3", "expo-constants": "~57.0.8", "expo-file-system": "~57.0.1", diff --git a/src/db/migrations.test.ts b/src/db/migrations.test.ts index 271a5a3..b387562 100644 --- a/src/db/migrations.test.ts +++ b/src/db/migrations.test.ts @@ -109,6 +109,19 @@ describe('migrations', () => { } }); + it('stores the liked timestamp without making existing stats rows invalid', () => { + const db = freshDatabase(); + const columns = db.prepare('PRAGMA table_info(track_stats)').all() as { + name: string; + notnull: number; + type: string; + }[]; + const favoriteAt = columns.find((column) => column.name === 'favorite_at'); + + expect(favoriteAt?.type.toLowerCase()).toBe('integer'); + expect(favoriteAt?.notnull).toBe(0); + }); + it('stores artwork as a path, never as bytes', () => { const db = freshDatabase(); for (const table of ['tracks', 'albums', 'playlists']) { diff --git a/src/db/migrations/0003_numerous_cannonball.sql b/src/db/migrations/0003_numerous_cannonball.sql new file mode 100644 index 0000000..efcffaf --- /dev/null +++ b/src/db/migrations/0003_numerous_cannonball.sql @@ -0,0 +1 @@ +ALTER TABLE `track_stats` ADD `favorite_at` integer; \ No newline at end of file diff --git a/src/db/migrations/meta/0003_snapshot.json b/src/db/migrations/meta/0003_snapshot.json new file mode 100644 index 0000000..c1d5b5f --- /dev/null +++ b/src/db/migrations/meta/0003_snapshot.json @@ -0,0 +1,795 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "691125bf-71f4-4eee-ac1b-f53109c68acc", + "prevId": "5e388170-1ca7-413d-b678-43cfbce1678b", + "tables": { + "albums": { + "name": "albums", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "artist_id": { + "name": "artist_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "year": { + "name": "year", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "artwork_path": { + "name": "artwork_path", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "albums_artist_idx": { + "name": "albums_artist_idx", + "columns": ["artist_id"], + "isUnique": false + }, + "albums_name_artist_unique": { + "name": "albums_name_artist_unique", + "columns": ["name", "artist_id"], + "isUnique": true + } + }, + "foreignKeys": { + "albums_artist_id_artists_id_fk": { + "name": "albums_artist_id_artists_id_fk", + "tableFrom": "albums", + "tableTo": "artists", + "columnsFrom": ["artist_id"], + "columnsTo": ["id"], + "onDelete": "set null", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "artists": { + "name": "artists", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "sort_name": { + "name": "sort_name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "artists_name_unique": { + "name": "artists_name_unique", + "columns": ["name"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "play_events": { + "name": "play_events", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "track_id": { + "name": "track_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "started_at_utc": { + "name": "started_at_utc", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "ms_played": { + "name": "ms_played", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "completed": { + "name": "completed", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "outcome": { + "name": "outcome", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'partial'" + }, + "source_type": { + "name": "source_type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "source_id": { + "name": "source_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "shuffle_algorithm": { + "name": "shuffle_algorithm", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "week_key": { + "name": "week_key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "month_key": { + "name": "month_key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "year_key": { + "name": "year_key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "play_events_started_idx": { + "name": "play_events_started_idx", + "columns": ["started_at_utc"], + "isUnique": false + }, + "play_events_track_idx": { + "name": "play_events_track_idx", + "columns": ["track_id"], + "isUnique": false + }, + "play_events_week_idx": { + "name": "play_events_week_idx", + "columns": ["week_key"], + "isUnique": false + }, + "play_events_month_idx": { + "name": "play_events_month_idx", + "columns": ["month_key"], + "isUnique": false + }, + "play_events_outcome_idx": { + "name": "play_events_outcome_idx", + "columns": ["outcome"], + "isUnique": false + } + }, + "foreignKeys": { + "play_events_track_id_tracks_id_fk": { + "name": "play_events_track_id_tracks_id_fk", + "tableFrom": "play_events", + "tableTo": "tracks", + "columnsFrom": ["track_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "playlist_tracks": { + "name": "playlist_tracks", + "columns": { + "playlist_id": { + "name": "playlist_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "track_id": { + "name": "track_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "position": { + "name": "position", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "added_at": { + "name": "added_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "playlist_tracks_pk": { + "name": "playlist_tracks_pk", + "columns": ["playlist_id", "position"], + "isUnique": true + }, + "playlist_tracks_track_idx": { + "name": "playlist_tracks_track_idx", + "columns": ["track_id"], + "isUnique": false + } + }, + "foreignKeys": { + "playlist_tracks_playlist_id_playlists_id_fk": { + "name": "playlist_tracks_playlist_id_playlists_id_fk", + "tableFrom": "playlist_tracks", + "tableTo": "playlists", + "columnsFrom": ["playlist_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + }, + "playlist_tracks_track_id_tracks_id_fk": { + "name": "playlist_tracks_track_id_tracks_id_fk", + "tableFrom": "playlist_tracks", + "tableTo": "tracks", + "columnsFrom": ["track_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "playlists": { + "name": "playlists", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "artwork_path": { + "name": "artwork_path", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "scan_folders": { + "name": "scan_folders", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "uri": { + "name": "uri", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "enabled": { + "name": "enabled", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + } + }, + "indexes": { + "scan_folders_uri_unique": { + "name": "scan_folders_uri_unique", + "columns": ["uri"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "stats_rollups": { + "name": "stats_rollups", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "period_type": { + "name": "period_type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "period_key": { + "name": "period_key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "entity_type": { + "name": "entity_type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "entity_id": { + "name": "entity_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "play_count": { + "name": "play_count", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "ms_played": { + "name": "ms_played", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "stats_rollups_unique": { + "name": "stats_rollups_unique", + "columns": ["period_type", "period_key", "entity_type", "entity_id"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "track_stats": { + "name": "track_stats", + "columns": { + "track_id": { + "name": "track_id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "play_count": { + "name": "play_count", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "skip_count": { + "name": "skip_count", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "ms_played_total": { + "name": "ms_played_total", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "last_played_at": { + "name": "last_played_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "is_favorite": { + "name": "is_favorite", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + }, + "favorite_at": { + "name": "favorite_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "track_stats_track_id_tracks_id_fk": { + "name": "track_stats_track_id_tracks_id_fk", + "tableFrom": "track_stats", + "tableTo": "tracks", + "columnsFrom": ["track_id"], + "columnsTo": ["id"], + "onDelete": "cascade", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "tracks": { + "name": "tracks", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "media_store_id": { + "name": "media_store_id", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "file_uri": { + "name": "file_uri", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "artist_id": { + "name": "artist_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "album_id": { + "name": "album_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "album_artist": { + "name": "album_artist", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "genre": { + "name": "genre", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "track_no": { + "name": "track_no", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "disc_no": { + "name": "disc_no", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "year": { + "name": "year", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "duration_ms": { + "name": "duration_ms", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "file_size": { + "name": "file_size", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "container": { + "name": "container", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "codec": { + "name": "codec", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "bitrate_kbps": { + "name": "bitrate_kbps", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "sample_rate_hz": { + "name": "sample_rate_hz", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "bit_depth": { + "name": "bit_depth", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "channels": { + "name": "channels", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "artwork_path": { + "name": "artwork_path", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "date_added": { + "name": "date_added", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "date_modified": { + "name": "date_modified", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "last_scanned_at": { + "name": "last_scanned_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "is_missing": { + "name": "is_missing", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + } + }, + "indexes": { + "tracks_file_uri_unique": { + "name": "tracks_file_uri_unique", + "columns": ["file_uri"], + "isUnique": true + }, + "tracks_artist_idx": { + "name": "tracks_artist_idx", + "columns": ["artist_id"], + "isUnique": false + }, + "tracks_album_idx": { + "name": "tracks_album_idx", + "columns": ["album_id"], + "isUnique": false + }, + "tracks_genre_idx": { + "name": "tracks_genre_idx", + "columns": ["genre"], + "isUnique": false + }, + "tracks_missing_idx": { + "name": "tracks_missing_idx", + "columns": ["is_missing"], + "isUnique": false + }, + "tracks_title_nocase_idx": { + "name": "tracks_title_nocase_idx", + "columns": ["\"title\" COLLATE NOCASE"], + "isUnique": false + } + }, + "foreignKeys": { + "tracks_artist_id_artists_id_fk": { + "name": "tracks_artist_id_artists_id_fk", + "tableFrom": "tracks", + "tableTo": "artists", + "columnsFrom": ["artist_id"], + "columnsTo": ["id"], + "onDelete": "set null", + "onUpdate": "no action" + }, + "tracks_album_id_albums_id_fk": { + "name": "tracks_album_id_albums_id_fk", + "tableFrom": "tracks", + "tableTo": "albums", + "columnsFrom": ["album_id"], + "columnsTo": ["id"], + "onDelete": "set null", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": { + "tracks_title_nocase_idx": { + "columns": { + "\"title\" COLLATE NOCASE": { + "isExpression": true + } + } + } + } + } +} diff --git a/src/db/migrations/meta/_journal.json b/src/db/migrations/meta/_journal.json index 7c8ebc4..011ed9b 100644 --- a/src/db/migrations/meta/_journal.json +++ b/src/db/migrations/meta/_journal.json @@ -22,6 +22,13 @@ "when": 1785500000000, "tag": "0002_forget_unknown_placeholders", "breakpoints": true + }, + { + "idx": 3, + "version": "6", + "when": 1785608213665, + "tag": "0003_numerous_cannonball", + "breakpoints": true } ] } diff --git a/src/db/migrations/migrations.js b/src/db/migrations/migrations.js index b85057e..9df28a0 100644 --- a/src/db/migrations/migrations.js +++ b/src/db/migrations/migrations.js @@ -4,6 +4,7 @@ import journal from './meta/_journal.json'; import m0000 from './0000_curved_odin.sql'; import m0001 from './0001_smooth_tana_nile.sql'; import m0002 from './0002_forget_unknown_placeholders.sql'; +import m0003 from './0003_numerous_cannonball.sql'; export default { journal, @@ -11,5 +12,6 @@ export default { m0000, m0001, m0002, + m0003, }, }; diff --git a/src/db/queries/playlists.ts b/src/db/queries/playlists.ts index e0398fe..b93ff20 100644 --- a/src/db/queries/playlists.ts +++ b/src/db/queries/playlists.ts @@ -1,13 +1,19 @@ -import { and, asc, eq, gt, max, sql } from 'drizzle-orm'; +import { and, asc, desc, eq, gt, max, sql } from 'drizzle-orm'; import { useLiveQuery } from 'drizzle-orm/expo-sqlite'; -import { foldPlaylistRows, reorder, type PlaylistSummary } from '@/services/playlists/order'; +import { + foldPlaylistRows, + LIKED_SONGS_ID, + reorder, + type PlaylistSummary, +} from '@/services/playlists/order'; import { db } from '../client'; import { albums, artists, playlistTracks, playlists, trackStats, tracks } from '../schema'; /* Re-exported so screens keep importing playlist types from the query module. */ export type { PlaylistSummary }; +export { LIKED_SONGS_ID }; /** * Playlists and their contents. @@ -31,6 +37,18 @@ export interface PlaylistEntry { isFavorite: boolean; } +const entrySelection = { + trackId: tracks.id, + fileUri: tracks.fileUri, + title: tracks.title, + artistName: artists.name, + albumName: albums.name, + durationMs: tracks.durationMs, + artworkPath: tracks.artworkPath, + playCount: sql`coalesce(${trackStats.playCount}, 0)`, + isFavorite: sql`coalesce(${trackStats.isFavorite}, 0)`.mapWith(Boolean), +}; + /** * Live list of playlists, newest first, with their sizes and mosaic covers. * @@ -67,22 +85,14 @@ export function usePlaylists(): PlaylistSummary[] { export function usePlaylistEntries(playlistId: number): PlaylistEntry[] { const query = db .select({ - trackId: tracks.id, position: playlistTracks.position, - fileUri: tracks.fileUri, - title: tracks.title, - artistName: artists.name, - albumName: albums.name, - durationMs: tracks.durationMs, - artworkPath: tracks.artworkPath, + ...entrySelection, /* * Real counts, not the zero this used to select. Shuffling a playlist * runs the same algorithms as shuffling the library, and `discovery` and * `favorites` both weight on these — fed constants they degrade silently * into `pure`, which looks like the setting being ignored. */ - playCount: sql`coalesce(${trackStats.playCount}, 0)`, - isFavorite: sql`coalesce(${trackStats.isFavorite}, 0)`.mapWith(Boolean), }) .from(playlistTracks) .innerJoin(tracks, eq(tracks.id, playlistTracks.trackId)) @@ -96,6 +106,41 @@ export function usePlaylistEntries(playlistId: number): PlaylistEntry[] { return data; } +/** + * Live favourite tracks, newest favourite first, presented as a virtual playlist. + * + * **`from(trackStats)`, and the order of the joins is the whole point.** + * `useLiveQuery` re-runs a query when a table changes, and the only table it + * watches is the one in `FROM` — it reads `query.config.table` and compares the + * name, so joined tables are invisible to it. Selecting `from(tracks)` meant + * this list watched `tracks` and never `track_stats`, and liking a song writes + * only to `track_stats`. + * + * The result was the reported split: the Playlists tab showed "0 tracks" beside + * Liked Songs while the detail screen showed the real ones. Neither number was + * computed differently — this is one query, used by both — the tab simply had + * a copy from whenever it last mounted and was never told anything had changed. + * + * Favourites are `track_stats` rows, so watching that table is also the honest + * description of what this list is. + */ +export function useFavoriteEntries(): PlaylistEntry[] { + const query = db + .select({ + position: sql`row_number() over (order by ${trackStats.favoriteAt} desc, ${tracks.id} desc) - 1`, + ...entrySelection, + }) + .from(trackStats) + .innerJoin(tracks, eq(tracks.id, trackStats.trackId)) + .leftJoin(artists, eq(artists.id, tracks.artistId)) + .leftJoin(albums, eq(albums.id, tracks.albumId)) + .where(and(eq(trackStats.isFavorite, 1), eq(tracks.isMissing, 0))) + .orderBy(desc(trackStats.favoriteAt), desc(tracks.id)); + + const { data } = useLiveQuery(query); + return data; +} + export async function createPlaylist(name: string): Promise { const trimmed = name.trim(); if (!trimmed) return null; diff --git a/src/db/queries/scanning.ts b/src/db/queries/scanning.ts index 9f87eca..4ed41ff 100644 --- a/src/db/queries/scanning.ts +++ b/src/db/queries/scanning.ts @@ -145,9 +145,8 @@ async function resolveAlbums( * doing five awaited round trips per track: insert artist, select artist, insert * album, select album, upsert track. At the default page size of 500 that is * ~2,500 sequential queries in one un-yielded block, and it is what "the - * automatic scan freezes the app" meant. Measured on the Pixel_7 AVD with a - * 528-file library, first scan: **859ms for one page of 500**, during which - * nothing renders and no touch is handled. + * large-library scan freezes the app" meant: a first scan skipped nothing and + * could hold the JS thread for the whole page. * * The fingerprint skip that was already here only helps a *re*-scan, where most * rows are unchanged. The first scan — the one a new user sees — skipped nothing @@ -216,8 +215,7 @@ export async function saveEnumerated(rows: EnumeratedRow[]): Promise { albumId: row.albumName === null ? null - : (albumIds.get(albumKey(row.albumName, albumArtistIdOf(row, artistIds))) ?? - null), + : (albumIds.get(albumKey(row.albumName, albumArtistIdOf(row, artistIds))) ?? null), albumArtist: row.albumArtist, genre: row.genre, trackNo: row.trackNo, @@ -398,7 +396,8 @@ export async function saveEnriched(rows: EnrichedRow[]): Promise { await db.transaction(async (tx) => { for (const { fileUri, fields } of rows) { - const artistId = fields.artistName === null ? null : (artistIds.get(fields.artistName) ?? null); + const artistId = + fields.artistName === null ? null : (artistIds.get(fields.artistName) ?? null); await tx .update(tracks) .set({ @@ -407,7 +406,8 @@ export async function saveEnriched(rows: EnrichedRow[]): Promise { albumId: fields.albumName === null ? null - : (albumIds.get(albumKey(fields.albumName, albumArtistIdOf(fields, artistIds))) ?? null), + : (albumIds.get(albumKey(fields.albumName, albumArtistIdOf(fields, artistIds))) ?? + null), albumArtist: fields.albumArtist, genre: fields.genre, trackNo: fields.trackNo, @@ -535,7 +535,7 @@ export function useScanFolders(): ScanFolder[] { * Forget a folder. * * The row goes; the tracks stay. Those tracks are ordinary MediaStore content - * that the automatic sweep would find anyway, so deleting them here would + * that a later library scan would find anyway, so deleting them here would * take playlist entries and play history with them to remove something the * next scan puts straight back. Removing a folder means "stop re-indexing * this path", not "delete this music". diff --git a/src/db/queries/tracks.ts b/src/db/queries/tracks.ts index 3b23afc..ef9d268 100644 --- a/src/db/queries/tracks.ts +++ b/src/db/queries/tracks.ts @@ -223,10 +223,11 @@ export function useIsFavorite(trackId: number | null): boolean { */ export async function setFavorite(trackId: number, isFavorite: boolean): Promise { const flag = isFavorite ? 1 : 0; + const favoriteAt = isFavorite ? Date.now() : null; await db .insert(trackStats) - .values({ trackId, isFavorite: flag }) - .onConflictDoUpdate({ target: trackStats.trackId, set: { isFavorite: flag } }); + .values({ trackId, isFavorite: flag, favoriteAt }) + .onConflictDoUpdate({ target: trackStats.trackId, set: { isFavorite: flag, favoriteAt } }); } /** @@ -358,12 +359,7 @@ export function useCollectionTracks(kind: 'artist' | 'album', id: number): Track .leftJoin(artists, eq(tracks.artistId, artists.id)) .leftJoin(albums, eq(tracks.albumId, albums.id)) .leftJoin(trackStats, eq(trackStats.trackId, tracks.id)) - .where( - and( - eq(tracks.isMissing, 0), - collectionPredicate, - ), - ) + .where(and(eq(tracks.isMissing, 0), collectionPredicate)) .orderBy( asc(sql`${tracks.discNo} IS NULL`), asc(tracks.discNo), diff --git a/src/db/schema.ts b/src/db/schema.ts index aa2f6cc..8681b35 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -92,6 +92,8 @@ export const trackStats = sqliteTable('track_stats', { msPlayedTotal: integer('ms_played_total').notNull().default(0), lastPlayedAt: integer('last_played_at'), isFavorite: integer('is_favorite').notNull().default(0), + /** Set only while favourited, so liked songs can be shown newest first. */ + favoriteAt: integer('favorite_at'), }); export const playlists = sqliteTable('playlists', { diff --git a/src/features/library/LibraryScreen.tsx b/src/features/library/LibraryScreen.tsx index 7b5b756..5a6e9e7 100644 --- a/src/features/library/LibraryScreen.tsx +++ b/src/features/library/LibraryScreen.tsx @@ -14,6 +14,7 @@ import { isPermissionError } from '@/services/scanner/permission'; import { CollectionGrid } from './components/CollectionGrid'; import { LibraryHeader } from './components/LibraryHeader'; +import { FolderImportModal } from './components/FolderImportModal'; import { ScanBanner } from './components/ScanBanner'; import { SearchField } from './components/SearchField'; import { LibraryTracks } from './LibraryTracks'; @@ -53,7 +54,17 @@ export function LibraryScreen() { const [search, setSearch] = useState(''); // The field stays instant; only the query waits. const { tracks, isLoading } = useTracks(useDebounced(search)); - const { progress, isScanning, isRefreshing, scanLibrary, addFolder, rescan, cancel } = useScan(); + const { + progress, + isScanning, + isRefreshing, + scanLibrary, + pickFolder, + importFolder, + isFolderImporting, + rescan, + cancel, + } = useScan(); const artists = useArtistCards(); const albums = useAlbumCards(); @@ -66,7 +77,17 @@ export function LibraryScreen() { /** True while the scan confirmation is on screen. */ const [confirmingScan, setConfirmingScan] = useState(false); + const [pendingFolder, setPendingFolder] = useState(null); const askToScan = useCallback(() => setConfirmingScan(true), []); + const chooseFolder = useCallback(async () => { + const uri = await pickFolder(); + if (uri) setPendingFolder(uri); + }, [pickFolder]); + const confirmFolderImport = useCallback(() => { + const uri = pendingFolder; + setPendingFolder(null); + if (uri) void importFolder(uri); + }, [importFolder, pendingFolder]); const hasFailed = !isScanning && progress.phase === 'failed'; const permissionFailed = isPermissionError(progress.error); @@ -95,8 +116,7 @@ export function LibraryScreen() { count={displayedTrackCount} isScanning={isScanning} onScan={askToScan} - onAddFolder={addFolder} - onStartSelecting={() => setView('tracks')} + onAddFolder={chooseFolder} /> @@ -112,7 +132,9 @@ export function LibraryScreen() { a search box, and hiding it makes that obvious rather than puzzling. */} {view === 'tracks' ? : null} - {isScanning ? : null} + {isScanning && !isFolderImporting ? ( + + ) : null} {hasFailed ? ( setConfirmingScan(false)} /> + setPendingFolder(null)} + /> + {isFolderImporting ? : null} ); } diff --git a/src/features/library/LibraryTracks.tsx b/src/features/library/LibraryTracks.tsx index 724240b..b7d938a 100644 --- a/src/features/library/LibraryTracks.tsx +++ b/src/features/library/LibraryTracks.tsx @@ -11,12 +11,10 @@ import * as perf from '@/services/perf'; import { AddToPlaylistSheet } from '../playlists/components/AddToPlaylistSheet'; import { useCurrentTrack, usePlaybackControls } from '../player/hooks/usePlayback'; import { toPlayable } from '../player/toPlayable'; -import { SelectionBar } from './components/SelectionBar'; import { TrackActionSheet, type TrackAction } from './components/TrackActionSheet'; import { TrackInfoSheet } from './components/TrackInfoSheet'; import { TrackList } from './components/TrackList'; import { TrackListSkeleton } from './components/TrackListSkeleton'; -import { useSelection } from './hooks/useSelection'; import { useTrackActions } from './hooks/useTrackActions'; export interface LibraryTracksProps { @@ -38,7 +36,7 @@ export interface LibraryTracksProps { * * Split out of `LibraryScreen`, which had reached exactly the 300-line limit * `AGENTS.md` sets. The division is by subject rather than by size: this owns - * *tracks* — selection, the action sheet, the info sheet, playing — and the + * *tracks* — the action sheet, the info sheet, playing — and the * screen above owns the *library*: scanning, searching, and which view is on * screen. */ @@ -57,17 +55,6 @@ export function LibraryTracks({ const { playFrom } = usePlaybackControls(); const currentTrack = useCurrentTrack(); - /* - * Destructured, not held as an object. Every callback below would otherwise - * list `selection` as a dependency and be rebuilt on every render, which - * rebuilt every visible row — 47 of them per checkbox tap, measured. The - * individual functions are stable; only `isActive` and `ids` actually move. - */ - const selection = useSelection(); - const { isActive: isSelecting, ids: selectedIdList, toggle: toggleSelected } = selection; - - /** The selection as a Set, built once per change rather than once per row. */ - const selectedIds = useMemo(() => new Set(selectedIdList), [selectedIdList]); const { addToQueue, playNext, toggleFavorite } = useTrackActions(); // Queue conversion belongs to a data change, not to a row press. const playableTracks = useMemo(() => { @@ -95,15 +82,10 @@ export function LibraryTracks({ /* * Playing a track makes the list it came from the queue, which is what a user - * means by tapping a row — not "play this one thing and stop". While - * selecting, the same tap ticks a box instead. + * means by tapping a row — not "play this one thing and stop". */ const onPress = useCallback( (id: number) => { - if (isSelecting) { - toggleSelected(id); - return; - } const index = trackIndexById.get(id); if (index === undefined) return; perf.mark('library.play.handler'); @@ -111,21 +93,10 @@ export function LibraryTracks({ playFrom(playableTracks, index); perf.measure('library.play.handler', playableTracks.length); }, - [trackIndexById, playFrom, playableTracks, isSelecting, toggleSelected], + [trackIndexById, playFrom, playableTracks], ); - const onLongPress = useCallback( - (id: number) => { - // Long-pressing during a selection extends it rather than opening a sheet - // about one row — the user is plainly in the middle of picking several. - if (isSelecting) { - toggleSelected(id); - return; - } - setActionTarget(find(id)); - }, - [isSelecting, toggleSelected, find], - ); + const onLongPress = useCallback((id: number) => setActionTarget(find(id)), [find]); const onSwipeToQueue = useCallback( (id: number) => { @@ -153,30 +124,17 @@ export function LibraryTracks({ case 'favorite': toggleFavorite(track); return; - case 'select': - selection.begin(track.id); - return; case 'info': setInfoTarget(track); return; } }, - [actionTarget, playNext, addToQueue, toggleFavorite, selection], + [actionTarget, playNext, addToQueue, toggleFavorite], ); - const onSelectionQueue = useCallback(() => { - // Resolved in selection order, so the queue takes them as they were picked. - const picked = selection.ids - .map(find) - .filter((track): track is TrackListItem => track !== null); - addToQueue(picked); - selection.clear(); - }, [addToQueue, selection, find]); - const closePlaylistSheet = useCallback(() => { setPlaylistTargets([]); - selection.clear(); - }, [selection]); + }, []); return ( <> @@ -187,8 +145,6 @@ export function LibraryTracks({ - {isSelecting ? ( - selection.toggleAll(tracks.map((track) => track.id))} - onAddToQueue={onSelectionQueue} - onAddToPlaylist={() => setPlaylistTargets(selection.ids)} - onCancel={selection.clear} - /> - ) : null} - >( ({ item }) => ( // Gutter as padding on the cell rather than a gap on the list: FlashList @@ -50,7 +54,14 @@ export function CollectionGrid({ kind, cards, icon, onPress, empty }: Collection renderItem={renderItem} keyExtractor={keyExtractor} numColumns={COLUMNS} - contentContainerClassName="px-4" + /* + One style rather than a class plus a style. The bottom inset is a + runtime measurement — the transport strip's height — which no Tailwind + class can carry, and mixing `contentContainerClassName` with + `contentContainerStyle` leaves which padding wins to NativeWind's merge + order. The values are still design-system tokens. + */ + contentContainerStyle={{ paddingHorizontal: SPACING[4], paddingBottom: bottomInset }} ListEmptyComponent={empty} /> ); diff --git a/src/features/library/components/FolderImportModal.tsx b/src/features/library/components/FolderImportModal.tsx new file mode 100644 index 0000000..79509e1 --- /dev/null +++ b/src/features/library/components/FolderImportModal.tsx @@ -0,0 +1,58 @@ +import { useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { Modal, Pressable, Text, View } from 'react-native'; + +import { ProgressBar } from '@/components/ui/ProgressBar'; +import { useMessages } from '@/i18n'; +import type { ScanProgress } from '@/services/scanner/scanner'; + +export interface FolderImportModalProps { + progress: ScanProgress; + onCancel: () => void; +} + +/** Blocks navigation during a chosen-folder import while scan batches keep yielding. */ +export function FolderImportModal({ progress, onCancel }: FolderImportModalProps) { + const { t } = useTranslation(); + const messages = useMessages('library.importing.messages'); + const [messageIndex] = useState(() => Math.floor(Math.random() * Math.max(messages.length, 1))); + const label = + progress.phase === 'enumerating' + ? t('library.scanning.enumerating') + : progress.phase === 'enriching' + ? t('library.scanning.enriching') + : t('library.importing.preparing'); + const ratio = progress.total > 0 ? progress.processed / progress.total : 0; + + return ( + + + + {t('library.importing.title')} + {messages[messageIndex] ?? ''} + + + {label} + {progress.total > 0 ? ( + + {progress.processed} / {progress.total} + + ) : null} + + + + + + {t('library.scanning.cancel')} + + + + + + ); +} diff --git a/src/features/library/components/LibraryHeader.tsx b/src/features/library/components/LibraryHeader.tsx index bd55145..cfbc6b1 100644 --- a/src/features/library/components/LibraryHeader.tsx +++ b/src/features/library/components/LibraryHeader.tsx @@ -1,4 +1,4 @@ -import { CheckSquare, FolderPlus, RefreshCw } from 'lucide-react-native'; +import { FolderPlus, RefreshCw } from 'lucide-react-native'; import { useTranslation } from 'react-i18next'; import { Pressable, Text, View } from 'react-native'; @@ -13,11 +13,10 @@ export interface LibraryHeaderProps { onScan: () => void; /** Open the system folder picker. */ onAddFolder: () => void; - onStartSelecting: () => void; } /** - * The count, and the three things you can do to the whole library. + * The count, and the two things you can do to the whole library. * * The count is always `tracks.length` of the array the list renders. Two live * queries over the same table drift, and the header once read "14 tracks" above @@ -30,13 +29,7 @@ export interface LibraryHeaderProps { * than only in the empty state, is the other half of that: a user who adds music * later needs to reach it without emptying their library first. */ -export function LibraryHeader({ - count, - isScanning, - onScan, - onAddFolder, - onStartSelecting, -}: LibraryHeaderProps) { +export function LibraryHeader({ count, isScanning, onScan, onAddFolder }: LibraryHeaderProps) { const { t } = useTranslation(); const colors = useThemeColors(); @@ -46,20 +39,6 @@ export function LibraryHeader({ {isScanning ? '' : t('library.trackCount', { count })} - {/* Selection is reachable from here as well as from a long press: the - gesture is faster once you know it, and invisible until you do. */} - - - - void; onLongPress: (id: number) => void; onSwipeToQueue: (id: number) => void; - isSelecting: boolean; - isSelected: boolean; isCurrent: boolean; /** Already translated, for the swipe action's screen-reader label. */ swipeLabel: string; @@ -28,8 +26,7 @@ export interface LibraryRowProps { * including `onSwipe={() => onSwipeToQueue(item.id)}` — a fresh closure per row * per render, which defeats every memo below it. * - * Memoized on primitives only. Nothing here takes the selection object, so a - * render caused by something unrelated to this row cannot reach it. + * Memoized on primitives only, so an unrelated screen render cannot reach it. */ const LibraryRowComponent = function LibraryRow({ track, @@ -37,8 +34,6 @@ const LibraryRowComponent = function LibraryRow({ onPress, onLongPress, onSwipeToQueue, - isSelecting, - isSelected, isCurrent, swipeLabel, }: LibraryRowProps) { @@ -51,21 +46,12 @@ const LibraryRowComponent = function LibraryRow({ locale={locale} onPress={onPress} onLongPress={onLongPress} - isSelecting={isSelecting} - isSelected={isSelected} isCurrent={isCurrent} /> ), - [track, locale, onPress, onLongPress, isSelecting, isSelected, isCurrent], + [track, locale, onPress, onLongPress, isCurrent], ); - /* - * No swipe while selecting. Two horizontal gestures on one row means the user - * aiming for a checkbox occasionally queues a track instead, and during a - * multi-select that is both wrong and hard to undo. - */ - if (isSelecting) return row; - return ( {row} @@ -80,8 +66,6 @@ function isSameRow(previous: LibraryRowProps, next: LibraryRowProps): boolean { previous.onPress === next.onPress && previous.onLongPress === next.onLongPress && previous.onSwipeToQueue === next.onSwipeToQueue && - previous.isSelecting === next.isSelecting && - previous.isSelected === next.isSelected && previous.isCurrent === next.isCurrent && previous.swipeLabel === next.swipeLabel ); diff --git a/src/features/library/components/SelectionBar.tsx b/src/features/library/components/SelectionBar.tsx deleted file mode 100644 index 4be59ef..0000000 --- a/src/features/library/components/SelectionBar.tsx +++ /dev/null @@ -1,91 +0,0 @@ -import { CheckCheck, ListEnd, ListMusic, X } from 'lucide-react-native'; -import { useTranslation } from 'react-i18next'; -import { Pressable, Text, View } from 'react-native'; - -import { useThemeColors } from '@/theme/useTheme'; - -export interface SelectionBarProps { - count: number; - /** Total rows on screen, for the select-all label. */ - total: number; - onSelectAll: () => void; - onAddToQueue: () => void; - onAddToPlaylist: () => void; - onCancel: () => void; -} - -/** - * What you can do with a selection. - * - * Sits at the bottom, where the thumb already is, rather than in the header — - * the selection is made by tapping rows, so the actions belong next to the - * hand doing the tapping. - * - * The two actions are disabled at zero rather than hidden. A bar whose buttons - * appear and disappear as the count crosses one is harder to aim at than a bar - * whose buttons are always in the same place. - */ -export function SelectionBar({ - count, - total, - onSelectAll, - onAddToQueue, - onAddToPlaylist, - onCancel, -}: SelectionBarProps) { - const { t } = useTranslation(); - const colors = useThemeColors(); - const empty = count === 0; - - return ( - - - - - - - {t('selection.count', { count })} - - - - 0 ? colors.signal : colors.label} - size={22} - strokeWidth={2} - /> - - - - - - - - - - - ); -} diff --git a/src/features/library/components/TrackActionSheet.tsx b/src/features/library/components/TrackActionSheet.tsx index bdd0445..a1fdd7b 100644 --- a/src/features/library/components/TrackActionSheet.tsx +++ b/src/features/library/components/TrackActionSheet.tsx @@ -1,12 +1,4 @@ -import { - CheckSquare, - Heart, - HeartOff, - Info, - ListEnd, - ListMusic, - ListStart, -} from 'lucide-react-native'; +import { Heart, HeartOff, Info, ListEnd, ListMusic, ListStart } from 'lucide-react-native'; import { useMemo } from 'react'; import { useTranslation } from 'react-i18next'; @@ -14,13 +6,7 @@ import { ActionSheet, type ActionSheetAction } from '@/components/ui/ActionSheet import type { TrackListItem } from '@/db/queries/tracks'; /** What the sheet can be asked to do. The screen decides how. */ -export type TrackAction = - | 'playNext' - | 'addToQueue' - | 'addToPlaylist' - | 'favorite' - | 'select' - | 'info'; +export type TrackAction = 'playNext' | 'addToQueue' | 'addToPlaylist' | 'favorite' | 'info'; export interface TrackActionSheetProps { /** The track, or null when the sheet is closed. */ @@ -47,8 +33,6 @@ export interface TrackActionSheetProps { * Both are listed here rather than silently dropped, because a reader comparing * this against the brief deserves the reason. * - * `select` is here as well as on the header, since long-press is where most - * people look for multi-select first. */ export function TrackActionSheet({ track, onSelect, onClose }: TrackActionSheetProps) { const { t } = useTranslation(); @@ -56,14 +40,13 @@ export function TrackActionSheet({ track, onSelect, onClose }: TrackActionSheetP const actions = useMemo( () => [ { id: 'playNext', label: t('track.playNext'), icon: ListStart, emphasis: true }, - { id: 'addToQueue', label: t('selection.addToQueue'), icon: ListEnd }, + { id: 'addToQueue', label: t('track.addToQueue'), icon: ListEnd }, { id: 'addToPlaylist', label: t('playlists.addTo'), icon: ListMusic }, { id: 'favorite', label: track?.isFavorite ? t('player.unfavorite') : t('player.favorite'), icon: track?.isFavorite ? HeartOff : Heart, }, - { id: 'select', label: t('track.select'), icon: CheckSquare }, { id: 'info', label: t('track.info'), icon: Info }, ], [t, track?.isFavorite], diff --git a/src/features/library/components/TrackList.tsx b/src/features/library/components/TrackList.tsx index 1c57b8d..4931a10 100644 --- a/src/features/library/components/TrackList.tsx +++ b/src/features/library/components/TrackList.tsx @@ -5,6 +5,7 @@ import { useTranslation } from 'react-i18next'; import { RefreshControl } from 'react-native'; import type { TrackListItem } from '@/db/queries/tracks'; +import { useMiniPlayerInset } from '@/features/player/playerLayerLayout'; import { useThemeColors } from '@/theme/useTheme'; import { LibraryRow } from './LibraryRow'; @@ -35,19 +36,9 @@ const DRAW_DISTANCE = 1_200; export interface TrackListProps { tracks: TrackListItem[]; locale: string; - /** True while the list is in selection mode. */ - isSelecting: boolean; - /** - * The selected ids as a Set. - * - * A Set rather than the whole selection object, because `renderItem` closes - * over whatever it is given: taking the object made every row depend on - * something that changed on every unrelated parent render. - */ - selectedIds: ReadonlySet; - /** Plays the track, or toggles it when selecting. */ + /** Plays the track. */ onPress: (id: number) => void; - /** Opens the action sheet, or starts selection. */ + /** Opens the action sheet. */ onLongPress: (id: number) => void; /** Swipe left on a row. */ onSwipeToQueue: (id: number) => void; @@ -62,15 +53,13 @@ export interface TrackListProps { * The library list itself. * * Split out of `LibraryScreen` because the screen had grown past what one - * component should hold once selection, swiping and the action sheet arrived — + * component should hold once swiping and the action sheet arrived — * `AGENTS.md` puts a hard limit at 300 lines. The screen now decides *what* * happens; this decides how rows are drawn. */ export function TrackList({ tracks, locale, - isSelecting, - selectedIds, onPress, onLongPress, onSwipeToQueue, @@ -81,8 +70,14 @@ export function TrackList({ }: TrackListProps) { const { t } = useTranslation(); const colors = useThemeColors(); + /* + * A runtime measurement, so it cannot be a Tailwind class — the config + * compiles anything outside the spacing scale to nothing at all. This is the + * exception `AGENTS.md` names. + */ + const bottomInset = useMiniPlayerInset(); - const swipeLabel = t('selection.addToQueue'); + const swipeLabel = t('track.addToQueue'); const renderItem = useCallback>( ({ item }) => { @@ -93,23 +88,12 @@ export function TrackList({ onPress={onPress} onLongPress={onLongPress} onSwipeToQueue={onSwipeToQueue} - isSelecting={isSelecting} - isSelected={selectedIds.has(item.id)} isCurrent={item.id === currentTrackId} swipeLabel={swipeLabel} /> ); }, - [ - locale, - onPress, - onLongPress, - onSwipeToQueue, - isSelecting, - selectedIds, - currentTrackId, - swipeLabel, - ], + [locale, onPress, onLongPress, onSwipeToQueue, currentTrackId, swipeLabel], ); return ( @@ -119,6 +103,8 @@ export function TrackList({ keyExtractor={keyExtractor} drawDistance={DRAW_DISTANCE} overrideItemLayout={setRowHeight} + // So the last rows clear the transport strip instead of sitting under it. + contentContainerStyle={{ paddingBottom: bottomInset }} /* Pull to refresh re-indexes and sweeps. Without it a user who has just copied files in has no way to make the app look again short of restarting diff --git a/src/features/library/components/TrackRow.tsx b/src/features/library/components/TrackRow.tsx index 174508c..497fbdf 100644 --- a/src/features/library/components/TrackRow.tsx +++ b/src/features/library/components/TrackRow.tsx @@ -1,5 +1,5 @@ import { Image } from 'expo-image'; -import { Check, Music } from 'lucide-react-native'; +import { Music } from 'lucide-react-native'; import { memo, useCallback } from 'react'; import { useTranslation } from 'react-i18next'; import { Pressable, Text, View } from 'react-native'; @@ -16,9 +16,6 @@ export interface TrackRowProps { onPress: (id: number) => void; /** Long press opens the track's actions. Also stable. */ onLongPress: (id: number) => void; - /** True while the list is in selection mode. Swaps artwork for a checkbox. */ - isSelecting?: boolean; - isSelected?: boolean; /** True when this is the track the engine is playing. Indigo marks it. */ isCurrent?: boolean; } @@ -35,8 +32,6 @@ const TrackRowComponent = function TrackRow({ locale, onPress, onLongPress, - isSelecting = false, - isSelected = false, isCurrent = false, }: TrackRowProps) { const { t } = useTranslation(); @@ -58,28 +53,13 @@ const TrackRowComponent = function TrackRow({ onPress={handlePress} onLongPress={handleLongPress} android_ripple={{ color: colors.etch }} - accessibilityRole={isSelecting ? 'checkbox' : 'button'} + accessibilityRole="button" accessibilityLabel={track.title} accessibilityHint={subtitle || undefined} - accessibilityState={isSelecting ? { checked: isSelected } : { selected: isCurrent }} + accessibilityState={{ selected: isCurrent }} className="h-16 flex-row items-center gap-3 px-6" > - {/* - The checkbox replaces the artwork rather than sitting beside it. Adding a - column would shift every title sideways the moment selection starts, - which makes the whole list appear to jump. - */} - {isSelecting ? ( - - {isSelected ? : null} - - ) : artworkUri ? ( + {artworkUri ? ( {track.title} @@ -139,8 +121,6 @@ function isSameRow(previous: TrackRowProps, next: TrackRowProps): boolean { previous.locale === next.locale && previous.onPress === next.onPress && previous.onLongPress === next.onLongPress && - previous.isSelecting === next.isSelecting && - previous.isSelected === next.isSelected && previous.isCurrent === next.isCurrent && previous.track.id === next.track.id && previous.track.title === next.track.title && diff --git a/src/features/library/hooks/useScan.ts b/src/features/library/hooks/useScan.ts index 0e7db13..6bc1318 100644 --- a/src/features/library/hooks/useScan.ts +++ b/src/features/library/hooks/useScan.ts @@ -62,8 +62,12 @@ export interface UseScanResult { * body about there being no automatic sweep. */ scanLibrary: () => Promise; - /** Opens the system folder picker, then scans what was chosen. */ - addFolder: () => Promise; + /** Opens the system folder picker without starting an import yet. */ + pickFolder: () => Promise; + /** Adds a confirmed folder and runs both scan stages. */ + importFolder: (treeUri: string) => Promise; + /** True from confirmed folder import until enrichment has settled. */ + isFolderImporting: boolean; /** * Re-index the known folders and sweep again. This is the pull-to-refresh * path: a user who has just copied files in should not have to restart the @@ -86,6 +90,7 @@ export interface UseScanResult { export function useScan(): UseScanResult { const [progress, setProgress] = useState(IDLE); const [pulled, setPulled] = useState(false); + const [isFolderImporting, setFolderImporting] = useState(false); const cancelled = useRef(false); const ports: ScannerPorts = useMemo( @@ -163,36 +168,20 @@ export function useScan(): UseScanResult { await run(); }, [ensurePermission, run]); - const addFolder = useCallback(async () => { + const pickFolder = useCallback(async (): Promise => { // Ask before opening the picker, not after. The tree the picker returns // grants access to that tree only — the scan queries MediaStore, which // needs the audio permission — so without it the pick is wasted work and // the user has chosen a folder for nothing. const permission = await ensurePermission(); - if (permission === 'denied') return; + if (permission === 'denied') return null; try { const directory = await Directory.pickDirectoryAsync(); - await addScanFolder(directory.uri); - - // Index the folder rather than walking it — see ADR 007. A tree walk - // would put SAF document URIs in `tracks.file_uri` alongside MediaStore - // ones, and every consumer would then have to know which it was holding. - await requestMediaScanFor(directory.uri); - - await run(); + return directory.uri; } catch (error) { if (isPickerDismissal(error)) { - /* - * A cancelled picker is not a failure — the user changed their mind, and - * the screen should look as it did before. With one exception: if the - * permission was granted a moment ago, nothing has ever read the library, - * so sweep once anyway. The user did ask for music to be added; they only - * changed their mind about *which folder*, and leaving a freshly - * permitted app on an empty library answers a question they did not ask. - */ - if (permission === 'granted-now') await run(); - return; + return null; } setProgress({ phase: 'failed', @@ -200,8 +189,26 @@ export function useScan(): UseScanResult { processed: 0, error: error instanceof Error ? error.message : String(error), }); + return null; } - }, [ensurePermission, run]); + }, [ensurePermission]); + + const importFolder = useCallback( + async (treeUri: string) => { + setFolderImporting(true); + try { + await addScanFolder(treeUri); + // Index the folder rather than walking it — see ADR 007. A tree walk + // would put SAF document URIs in `tracks.file_uri` alongside MediaStore + // ones, and every consumer would then have to know which it was holding. + await requestMediaScanFor(treeUri); + await run(); + } finally { + setFolderImporting(false); + } + }, + [run], + ); const runRescan = useCallback(async () => { if ((await ensurePermission()) === 'denied') return; @@ -268,7 +275,9 @@ export function useScan(): UseScanResult { isScanning, isRefreshing: pulled && isScanning, scanLibrary, - addFolder, + pickFolder, + importFolder, + isFolderImporting, rescan, cancel, }; @@ -292,4 +301,3 @@ async function requestMediaScanFor(treeUri: string): Promise { // Indexing is best-effort; the sweep below still runs. } } - diff --git a/src/features/library/hooks/useSelection.ts b/src/features/library/hooks/useSelection.ts deleted file mode 100644 index 90fca62..0000000 --- a/src/features/library/hooks/useSelection.ts +++ /dev/null @@ -1,97 +0,0 @@ -import { useCallback, useMemo, useState } from 'react'; - -import { commitFeedback, liftFeedback, tapFeedback } from '@/services/haptics'; - -export interface Selection { - /** True while the list is in selection mode, even with nothing selected. */ - isActive: boolean; - /** Selected ids, in the order they were picked. */ - ids: number[]; - has: (id: number) => boolean; - toggle: (id: number) => void; - /** Enter selection mode with nothing picked. From the header button. */ - activate: () => void; - /** Enter selection mode with this id already picked. From a long press. */ - begin: (id: number) => void; - /** Select every id given, or clear if they are all already selected. */ - toggleAll: (ids: number[]) => void; - /** Leave selection mode and forget everything. */ - clear: () => void; -} - -/** - * Which rows are selected, and whether selection mode is on at all. - * - * Insertion order is kept rather than sorted by id. Selecting four tracks and - * adding them to a playlist should put them in the playlist in the order they - * were tapped — sorting by primary key would reorder them by when they were - * scanned, which is arbitrary from the user's side. - * - * `isActive` is separate from `ids.length > 0` on purpose. Deselecting the last - * row must not drop out of selection mode: the user is mid-task and about to - * pick a different row, and having the checkboxes vanish under them is the - * behaviour that makes multi-select feel unusable. - */ -export function useSelection(): Selection { - const [isActive, setActive] = useState(false); - const [ids, setIds] = useState([]); - - const selected = useMemo(() => new Set(ids), [ids]); - - const has = useCallback((id: number) => selected.has(id), [selected]); - - const toggle = useCallback((id: number) => { - tapFeedback(); - setIds((current) => - current.includes(id) ? current.filter((entry) => entry !== id) : [...current, id], - ); - }, []); - - /* - * Two entry points rather than one with a sentinel id. - * - * This was `begin(-1)` from the header, on the theory that -1 matches no row. - * It does not match a row and it *is* still added to the list, so the bar - * opened reading "1 selected" with nothing ticked, and `toggleAll`'s - * length comparison was permanently off by one. Caught on the emulator, not - * by a type. - */ - const activate = useCallback(() => { - liftFeedback(); - setActive(true); - }, []); - - const begin = useCallback((id: number) => { - liftFeedback(); - setActive(true); - setIds((current) => (current.includes(id) ? current : [...current, id])); - }, []); - - const toggleAll = useCallback((all: number[]) => { - commitFeedback(); - setIds((current) => (current.length === all.length ? [] : all)); - }, []); - - const clear = useCallback(() => { - setActive(false); - setIds([]); - }, []); - - /* - * Memoized, and it matters more than it looks. - * - * This used to return a fresh object literal on every render. Anything holding - * it — `LibraryScreen`'s row callbacks, `TrackList`'s `renderItem` — then had a - * dependency that changed on *every* parent render, including ones with - * nothing to do with selection. Measured on the Pixel_7 AVD: 47 row renders per - * checkbox tap, each rebuilding a Pan gesture, which is what "the app freezes - * when you open the checkboxes" was. - * - * Every function below is `useCallback([])` and therefore already stable, so in - * practice this changes identity only when `isActive` or `ids` really change. - */ - return useMemo( - () => ({ isActive, ids, has, toggle, activate, begin, toggleAll, clear }), - [isActive, ids, has, toggle, activate, begin, toggleAll, clear], - ); -} diff --git a/src/features/library/hooks/useTrackActions.ts b/src/features/library/hooks/useTrackActions.ts index 1165075..9058c17 100644 --- a/src/features/library/hooks/useTrackActions.ts +++ b/src/features/library/hooks/useTrackActions.ts @@ -21,10 +21,9 @@ export interface TrackActions { /** * The queue and favourite actions, in one place. * - * Both the swipe gesture, the long-press sheet and the selection bar do these, - * and each one wants the same haptic and the same empty-input guard. Duplicating - * that across three call sites is how one of them ends up silently doing - * nothing. + * Both the swipe gesture and the long-press sheet use these, and each one wants + * the same haptic and the same empty-input guard. Duplicating that across call + * sites is how one of them ends up silently doing nothing. * * An empty list gets a rejection buzz rather than a success one. "Add to queue" * with nothing selected is a press that cannot work, and confirming it is worse diff --git a/src/features/player/PlayerLayer.tsx b/src/features/player/PlayerLayer.tsx new file mode 100644 index 0000000..bfedc4d --- /dev/null +++ b/src/features/player/PlayerLayer.tsx @@ -0,0 +1,143 @@ +import { useSegments } from 'expo-router'; +import type { ReactNode } from 'react'; +import { useCallback, useEffect, useState } from 'react'; +import { BackHandler, View, type LayoutChangeEvent } from 'react-native'; +import Animated, { runOnJS, useAnimatedStyle, withSpring } from 'react-native-reanimated'; +import { SafeAreaView } from 'react-native-safe-area-context'; + +import { MiniPlayer } from './components/MiniPlayer'; +import { NowPlayingOverlay } from './components/NowPlayingOverlay'; +import { QueueOverlay } from './components/QueueOverlay'; +import { playerExpansion } from './playerExpansion'; +import { setMiniPlayerHeight, usePlayerTabBarHeight } from './playerLayerLayout'; + +/** + * The settle after the finger leaves. + * + * Softer and heavier than the spring it replaces (`damping: 24, + * stiffness: 260`, which is ζ ≈ 0.75 with no mass term and arrives with a + * snap). At ζ ≈ 0.86 this overshoots by a hair and comes to rest, which is what + * a sheet does. The number that matters more than either is `velocity`: the + * spring used to start from rest however hard the strip was flicked, so a fast + * throw and a slow drag opened at exactly the same speed and the gesture + * appeared not to be connected to the animation at all. + */ +const SPRING = { damping: 22, stiffness: 180, mass: 0.9 } as const; + +export interface PlayerLayerProps { + children: ReactNode; +} + +/** Keeps transport visible above every route and owns the one player expansion value. */ +export function PlayerLayer({ children }: PlayerLayerProps) { + const segments = useSegments(); + const tabBarHeight = usePlayerTabBarHeight(); + const [visible, setVisible] = useState(false); + const [expanded, setExpanded] = useState(false); + const [queueOpen, setQueueOpen] = useState(false); + const isTabRoute = segments[0] === '(tabs)'; + + const prepareOpen = useCallback(() => setVisible(true), []); + const openQueue = useCallback(() => setQueueOpen(true), []); + const closeQueue = useCallback(() => setQueueOpen(false), []); + + /* + * Publish how much of every screen the strip covers, so lists can pad for it + * from one number instead of each guessing. Measured on the wrapper, which + * includes the route-dependent safe-area padding below the strip. + */ + const onStripLayout = useCallback((event: LayoutChangeEvent) => { + setMiniPlayerHeight(event.nativeEvent.layout.height); + }, []); + + useEffect(() => () => setMiniPlayerHeight(0), []); + + /** + * `velocity` is in expansion units per second — the gesture's px/s divided by + * the screen height — because that is what the shared value is measured in. + */ + const onExpandedChange = useCallback((nextExpanded: boolean, velocity = 0) => { + if (nextExpanded) { + setVisible(true); + setExpanded(true); + playerExpansion.value = withSpring(1, { ...SPRING, velocity }); + return; + } + + setExpanded(false); + playerExpansion.value = withSpring(0, { ...SPRING, velocity }, (finished) => { + if (finished) runOnJS(setVisible)(false); + }); + }, []); + + /* + * Back closes the topmost player surface. + * + * The queue used to be a route, so the navigator gave it this for free. Now + * that neither surface is one, nothing else can: they are mounted outside the + * router, so back would pop the screen *underneath* an open player and leave + * it open over a screen the user never chose. Innermost first, which is the + * order they are stacked in. + * + * Declared after `onExpandedChange` deliberately. A dependency array is built + * during render, so referencing it above its own `const` is a temporal dead + * zone error on every render, not a lint nit. + */ + useEffect(() => { + if (!queueOpen && !expanded) return; + + const subscription = BackHandler.addEventListener('hardwareBackPress', () => { + if (queueOpen) { + setQueueOpen(false); + return true; + } + onExpandedChange(false); + return true; + }); + + return () => subscription.remove(); + }, [queueOpen, expanded, onExpandedChange]); + + /* + * The whole strip fades, not just its contents. + * + * This lived on the row inside `MiniPlayer` and left three things behind at + * full opacity: the strip's own `bg-surface-elevated`, its top hairline, and + * `MiniProgress`, which sits outside the gesture wrapper on purpose. All + * three kept drawing over an open Now Playing — the panel and progress bar + * across the bottom of the expanded player were the mini player, still there. + */ + const strip = useAnimatedStyle(() => ({ opacity: 1 - playerExpansion.value })); + + return ( + + {children} + + + + + + + + {/* + Last, and at the highest layer. Paint order and `zIndex` now agree: the + overlay used to be `z-10` under the strip's `z-20`, so the surface it is + supposed to cover was drawn on top of it. + */} + + + {/* Above Now Playing, and outside its transformed container. */} + + + ); +} diff --git a/src/features/player/PlayerScreen.tsx b/src/features/player/PlayerScreen.tsx index 439d48f..4da902f 100644 --- a/src/features/player/PlayerScreen.tsx +++ b/src/features/player/PlayerScreen.tsx @@ -1,4 +1,3 @@ -import { useRouter } from 'expo-router'; import { ChevronDown, ListMusic, @@ -14,7 +13,6 @@ import { import { useCallback, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Pressable, Text, View } from 'react-native'; -import { SafeAreaView } from 'react-native-safe-area-context'; import { EmptyState } from '@/components/ui/EmptyState'; import { AudioEngine } from '@/services/audio/AudioEngine'; @@ -34,13 +32,18 @@ import { useQueueNeighbours } from './hooks/useQueueNeighbours'; /** * Now Playing. * - * A modal route rather than a tab: it is a place you go from something, and - * you leave it by dismissing rather than by choosing a different destination. + * The root player overlay rather than a route. It stays mounted above every + * screen so the mini player and this surface share one gesture progress value. */ -export function PlayerScreen() { +export interface PlayerScreenProps { + onExpandedChange: (expanded: boolean, velocity?: number) => void; + /** The queue is a root-level surface, not a route. See `QueueOverlay`. */ + onOpenQueue: () => void; +} + +export function PlayerScreen({ onExpandedChange, onOpenQueue }: PlayerScreenProps) { const { t, i18n } = useTranslation(); const colors = useThemeColors(); - const router = useRouter(); const { phase, track, positionMs, durationMs, error } = usePlayback(); const { toggle, toggleShuffle, next, previous, seekTo } = usePlaybackControls(); @@ -48,10 +51,7 @@ export function PlayerScreen() { const [repeat, setRepeatState] = useState(() => AudioEngine.getRepeat()); const [shuffled, setShuffledState] = useState(() => AudioEngine.isShuffled()); - const close = useCallback(() => router.back(), [router]); - // `navigate` rather than `push`, for the same reason as the player itself: - // a double press must not leave two identical screens on the stack. - const openQueue = useCallback(() => router.navigate('/queue'), [router]); + const close = useCallback(() => onExpandedChange(false), [onExpandedChange]); const onShufflePress = useCallback(() => { toggleShuffle(); @@ -66,10 +66,10 @@ export function PlayerScreen() { if (track === null) { return ( - -
+ +
- + ); } @@ -79,10 +79,15 @@ export function PlayerScreen() { const RepeatIcon = repeat === 'one' ? Repeat1 : Repeat; return ( - -
- - + +
+ + {/* + `gap-6`, not `gap-8`. Three gaps of 32 between four blocks was 96 dp of + air the column did not have — see `artworkSize.ts` for what the overflow + did to both ends of the screen. + */} + {/* The artwork carries the gestures, not the whole screen: the scrubber below owns a pan of its own, and two competing pans on one surface @@ -96,7 +101,7 @@ export function PlayerScreen() { neighbours={neighbours} onNext={next} onPrevious={previous} - onDismiss={close} + onExpandedChange={onExpandedChange} /> @@ -202,7 +207,7 @@ export function PlayerScreen() { - + ); } diff --git a/src/features/player/QueueScreen.tsx b/src/features/player/QueueScreen.tsx index 48a1b3b..ef57c11 100644 --- a/src/features/player/QueueScreen.tsx +++ b/src/features/player/QueueScreen.tsx @@ -1,5 +1,4 @@ import { FlashList, type ListRenderItem } from '@shopify/flash-list'; -import { useRouter } from 'expo-router'; import { ChevronDown, ListX, Music } from 'lucide-react-native'; import { useCallback, useSyncExternalStore } from 'react'; import { useTranslation } from 'react-i18next'; @@ -26,21 +25,24 @@ interface QueueItem { * emitted twice a second for the position, and re-rendering a few hundred rows * at that rate is exactly the jank the performance rules exist to prevent. */ -export function QueueScreen() { +export interface QueueScreenProps { + /** Dismisses the sheet. Not `router.back()` — this is not a route. */ + onClose: () => void; +} + +export function QueueScreen({ onClose }: QueueScreenProps) { const { t, i18n } = useTranslation(); const colors = useThemeColors(); - const router = useRouter(); const snapshot = useSyncExternalStore(subscribeQueue, getQueueSnapshot); - const close = useCallback(() => router.back(), [router]); const playAt = useCallback((position: number) => void AudioEngine.jumpTo(position), []); const removeAt = useCallback((position: number) => void AudioEngine.removeAt(position), []); const clear = useCallback(() => { void AudioEngine.clearQueue(); - router.back(); - }, [router]); + onClose(); + }, [onClose]); const items: QueueItem[] = snapshot.tracks.map((track, position) => ({ track, position })); @@ -60,10 +62,13 @@ export function QueueScreen() { ); return ( - + /* `bottom` as well as `top`: this is a full-screen sheet now, not a route + inside a navigator that was insetting it, so the last queue row would sit + under the navigation bar the way the transport row did. */ + { + it('is the width minus both gutters when there is height to spare', () => { + expect(artworkSize(393, 600)).toBe(393 - ARTWORK_GUTTER * 2); + }); + + it('is the available height when height is the tighter bound', () => { + expect(artworkSize(393, 266)).toBe(266); + }); + + it('never exceeds the space it was given', () => { + for (const width of [320, 360, 393, 412, 480]) { + for (const height of [0, 120, 266, 345, 700]) { + const size = artworkSize(width, height); + expect(size).toBeLessThanOrEqual(width - ARTWORK_GUTTER * 2); + if (height > 0) expect(size).toBeLessThanOrEqual(height); + } + } + }); + + it('falls back to the width bound before the first layout', () => { + // Zero would draw nothing for a frame and then pop to full size. + expect(artworkSize(393, 0)).toBe(345); + }); + + it('never goes negative on an implausibly narrow screen', () => { + expect(artworkSize(20, 100)).toBe(0); + expect(artworkSize(20, 0)).toBe(0); + }); +}); diff --git a/src/features/player/artworkSize.ts b/src/features/player/artworkSize.ts new file mode 100644 index 0000000..1af2deb --- /dev/null +++ b/src/features/player/artworkSize.ts @@ -0,0 +1,33 @@ +/** + * How big the Now Playing cover may be. + * + * The cover used to be sized from the screen width alone — a square of + * `width - 2 × gutter`, whatever that came to. On a 393 × 851 dp phone that is + * 345 dp of artwork above roughly 440 dp of title, spec strip, scrubber and + * transport, inside about 780 dp of usable height. The column overflowed by a + * good 50 dp, and `justify-center` split the overflow between both ends: the + * header was clipped off the top and the transport row was pushed under the + * navigation bar. Two bug reports, one cause — the artwork was never asked + * whether it fit. + * + * So the cover is bounded by both axes. The height comes from a real + * measurement of the space flex left over rather than from a constant, because + * the chrome below it is not a fixed height: the title wraps to two lines, and + * Turkish runs 10–20% longer than English. + */ + +/** `px-6` either side of the cover, matching the rest of the screen. */ +export const ARTWORK_GUTTER = 24; + +/** + * The side of the cover square. + * + * `availableHeight` of zero means layout has not run yet, where the width bound + * is the better guess — starting at zero would draw an invisible cover for a + * frame and then pop. + */ +export function artworkSize(screenWidth: number, availableHeight: number): number { + const byWidth = Math.max(0, screenWidth - ARTWORK_GUTTER * 2); + if (availableHeight <= 0) return byWidth; + return Math.max(0, Math.min(byWidth, availableHeight)); +} diff --git a/src/features/player/components/ArtworkCarousel.tsx b/src/features/player/components/ArtworkCarousel.tsx index 9a74431..99b5239 100644 --- a/src/features/player/components/ArtworkCarousel.tsx +++ b/src/features/player/components/ArtworkCarousel.tsx @@ -1,8 +1,8 @@ import { Image } from 'expo-image'; import { Music } from 'lucide-react-native'; -import { useMemo } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { useWindowDimensions, View } from 'react-native'; +import { useWindowDimensions, View, type LayoutChangeEvent } from 'react-native'; import { Gesture, GestureDetector } from 'react-native-gesture-handler'; import Animated, { runOnJS, @@ -15,7 +15,9 @@ import type { PlayableTrack } from '@/services/audio/types'; import { useReducedMotion } from '@/theme/useReducedMotion'; import { useThemeColors } from '@/theme/useTheme'; +import { artworkSize } from '../artworkSize'; import type { QueueNeighbours } from '../hooks/useQueueNeighbours'; +import { setPlayerExpansion } from '../playerExpansion'; /** Fraction of a page the finger must cover to commit without a flick. */ const DISTANCE_THRESHOLD = 0.28; @@ -54,8 +56,11 @@ export interface ArtworkCarouselProps { neighbours: QueueNeighbours; onNext: () => void; onPrevious: () => void; - /** Swipe down on the artwork dismisses the player. */ - onDismiss: () => void; + /** + * Releases the shared overlay at either end of a vertical drag. `velocity` is + * in expansion units per second, so a thrown screen keeps its speed. + */ + onExpandedChange: (expanded: boolean, velocity?: number) => void; } /** @@ -85,13 +90,25 @@ export function ArtworkCarousel({ neighbours, onNext, onPrevious, - onDismiss, + onExpandedChange, }: ArtworkCarouselProps) { - const { width } = useWindowDimensions(); + const { width, height } = useWindowDimensions(); const reducedMotion = useReducedMotion(); + /* + * The cover is bounded by the height flex left over, not by the screen width + * alone. See `artworkSize.ts`: the width-only square overflowed the column, + * which is what clipped the header and pushed the transport row under the + * navigation bar. Measured rather than computed from a constant because the + * chrome below it changes height when the title wraps. + */ + const [boxHeight, setBoxHeight] = useState(0); + const onLayout = useCallback((event: LayoutChangeEvent) => { + setBoxHeight(event.nativeEvent.layout.height); + }, []); + const size = artworkSize(width, boxHeight); + const offsetX = useSharedValue(0); - const offsetY = useSharedValue(0); /** 0 undecided, 1 horizontal, 2 vertical. Fixed once per gesture. */ const axis = useSharedValue(0); @@ -123,8 +140,7 @@ export function ArtworkCarousel({ } if (axis.value === 2) { - // Down only. Dragging up from the player has no meaning. - offsetY.value = Math.max(0, event.translationY); + setPlayerExpansion(Math.min(1, Math.max(0, 1 - Math.max(0, event.translationY) / height))); return; } @@ -136,9 +152,10 @@ export function ArtworkCarousel({ }) .onEnd((event) => { if (axis.value === 2) { - if (event.translationY > DISMISS_DISTANCE || event.velocityY > DISMISS_VELOCITY) { - runOnJS(onDismiss)(); - } + const dismiss = event.translationY > DISMISS_DISTANCE || event.velocityY > DISMISS_VELOCITY; + // Downwards is positive in gesture space and negative in expansion + // space, so the throw carries into the spring rather than stopping dead. + runOnJS(onExpandedChange)(!dismiss, -event.velocityY / height); return; } @@ -176,21 +193,23 @@ export function ArtworkCarousel({ }) .onFinalize(() => { axis.value = 0; - offsetY.value = withSpring(0, SPRING); }); const stripStyle = useAnimatedStyle(() => ({ - transform: [{ translateX: offsetX.value }, { translateY: offsetY.value }], + transform: [{ translateX: offsetX.value }], })); return ( - {/* Clips the neighbours to the visible slot. */} - - - - - + {/* Takes the height flex leaves it, and clips the neighbours. */} + + + + + @@ -200,6 +219,8 @@ export function ArtworkCarousel({ interface SlotProps { track: PlayableTrack | null; width: number; + /** The side of the cover square, already bounded by both axes. */ + size: number; /** Where this slot sits relative to the centre one. */ offset: number; } @@ -210,21 +231,29 @@ interface SlotProps { * Absolutely positioned rather than laid out in the row, so the strip is exactly * one screen wide and the neighbours hang off either edge — a three-wide flex row * would make the container three screens wide and push the layout around. + * + * The square takes its side from `size` rather than from `aspect-square w-full`. + * A width-derived square is only square while the width is the tighter of the + * two bounds, and on this screen it was not. */ -function Slot({ track, width, offset }: SlotProps) { +function Slot({ track, width, size, offset }: SlotProps) { const { t } = useTranslation(); const colors = useThemeColors(); const artworkUri = track?.artworkPath ? `file://${track.artworkPath}` : null; const isCentre = offset === 0; - const style = useMemo( + const slotStyle = useMemo( () => ({ width, left: offset, position: 'absolute' as const, top: 0, bottom: 0 }), [width, offset], ); + const squareStyle = useMemo(() => ({ width: size, height: size }), [size]); const content = ( - + {artworkUri ? ( ); - /* - * The centre slot is in the layout and gives the container its height; the - * neighbours are absolute and contribute none. Doing it the other way round - * would make the player's height depend on whether a next track exists. - */ if (isCentre) { return ( {content} @@ -260,7 +287,7 @@ function Slot({ track, width, offset }: SlotProps) { } return ( - + {content} ); diff --git a/src/features/player/components/MiniPlayer.tsx b/src/features/player/components/MiniPlayer.tsx index e886d57..1ac595e 100644 --- a/src/features/player/components/MiniPlayer.tsx +++ b/src/features/player/components/MiniPlayer.tsx @@ -1,9 +1,8 @@ import { Image } from 'expo-image'; -import { useRouter } from 'expo-router'; import { Music, Pause, Play, SkipBack, SkipForward } from 'lucide-react-native'; import { useCallback, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; -import { Pressable, Text, View } from 'react-native'; +import { Pressable, Text, useWindowDimensions, View } from 'react-native'; import { Gesture, GestureDetector } from 'react-native-gesture-handler'; import Animated, { runOnJS, @@ -15,14 +14,12 @@ import Animated, { import { tapFeedback } from '@/services/haptics'; import * as perf from '@/services/perf'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; -import { useReducedMotion } from '@/theme/useReducedMotion'; import { useThemeColors } from '@/theme/useTheme'; import { useCurrentTrack, usePlaybackControls, usePlaybackPhase } from '../hooks/usePlayback'; +import { playerExpansion, setPlayerExpansion } from '../playerExpansion'; import { MiniProgress } from './MiniProgress'; -/** How far up the strip must be dragged to open the player. */ -const OPEN_DISTANCE = 40; /** px/s upward that opens it regardless of distance. */ const OPEN_VELOCITY = 600; /** Sideways travel that changes track. Shorter than the player's — less room. */ @@ -43,6 +40,19 @@ const FOLLOW_RATIO = 0.4; const SPRING = { damping: 20, stiffness: 220 } as const; +export interface MiniPlayerProps { + /** Mounts Now Playing as soon as a vertical drag begins, or a finger lands. */ + onPrepareOpen: () => void; + /** + * Settles the root overlay after a tap or released drag. + * + * `velocity` is in expansion units per second — the gesture's px/s over the + * screen height — so a flick hands its speed to the spring instead of the + * spring always starting from rest. + */ + onExpandedChange: (expanded: boolean, velocity?: number) => void; +} + /** * The persistent transport strip above the tab bar. * @@ -55,12 +65,11 @@ const SPRING = { damping: 20, stiffness: 220 } as const; * flick it. The gesture is deliberately additive — every one of its actions has * a button beside it, so nothing here is reachable only by knowing a secret. */ -export function MiniPlayer() { +export function MiniPlayer({ onPrepareOpen, onExpandedChange }: MiniPlayerProps) { useLifecycleTrace('MiniPlayer'); const { t } = useTranslation(); const colors = useThemeColors(); - const router = useRouter(); - const reducedMotion = useReducedMotion(); + const { height } = useWindowDimensions(); /* * Phase and track, never position. @@ -80,23 +89,12 @@ export function MiniPlayer() { if (track !== null) perf.measure('library.play.toMiniPlayer', track.id); }, [track]); - /* - * `navigate`, not `push`. - * - * The strip offers the same action three ways — tap, drag, flick — and a - * drag fires the pan's handler while the underlying Pressable can still - * register its own press. `push` stacked two copies of the player, and the - * symptom was baffling: swipe down to dismiss appeared to do nothing, and so - * did the close button, because each was correctly popping one of two - * identical screens. `navigate` reuses the route that is already there. - */ const openPlayer = useCallback(() => { tapFeedback(); - router.navigate('/player'); - }, [router]); + onExpandedChange(true); + }, [onExpandedChange]); const offsetX = useSharedValue(0); - const offsetY = useSharedValue(0); const axis = useSharedValue(0); /* @@ -132,14 +130,14 @@ export function MiniPlayer() { // Undecided until the finger has actually gone somewhere. if (Math.max(dx, dy) < AXIS_LOCK_SLOP) return; axis.value = dx > dy ? 1 : 2; + if (axis.value === 2) runOnJS(onPrepareOpen)(); } if (axis.value === 1) { offsetX.value = event.translationX * FOLLOW_RATIO; return; } - // Up only. Dragging the strip down has nowhere to go. - offsetY.value = Math.min(0, event.translationY) * FOLLOW_RATIO; + setPlayerExpansion(Math.min(1, Math.max(0, -event.translationY / height))); }) .onEnd((event) => { if (axis.value === 1) { @@ -148,20 +146,24 @@ export function MiniPlayer() { return; } - // Distance or velocity, so a short flick opens it as readily as a - // deliberate drag. - const far = event.translationY <= -OPEN_DISTANCE; + const far = playerExpansion.value >= 0.18; const fast = event.velocityY <= -OPEN_VELOCITY; - if (far || fast) runOnJS(openPlayer)(); + // Upwards is negative in gesture space and positive in expansion space. + runOnJS(onExpandedChange)(far || fast, -event.velocityY / height); }) .onFinalize(() => { axis.value = 0; offsetX.value = withSpring(0, SPRING); - offsetY.value = withSpring(0, SPRING); }); + /* + * Only the sideways follow. The fade moved up to `PlayerLayer`, which wraps + * the whole strip — this style covered the row but not the strip's panel + * background, its top hairline or `MiniProgress`, and those three carried on + * drawing over an open Now Playing. + */ const followStyle = useAnimatedStyle(() => ({ - transform: [{ translateX: offsetX.value }, { translateY: offsetY.value }], + transform: [{ translateX: offsetX.value }], })); if (phase === 'idle' || track === null) return null; @@ -182,10 +184,18 @@ export function MiniPlayer() { - + void; + /** Opens the queue, which is a root-level surface rather than a route. */ + onOpenQueue: () => void; +} + +/** Root-mounted Now Playing surface driven directly by the mini-player gesture. */ +export function NowPlayingOverlay({ + visible, + expanded, + onExpandedChange, + onOpenQueue, +}: NowPlayingOverlayProps) { + const { height } = useWindowDimensions(); + const style = useAnimatedStyle(() => ({ + opacity: interpolate( + playerExpansion.value, + [0, FADE_COMPLETE_AT], + [0, 1], + Extrapolation.CLAMP, + ), + // Linear against the shared value on purpose: the easing belongs to the + // spring driving that value, and putting a curve here too would compound + // the two into something that reads as a stutter. + transform: [{ translateY: interpolate(playerExpansion.value, [0, 1], [height, 0]) }], + })); + + return ( + + {visible ? ( + + + + ) : null} + + ); +} diff --git a/src/features/player/components/QueueOverlay.tsx b/src/features/player/components/QueueOverlay.tsx new file mode 100644 index 0000000..0dd8b2c --- /dev/null +++ b/src/features/player/components/QueueOverlay.tsx @@ -0,0 +1,45 @@ +import Animated, { SlideInDown, SlideOutDown } from 'react-native-reanimated'; + +import { useReducedMotion } from '@/theme/useReducedMotion'; + +import { QueueScreen } from '../QueueScreen'; + +export interface QueueOverlayProps { + visible: boolean; + onClose: () => void; +} + +/** + * The queue, as a root-level sheet rather than a route. + * + * It used to be `app/queue.tsx`, pushed with `router.navigate('/queue')`, and + * it opened without ever becoming visible. Nothing was wrong with the screen: + * `PlayerLayer` mounts the Now Playing overlay *outside* the router, absolutely + * positioned over the whole app, and an opaque full-screen surface at that + * level covers anything the navigator puts underneath it. The queue was + * rendering correctly, one layer down, behind the player that opened it. + * + * That is a property of the overlay rather than of this screen — **any** route + * pushed while Now Playing is open would have disappeared the same way — so the + * fix is to give the queue the same treatment as Now Playing itself: a sibling + * at the root, one layer above it, outside the overlay's transformed and + * clipped container. + * + * Mounted only while open. It carries a FlashList of the whole queue, and the + * player has no reason to pay for that while nobody is looking at it. + */ +export function QueueOverlay({ visible, onClose }: QueueOverlayProps) { + const reducedMotion = useReducedMotion(); + + if (!visible) return null; + + return ( + + + + ); +} diff --git a/src/features/player/listenRecorder.ts b/src/features/player/listenRecorder.ts index b613adc..a3edb93 100644 --- a/src/features/player/listenRecorder.ts +++ b/src/features/player/listenRecorder.ts @@ -21,7 +21,8 @@ export function startListenRecording(): () => void { void recordListen( { trackId: listen.track.id, - durationMs: listen.track.durationMs, + // The engine's duration, not the track row's. See `FinishedListen`. + durationMs: listen.durationMs, msPlayed: listen.msPlayed, startedAt: listen.startedAt, sourceType: listen.source.type, diff --git a/src/features/player/playerExpansion.ts b/src/features/player/playerExpansion.ts new file mode 100644 index 0000000..56a52b9 --- /dev/null +++ b/src/features/player/playerExpansion.ts @@ -0,0 +1,10 @@ +import { makeMutable } from 'react-native-reanimated'; + +/** The one root-owned progress value for the mini player and Now Playing overlay. */ +export const playerExpansion = makeMutable(0); + +/** Updates root player progress from a Reanimated worklet. */ +export function setPlayerExpansion(value: number): void { + 'worklet'; + playerExpansion.value = value; +} diff --git a/src/features/player/playerLayerLayout.ts b/src/features/player/playerLayerLayout.ts new file mode 100644 index 0000000..a34b959 --- /dev/null +++ b/src/features/player/playerLayerLayout.ts @@ -0,0 +1,84 @@ +import { useSyncExternalStore } from 'react'; + +/** + * The heights the root player layer measures and everything else needs. + * + * Both are real measurements rather than constants, and neither can be a + * design-system spacing value: the tab bar's height comes from the system font + * scale and the navigation-bar inset, and the mini player's from its own + * content plus whatever safe area the current route leaves it. + * + * A module-level store read through `useSyncExternalStore` rather than context, + * for the same reason the engine is: this changes on rotation and on the + * player appearing, which is rare, and a context provider around every screen + * would re-render the tree on each measurement. + */ + +let tabBarHeight = 0; +let miniPlayerHeight = 0; +const listeners = new Set<() => void>(); + +function emit(): void { + for (const listener of listeners) listener(); +} + +/** Records the measured tab-bar height for the root player layer. */ +export function setPlayerTabBarHeight(nextHeight: number): void { + if (tabBarHeight === nextHeight) return; + tabBarHeight = nextHeight; + emit(); +} + +/** + * Records how much of the screen the transport strip covers. + * + * Zero while nothing is playing, because the mini player renders nothing at + * all then — so a list gets its full height back rather than a strip of dead + * space under it. + */ +export function setMiniPlayerHeight(nextHeight: number): void { + if (miniPlayerHeight === nextHeight) return; + miniPlayerHeight = nextHeight; + emit(); +} + +/** Returns the current tab-bar height without polling layout from every route. */ +export function usePlayerTabBarHeight(): number { + return useSyncExternalStore(subscribe, getTabBarHeight); +} + +/** + * Bottom padding a scrollable screen needs so its last row clears the player. + * + * The mini player is always visible once something is playing, and it is + * absolutely positioned over the routes rather than laid out with them — so + * without this the last few rows of every list sit behind it, permanently + * unreachable. Every list and scroll view applies this to its **content + * container**, so the strip still has content sliding under it rather than a + * hard edge. + * + * One number, measured once, rather than a hand-tuned `pb-` on each screen: + * five screens with five guesses is five things to get wrong, and four of them + * were. + * + * The tab bar is deliberately *not* in it. On a tab route the screen already + * ends where the bar begins; on a pushed route there is no bar and the strip + * measures its own safe-area padding instead. Either way the mini player's + * measured height is exactly the overlap. + */ +export function useMiniPlayerInset(): number { + return useSyncExternalStore(subscribe, getMiniPlayerHeight); +} + +function subscribe(listener: () => void): () => void { + listeners.add(listener); + return () => listeners.delete(listener); +} + +function getTabBarHeight(): number { + return tabBarHeight; +} + +function getMiniPlayerHeight(): number { + return miniPlayerHeight; +} diff --git a/src/features/playlists/PlaylistDetailScreen.tsx b/src/features/playlists/PlaylistDetailScreen.tsx index 28e3ae0..0c831f0 100644 --- a/src/features/playlists/PlaylistDetailScreen.tsx +++ b/src/features/playlists/PlaylistDetailScreen.tsx @@ -8,21 +8,22 @@ import { SafeAreaView } from 'react-native-safe-area-context'; import { EmptyState } from '@/components/ui/EmptyState'; import { - addTracksToPlaylist, deletePlaylist, + LIKED_SONGS_ID, movePlaylistEntry, removeFromPlaylist, renamePlaylist, + useFavoriteEntries, usePlaylistEntries, usePlaylists, type PlaylistEntry, } from '@/db/queries/playlists'; +import { useMiniPlayerInset } from '@/features/player/playerLayerLayout'; import { useMessages } from '@/i18n'; import { AudioEngine } from '@/services/audio/AudioEngine'; -import type { PlayableTrack, QueueSource } from '@/services/audio/types'; +import { LIBRARY_SOURCE, type PlayableTrack, type QueueSource } from '@/services/audio/types'; import { getShuffleAlgorithm } from '@/services/settings'; -import { AddTracksSheet } from './components/AddTracksSheet'; import { NamePlaylistDialog } from './components/NamePlaylistDialog'; import { PlaylistDetailHeader } from './components/PlaylistDetailHeader'; import { PlaylistEntryRow } from './components/PlaylistEntryRow'; @@ -36,23 +37,26 @@ export interface PlaylistDetailScreenProps { export function PlaylistDetailScreen({ playlistId }: PlaylistDetailScreenProps) { const { t, i18n } = useTranslation(); const router = useRouter(); - const messages = useMessages('playlists.detailEmpty'); + const isLiked = playlistId === LIKED_SONGS_ID; + const bottomInset = useMiniPlayerInset(); + const detailMessages = useMessages(isLiked ? 'playlists.likedEmpty' : 'playlists.detailEmpty'); - const entries = usePlaylistEntries(playlistId); + const playlistEntries = usePlaylistEntries(playlistId); + const likedEntries = useFavoriteEntries(); + const entries = isLiked ? likedEntries : playlistEntries; const playlist = usePlaylists().find((entry) => entry.id === playlistId); const [renaming, setRenaming] = useState(false); - const [adding, setAdding] = useState(false); /* - * Every entry point here declares the playlist as the queue's source, which is + * User playlists declare themselves as the queue's source, which is * what puts rows in `stats_rollups` under entity type 'playlist'. Without it * the top-playlists list is permanently empty and looks like a user who never * plays playlists. */ const source = useMemo( - () => ({ type: 'playlist', id: playlistId }), - [playlistId], + () => (isLiked ? LIBRARY_SOURCE : { type: 'playlist', id: playlistId }), + [isLiked, playlistId], ); const playAll = useCallback(() => { @@ -90,14 +94,6 @@ export function PlaylistDetailScreen({ playlistId }: PlaylistDetailScreenProps) [playlistId], ); - const onAddTracks = useCallback( - (trackIds: number[]) => { - setAdding(false); - void addTracksToPlaylist(playlistId, trackIds); - }, - [playlistId], - ); - const onRename = useCallback( (name: string) => { setRenaming(false); @@ -114,69 +110,68 @@ export function PlaylistDetailScreen({ playlistId }: PlaylistDetailScreenProps) }, [router, playlistId]); const renderItem = useCallback>( - ({ item, index }) => ( - - - - ), - [entries.length, move, playAt, remove, i18n.language, t], + ({ item, index }) => + isLiked ? ( + + ) : ( + + + + ), + [entries.length, isLiked, move, playAt, remove, i18n.language, t], ); + const name = isLiked ? t('playlists.likedSongs') : (playlist?.name ?? ''); + const covers = isLiked + ? entries.flatMap((entry) => (entry.artworkPath ? [entry.artworkPath] : [])).slice(0, 4) + : (playlist?.mosaic ?? []); + return ( void shuffleAll()} - onAddTracks={() => setAdding(true)} - onRename={() => setRenaming(true)} - onDelete={onDelete} + onRename={isLiked ? undefined : () => setRenaming(true)} + onDelete={isLiked ? undefined : onDelete} /> {/* Bounded, so the list re-lays out when the rows above it change. */} {entries.length === 0 ? ( - setAdding(true)} - /> + ) : ( )} - setAdding(false)} - /> - - setRenaming(false)} - onSubmit={onRename} - /> + {isLiked ? null : ( + setRenaming(false)} + onSubmit={onRename} + /> + )} ); } diff --git a/src/features/playlists/PlaylistsScreen.tsx b/src/features/playlists/PlaylistsScreen.tsx index 44948fe..ac60433 100644 --- a/src/features/playlists/PlaylistsScreen.tsx +++ b/src/features/playlists/PlaylistsScreen.tsx @@ -6,13 +6,21 @@ import { FlatList, Pressable, Text, View } from 'react-native'; import { EmptyState } from '@/components/ui/EmptyState'; import { Screen } from '@/components/ui/Screen'; -import { createPlaylist, usePlaylists, type PlaylistSummary } from '@/db/queries/playlists'; +import { + createPlaylist, + useFavoriteEntries, + usePlaylists, + type PlaylistSummary, +} from '@/db/queries/playlists'; +import { useMiniPlayerInset } from '@/features/player/playerLayerLayout'; import { useMessages } from '@/i18n'; +import { SPACING } from '@/theme/tokens'; import { useThemeColors } from '@/theme/useTheme'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { NamePlaylistDialog } from './components/NamePlaylistDialog'; import { PlaylistRow } from './components/PlaylistRow'; +import { buildPlaylistRows, shouldShowEmptyState } from './playlistRows'; /** * The user's playlists. @@ -25,10 +33,12 @@ export function PlaylistsScreen() { useLifecycleTrace('PlaylistsScreen'); const { t } = useTranslation(); const messages = useMessages('playlists.empty'); + const bottomInset = useMiniPlayerInset(); const colors = useThemeColors(); const router = useRouter(); const playlists = usePlaylists(); + const likedEntries = useFavoriteEntries(); const [naming, setNaming] = useState(false); const openNaming = useCallback(() => setNaming(true), []); @@ -45,21 +55,21 @@ export function PlaylistsScreen() { [router], ); - const openPlaylist = useCallback( - (id: number) => router.push(`/playlist/${id}`), - [router], - ); + const openPlaylist = useCallback((id: number) => router.push(`/playlist/${id}`), [router]); const renderItem = useCallback( ({ item }: { item: PlaylistSummary }) => , [openPlaylist], ); + const rows = buildPlaylistRows(likedEntries, playlists, t('playlists.likedSongs')); + return ( + {/* `rows.length`, so the number describes the list under it. */} - {t('playlists.count', { count: playlists.length })} + {t('playlists.count', { count: rows.length })} - {playlists.length === 0 ? ( - - ) : ( - - )} + + ) : null + } + /> void; @@ -57,12 +56,7 @@ export function AddToPlaylistSheet({ trackIds, onClose }: AddToPlaylistSheetProp ); return ( - 0} - transparent - animationType="slide" - onRequestClose={onClose} - > + 0} transparent animationType="slide" onRequestClose={onClose}> - - {t('playlists.create')} - + {t('playlists.create')} diff --git a/src/features/playlists/components/AddTracksSheet.tsx b/src/features/playlists/components/AddTracksSheet.tsx deleted file mode 100644 index b83f3ae..0000000 --- a/src/features/playlists/components/AddTracksSheet.tsx +++ /dev/null @@ -1,193 +0,0 @@ -import { FlashList, type ListRenderItem } from '@shopify/flash-list'; -import { Check } from 'lucide-react-native'; -import { memo, useCallback, useState } from 'react'; -import { useTranslation } from 'react-i18next'; -import { Modal, Pressable, Text, View } from 'react-native'; - -import type { TrackListItem } from '@/db/queries/tracks'; -import { commitFeedback, tapFeedback } from '@/services/haptics'; -import { useThemeColors } from '@/theme/useTheme'; - -import { SearchField } from '../../library/components/SearchField'; -import { useDebounced } from '../../library/hooks/useDebounced'; -import { useTracks } from '../../library/hooks/useLibrary'; - -export interface AddTracksSheetProps { - visible: boolean; - /** Called with the picked ids, in the order they were ticked. */ - onAdd: (trackIds: number[]) => void; - onClose: () => void; -} - -/** - * Pick tracks from the library to add to a playlist. - * - * This is the missing direction. Adding a track to a playlist could only be done - * *from* the library, which means filling a playlist meant knowing every track - * you wanted before you started — the playlist itself, the screen where you can - * see what is already in it, had no way to add anything. - * - * Searchable, because picking from a 10,000-track library by scrolling is not - * picking. Multi-select, because nobody adds one track at a time. - * - * Mounted only while open (`visible` guards the whole subtree) rather than - * hidden behind a Modal's own visibility. It runs the full library query, and - * keeping that live on the playlist screen forever would put a second copy of the - * most expensive query in the app behind a sheet nobody has opened. - */ -export function AddTracksSheet({ visible, onAdd, onClose }: AddTracksSheetProps) { - return ( - - {visible ? : null} - - ); -} - -interface PickerProps { - onAdd: (trackIds: number[]) => void; - onClose: () => void; -} - -function Picker({ onAdd, onClose }: PickerProps) { - const { t } = useTranslation(); - - const [search, setSearch] = useState(''); - const { tracks } = useTracks(useDebounced(search)); - const [picked, setPicked] = useState([]); - - const toggle = useCallback((id: number) => { - tapFeedback(); - setPicked((current) => - current.includes(id) ? current.filter((entry) => entry !== id) : [...current, id], - ); - }, []); - - const confirm = useCallback(() => { - if (picked.length === 0) return; - commitFeedback(); - onAdd(picked); - }, [picked, onAdd]); - - const renderItem = useCallback>( - ({ item }) => ( - - ), - [picked, toggle], - ); - - return ( - - - - - {t('playlists.addTracks')} - - - {t('common.cancel')} - - - - - - {/* Bounded, so the virtualized list inside gets a real height. */} - - - - - - - {t('playlists.addSelected', { count: picked.length })} - - - - - ); -} - -interface PickRowProps { - track: TrackListItem; - isPicked: boolean; - onToggle: (id: number) => void; -} - -/** A library track with a tick box. Deliberately plainer than `TrackRow`. */ -const PickRowComponent = function PickRow({ track, isPicked, onToggle }: PickRowProps) { - const { t } = useTranslation(); - const colors = useThemeColors(); - const handlePress = useCallback(() => onToggle(track.id), [onToggle, track.id]); - const subtitle = [ - track.artistName ?? t('common.unknownArtist'), - track.albumName ?? t('common.unknownAlbum'), - ].join(' — '); - - return ( - - - {isPicked ? : null} - - - - - {track.title} - - {subtitle ? ( - - {subtitle} - - ) : null} - - - ); -}; - -function isSamePickRow(previous: PickRowProps, next: PickRowProps): boolean { - return ( - previous.isPicked === next.isPicked && - previous.onToggle === next.onToggle && - previous.track.id === next.track.id && - previous.track.title === next.track.title && - previous.track.artistName === next.track.artistName && - previous.track.albumName === next.track.albumName - ); -} - -const PickRow = memo(PickRowComponent, isSamePickRow); - -function keyExtractor(track: TrackListItem): string { - return String(track.id); -} diff --git a/src/features/playlists/components/PlaylistDetailHeader.tsx b/src/features/playlists/components/PlaylistDetailHeader.tsx index c5ba05a..c01653c 100644 --- a/src/features/playlists/components/PlaylistDetailHeader.tsx +++ b/src/features/playlists/components/PlaylistDetailHeader.tsx @@ -1,5 +1,5 @@ import { useRouter } from 'expo-router'; -import { ChevronLeft, Pencil, Plus, Shuffle, Trash2 } from 'lucide-react-native'; +import { ChevronLeft, Pencil, Shuffle, Trash2 } from 'lucide-react-native'; import { useCallback } from 'react'; import { useTranslation } from 'react-i18next'; import { Pressable, Text, View } from 'react-native'; @@ -14,9 +14,8 @@ export interface PlaylistDetailHeaderProps { covers: readonly string[]; onPlay: () => void; onShuffle: () => void; - onAddTracks: () => void; - onRename: () => void; - onDelete: () => void; + onRename?: () => void; + onDelete?: () => void; } /** @@ -36,7 +35,6 @@ export function PlaylistDetailHeader({ covers, onPlay, onShuffle, - onAddTracks, onRename, onDelete, }: PlaylistDetailHeaderProps) { @@ -60,32 +58,27 @@ export function PlaylistDetailHeader({ - - - - - - - + {onRename ? ( + + + + ) : null} - - - + {onDelete ? ( + + + + ) : null} diff --git a/src/features/playlists/components/PlaylistEntryRow.tsx b/src/features/playlists/components/PlaylistEntryRow.tsx index 313be32..2e5eb5a 100644 --- a/src/features/playlists/components/PlaylistEntryRow.tsx +++ b/src/features/playlists/components/PlaylistEntryRow.tsx @@ -12,7 +12,7 @@ export interface PlaylistEntryRowProps { entry: PlaylistEntry; locale: string; onPress: (position: number) => void; - onRemove: (position: number) => void; + onRemove?: (position: number) => void; } /** @@ -31,7 +31,7 @@ export const PlaylistEntryRow = memo(function PlaylistEntryRow({ const colors = useThemeColors(); const handlePress = useCallback(() => onPress(entry.position), [onPress, entry.position]); - const handleRemove = useCallback(() => onRemove(entry.position), [onRemove, entry.position]); + const handleRemove = useCallback(() => onRemove?.(entry.position), [onRemove, entry.position]); const artworkUri = entry.artworkPath ? `file://${entry.artworkPath}` : null; const subtitle = [ @@ -78,14 +78,16 @@ export const PlaylistEntryRow = memo(function PlaylistEntryRow({ - - - + {onRemove ? ( + + + + ) : null} ); }); diff --git a/src/features/playlists/playlistRows.test.ts b/src/features/playlists/playlistRows.test.ts new file mode 100644 index 0000000..0ae67aa --- /dev/null +++ b/src/features/playlists/playlistRows.test.ts @@ -0,0 +1,78 @@ +import { LIKED_SONGS_ID, type PlaylistSummary } from '@/services/playlists/order'; + +import { buildPlaylistRows, shouldShowEmptyState, type FavoriteRow } from './playlistRows'; + +const LIKED = 'Liked Songs'; + +function playlist(id: number, trackCount = 0): PlaylistSummary { + return { id, name: `list-${id}`, trackCount, artworkPath: null, mosaic: [] }; +} + +function favorites(count: number, withArtwork = 0): FavoriteRow[] { + return Array.from({ length: count }, (_, index) => ({ + artworkPath: index < withArtwork ? `/covers/${index}.jpg` : null, + })); +} + +describe('buildPlaylistRows', () => { + it('counts the liked songs it was given, not zero', () => { + const rows = buildPlaylistRows(favorites(7), [], LIKED); + + expect(rows[0]).toMatchObject({ id: LIKED_SONGS_ID, name: LIKED, trackCount: 7 }); + }); + + it('leaves Liked Songs out while nothing is liked', () => { + expect(buildPlaylistRows([], [playlist(1)], LIKED)).toEqual([playlist(1)]); + }); + + it('puts Liked Songs first, then the user playlists in the order given', () => { + const rows = buildPlaylistRows(favorites(1), [playlist(3), playlist(2)], LIKED); + + expect(rows.map((row) => row.id)).toEqual([LIKED_SONGS_ID, 3, 2]); + }); + + it('takes at most four covers for the mosaic, skipping tracks without one', () => { + const rows = buildPlaylistRows(favorites(9, 6), [], LIKED); + + expect(rows[0]?.mosaic).toHaveLength(4); + }); +}); + +describe('shouldShowEmptyState', () => { + /* + * The regression this pins: the tab rendered "there is nothing here, make a + * playlist" *underneath* a visible row, because the empty state asked + * `playlists.length` while the list rendered something else. Whatever the + * rows are built from, the empty state answers a question about the rows. + */ + it('is false whenever the list has a row — including a liked-songs-only list', () => { + const rows = buildPlaylistRows(favorites(12), [], LIKED); + + expect(rows).not.toHaveLength(0); + expect(rows[0]?.trackCount).toBeGreaterThan(0); + expect(shouldShowEmptyState(rows)).toBe(false); + }); + + it('is false when the user has playlists but nothing liked', () => { + expect(shouldShowEmptyState(buildPlaylistRows([], [playlist(1)], LIKED))).toBe(false); + }); + + it('is true only when there is genuinely nothing', () => { + expect(shouldShowEmptyState(buildPlaylistRows([], [], LIKED))).toBe(true); + }); + + it('never disagrees with the count the header shows', () => { + for (const likedCount of [0, 1, 5]) { + for (const listCount of [0, 1, 4]) { + const rows = buildPlaylistRows( + favorites(likedCount), + Array.from({ length: listCount }, (_, index) => playlist(index + 1)), + LIKED, + ); + + // The header renders `rows.length`, so the two can only ever agree. + expect(shouldShowEmptyState(rows)).toBe(rows.length === 0); + } + } + }); +}); diff --git a/src/features/playlists/playlistRows.ts b/src/features/playlists/playlistRows.ts new file mode 100644 index 0000000..bc752f9 --- /dev/null +++ b/src/features/playlists/playlistRows.ts @@ -0,0 +1,65 @@ +import { LIKED_SONGS_ID, MOSAIC_SIZE, type PlaylistSummary } from '@/services/playlists/order'; + +/** + * What the Playlists tab actually lists. + * + * Pure, and separate from the screen, because three things on that screen have + * to agree and were each reading something different: the header count said + * "0 lists", the Liked Songs row said "0 tracks", and the empty state said + * there was nothing here — above a list that was showing a row. + * + * The header count came from `playlists.length`, which excludes Liked Songs; + * the empty state came from the same number; the list rendered `rows`. Three + * views of one collection, computed three times. + * + * `AGENTS.md` states the rule this breaks: *a count and the list it describes + * come from one query*. So one function builds the rows, and the count and the + * empty state are both facts about what it returned. + */ + +/** One favourited track, as `useFavoriteEntries` returns it. */ +export interface FavoriteRow { + artworkPath: string | null; +} + +/** + * The rows, Liked Songs first. + * + * Liked Songs is included **only when it holds something**. It is a virtual + * playlist with no existence of its own, and an always-present row reading + * "0 tracks" is what made the counts contradict each other: either the header + * counted it and disagreed with `playlists.length`, or it did not and the + * screen showed a row it claimed was not there. A destination with nothing in + * it is not a destination yet, and the moment anything is liked it appears. + */ +export function buildPlaylistRows( + favorites: readonly FavoriteRow[], + playlists: readonly PlaylistSummary[], + likedSongsName: string, +): PlaylistSummary[] { + if (favorites.length === 0) return [...playlists]; + + return [ + { + id: LIKED_SONGS_ID, + name: likedSongsName, + trackCount: favorites.length, + mosaic: favorites + .flatMap((entry) => (entry.artworkPath ? [entry.artworkPath] : [])) + .slice(0, MOSAIC_SIZE), + artworkPath: null, + }, + ...playlists, + ]; +} + +/** + * Whether the screen has nothing to show. + * + * The one condition, taken from the rendered rows rather than from any of the + * collections behind them. An empty state above visible content is a straight + * contradiction, and it shipped. + */ +export function shouldShowEmptyState(rows: readonly PlaylistSummary[]): boolean { + return rows.length === 0; +} diff --git a/src/features/settings/SettingsScreen.tsx b/src/features/settings/SettingsScreen.tsx index a49d823..ad89837 100644 --- a/src/features/settings/SettingsScreen.tsx +++ b/src/features/settings/SettingsScreen.tsx @@ -18,6 +18,7 @@ import { SegmentedControl, type SegmentedControlOption } from '@/components/ui/S import { SettingGroup } from '@/components/ui/SettingGroup'; import { SettingRow } from '@/components/ui/SettingRow'; import { SettingSwitch } from '@/components/ui/SettingSwitch'; +import { useMiniPlayerInset } from '@/features/player/playerLayerLayout'; import { changeLanguage } from '@/i18n'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { @@ -34,6 +35,7 @@ import { type ThemePreference, } from '@/services/settings'; import { SHUFFLE_ALGORITHMS, type ShuffleAlgorithm } from '@/services/shuffle'; +import { SPACING } from '@/theme/tokens'; import { useTheme } from '@/theme/useTheme'; import { DevTools } from './components/DevTools'; @@ -61,6 +63,7 @@ export function SettingsScreen() { useLifecycleTrace('SettingsScreen'); const { t } = useTranslation(); const { preference: theme, setPreference: setTheme } = useTheme(); + const bottomInset = useMiniPlayerInset(); const [language, setLanguage] = useState(getLanguagePreference); const [shuffle, setShuffle] = useState(getShuffleAlgorithm); @@ -107,7 +110,13 @@ export function SettingsScreen() { return ( - + ('week'); // The key for "now" in the selected period. Recomputed per render rather @@ -69,7 +72,13 @@ export function StatsScreen() { {hasData ? ( - + void; type ListenReporter = (listen: FinishedListen) => void; @@ -120,6 +126,14 @@ class Engine { */ private playWhenReady = false; + /** + * Whether this player is already the lock screen's active player. + * + * Claiming it is a session rebuild; keeping it is a metadata update. See + * `bindLockScreen`. Cleared by `stop()`, which hands the session back. + */ + private lockScreenBound = false; + /** * Claim the audio session. * @@ -138,6 +152,10 @@ class Engine { shouldPlayInBackground: true, }); + // Unpacked before the first track binds the lock screen, so a track with + // no cover has the app's placeholder to show rather than a blank square. + await prepareNotificationArtwork(); + /* * Pause when the audio route changes to the speaker. * @@ -330,6 +348,11 @@ class Engine { if (track !== null && listen !== null) { this.reportListen?.({ track, + // The engine's duration, falling back to the scanner's only when the + // file never opened. See `FinishedListen.durationMs`: classifying + // against a MediaStore duration of zero turns every listen into a + // `partial` and loses it from the counts. + durationMs: this.state.durationMs > 0 ? this.state.durationMs : track.durationMs, msPlayed: listen.msPlayed, startedAt: listen.startedAt, completed, @@ -397,18 +420,42 @@ class Engine { * three minutes in the background — it is what promotes the session to a * foreground media service, not merely what draws the controls. The tech * stack doc flags this as the single Android gotcha of this library. + * + * **Called once, then updated in place.** This used to run on every track + * load, and `setActiveForLockScreen` on an already-active player does not + * refresh a session — it releases the MediaSession and builds a new one on + * the main queue. Between the release and the rebuild there is a window with + * no live session, and the notification's play/pause icon is drawn from + * `session.player.isPlaying` at the moment it is posted. A state change + * landing in that window is drawn against a released session and then never + * corrected, which is how the notification came to show "playing" for audio + * that had stopped. It is also why `dumpsys media_session` reports nonsense + * for this app, which `docs/player.md` records as an unexplained quirk: it + * was catching the swap. + * + * `updateLockScreenMetadata` changes the metadata on the live session and + * re-posts the notification, with no session release and no window. */ private bindLockScreen(track: PlayableTrack): void { - this.player?.setActiveForLockScreen( - true, - { - title: track.title, - artist: track.artistName ?? undefined, - albumTitle: track.albumName ?? undefined, - artworkUrl: track.artworkPath ? `file://${track.artworkPath}` : undefined, - }, - { showSeekForward: true, showSeekBackward: true }, - ); + const metadata = { + title: track.title, + artist: track.artistName ?? undefined, + albumTitle: track.albumName ?? undefined, + // The app's own placeholder rather than nothing, so a track without a + // cover looks the same in the notification as it does on screen. + artworkUrl: lockScreenArtworkUri(track.artworkPath), + }; + + if (this.lockScreenBound) { + this.player?.updateLockScreenMetadata(metadata); + return; + } + + this.lockScreenBound = true; + this.player?.setActiveForLockScreen(true, metadata, { + showSeekForward: true, + showSeekBackward: true, + }); } /** Start playback once the source is actually open. Safe to call repeatedly. */ @@ -466,8 +513,7 @@ class Engine { positionMs, // expo-audio reports -1 or 0 before the file is open; the scanner's // figure is the better answer until then. - durationMs: - status.duration > 0 ? Math.round(status.duration * 1000) : this.state.durationMs, + durationMs: status.duration > 0 ? Math.round(status.duration * 1000) : this.state.durationMs, }); }; @@ -510,6 +556,9 @@ class Engine { // Repeat-one on a finished track: same index, so seek rather than reload. if (next === this.index && !explicit) { + // `onStatus` just banked the completed listen. Unlike loadIndex, this + // path keeps the same source, so it must explicitly open the next pass. + this.listenCycle.open(); await this.seekTo(0); this.play(); return; @@ -521,12 +570,12 @@ class Engine { /** * The previous track, or the start of this one. * - * The three-second rule lives here rather than in the queue because it needs + * The ten-second rule lives here rather than in the queue because it needs * the playback position, which the queue does not have. It is what every * other player does and what the button is expected to do. */ async previous(): Promise { - if (this.state.positionMs > RESTART_THRESHOLD_MS) { + if (shouldRestartCurrentTrack(this.state.positionMs)) { await this.seekTo(0); return; } @@ -607,6 +656,7 @@ class Engine { this.flushListen(false); this.player?.pause(); this.player?.clearLockScreenControls(); + this.lockScreenBound = false; this.index = -1; this.emitQueue(); this.emit({ ...IDLE_PLAYBACK }); diff --git a/src/services/audio/listenRecording.test.ts b/src/services/audio/listenRecording.test.ts new file mode 100644 index 0000000..c674b29 --- /dev/null +++ b/src/services/audio/listenRecording.test.ts @@ -0,0 +1,284 @@ +jest.mock('audio-focus', () => ({ + onAudioBecomingNoisy: () => () => undefined, + hasAudioFocusEvents: false, +})); + +jest.mock('expo-audio', () => ({ + // eslint-disable-next-line @typescript-eslint/no-require-imports + createAudioPlayer: () => require('./testing/fakeAudioPlayer').makeFakePlayer(), + setAudioModeAsync: async () => undefined, + setIsAudioActiveAsync: async () => undefined, +})); + +// Below the mocks on purpose: `jest.mock` is hoisted above imports, and the +// engine reads `expo-audio` at module scope, so an import here that ran first +// would load the real native module and throw. +// eslint-disable-next-line import/first +import { startPlayback, track, type PlaybackHarness } from './testing/playbackHarness'; + +/** + * The listen-counting matrix, run against the real engine. + * + * This exists because the same defect was reported three times and "fixed and + * verified against the database" twice. Both of those verifications were real + * device sessions; neither could be re-run, so neither caught the next + * regression. `ListenCycle` and `isRewindToRestart` each had good unit tests + * and each was correct in isolation — every miscount lived in the wiring + * between them inside `AudioEngine.onStatus`, which nothing covered. + * + * The scenarios are the ones a person would actually perform, named as such, + * because the expected answers come from ADR 005 and ADR 011 rather than from + * whatever the code happens to do. + * + * A 30-second track puts both thresholds in easy reach: + * play at `min(30s, 15s)` = **15,000 ms**, skip below `0.2 × 30s` = **6,000 ms**. + */ + +const DURATION_MS = 30_000; +const PLAY_THRESHOLD_MS = 15_000; +/** One status tick. Accumulated time is measured between ticks, so it rounds. */ +const TICK_MS = 500; + +beforeEach(() => { + jest.useFakeTimers(); + jest.setSystemTime(new Date('2026-08-02T12:00:00Z')); +}); + +afterEach(() => { + jest.useRealTimers(); +}); + +function start(): Promise { + return startPlayback([track(1, DURATION_MS), track(2, DURATION_MS)]); +} + +describe('a — one track, start to finish', () => { + it('records exactly one play', async () => { + const harness = await start(); + + await harness.playFor(DURATION_MS - TICK_MS); + await harness.finishTrack(); + + expect(harness.listens).toHaveLength(1); + expect(harness.listens[0]).toMatchObject({ trackId: 1, outcome: 'play', completed: true }); + expect(harness.listens[0]?.msPlayed).toBeGreaterThanOrEqual(DURATION_MS - 2 * TICK_MS); + }); +}); + +describe('b — repeat-one, three times round', () => { + it('records three plays, not one', async () => { + const harness = await start(); + harness.setRepeat('one'); + + for (let pass = 0; pass < 3; pass += 1) { + await harness.playFor(DURATION_MS - TICK_MS); + await harness.finishTrack(); + } + + expect(harness.listens).toHaveLength(3); + for (const listen of harness.listens) { + expect(listen).toMatchObject({ trackId: 1, outcome: 'play', completed: true }); + } + }); + + it('gives each pass its own start time, so a loop across midnight splits', async () => { + const harness = await start(); + harness.setRepeat('one'); + + await harness.playFor(DURATION_MS - TICK_MS); + await harness.finishTrack(); + await harness.playFor(DURATION_MS - TICK_MS); + await harness.finishTrack(); + + const [first, second] = harness.listens; + expect(second?.startedAt).toBeGreaterThan(first?.startedAt ?? 0); + }); +}); + +describe('c — heard past the play mark, then dragged back to the start', () => { + /* + * ADR 011: a rewind to at or below 25% of the track, by a listen that has + * already earned a play, ends that listen and begins another. Two events, not + * one — which is what someone who played a song twice means. + */ + it('records two listens', async () => { + const harness = await start(); + + await harness.playFor(18_000); + await harness.seekTo(0); + await harness.playFor(18_000); + await harness.stop(); + + expect(harness.listens).toHaveLength(2); + expect(harness.listens.map((listen) => listen.outcome)).toEqual(['play', 'play']); + }); + + it('does not split when the rewind is short of the mark', async () => { + const harness = await start(); + + // Back to 26% — past three quarters of the way in, this is an adjustment. + await harness.playFor(18_000); + await harness.seekTo(8_000); + await harness.playFor(6_000); + await harness.stop(); + + expect(harness.listens).toHaveLength(1); + }); + + it('does not split a rewind that has not yet earned a play', async () => { + const harness = await start(); + + await harness.playFor(6_000); + await harness.seekTo(0); + await harness.playFor(6_000); + await harness.stop(); + + expect(harness.listens).toHaveLength(1); + expect(harness.listens[0]?.msPlayed).toBeLessThan(PLAY_THRESHOLD_MS); + }); +}); + +describe('d — abandoned early, skipped forward, finished there', () => { + /* + * Ten per cent heard, a jump to 60%, then played out. Only the time actually + * heard accumulates — the skipped middle is not listening — so this lands on + * the play threshold from below rather than being credited the whole track. + */ + it('records one listen, counting only the audio that was heard', async () => { + const harness = await start(); + + await harness.playFor(3_000); + await harness.seekTo(18_000); + await harness.playFor(11_000); + await harness.finishTrack(); + + expect(harness.listens).toHaveLength(1); + expect(harness.listens[0]?.msPlayed).toBeLessThan(DURATION_MS - 10_000); + expect(harness.listens[0]?.completed).toBe(true); + }); + + it('is a skip when almost none of it was heard', async () => { + const harness = await start(); + + await harness.playFor(2_000); + await harness.seekTo(29_000); + await harness.finishTrack(); + + expect(harness.listens).toHaveLength(1); + expect(harness.listens[0]?.outcome).toBe('skip'); + }); +}); + +describe('e — scrubbing back and forth', () => { + it('produces no extra events', async () => { + const harness = await start(); + + await harness.playFor(18_000); + for (const positionMs of [15_000, 20_000, 16_000, 22_000, 17_000, 21_000]) { + await harness.seekTo(positionMs); + await harness.playFor(1_000); + } + await harness.stop(); + + expect(harness.listens).toHaveLength(1); + }); + + it('produces no extra events while scrubbing inside the first seconds', async () => { + const harness = await start(); + + await harness.playFor(2_000); + for (const positionMs of [0, 4_000, 1_000, 5_000, 500]) { + await harness.seekTo(positionMs); + await harness.playFor(1_000); + } + await harness.stop(); + + expect(harness.listens).toHaveLength(1); + }); +}); + +describe('a track whose stored duration is wrong', () => { + /* + * MediaStore returns a null duration for a file it indexed before reading + * its metadata — a track copied onto the device and played straight away. + * The scanner stores the zero, and `classifyListen` given a duration of zero + * returns `partial` however much was heard. Listening to such a track from + * end to end moved neither counter and vanished from the counts. + * + * The engine knows better the moment the file is open, which is always well + * before a listen ends. + */ + it('counts a full listen as a play when the scanner stored no duration', async () => { + const harness = await startPlayback([track(1, 0), track(2, 0)], { + reportedDurationMs: DURATION_MS, + }); + + await harness.playFor(DURATION_MS - TICK_MS); + await harness.finishTrack(); + + expect(harness.listens).toHaveLength(1); + expect(harness.listens[0]?.outcome).toBe('play'); + }); + + it('still splits a repeat when the scanner stored no duration', async () => { + const harness = await startPlayback([track(1, 0), track(2, 0)], { + reportedDurationMs: DURATION_MS, + }); + harness.setRepeat('one'); + + await harness.playFor(DURATION_MS - TICK_MS); + await harness.finishTrack(); + await harness.playFor(DURATION_MS - TICK_MS); + await harness.finishTrack(); + + expect(harness.listens).toHaveLength(2); + expect(harness.listens.map((listen) => listen.outcome)).toEqual(['play', 'play']); + }); + + it('prefers the engine when the scanner stored a duration that is too short', async () => { + // A five-second claim against a thirty-second file. The play threshold + // would be 2.5s, so almost anything would count — including a real skip. + const harness = await startPlayback([track(1, 5_000), track(2, 5_000)], { + reportedDurationMs: DURATION_MS, + }); + + await harness.playFor(3_000); + await harness.next(); + + expect(harness.listens[0]?.outcome).toBe('skip'); + }); +}); + +describe('the queue moving on', () => { + it('closes the outgoing listen exactly once when a track ends', async () => { + const harness = await start(); + + await harness.playFor(DURATION_MS - TICK_MS); + await harness.finishTrack(); + await harness.playFor(5_000); + await harness.stop(); + + expect(harness.listens.map((listen) => listen.trackId)).toEqual([1, 2]); + }); + + it('closes the outgoing listen exactly once when the user skips', async () => { + const harness = await start(); + + await harness.playFor(18_000); + await harness.next(); + await harness.playFor(5_000); + await harness.stop(); + + expect(harness.listens.map((listen) => listen.trackId)).toEqual([1, 2]); + expect(harness.listens[0]?.completed).toBe(false); + }); + + it('records nothing for a track that never played', async () => { + const harness = await start(); + + await harness.next(); + await harness.stop(); + + expect(harness.listens).toHaveLength(0); + }); +}); diff --git a/src/services/audio/notificationArtwork.ts b/src/services/audio/notificationArtwork.ts new file mode 100644 index 0000000..21e51e6 --- /dev/null +++ b/src/services/audio/notificationArtwork.ts @@ -0,0 +1,53 @@ +import { Asset } from 'expo-asset'; + +import placeholderModule from '@/assets/images/notification-artwork.png'; + +/** + * The cover Android shows when a track has none. + * + * Without it the system media notification drew no artwork at all while the + * app drew its music-note placeholder — the same track looking like two + * different things depending on which surface you were looking at. This is + * that placeholder as a bitmap: lucide's `music` mark, at the same stroke + * weight, on `--color-panel`, so the notification and the app agree. + * + * It has to reach expo-audio as a `file://` URL. The service loads artwork + * with `java.net.URL(...).openConnection()`, which knows nothing about + * `asset://` or a Metro URL, so the asset is unpacked to the cache directory + * once and the resulting path is reused. + * + * Resolving it is asynchronous and binding the lock screen is not, so the URI + * is prepared up front and read synchronously afterwards. A track that loads + * before it lands simply has no placeholder for that moment; the next metadata + * push carries it. + */ + +let placeholderUri: string | null = null; +let preparing: Promise | null = null; + +/** Unpack the placeholder to a real file. Safe to call repeatedly. */ +export function prepareNotificationArtwork(): Promise { + if (placeholderUri !== null) return Promise.resolve(placeholderUri); + + preparing ??= Asset.fromModule(placeholderModule) + .downloadAsync() + .then((asset) => { + // `localUri` is a `file://` path once downloaded. Anything else — a + // Metro URL that never resolved, say — is not something the notification + // can load, so it is better to show nothing than to show a broken icon. + placeholderUri = asset.localUri?.startsWith('file://') ? asset.localUri : null; + return placeholderUri; + }) + .catch(() => null); + + return preparing; +} + +/** + * What to hand the lock screen for this track: its own cover, or the + * placeholder, or nothing if the placeholder is not ready yet. + */ +export function lockScreenArtworkUri(artworkPath: string | null): string | undefined { + if (artworkPath !== null) return `file://${artworkPath}`; + return placeholderUri ?? undefined; +} diff --git a/src/services/audio/queue.test.ts b/src/services/audio/queue.test.ts index cc4ce95..868cbe4 100644 --- a/src/services/audio/queue.test.ts +++ b/src/services/audio/queue.test.ts @@ -4,6 +4,7 @@ import { nextIndex, playNextIndex, previousIndex, + shouldRestartCurrentTrack, shiftForInsert, } from './queue'; import type { RepeatMode } from './types'; @@ -77,6 +78,17 @@ describe('previousIndex', () => { }); }); +describe('shouldRestartCurrentTrack', () => { + it('moves to the previous track before ten seconds', () => { + expect(shouldRestartCurrentTrack(9_900)).toBe(false); + }); + + it('restarts the current track at and after ten seconds', () => { + expect(shouldRestartCurrentTrack(10_000)).toBe(true); + expect(shouldRestartCurrentTrack(10_100)).toBe(true); + }); +}); + describe('isPlayable', () => { it('accepts indexes inside the queue', () => { expect(isPlayable(0, 3)).toBe(true); diff --git a/src/services/audio/queue.ts b/src/services/audio/queue.ts index b98e6b5..a4c354f 100644 --- a/src/services/audio/queue.ts +++ b/src/services/audio/queue.ts @@ -10,6 +10,14 @@ import type { RepeatMode } from './types'; +/** Previous restarts the current track at and after this position. */ +export const RESTART_THRESHOLD_MS = 10_000; + +/** Whether Previous must seek to zero instead of moving through the queue. */ +export function shouldRestartCurrentTrack(positionMs: number): boolean { + return positionMs >= RESTART_THRESHOLD_MS; +} + export interface QueuePosition { /** Index into the queue, or -1 when nothing is queued. */ index: number; @@ -25,7 +33,10 @@ export interface QueuePosition { * moves on — repeating the same track when someone asks for the next one * reads as a broken button, not as a respected setting. */ -export function nextIndex({ index, length, repeat }: QueuePosition, explicit: boolean): number | null { +export function nextIndex( + { index, length, repeat }: QueuePosition, + explicit: boolean, +): number | null { if (length === 0 || index < 0) return null; if (repeat === 'one' && !explicit) return index; @@ -39,8 +50,8 @@ export function nextIndex({ index, length, repeat }: QueuePosition, explicit: bo /** * The previous index, or null when there is nowhere to go. * - * Deliberately does not implement "restart the current track if more than - * three seconds in" — that rule belongs to the button, which knows the + * Deliberately does not implement "restart the current track at ten seconds" — + * that rule belongs to the button, which knows the * playback position, not to the queue, which does not. */ export function previousIndex({ index, length, repeat }: QueuePosition): number | null { diff --git a/src/services/audio/testing/fakeAudioPlayer.ts b/src/services/audio/testing/fakeAudioPlayer.ts new file mode 100644 index 0000000..c076650 --- /dev/null +++ b/src/services/audio/testing/fakeAudioPlayer.ts @@ -0,0 +1,127 @@ +import type { AudioStatus } from 'expo-audio'; + +/** + * A stand-in for one `expo-audio` player, driven by a virtual clock. + * + * The engine's listen counting is a state machine fed by a stream of status + * ticks, and until now nothing exercised it end to end — `ListenCycle` and + * `isRewindToRestart` were unit tested in isolation while the wiring between + * them, which is where every reported miscount actually lived, was only ever + * checked by hand on a phone. This replays a tick stream through the real + * engine so those reports become assertions. + * + * Deliberately not a general expo-audio mock: it implements the handful of + * members `AudioEngine` touches and nothing else, so a method the engine starts + * calling shows up as a missing function rather than as a silent no-op. + */ +export class FakeAudioPlayer { + isLoaded = true; + playing = false; + /** Seconds, matching expo-audio's unit. */ + currentTime = 0; + duration = 0; + + private listener: ((status: AudioStatus) => void) | null = null; + + /** Every call the engine made, in order, for asserting on side effects. */ + readonly calls: string[] = []; + + addListener(event: string, listener: (status: AudioStatus) => void): { remove(): void } { + if (event === 'playbackStatusUpdate') this.listener = listener; + return { remove: () => undefined }; + } + + play(): void { + this.calls.push('play'); + if (this.isLoaded) this.playing = true; + } + + pause(): void { + this.calls.push('pause'); + this.playing = false; + } + + /** + * Swapping the source is asynchronous on a device, so the fake mirrors it: + * `isLoaded` goes false until `finishLoading()`. + */ + replace(_source: unknown): void { + this.calls.push('replace'); + this.isLoaded = false; + this.playing = false; + this.currentTime = 0; + } + + async seekTo(seconds: number): Promise { + this.calls.push(`seekTo:${seconds}`); + this.currentTime = seconds; + } + + setActiveForLockScreen(active: boolean): void { + this.calls.push(`setActiveForLockScreen:${String(active)}`); + } + + updateLockScreenMetadata(): void { + this.calls.push('updateLockScreenMetadata'); + } + + setPlaybackStateForLockScreen(playing: boolean): void { + this.calls.push(`lockScreenPlaying:${String(playing)}`); + } + + clearLockScreenControls(): void { + this.calls.push('clearLockScreenControls'); + } + + remove(): void { + this.calls.push('remove'); + } + + /** The source finished opening. Mirrors expo-audio reporting `isLoaded`. */ + finishLoading(durationSec: number): void { + this.isLoaded = true; + this.duration = durationSec; + } + + emit(overrides: Partial = {}): void { + this.listener?.({ + id: 'fake', + currentTime: this.currentTime, + playbackState: this.playing ? 'readyToPlay' : 'paused', + timeControlStatus: this.playing ? 'playing' : 'paused', + reasonForWaitingToPlay: '', + mute: false, + duration: this.duration, + playing: this.playing, + loop: false, + didJustFinish: false, + isBuffering: false, + isLoaded: this.isLoaded, + playbackRate: 1, + shouldCorrectPitch: true, + isLive: false, + liveOffset: null, + ...overrides, + } as AudioStatus); + } +} + +/** + * The one player the engine builds, shared with the tests that drive it. + * + * A module-level handle rather than a return value because `createAudioPlayer` + * is called from inside the engine, where a test cannot see it. + */ +let live: FakeAudioPlayer | null = null; + +/** Called by the `expo-audio` mock in place of `createAudioPlayer`. */ +export function makeFakePlayer(): FakeAudioPlayer { + live = new FakeAudioPlayer(); + return live; +} + +/** The player the engine is currently driving. */ +export function currentFakePlayer(): FakeAudioPlayer { + if (live === null) throw new Error('The engine has not created a player yet.'); + return live; +} diff --git a/src/services/audio/testing/playbackHarness.ts b/src/services/audio/testing/playbackHarness.ts new file mode 100644 index 0000000..968f6c6 --- /dev/null +++ b/src/services/audio/testing/playbackHarness.ts @@ -0,0 +1,210 @@ +import { classifyListen, type ListenOutcome } from '@/services/stats/playCounting'; + +import { AudioEngine } from '../AudioEngine'; +import type { FinishedListen, PlayableTrack, QueueSource, RepeatMode } from '../types'; +import { currentFakePlayer, type FakeAudioPlayer } from './fakeAudioPlayer'; + +/** + * Drive the real `AudioEngine` from a scripted status stream. + * + * Time is the fake timer clock, which `ListenCycle` reads through `Date.now()`, + * so a listen's `msPlayed` here is the same arithmetic that runs on a phone. + * + * The engine is a module-level singleton on purpose — playback outlives every + * screen — so each harness resets it through its own public API rather than by + * reloading the module. That also keeps one fake player alive across a suite, + * which is what a device does: `createAudioPlayer` happens once and every track + * after it arrives through `replace()`. + */ + +/** What `recordListen` would write for one reported listen. */ +export interface RecordedListen { + trackId: number; + msPlayed: number; + outcome: ListenOutcome; + completed: boolean; + startedAt: number; +} + +/** expo-audio's own default, and what the engine asks for. */ +const TICK_MS = 500; + +export interface PlaybackHarness { + /** Every listen the engine reported, in order. One per `play_events` row. */ + readonly listens: RecordedListen[]; + /** Play for this long, in ticks, advancing the clock and the position. */ + playFor(ms: number): Promise; + /** Paused wall-clock time. Ticks still arrive; the position does not move. */ + pauseFor(ms: number): Promise; + /** Run to the end of the current track and report `didJustFinish`. */ + finishTrack(): Promise; + /** Seek the way the scrubber does — through the engine, not the player. */ + seekTo(ms: number): Promise; + /** Press play/pause. */ + toggle(): Promise; + /** Press next. */ + next(): Promise; + /** Press previous. */ + previous(): Promise; + setRepeat(mode: RepeatMode): void; + stop(): Promise; + /** Position the engine currently believes it is at, in ms. */ + positionMs(): number; + /** The fake player the engine is driving. */ + player(): FakeAudioPlayer; +} + +export interface StartOptions { + startIndex?: number; + source?: QueueSource; + repeat?: RepeatMode; + /** + * What the player reports once the file is open, when that differs from what + * the scanner stored. MediaStore returns null durations often enough that + * this is an ordinary case, not a contrived one. + */ + reportedDurationMs?: number; +} + +/** Build a track with sane defaults; only `durationMs` usually matters. */ +export function track(id: number, durationMs: number, title = `track-${id}`): PlayableTrack { + return { + id, + uri: `content://media/external/audio/media/${id}`, + title, + artistName: null, + albumName: null, + durationMs, + artworkPath: null, + playCount: 0, + isFavorite: false, + }; +} + +/** Start playback and return the driver. Resets the engine first. */ +export async function startPlayback( + tracks: PlayableTrack[], + options: StartOptions = {}, +): Promise { + // Drop whatever a previous test left loaded before anything is listening, so + // its final flush is not attributed to this one. + AudioEngine.setListenReporter(null); + AudioEngine.setRepeat('off'); + await AudioEngine.clearQueue(); + await flush(); + + const listens: RecordedListen[] = []; + AudioEngine.setListenReporter((listen: FinishedListen) => { + listens.push({ + trackId: listen.track.id, + msPlayed: listen.msPlayed, + // The same call `recordListen` makes, on the same duration it is given, + // so an outcome here is the outcome that would be written to the row. + outcome: classifyListen(listen.msPlayed, listen.durationMs), + completed: listen.completed, + startedAt: listen.startedAt.getTime(), + }); + }); + + if (options.repeat) AudioEngine.setRepeat(options.repeat); + + await AudioEngine.setQueue(tracks, options.startIndex ?? 0, options.source); + await flush(); + await settleLoad(options.reportedDurationMs); + + return { + listens, + player: currentFakePlayer, + + positionMs() { + return AudioEngine.getState().positionMs; + }, + + async playFor(ms: number) { + for (let elapsed = 0; elapsed < ms; elapsed += TICK_MS) { + jest.advanceTimersByTime(TICK_MS); + const live = currentFakePlayer(); + if (live.playing) live.currentTime += TICK_MS / 1000; + live.emit(); + await flush(); + } + }, + + async pauseFor(ms: number) { + for (let elapsed = 0; elapsed < ms; elapsed += TICK_MS) { + jest.advanceTimersByTime(TICK_MS); + currentFakePlayer().emit(); + await flush(); + } + }, + + async finishTrack() { + const live = currentFakePlayer(); + jest.advanceTimersByTime(TICK_MS); + live.currentTime = live.duration; + live.playing = false; + live.emit({ didJustFinish: true }); + await flush(); + await settleLoad(options.reportedDurationMs); + }, + + async seekTo(ms: number) { + await AudioEngine.seekTo(ms); + await flush(); + }, + + async toggle() { + AudioEngine.toggle(); + await flush(); + }, + + async next() { + await AudioEngine.advance(true); + await flush(); + await settleLoad(options.reportedDurationMs); + }, + + async previous() { + await AudioEngine.previous(); + await flush(); + await settleLoad(options.reportedDurationMs); + }, + + setRepeat(mode: RepeatMode) { + AudioEngine.setRepeat(mode); + }, + + async stop() { + await AudioEngine.stop(); + await flush(); + }, + }; +} + +/** + * Let a `replace()` finish opening and report it, the way a device would. + * + * Modelling the swap as instant would hide the `playWhenReady` race the engine + * exists to handle — and that race is the reason playback used to stop dead on + * the second track of every queue. + */ +async function settleLoad(reportedDurationMs?: number): Promise { + const live = currentFakePlayer(); + if (live.isLoaded) return; + const current = AudioEngine.getState().track; + if (current === null) return; + live.finishLoading((reportedDurationMs ?? current.durationMs) / 1000); + live.emit(); + await flush(); +} + +/** + * Let the engine's un-awaited promise chains run out. + * + * `onStatus` fires `void this.advance(false)` on a finished track, so the + * interesting work happens in microtasks the caller never sees. Twenty turns is + * far more than the deepest chain and costs nothing. + */ +export async function flush(): Promise { + for (let turn = 0; turn < 20; turn += 1) await Promise.resolve(); +} diff --git a/src/services/audio/types.ts b/src/services/audio/types.ts index 59e07f1..969407f 100644 --- a/src/services/audio/types.ts +++ b/src/services/audio/types.ts @@ -83,6 +83,22 @@ export const LIBRARY_SOURCE: QueueSource = { type: 'library' }; */ export interface FinishedListen { track: PlayableTrack; + /** + * The duration the play/skip rule must be applied against. + * + * Not `track.durationMs`. That is the scanner's figure, out of MediaStore, + * and MediaStore returns null for it more often than anyone expects — a file + * copied onto the device and indexed before its metadata was read has no + * duration at all. `classifyListen` given a duration of zero returns + * `partial` whatever was actually heard, so a full listen to such a track + * moved neither counter and simply vanished from the counts. + * + * The engine's own figure is authoritative once the file is open, which it + * always is by the time a listen ends. `PlaybackState.durationMs` already + * preferred it; the recorded listen did not, and that gap is the whole + * defect. + */ + durationMs: number; msPlayed: number; /** When this track started. Period keys derive from it, not from "now". */ startedAt: Date; diff --git a/src/services/haptics/index.ts b/src/services/haptics/index.ts index f26ecbb..ff3cd97 100644 --- a/src/services/haptics/index.ts +++ b/src/services/haptics/index.ts @@ -23,7 +23,7 @@ function fire(run: () => Promise): void { }); } -/** A discrete confirmation: play, pause, favourite, a row selected. */ +/** A discrete confirmation: play, pause or favourite. */ export function tapFeedback(): void { fire(() => Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Light)); } @@ -33,12 +33,12 @@ export function liftFeedback(): void { fire(() => Haptics.impactAsync(Haptics.ImpactFeedbackStyle.Medium)); } -/** A committed change: reorder dropped, track queued, selection applied. */ +/** A committed change: reorder dropped or track queued. */ export function commitFeedback(): void { fire(() => Haptics.notificationAsync(Haptics.NotificationFeedbackType.Success)); } -/** A refused action: swiping past the end, an empty selection. */ +/** A refused action: swiping past the end or an empty action. */ export function rejectFeedback(): void { fire(() => Haptics.notificationAsync(Haptics.NotificationFeedbackType.Warning)); } diff --git a/src/services/playlists/order.ts b/src/services/playlists/order.ts index f1167f7..5b41754 100644 --- a/src/services/playlists/order.ts +++ b/src/services/playlists/order.ts @@ -15,6 +15,16 @@ /** How many covers the mosaic grid can show. */ export const MOSAIC_SIZE = 4; +/** + * Virtual route id for liked songs; it never exists in `playlists`. + * + * Here rather than beside the queries for the reason above: importing + * `src/db/queries/playlists.ts` opens SQLite, so a constant kept there drags a + * database into every test that needs to name this playlist. Re-exported from + * the query module, so screens still import it from where they always did. + */ +export const LIKED_SONGS_ID = -1; + export interface PlaylistSummary { id: number; name: string; diff --git a/src/services/stats/listenCycle.test.ts b/src/services/stats/listenCycle.test.ts index 8e44fdc..5eaf94e 100644 --- a/src/services/stats/listenCycle.test.ts +++ b/src/services/stats/listenCycle.test.ts @@ -75,6 +75,24 @@ describe('ListenCycle', () => { expect(banked).toEqual([trackMs, trackMs, trackMs, trackMs, trackMs]); }); + it('can start the next repeat-one pass after the finished pass closes', () => { + const cycle = new ListenCycle(); + cycle.open(new Date(T0)); + cycle.tick(true, T0); + cycle.tick(true, T0 + 6_000); + + expect(cycle.close(T0 + 6_000)?.msPlayed).toBe(6_000); + + // AudioEngine takes this route when expo-audio reports didJustFinish: + // it banks the completed pass, seeks the same source to zero, then opens + // a fresh cycle before the next status tick. + cycle.open(new Date(T0 + 6_000)); + cycle.tick(true, T0 + 6_000); + cycle.tick(true, T0 + 12_000); + + expect(cycle.close(T0 + 12_000)?.msPlayed).toBe(6_000); + }); + it('dates each pass from when that pass began, not when the first did', () => { const cycle = new ListenCycle(); cycle.open(new Date(T0)); diff --git a/tsconfig.json b/tsconfig.json index 5f4f074..19c1292 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -6,9 +6,14 @@ "noImplicitOverride": true, "noFallthroughCasesInSwitch": true, "types": ["jest", "node"], + // Most specific first. TypeScript picks the best pattern whatever the + // order, but jest-expo turns these into a Jest moduleNameMapper in this + // order and Jest takes the first pattern that matches — with "@/*" listed + // first, "@/assets/x" resolved to "src/assets/x" and no asset import + // resolved under test. "paths": { - "@/*": ["./src/*"], "@/assets/*": ["./assets/*"], + "@/*": ["./src/*"], "audio-tags": ["./modules/audio-tags/index.ts"], "audio-focus": ["./modules/audio-focus/index.ts"] }