From b2f98eb2f9c4293d068fbe29863579cb1c6ff217 Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sat, 1 Aug 2026 22:00:36 +0300 Subject: [PATCH 01/13] - --- app/(tabs)/_layout.tsx | 14 +- app/_layout.tsx | 15 +- app/player.tsx | 5 - docs/01-TECH-STACK.md | 4 +- docs/adr/006-manual-add-is-first-class.md | 7 +- .../007-saf-folders-go-through-mediastore.md | 14 +- .../008-permission-is-asked-not-assumed.md | 2 +- docs/adr/009-expo-audio-and-our-own-queue.md | 4 +- docs/components.md | 10 +- docs/database.md | 11 +- docs/player.md | 4 +- docs/scanner.md | 13 +- docs/stats.md | 5 +- src/db/migrations.test.ts | 13 + .../migrations/0003_numerous_cannonball.sql | 1 + src/db/migrations/meta/0003_snapshot.json | 795 ++++++++++++++++++ src/db/migrations/meta/_journal.json | 7 + src/db/migrations/migrations.js | 2 + src/db/queries/playlists.ts | 45 +- src/db/queries/scanning.ts | 16 +- src/db/queries/tracks.ts | 12 +- src/db/schema.ts | 2 + src/features/library/LibraryScreen.tsx | 39 +- src/features/library/LibraryTracks.tsx | 67 +- .../library/components/FolderImportModal.tsx | 58 ++ .../library/components/LibraryHeader.tsx | 27 +- .../library/components/LibraryRow.tsx | 22 +- .../library/components/SelectionBar.tsx | 91 -- .../library/components/TrackActionSheet.tsx | 23 +- src/features/library/components/TrackList.tsx | 33 +- src/features/library/components/TrackRow.tsx | 34 +- src/features/library/hooks/useScan.ts | 58 +- src/features/library/hooks/useSelection.ts | 97 --- src/features/library/hooks/useTrackActions.ts | 7 +- src/features/player/PlayerLayer.tsx | 62 ++ src/features/player/PlayerScreen.tsx | 26 +- .../player/components/ArtworkCarousel.tsx | 21 +- src/features/player/components/MiniPlayer.tsx | 50 +- .../player/components/NowPlayingOverlay.tsx | 37 + src/features/player/playerExpansion.ts | 10 + src/features/player/playerLayerLayout.ts | 25 + .../playlists/PlaylistDetailScreen.tsx | 106 ++- src/features/playlists/PlaylistsScreen.tsx | 58 +- .../components/AddToPlaylistSheet.tsx | 16 +- .../playlists/components/AddTracksSheet.tsx | 193 ----- .../components/PlaylistDetailHeader.tsx | 53 +- .../playlists/components/PlaylistEntryRow.tsx | 22 +- src/i18n/locales/en.json | 33 +- src/i18n/locales/tr.json | 33 +- src/services/audio/AudioEngine.ts | 22 +- src/services/audio/queue.test.ts | 12 + src/services/audio/queue.ts | 17 +- src/services/haptics/index.ts | 6 +- src/services/stats/listenCycle.test.ts | 18 + 54 files changed, 1483 insertions(+), 894 deletions(-) delete mode 100644 app/player.tsx create mode 100644 src/db/migrations/0003_numerous_cannonball.sql create mode 100644 src/db/migrations/meta/0003_snapshot.json create mode 100644 src/features/library/components/FolderImportModal.tsx delete mode 100644 src/features/library/components/SelectionBar.tsx delete mode 100644 src/features/library/hooks/useSelection.ts create mode 100644 src/features/player/PlayerLayer.tsx create mode 100644 src/features/player/components/NowPlayingOverlay.tsx create mode 100644 src/features/player/playerExpansion.ts create mode 100644 src/features/player/playerLayerLayout.ts delete mode 100644 src/features/playlists/components/AddTracksSheet.tsx 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..a0df7f4 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,12 @@ 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. */} - - - - + + + + + + ); } 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/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/components.md b/docs/components.md index 12aa94f..b3e0a56 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,7 +71,7 @@ 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. | +| `PlayerLayer` / `PlayerScreen` | Root-mounted Now Playing overlay and its content. The mini player and full screen share one Reanimated expansion value; no route transition sits between them. | | `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. | | `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. | @@ -91,7 +90,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/player.md b/docs/player.md index 7630d83..399d245 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 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..a6bea93 100644 --- a/docs/stats.md +++ b/docs/stats.md @@ -3,8 +3,9 @@ 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 device check is recorded only after +> it has been run against the on-device database. --- 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..d3f228e 100644 --- a/src/db/queries/playlists.ts +++ b/src/db/queries/playlists.ts @@ -1,4 +1,4 @@ -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'; @@ -31,6 +31,21 @@ export interface PlaylistEntry { isFavorite: boolean; } +/** Virtual route id for liked songs; it never exists in `playlists`. */ +export const LIKED_SONGS_ID = -1; + +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 +82,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 +103,24 @@ export function usePlaylistEntries(playlistId: number): PlaylistEntry[] { return data; } +/** Live favourite tracks, newest favourite first, presented as a virtual playlist. */ +export function useFavoriteEntries(): PlaylistEntry[] { + const query = db + .select({ + position: sql`row_number() over (order by ${trackStats.favoriteAt} desc, ${tracks.id} desc) - 1`, + ...entrySelection, + }) + .from(tracks) + .innerJoin(trackStats, eq(trackStats.trackId, tracks.id)) + .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} - 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..787252b 100644 --- a/src/features/library/components/TrackList.tsx +++ b/src/features/library/components/TrackList.tsx @@ -35,19 +35,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 +52,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, @@ -82,7 +70,7 @@ export function TrackList({ const { t } = useTranslation(); const colors = useThemeColors(); - const swipeLabel = t('selection.addToQueue'); + const swipeLabel = t('track.addToQueue'); const renderItem = useCallback>( ({ item }) => { @@ -93,23 +81,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 ( 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..3335428 --- /dev/null +++ b/src/features/player/PlayerLayer.tsx @@ -0,0 +1,62 @@ +import { useSegments } from 'expo-router'; +import type { ReactNode } from 'react'; +import { useCallback, useState } from 'react'; +import { View } from 'react-native'; +import { runOnJS, withSpring } from 'react-native-reanimated'; +import { SafeAreaView } from 'react-native-safe-area-context'; + +import { MiniPlayer } from './components/MiniPlayer'; +import { NowPlayingOverlay } from './components/NowPlayingOverlay'; +import { playerExpansion } from './playerExpansion'; +import { usePlayerTabBarHeight } from './playerLayerLayout'; + +const SPRING = { damping: 24, stiffness: 260 } 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 isTabRoute = segments[0] === '(tabs)'; + + const prepareOpen = useCallback(() => setVisible(true), []); + + const onExpandedChange = useCallback((nextExpanded: boolean) => { + if (nextExpanded) { + setVisible(true); + setExpanded(true); + playerExpansion.value = withSpring(1, SPRING); + return; + } + + setExpanded(false); + playerExpansion.value = withSpring(0, SPRING, (finished) => { + if (finished) runOnJS(setVisible)(false); + }); + }, []); + + return ( + + {children} + + + + + + + + ); +} diff --git a/src/features/player/PlayerScreen.tsx b/src/features/player/PlayerScreen.tsx index 439d48f..d69ede1 100644 --- a/src/features/player/PlayerScreen.tsx +++ b/src/features/player/PlayerScreen.tsx @@ -14,7 +14,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,10 +33,14 @@ 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) => void; +} + +export function PlayerScreen({ onExpandedChange }: PlayerScreenProps) { const { t, i18n } = useTranslation(); const colors = useThemeColors(); const router = useRouter(); @@ -48,9 +51,8 @@ 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 close = useCallback(() => onExpandedChange(false), [onExpandedChange]); + // A double press must not leave two identical queue screens on the stack. const openQueue = useCallback(() => router.navigate('/queue'), [router]); const onShufflePress = useCallback(() => { @@ -66,10 +68,10 @@ export function PlayerScreen() { if (track === null) { return ( - +
- + ); } @@ -79,7 +81,7 @@ export function PlayerScreen() { const RepeatIcon = repeat === 'one' ? Repeat1 : Repeat; return ( - +
@@ -96,7 +98,7 @@ export function PlayerScreen() { neighbours={neighbours} onNext={next} onPrevious={previous} - onDismiss={close} + onExpandedChange={onExpandedChange} /> @@ -202,7 +204,7 @@ export function PlayerScreen() { - + ); } diff --git a/src/features/player/components/ArtworkCarousel.tsx b/src/features/player/components/ArtworkCarousel.tsx index 9a74431..c456bd8 100644 --- a/src/features/player/components/ArtworkCarousel.tsx +++ b/src/features/player/components/ArtworkCarousel.tsx @@ -16,6 +16,7 @@ import { useReducedMotion } from '@/theme/useReducedMotion'; import { useThemeColors } from '@/theme/useTheme'; 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 +55,8 @@ 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. */ + onExpandedChange: (expanded: boolean) => void; } /** @@ -85,13 +86,12 @@ export function ArtworkCarousel({ neighbours, onNext, onPrevious, - onDismiss, + onExpandedChange, }: ArtworkCarouselProps) { - const { width } = useWindowDimensions(); + const { width, height } = useWindowDimensions(); const reducedMotion = useReducedMotion(); const offsetX = useSharedValue(0); - const offsetY = useSharedValue(0); /** 0 undecided, 1 horizontal, 2 vertical. Fixed once per gesture. */ const axis = useSharedValue(0); @@ -123,8 +123,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 +135,8 @@ 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; + runOnJS(onExpandedChange)(!dismiss); return; } @@ -176,11 +174,10 @@ 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 ( diff --git a/src/features/player/components/MiniPlayer.tsx b/src/features/player/components/MiniPlayer.tsx index e886d57..4552083 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,13 @@ 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. */ + onPrepareOpen: () => void; + /** Settles the root overlay after a tap or released drag. */ + onExpandedChange: (expanded: boolean) => void; +} + /** * The persistent transport strip above the tab bar. * @@ -55,12 +59,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 +83,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 +124,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 +140,18 @@ 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)(); + runOnJS(onExpandedChange)(far || fast); }) .onFinalize(() => { axis.value = 0; offsetX.value = withSpring(0, SPRING); - offsetY.value = withSpring(0, SPRING); }); const followStyle = useAnimatedStyle(() => ({ - transform: [{ translateX: offsetX.value }, { translateY: offsetY.value }], + opacity: 1 - playerExpansion.value, + transform: [{ translateX: offsetX.value }], })); if (phase === 'idle' || track === null) return null; @@ -182,7 +172,7 @@ export function MiniPlayer() { - + void; +} + +/** Root-mounted Now Playing surface driven directly by the mini-player gesture. */ +export function NowPlayingOverlay({ visible, expanded, onExpandedChange }: NowPlayingOverlayProps) { + const { height } = useWindowDimensions(); + const style = useAnimatedStyle(() => ({ + opacity: playerExpansion.value, + transform: [{ translateY: interpolate(playerExpansion.value, [0, 1], [height, 0]) }], + })); + + return ( + + {visible ? ( + + + + ) : null} + + ); +} 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..bbe69dd --- /dev/null +++ b/src/features/player/playerLayerLayout.ts @@ -0,0 +1,25 @@ +import { useSyncExternalStore } from 'react'; + +let tabBarHeight = 0; +const listeners = new Set<() => void>(); + +/** Records the measured tab-bar height for the root player layer. */ +export function setPlayerTabBarHeight(nextHeight: number): void { + if (tabBarHeight === nextHeight) return; + tabBarHeight = nextHeight; + for (const listener of listeners) listener(); +} + +/** Returns the current tab-bar height without polling layout from every route. */ +export function usePlayerTabBarHeight(): number { + return useSyncExternalStore(subscribe, getSnapshot); +} + +function subscribe(listener: () => void): () => void { + listeners.add(listener); + return () => listeners.delete(listener); +} + +function getSnapshot(): number { + return tabBarHeight; +} diff --git a/src/features/playlists/PlaylistDetailScreen.tsx b/src/features/playlists/PlaylistDetailScreen.tsx index 28e3ae0..4860047 100644 --- a/src/features/playlists/PlaylistDetailScreen.tsx +++ b/src/features/playlists/PlaylistDetailScreen.tsx @@ -8,21 +8,21 @@ 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 { 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 +36,25 @@ 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 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 +92,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,46 +108,48 @@ 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..66b1f2d 100644 --- a/src/features/playlists/PlaylistsScreen.tsx +++ b/src/features/playlists/PlaylistsScreen.tsx @@ -6,7 +6,13 @@ 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, + LIKED_SONGS_ID, + useFavoriteEntries, + usePlaylists, + type PlaylistSummary, +} from '@/db/queries/playlists'; import { useMessages } from '@/i18n'; import { useThemeColors } from '@/theme/useTheme'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; @@ -29,6 +35,7 @@ export function PlaylistsScreen() { const router = useRouter(); const playlists = usePlaylists(); + const likedEntries = useFavoriteEntries(); const [naming, setNaming] = useState(false); const openNaming = useCallback(() => setNaming(true), []); @@ -45,16 +52,26 @@ 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 = [ + { + id: LIKED_SONGS_ID, + name: t('playlists.likedSongs'), + trackCount: likedEntries.length, + mosaic: likedEntries + .flatMap((entry) => (entry.artworkPath ? [entry.artworkPath] : [])) + .slice(0, 4), + artworkPath: null, + }, + ...playlists, + ]; + return ( @@ -73,21 +90,22 @@ export function PlaylistsScreen() { - {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/i18n/locales/en.json b/src/i18n/locales/en.json index 170d9be..b615ad7 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -33,10 +33,24 @@ "noResults": "Nothing matches “{{term}}”.", "scan": "Scan", "addFolder": "Add a folder", + "import": "Import", "scanConfirm": { "title": "Scan for music?", "body": "Mufify will read every audio file Android has indexed on this device. On a large library this takes a while. You can stop it at any point, and anything already found is kept." }, + "folderImportConfirm": { + "title": "Import this folder?", + "body": "Mufify will index the selected folder and read its tags. This can take a while on a large library and may temporarily affect device performance. You can stop it at any time." + }, + "importing": { + "title": "Importing music", + "preparing": "Preparing the import…", + "messages": [ + "Reading the music that is already on this device.", + "Building the library without sending anything anywhere.", + "Covers and tags are being prepared for the library." + ] + }, "view": { "label": "Library view", "tracks": "Tracks", @@ -46,6 +60,7 @@ }, "playlists": { "title": "Playlists", + "likedSongs": "Liked songs", "create": "New playlist", "rename": "Rename", "delete": "Delete playlist", @@ -66,14 +81,12 @@ "Nothing in here yet. Add tracks from the library.", "An empty playlist. Long-press a track to put it here." ], + "likedEmpty": ["No liked songs yet. Use the heart on a track to keep it here."], "addCount_one": "Add {{count}} track to a playlist", "addCount_other": "Add {{count}} tracks to a playlist", "shuffleAll": "Shuffle", "addTracks": "Add tracks", - "reorder": "Reorder {{title}}", - "addSelected_zero": "Pick some tracks", - "addSelected_one": "Add {{count}} track", - "addSelected_other": "Add {{count}} tracks" + "reorder": "Reorder {{title}}" }, "stats": { "title": "Stats", @@ -198,19 +211,9 @@ "remaining_one": "{{count}} track left", "remaining_other": "{{count}} tracks left" }, - "selection": { - "count_zero": "Select tracks", - "count_one": "{{count}} selected", - "count_other": "{{count}} selected", - "all": "Select all", - "none": "Clear selection", - "cancel": "Leave selection", - "addToQueue": "Add to queue", - "addToPlaylist": "Add to playlist" - }, "track": { "playNext": "Play next", - "select": "Select", + "addToQueue": "Add to queue", "info": "Track info", "field": { "title": "Title", diff --git a/src/i18n/locales/tr.json b/src/i18n/locales/tr.json index b7363eb..0dae65c 100644 --- a/src/i18n/locales/tr.json +++ b/src/i18n/locales/tr.json @@ -33,10 +33,24 @@ "noResults": "“{{term}}” ile eşleşen bir şey yok.", "scan": "Tara", "addFolder": "Klasör ekle", + "import": "İçe aktar", "scanConfirm": { "title": "Müzik taransın mı?", "body": "Mufify, Android'in bu cihazda indekslediği tüm ses dosyalarını okuyacak. Büyük bir kütüphanede bu biraz sürer. İstediğin an durdurabilirsin, o ana kadar bulunanlar kalır." }, + "folderImportConfirm": { + "title": "Bu klasör içe aktarılsın mı?", + "body": "Mufify seçilen klasörü indeksleyecek ve etiketlerini okuyacak. Büyük bir kitaplıkta bu sürebilir ve cihaz performansını geçici olarak etkileyebilir. İstediğin an durdurabilirsin." + }, + "importing": { + "title": "Müzik içe aktarılıyor", + "preparing": "İçe aktarma hazırlanıyor…", + "messages": [ + "Bu cihazdaki müzikler okunuyor.", + "Kitaplık, hiçbir yere veri göndermeden oluşturuluyor.", + "Kapaklar ve etiketler kitaplık için hazırlanıyor." + ] + }, "view": { "label": "Kitaplık görünümü", "tracks": "Parçalar", @@ -46,6 +60,7 @@ }, "playlists": { "title": "Listeler", + "likedSongs": "Beğenilenler", "create": "Yeni liste", "rename": "Yeniden adlandır", "delete": "Listeyi sil", @@ -66,14 +81,12 @@ "Burada henüz bir şey yok. Kitaplıktan parça ekle.", "Boş bir liste. Bir parçaya uzun bas ve buraya koy." ], + "likedEmpty": ["Henüz beğenilen parça yok. Burada tutmak için bir parçadaki kalbi kullan."], "addCount_one": "{{count}} parçayı bir listeye ekle", "addCount_other": "{{count}} parçayı bir listeye ekle", "shuffleAll": "Karıştır", "addTracks": "Parça ekle", - "reorder": "{{title}} sırasını değiştir", - "addSelected_zero": "Birkaç parça seç", - "addSelected_one": "{{count}} parça ekle", - "addSelected_other": "{{count}} parça ekle" + "reorder": "{{title}} sırasını değiştir" }, "stats": { "title": "İstatistik", @@ -198,19 +211,9 @@ "remaining_one": "{{count}} parça kaldı", "remaining_other": "{{count}} parça kaldı" }, - "selection": { - "count_zero": "Parça seç", - "count_one": "{{count}} seçildi", - "count_other": "{{count}} seçildi", - "all": "Tümünü seç", - "none": "Seçimi temizle", - "cancel": "Seçimden çık", - "addToQueue": "Sıraya ekle", - "addToPlaylist": "Listeye ekle" - }, "track": { "playNext": "Sıradaki çal", - "select": "Seç", + "addToQueue": "Sıraya ekle", "info": "Parça bilgisi", "field": { "title": "Parça", diff --git a/src/services/audio/AudioEngine.ts b/src/services/audio/AudioEngine.ts index adc886b..51d113d 100644 --- a/src/services/audio/AudioEngine.ts +++ b/src/services/audio/AudioEngine.ts @@ -11,7 +11,14 @@ import { shuffleTracks, type ShuffleAlgorithm } from '@/services/shuffle'; import { ListenCycle, type BankedListen } from '@/services/stats/listenCycle'; import { isRewindToRestart } from '@/services/stats/repeatListen'; -import { isPlayable, nextIndex, playNextIndex, previousIndex, shiftForInsert } from './queue'; +import { + isPlayable, + nextIndex, + playNextIndex, + previousIndex, + shouldRestartCurrentTrack, + shiftForInsert, +} from './queue'; import { IDLE_PLAYBACK, LIBRARY_SOURCE, @@ -39,9 +46,6 @@ import { /** How often the engine reports position. 500ms is expo-audio's own default. */ const STATUS_INTERVAL_MS = 500; -/** Pressing previous past this point restarts the track instead of going back. */ -const RESTART_THRESHOLD_MS = 3_000; - type Listener = (state: PlaybackState) => void; type ListenReporter = (listen: FinishedListen) => void; @@ -466,8 +470,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 +513,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 +527,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; } 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/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/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)); From a538f920ec53728ab22617e2e1a4e1e888d0e3a0 Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:23:50 +0300 Subject: [PATCH 02/13] test(jest): keep worktree checkouts out of the module map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `.claude/worktrees/*` holds full checkouts of other branches, each with its own `modules/audio-focus/package.json`. Four packages claiming the name `audio-focus` makes the Haste map ambiguous, so `jest.mock('audio-focus')` cannot resolve it at all — the new engine suite failed to run before its first line — and every other run printed a duplicate-name warning nobody could act on. `testPathIgnorePatterns` already skipped the *tests* there. It does not touch the module map, which is a separate index. Co-Authored-By: Claude Opus 5 --- jest.config.js | 6 ++++++ 1 file changed, 6 insertions(+) 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'], }; From 33dd423204f843bc40d65749287a5c93ed1ea3dc Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:24:08 +0300 Subject: [PATCH 03/13] test(player): pin the listen-counting matrix to the real engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same miscount has been reported three times and twice recorded as "fixed and verified against the database". Both verifications were real device sessions and both were true when they were written; neither could be re-run, so neither caught what came after. The gap they left is specific. `ListenCycle` and `isRewindToRestart` each have good unit tests and each is correct in isolation. Nothing covered the wiring between them in `AudioEngine.onStatus` — which is where a listen is opened, banked and reopened, and where every reported defect has actually lived. So this replays a scripted status stream through the real engine, with `expo-audio` behind a fake player and time on the fake timer clock that `ListenCycle` already reads through `Date.now()`. The scenarios are the ones a person performs, and their expected answers come from ADR 005 and ADR 011 rather than from whatever the code currently does: a played start to finish -> one play b repeat-one, three times round -> three plays, three start times c past the play mark, dragged back -> two listens rewind short of the 25% mark -> still one rewind before it has earned a play -> still one d abandoned early, skipped forward -> one listen, only the audio heard e scrubbing back and forth -> no extra events Plus the queue moving on, which is the other way a listen closes: a track ending and a user skipping each bank exactly one, and a track that never played banks nothing. All thirteen pass against the engine as it stands, which is itself a finding — the counting state machine is right for every tick stream a phone can produce, so the next place to look for the miscount is the stream, not the machine. The fake deliberately implements only what the engine calls. A method the engine starts using shows up as a missing function rather than a silent no-op. Co-Authored-By: Claude Opus 5 --- src/services/audio/listenRecording.test.ts | 232 ++++++++++++++++++ src/services/audio/testing/fakeAudioPlayer.ts | 127 ++++++++++ src/services/audio/testing/playbackHarness.ts | 204 +++++++++++++++ 3 files changed, 563 insertions(+) create mode 100644 src/services/audio/listenRecording.test.ts create mode 100644 src/services/audio/testing/fakeAudioPlayer.ts create mode 100644 src/services/audio/testing/playbackHarness.ts diff --git a/src/services/audio/listenRecording.test.ts b/src/services/audio/listenRecording.test.ts new file mode 100644 index 0000000..64d7fd1 --- /dev/null +++ b/src/services/audio/listenRecording.test.ts @@ -0,0 +1,232 @@ +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('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/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..28f4c7d --- /dev/null +++ b/src/services/audio/testing/playbackHarness.ts @@ -0,0 +1,204 @@ +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; +} + +/** 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, so an outcome here is the outcome + // that would be written to the row. + outcome: classifyListen(listen.msPlayed, listen.track.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(); + + 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(); + }, + + 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(); + }, + + async previous() { + await AudioEngine.previous(); + await flush(); + await settleLoad(); + }, + + 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(): Promise { + const live = currentFakePlayer(); + if (live.isLoaded) return; + const current = AudioEngine.getState().track; + if (current === null) return; + live.finishLoading(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(); +} From 1e703e3c7d7af153be916a8be880a86e0c155482 Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:25:35 +0300 Subject: [PATCH 04/13] fix(player): sort out what the root player layer stacks over what MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three reports, one layer. `PlayerLayer` mounts the transport strip and Now Playing outside the router, absolutely positioned over every route, and the order it drew them in was wrong in three separate ways. **The mini player drew over an open Now Playing.** The overlay was `z-10` under the strip's `z-20`, so the surface meant to cover everything was painted beneath the thing it covers. The fade hid only part of it: `opacity` lived on the row inside `MiniPlayer`, which left the strip's own panel background, its top hairline and `MiniProgress` at full opacity — the panel and progress bar across the bottom of the expanded player were the mini player, still there. The fade moves up to the wrapper, so the whole strip goes together. **The queue opened and was never seen.** `app/queue.tsx` was a modal route, and an opaque full-screen overlay mounted above the navigator covers whatever the navigator puts under it. Nothing was wrong with `QueueScreen`; it was rendering correctly, one layer down, behind the player that opened it. That is a property of the overlay rather than of that screen — any route pushed while Now Playing is open would have vanished the same way — so the queue gets the same treatment as Now Playing itself: a root-level sheet, one layer above it, outside the overlay's transformed and clipped container. The route is gone along with `router.navigate('/queue')`. **The opening did not feel connected to the finger.** The gesture already derived one shared value from `translationY` and the overlay was already always mounted, so the structure was right. What was missing was everything after the release: the spring started from rest however hard the strip was flicked, so a throw and a slow drag opened at identical speed. Both ends of the gesture now hand their velocity over, in expansion units per second. The spring is softer and heavier with it (ζ ≈ 0.86 against 0.75, and no mass term before), and opacity finishes at 40% of the travel instead of tracking the whole thing — a sheet arriving, not two screens cross-fading. A tap also mounts the player on press-in rather than on press, so the first frames of the animation are not spent waiting for a render the animation itself triggered. `playerLayerLayout` grows a second measurement, the strip's height, which nothing consumes yet. Co-Authored-By: Claude Opus 5 --- app/_layout.tsx | 7 +- app/queue.tsx | 5 -- src/features/player/PlayerLayer.tsx | 85 +++++++++++++++---- src/features/player/PlayerScreen.tsx | 12 ++- src/features/player/QueueScreen.tsx | 16 ++-- .../player/components/ArtworkCarousel.tsx | 11 ++- src/features/player/components/MiniPlayer.tsx | 30 +++++-- .../player/components/NowPlayingOverlay.tsx | 38 +++++++-- .../player/components/QueueOverlay.tsx | 45 ++++++++++ src/features/player/playerLayerLayout.ts | 65 +++++++++++++- 10 files changed, 261 insertions(+), 53 deletions(-) delete mode 100644 app/queue.tsx create mode 100644 src/features/player/components/QueueOverlay.tsx diff --git a/app/_layout.tsx b/app/_layout.tsx index a0df7f4..c341692 100644 --- a/app/_layout.tsx +++ b/app/_layout.tsx @@ -58,9 +58,14 @@ export default function RootLayout() { + {/* + 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/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/src/features/player/PlayerLayer.tsx b/src/features/player/PlayerLayer.tsx index 3335428..28b5f81 100644 --- a/src/features/player/PlayerLayer.tsx +++ b/src/features/player/PlayerLayer.tsx @@ -1,16 +1,28 @@ import { useSegments } from 'expo-router'; import type { ReactNode } from 'react'; -import { useCallback, useState } from 'react'; -import { View } from 'react-native'; -import { runOnJS, withSpring } from 'react-native-reanimated'; +import { useCallback, useEffect, useState } from 'react'; +import { 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 { usePlayerTabBarHeight } from './playerLayerLayout'; +import { setMiniPlayerHeight, usePlayerTabBarHeight } from './playerLayerLayout'; -const SPRING = { damping: 24, stiffness: 260 } as const; +/** + * 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; @@ -22,41 +34,82 @@ export function PlayerLayer({ children }: PlayerLayerProps) { 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), []); - const onExpandedChange = useCallback((nextExpanded: boolean) => { + /* + * 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); + playerExpansion.value = withSpring(1, { ...SPRING, velocity }); return; } setExpanded(false); - playerExpansion.value = withSpring(0, SPRING, (finished) => { + playerExpansion.value = withSpring(0, { ...SPRING, velocity }, (finished) => { if (finished) runOnJS(setVisible)(false); }); }, []); + /* + * 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 d69ede1..2ee30f3 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, @@ -37,13 +36,14 @@ import { useQueueNeighbours } from './hooks/useQueueNeighbours'; * screen so the mini player and this surface share one gesture progress value. */ export interface PlayerScreenProps { - onExpandedChange: (expanded: boolean) => void; + onExpandedChange: (expanded: boolean, velocity?: number) => void; + /** The queue is a root-level surface, not a route. See `QueueOverlay`. */ + onOpenQueue: () => void; } -export function PlayerScreen({ onExpandedChange }: PlayerScreenProps) { +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(); @@ -52,8 +52,6 @@ export function PlayerScreen({ onExpandedChange }: PlayerScreenProps) { const [shuffled, setShuffledState] = useState(() => AudioEngine.isShuffled()); const close = useCallback(() => onExpandedChange(false), [onExpandedChange]); - // A double press must not leave two identical queue screens on the stack. - const openQueue = useCallback(() => router.navigate('/queue'), [router]); const onShufflePress = useCallback(() => { toggleShuffle(); @@ -69,7 +67,7 @@ export function PlayerScreen({ onExpandedChange }: PlayerScreenProps) { if (track === null) { return ( -
+
); diff --git a/src/features/player/QueueScreen.tsx b/src/features/player/QueueScreen.tsx index 48a1b3b..c87c497 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 })); @@ -63,7 +65,7 @@ export function QueueScreen() { void; onPrevious: () => void; - /** Releases the shared overlay at either end of a vertical drag. */ - onExpandedChange: (expanded: boolean) => 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; } /** @@ -136,7 +139,9 @@ export function ArtworkCarousel({ .onEnd((event) => { if (axis.value === 2) { const dismiss = event.translationY > DISMISS_DISTANCE || event.velocityY > DISMISS_VELOCITY; - runOnJS(onExpandedChange)(!dismiss); + // 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; } diff --git a/src/features/player/components/MiniPlayer.tsx b/src/features/player/components/MiniPlayer.tsx index 4552083..1ac595e 100644 --- a/src/features/player/components/MiniPlayer.tsx +++ b/src/features/player/components/MiniPlayer.tsx @@ -41,10 +41,16 @@ 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. */ + /** 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. */ - onExpandedChange: (expanded: boolean) => 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; } /** @@ -142,15 +148,21 @@ export function MiniPlayer({ onPrepareOpen, onExpandedChange }: MiniPlayerProps) const far = playerExpansion.value >= 0.18; const fast = event.velocityY <= -OPEN_VELOCITY; - runOnJS(onExpandedChange)(far || fast); + // 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); }); + /* + * 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(() => ({ - opacity: 1 - playerExpansion.value, transform: [{ translateX: offsetX.value }], })); @@ -176,6 +188,14 @@ export function MiniPlayer({ onPrepareOpen, onExpandedChange }: MiniPlayerProps) void; + onExpandedChange: (expanded: boolean, velocity?: number) => 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 }: NowPlayingOverlayProps) { +export function NowPlayingOverlay({ + visible, + expanded, + onExpandedChange, + onOpenQueue, +}: NowPlayingOverlayProps) { const { height } = useWindowDimensions(); const style = useAnimatedStyle(() => ({ - opacity: playerExpansion.value, + 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]) }], })); @@ -25,11 +51,11 @@ export function NowPlayingOverlay({ visible, expanded, onExpandedChange }: NowPl {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/playerLayerLayout.ts b/src/features/player/playerLayerLayout.ts index bbe69dd..a34b959 100644 --- a/src/features/player/playerLayerLayout.ts +++ b/src/features/player/playerLayerLayout.ts @@ -1,18 +1,73 @@ 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; - for (const listener of listeners) listener(); + 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, getSnapshot); + 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 { @@ -20,6 +75,10 @@ function subscribe(listener: () => void): () => void { return () => listeners.delete(listener); } -function getSnapshot(): number { +function getTabBarHeight(): number { return tabBarHeight; } + +function getMiniPlayerHeight(): number { + return miniPlayerHeight; +} From b7861d223c97f41e5a5f94a91a5e68236896294e Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:26:00 +0300 Subject: [PATCH 05/13] fix(player): keep the Now Playing column inside the screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header was clipped off the top and the transport row sat under the navigation bar. Two reports, one cause, and it is not the safe area — that was being applied. The column simply did not fit. The cover was 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. `justify-center` then split the overflow between both ends, which is exactly the two symptoms. The cover was never asked whether it fit. So it is bounded on both axes now, and the height bound is measured rather than computed from a constant: the chrome below it is not a fixed height, because the title wraps to two lines and Turkish runs 10–20% longer than English. `ArtworkCarousel` takes the height flex leaves it and draws a square of `min(width - gutters, that height)`. `aspect-square w-full` is gone with it. A width-derived square is only square while the width is the tighter of the two bounds, and on this screen it was not — which is the same bug wearing the other reporter's words, "the artwork is the wrong size". Three gaps of 32 between four blocks was also 96 dp of air the column did not have; `gap-6` gives 24 back, and `pb-4` keeps the transport off the navigation bar rather than merely clear of it. Co-Authored-By: Claude Opus 5 --- src/features/player/PlayerScreen.tsx | 11 +++- src/features/player/artworkSize.test.ts | 36 ++++++++++++ src/features/player/artworkSize.ts | 33 +++++++++++ .../player/components/ArtworkCarousel.tsx | 58 +++++++++++++------ 4 files changed, 117 insertions(+), 21 deletions(-) create mode 100644 src/features/player/artworkSize.test.ts create mode 100644 src/features/player/artworkSize.ts diff --git a/src/features/player/PlayerScreen.tsx b/src/features/player/PlayerScreen.tsx index 2ee30f3..4da902f 100644 --- a/src/features/player/PlayerScreen.tsx +++ b/src/features/player/PlayerScreen.tsx @@ -80,9 +80,14 @@ export function PlayerScreen({ onExpandedChange, onOpenQueue }: PlayerScreenProp 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 diff --git a/src/features/player/artworkSize.test.ts b/src/features/player/artworkSize.test.ts new file mode 100644 index 0000000..1943197 --- /dev/null +++ b/src/features/player/artworkSize.test.ts @@ -0,0 +1,36 @@ +import { ARTWORK_GUTTER, artworkSize } from './artworkSize'; + +/** + * The cover has to stay square and has to fit. It was doing neither: sized from + * the width alone it overflowed the column, and `justify-center` then pushed + * the header off the top and the transport under the navigation bar. + */ +describe('artworkSize', () => { + 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 328c693..89aa21b 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,6 +15,7 @@ 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'; @@ -94,6 +95,19 @@ export function ArtworkCarousel({ 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); /** 0 undecided, 1 horizontal, 2 vertical. Fixed once per gesture. */ const axis = useSharedValue(0); @@ -187,12 +201,15 @@ export function ArtworkCarousel({ return ( - {/* Clips the neighbours to the visible slot. */} - - - - - + {/* Takes the height flex leaves it, and clips the neighbours. */} + + + + + @@ -202,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; } @@ -212,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} @@ -262,7 +284,7 @@ function Slot({ track, width, offset }: SlotProps) { } return ( - + {content} ); From 841150654bfc4ce0d37a376269295248300c9b0d Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:26:20 +0300 Subject: [PATCH 06/13] fix(playlists): one query behind the liked-songs count and the list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Playlists tab showed "0 tracks" beside Liked Songs while opening it showed the real ones — and the tab also drew "there is nothing here, make a playlist" underneath a visible row. Both come from the same habit, and neither is a second query computing the number differently. **The count was never told anything had changed.** `useLiveQuery` re-runs 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. `useFavoriteEntries` selected `from(tracks)` and joined `track_stats`, so it watched `tracks` and never `track_stats`. Liking a song writes only to `track_stats`. The tab therefore held whatever the query returned when it last mounted; the detail screen agreed with reality because it had just mounted. Selecting from `track_stats` is also the honest description of what the list is: favourites are `track_stats` rows. **The empty state was answering a different question from the list.** It asked `playlists.length`, which excludes Liked Songs, while the list rendered `rows`, which included it. So did the header count. Three views of one collection, computed three times — the thing `AGENTS.md` names outright: *a count and the list it describes come from one query*. `buildPlaylistRows` now builds the rows once, the header renders `rows.length`, and the empty state is a fact about the same array. Liked Songs is in it only when it holds something: a virtual playlist with nothing in it is not a destination yet, and an always-present "0 tracks" row is what forced the count and the list to disagree in the first place. `LIKED_SONGS_ID` moves to `services/playlists/order` and is re-exported. Importing it from the query module drags SQLite into every test that needs to name this playlist — the module opens the database at import time, which the new test hit immediately. The regression is pinned: whatever the rows are built from, the empty state can only be true when there are none of them. Also carries this screen's share of the mini-player bottom inset, which the next commit applies everywhere else — the two changes are three lines apart in the same list. Co-Authored-By: Claude Opus 5 --- src/db/queries/playlists.ts | 34 +++++++-- src/features/playlists/PlaylistsScreen.tsx | 24 +++---- src/features/playlists/playlistRows.test.ts | 78 +++++++++++++++++++++ src/features/playlists/playlistRows.ts | 65 +++++++++++++++++ src/services/playlists/order.ts | 10 +++ 5 files changed, 189 insertions(+), 22 deletions(-) create mode 100644 src/features/playlists/playlistRows.test.ts create mode 100644 src/features/playlists/playlistRows.ts diff --git a/src/db/queries/playlists.ts b/src/db/queries/playlists.ts index d3f228e..b93ff20 100644 --- a/src/db/queries/playlists.ts +++ b/src/db/queries/playlists.ts @@ -1,13 +1,19 @@ 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,9 +37,6 @@ export interface PlaylistEntry { isFavorite: boolean; } -/** Virtual route id for liked songs; it never exists in `playlists`. */ -export const LIKED_SONGS_ID = -1; - const entrySelection = { trackId: tracks.id, fileUri: tracks.fileUri, @@ -103,15 +106,32 @@ export function usePlaylistEntries(playlistId: number): PlaylistEntry[] { return data; } -/** Live favourite tracks, newest favourite first, presented as a virtual playlist. */ +/** + * 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(tracks) - .innerJoin(trackStats, eq(trackStats.trackId, tracks.id)) + .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))) diff --git a/src/features/playlists/PlaylistsScreen.tsx b/src/features/playlists/PlaylistsScreen.tsx index 66b1f2d..1254b5d 100644 --- a/src/features/playlists/PlaylistsScreen.tsx +++ b/src/features/playlists/PlaylistsScreen.tsx @@ -13,12 +13,15 @@ import { 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. @@ -31,6 +34,7 @@ export function PlaylistsScreen() { useLifecycleTrace('PlaylistsScreen'); const { t } = useTranslation(); const messages = useMessages('playlists.empty'); + const bottomInset = useMiniPlayerInset(); const colors = useThemeColors(); const router = useRouter(); @@ -59,24 +63,14 @@ export function PlaylistsScreen() { [openPlaylist], ); - const rows = [ - { - id: LIKED_SONGS_ID, - name: t('playlists.likedSongs'), - trackCount: likedEntries.length, - mosaic: likedEntries - .flatMap((entry) => (entry.artworkPath ? [entry.artworkPath] : [])) - .slice(0, 4), - artworkPath: null, - }, - ...playlists, - ]; + 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 })} ({ + 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/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; From 73055fde1f8a6e3f96cfb14b3ba23121f6b30d33 Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:26:34 +0300 Subject: [PATCH 07/13] fix(ui): pad every scrollable screen for the mini player MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The transport strip is always visible once something is playing, and it is absolutely positioned over the routes rather than laid out with them. So the last rows of every list sat behind it, permanently unreachable — the bottom of the library, the bottom of a playlist, the last setting. One measured number, published by the layer that owns the strip and read through `useMiniPlayerInset`, rather than a hand-tuned `pb-` on each screen: six screens with six guesses is six things to get wrong, and five of them were simply absent. It is a runtime measurement, so it cannot be a Tailwind class — the config overrides the spacing scale and compiles anything outside it to nothing at all. This is the exception `AGENTS.md` names. Where a content container needed both, the whole padding moved into the style object using `SPACING` tokens, rather than leaving which one wins to NativeWind's merge order between `contentContainerClassName` and `contentContainerStyle`. The tab bar is deliberately not in the number. 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 strip's measured height is exactly the overlap. It is zero while nothing is playing, because the mini player renders nothing then — so a list gets its full height back rather than a band of dead space under it. Co-Authored-By: Claude Opus 5 --- src/features/library/components/CollectionGrid.tsx | 13 ++++++++++++- src/features/library/components/TrackList.tsx | 9 +++++++++ src/features/playlists/PlaylistDetailScreen.tsx | 3 +++ src/features/settings/SettingsScreen.tsx | 11 ++++++++++- src/features/stats/StatsScreen.tsx | 11 ++++++++++- 5 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/features/library/components/CollectionGrid.tsx b/src/features/library/components/CollectionGrid.tsx index ec7438d..5b604fc 100644 --- a/src/features/library/components/CollectionGrid.tsx +++ b/src/features/library/components/CollectionGrid.tsx @@ -5,6 +5,8 @@ import { useCallback } from 'react'; import { View } from 'react-native'; import type { CollectionCard as Card } from '@/db/queries/tracks'; +import { useMiniPlayerInset } from '@/features/player/playerLayerLayout'; +import { SPACING } from '@/theme/tokens'; import { CollectionCard } from './CollectionCard'; @@ -33,6 +35,8 @@ export interface CollectionGridProps { * and reuses it. */ export function CollectionGrid({ kind, cards, icon, onPress, empty }: CollectionGridProps) { + const bottomInset = useMiniPlayerInset(); + const renderItem = useCallback>( ({ 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/TrackList.tsx b/src/features/library/components/TrackList.tsx index 787252b..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'; @@ -69,6 +70,12 @@ 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('track.addToQueue'); @@ -96,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/playlists/PlaylistDetailScreen.tsx b/src/features/playlists/PlaylistDetailScreen.tsx index 4860047..0c831f0 100644 --- a/src/features/playlists/PlaylistDetailScreen.tsx +++ b/src/features/playlists/PlaylistDetailScreen.tsx @@ -18,6 +18,7 @@ import { usePlaylists, type PlaylistEntry, } from '@/db/queries/playlists'; +import { useMiniPlayerInset } from '@/features/player/playerLayerLayout'; import { useMessages } from '@/i18n'; import { AudioEngine } from '@/services/audio/AudioEngine'; import { LIBRARY_SOURCE, type PlayableTrack, type QueueSource } from '@/services/audio/types'; @@ -37,6 +38,7 @@ export function PlaylistDetailScreen({ playlistId }: PlaylistDetailScreenProps) const { t, i18n } = useTranslation(); const router = useRouter(); const isLiked = playlistId === LIKED_SONGS_ID; + const bottomInset = useMiniPlayerInset(); const detailMessages = useMessages(isLiked ? 'playlists.likedEmpty' : 'playlists.detailEmpty'); const playlistEntries = usePlaylistEntries(playlistId); @@ -156,6 +158,7 @@ export function PlaylistDetailScreen({ playlistId }: PlaylistDetailScreenProps) renderItem={renderItem} keyExtractor={keyExtractor} overrideItemLayout={setEntryHeight} + contentContainerStyle={{ paddingBottom: bottomInset }} /> )} 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 ? ( - + Date: Sun, 2 Aug 2026 22:44:30 +0300 Subject: [PATCH 08/13] fix(stats): count a listen against the duration the engine reports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The third report of "the loop count is wrong", and the first one with a mechanism that explains why it was *sometimes* right. Every threshold in the counting rule is a fraction of the track's duration, and there are two durations that disagree. The scanner's comes from MediaStore and 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 comes from the open file and is authoritative. `classifyListen(msPlayed, 0)` returns `partial`; that is its first line. So a track with no stored duration recorded a `play_event` for every listen, moved neither counter, and vanished from the play counts. Ten listens produced ten honest-looking rows and a play count of zero. `PlaybackState.durationMs` already preferred the engine's figure — the comment saying MediaStore "is occasionally wrong" has been in `docs/player.md` all along. The listen handed to the recorder did not: it carried `track.durationMs` straight off the row. That gap is the defect. `FinishedListen` now carries the duration explicitly, and the engine fills it with its own, falling back to the scanner's only when the file never opened. This is why two rounds of device verification came back clean. Both used tracks whose stored duration was fine, and those counted correctly the whole time. The failure needs a track MediaStore has not finished reading, which is exactly the state a freshly copied file is in — and freshly copied files are what you reach for when testing playback. Three cases in `listenRecording.test.ts` cover it, and all three fail against the previous behaviour: a full listen to a track with no stored duration is a play, a repeat of one still splits in two, and a stored duration that is far too short no longer turns a genuine skip into a play. Co-Authored-By: Claude Opus 5 --- docs/stats.md | 47 ++++++++++++++ src/features/player/listenRecorder.ts | 3 +- src/services/audio/AudioEngine.ts | 64 ++++++++++++++++--- src/services/audio/listenRecording.test.ts | 52 +++++++++++++++ src/services/audio/testing/playbackHarness.ts | 24 ++++--- src/services/audio/types.ts | 16 +++++ 6 files changed, 186 insertions(+), 20 deletions(-) diff --git a/docs/stats.md b/docs/stats.md index a6bea93..0b85d4b 100644 --- a/docs/stats.md +++ b/docs/stats.md @@ -122,6 +122,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 @@ -165,6 +177,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 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/services/audio/AudioEngine.ts b/src/services/audio/AudioEngine.ts index 51d113d..61793ea 100644 --- a/src/services/audio/AudioEngine.ts +++ b/src/services/audio/AudioEngine.ts @@ -11,6 +11,8 @@ import { shuffleTracks, type ShuffleAlgorithm } from '@/services/shuffle'; import { ListenCycle, type BankedListen } from '@/services/stats/listenCycle'; import { isRewindToRestart } from '@/services/stats/repeatListen'; +import { lockScreenArtworkUri, prepareNotificationArtwork } from './notificationArtwork'; + import { isPlayable, nextIndex, @@ -124,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. * @@ -142,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. * @@ -334,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, @@ -401,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. */ @@ -613,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 index 64d7fd1..c674b29 100644 --- a/src/services/audio/listenRecording.test.ts +++ b/src/services/audio/listenRecording.test.ts @@ -197,6 +197,58 @@ describe('e — scrubbing back and forth', () => { }); }); +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(); diff --git a/src/services/audio/testing/playbackHarness.ts b/src/services/audio/testing/playbackHarness.ts index 28f4c7d..968f6c6 100644 --- a/src/services/audio/testing/playbackHarness.ts +++ b/src/services/audio/testing/playbackHarness.ts @@ -58,6 +58,12 @@ 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. */ @@ -92,9 +98,9 @@ export async function startPlayback( listens.push({ trackId: listen.track.id, msPlayed: listen.msPlayed, - // The same call `recordListen` makes, so an outcome here is the outcome - // that would be written to the row. - outcome: classifyListen(listen.msPlayed, listen.track.durationMs), + // 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(), }); @@ -104,7 +110,7 @@ export async function startPlayback( await AudioEngine.setQueue(tracks, options.startIndex ?? 0, options.source); await flush(); - await settleLoad(); + await settleLoad(options.reportedDurationMs); return { listens, @@ -139,7 +145,7 @@ export async function startPlayback( live.playing = false; live.emit({ didJustFinish: true }); await flush(); - await settleLoad(); + await settleLoad(options.reportedDurationMs); }, async seekTo(ms: number) { @@ -155,13 +161,13 @@ export async function startPlayback( async next() { await AudioEngine.advance(true); await flush(); - await settleLoad(); + await settleLoad(options.reportedDurationMs); }, async previous() { await AudioEngine.previous(); await flush(); - await settleLoad(); + await settleLoad(options.reportedDurationMs); }, setRepeat(mode: RepeatMode) { @@ -182,12 +188,12 @@ export async function startPlayback( * exists to handle — and that race is the reason playback used to stop dead on * the second track of every queue. */ -async function settleLoad(): Promise { +async function settleLoad(reportedDurationMs?: number): Promise { const live = currentFakePlayer(); if (live.isLoaded) return; const current = AudioEngine.getState().track; if (current === null) return; - live.finishLoading(current.durationMs / 1000); + live.finishLoading((reportedDurationMs ?? current.durationMs) / 1000); live.emit(); await flush(); } 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; From fef2740a664cb6e8fa47a380ff111af3277c5ccc Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:44:50 +0300 Subject: [PATCH 09/13] fix(player): keep the media notification in step with what is playing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The system notification showed "playing" for audio that had stopped, and drew no artwork at all for a track without a cover while the app drew its music-note placeholder. **The state.** `setActiveForLockScreen` was called on every track load. On a player that is already active it does not refresh a session — inside expo-audio 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. Since our engine keeps one player for the app's lifetime and swaps sources through `replace()`, that window opened on every single track change. It is claimed once now and updated in place with `updateLockScreenMetadata`, which changes metadata on the live session and re-posts the notification with no release and no gap. `stop()` hands the session back and clears the flag. That also explains an entry in `docs/player.md` that had been filed as an unexplained quirk — `dumpsys media_session` reporting `state=NONE` with a stale title. It was catching the swap, and the swap was happening constantly. **The artwork.** A generated PNG of lucide's `music` mark, same stroke weight, on `--color-panel`, unpacked from the bundle to a `file://` path — the service loads artwork through `java.net.URL(...).openConnection()`, which knows nothing about `asset://` or a Metro URL. So a track with no cover looks the same in the notification as it does on screen. **No favourite button.** It cannot be done through this engine and ADR 015 records why in full: `AudioLockScreenOptions` has three fields and none is a custom action, the MediaSession is a private field of expo-audio's own service, and the routes that remain are a private fork of the library or reflection into its internals. Deferred to whenever the engine question in ADR 009 is reopened, and reported as not delivered rather than quietly dropped. ADR 014 records the other decision this branch made: the queue is a root-level sheet, because a route cannot be seen from under a root-mounted player overlay. `tsconfig` paths are reordered, most specific first. TypeScript picks the best pattern whatever the order; jest-expo turns them into a Jest moduleNameMapper in this order and Jest takes the first match, so `@/assets/x` was resolving to `src/assets/x` and no asset import resolved under test. Co-Authored-By: Claude Opus 5 --- assets.d.ts | 20 +++++ assets/images/notification-artwork.png | Bin 0 -> 3222 bytes .../014-queue-is-a-root-sheet-not-a-route.md | 70 +++++++++++++++ ...ourite-button-in-the-media-notification.md | 85 ++++++++++++++++++ docs/player.md | 34 +++++++ package.json | 1 + src/services/audio/notificationArtwork.ts | 53 +++++++++++ tsconfig.json | 7 +- 8 files changed, 269 insertions(+), 1 deletion(-) create mode 100644 assets.d.ts create mode 100644 assets/images/notification-artwork.png create mode 100644 docs/adr/014-queue-is-a-root-sheet-not-a-route.md create mode 100644 docs/adr/015-no-favourite-button-in-the-media-notification.md create mode 100644 src/services/audio/notificationArtwork.ts 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 0000000000000000000000000000000000000000..50ffe47e5ccd60eb77a6d0b3acde30e3fe51b428 GIT binary patch literal 3222 zcmb_ec~p~E7QYD*A`}cz5f=(>P}D*YS;}I9f*55mV--}8MIECcL@ENZ<--*QG ze_uoWS^5BANL%f_9srM)cpz#;(yWy?0Y2MG^Y+@1(9+jNiJVO{uj)w7SbS^s$ttI* z#pLSaF&D5!;$_ywMDrQeBw}>d)|&o3@oVx2Tl4B?l*`FP&ey4$YwAb(wn+yH_Gbs~n@NSRF|pc~ zrWpZj^>Z)%muPK#vUQ_ zi)}SP9cF4F$74|CUB^dklb`&X3X9Pg;HY4JGrQnbZLDMe6u^aTiHl=U%qTWaKM%Z- zX9o_Mze6C9(fR&iPE#>kBrlpy0s@xNKbAF6qT$r3+_cIcP$~JxlhCrAb;!L(m^_|& zH;=hY9D-WwnjN%dX|>$9FzlqwgZb;w8;WP3_S4tpz8S}K#|#S}4`Jm9D=oE{sVx%a zV4u>pR*O7LwwXWx90^bu^g$s0^M}_k4;wck!K3AwfXA8{#w57$QIM5O@OaqC@u5PM zghwR8`4lr84v^2Mz{m%KKWdB{kB0#fL%`67h|+!hL2uyyiT^C!q@@e% zsIN${TO z5|a)%73@%;Ucw;OMHTr{W~{Xub(3~$+YC>B@=?-*FFy-K#S^$c+KVa#U#rkLQ$asb z)3N>TuZs2;8g`8qs#SP&zvOXe$-bv;ct*_0KM=W8WjTkFgP5T5(LEWtWVSh4 z78NzHP_+PEm4FIn?fzA$Xl|~BCgT#GQ>qziVw5rFB&_b!PP)&ti@Y2fVS%S9`)qfi zCxS&T=?;yRjSLR64y5}a<|Jdoj>=R1;8|9@Tcx+!t=0|!n8H^0_bqj$RSs5n&?;-D zquaNhk^Car_Rb2P{ciR0q}{nsr7XW_+Q1K!!pxk9`)=As<6 zX49CT=R)g#(QB)HSDLe9w>s;k8AD!bCyTIse-UG!^qNH1)$k5IjRcp*PLov^C5Lyo zRL!nl5fX0xUASmCsd7@?ZnF>j zr8FDo%UDbBR0@N7?j>urqCM>g-2GAFVQb2|+WaCJiv)@8_~31tx!rhgAD!#%K~Y)XF2|lKBKIe{IW=FCDN`&Z4!IQ=OT1qWS?B$}$uAqq3#z%Z`bRVXws< zvk<}(z1Z?gi9OG$tzWQ&uOo!7APXIA!Qy1&_riRI)Th%o1$D35)}Vfx>wKhXU^WV5 z-|vzRy;S+a-p{z12H`Jc)vgQeD93=R5nS=R@Q=%U!|NYb(Sz0mCmG?e4*oPQPv?q# zC_hRqb*isvpuZbpkih(><%ezV;Hf@!hs(aU z|FR-8w*n2H_~>2Ta=o7f@wGOH?})BJ20Ns?Iw{2qu5I<8X@);U2Dz_rP>64A-beyn zSp@e%9^Kcm$8HGLJik#)gB4lAt<8 literal 0 HcmV?d00001 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/player.md b/docs/player.md index 399d245..5ce9bf8 100644 --- a/docs/player.md +++ b/docs/player.md @@ -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/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/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/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"] } From 1906a48c56d53f24550bb67b2f012731d54c9269 Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:44:59 +0300 Subject: [PATCH 10/13] chore(playlists): drop the import the row builder took over `LIKED_SONGS_ID` moved into `buildPlaylistRows`; the screen no longer names it. Co-Authored-By: Claude Opus 5 --- src/features/playlists/PlaylistsScreen.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/features/playlists/PlaylistsScreen.tsx b/src/features/playlists/PlaylistsScreen.tsx index 1254b5d..ac60433 100644 --- a/src/features/playlists/PlaylistsScreen.tsx +++ b/src/features/playlists/PlaylistsScreen.tsx @@ -8,7 +8,6 @@ import { EmptyState } from '@/components/ui/EmptyState'; import { Screen } from '@/components/ui/Screen'; import { createPlaylist, - LIKED_SONGS_ID, useFavoriteEntries, usePlaylists, type PlaylistSummary, From 52be8ff5b17d1526f020d22760969cf5fc4dcd60 Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:47:41 +0300 Subject: [PATCH 11/13] docs: correct the handoff's branch warning and the component tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `HANDOFF.md` told the next session that `main` holds three files and must never be branched from. PR #2 merged the project into `main` on 2026-08-02, so that warning now points the wrong way — and following it blindly would be its own mistake, because the newest four commits still sat ahead of `main` on `fix/ux-round-2`. It says "check, in both directions" now, with the command. `docs/components.md` gains `QueueOverlay`, splits `PlayerLayer` from `NowPlayingOverlay`, and records the stacking order in the one place that decides it. Test count 292 -> 335. Co-Authored-By: Claude Opus 5 --- HANDOFF.md | 28 ++++++++++++++++++++-------- docs/components.md | 8 +++++--- 2 files changed, 25 insertions(+), 11 deletions(-) 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/docs/components.md b/docs/components.md index b3e0a56..9b2e23a 100644 --- a/docs/components.md +++ b/docs/components.md @@ -71,14 +71,16 @@ it is repeated here because that is the thing a reader needs before touching it. | Component | What it is for | |---|---| -| `PlayerLayer` / `PlayerScreen` | Root-mounted Now Playing overlay and its content. The mini player and full screen share one Reanimated expansion value; no route transition sits between them. | -| `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. | --- From 5f399edb36ff6df13a7c2f1934969696359c52c6 Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 22:51:47 +0300 Subject: [PATCH 12/13] fix(player): close the player surfaces on back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing the queue route took hardware back with it. Both Now Playing and the queue are mounted outside the navigator, so nothing else can handle it: back would pop the screen *underneath* an open player and leave the player sitting over a screen the user never chose. `PlayerLayer` is the only place that knows both states, so it takes the press and closes the innermost one — queue first, then the overlay. Registered only while one of them is open, so an ordinary screen keeps its own back behaviour. The handler is declared after `onExpandedChange` on purpose. A dependency array is built during render, so naming that `const` above its own declaration is a temporal dead zone error on every render rather than a lint nit — which is how this was found. Also `h-full` rather than `flex-1` on the centre artwork slot: it sits in a row, so `flex-1` put a flex basis of zero against the explicit width and left which one wins to the shrink factor. Co-Authored-By: Claude Opus 5 --- src/features/player/PlayerLayer.tsx | 30 ++++++++++++++++++- .../player/components/ArtworkCarousel.tsx | 5 +++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/features/player/PlayerLayer.tsx b/src/features/player/PlayerLayer.tsx index 28b5f81..bfedc4d 100644 --- a/src/features/player/PlayerLayer.tsx +++ b/src/features/player/PlayerLayer.tsx @@ -1,7 +1,7 @@ import { useSegments } from 'expo-router'; import type { ReactNode } from 'react'; import { useCallback, useEffect, useState } from 'react'; -import { View, type LayoutChangeEvent } from 'react-native'; +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'; @@ -70,6 +70,34 @@ export function PlayerLayer({ children }: PlayerLayerProps) { }); }, []); + /* + * 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. * diff --git a/src/features/player/components/ArtworkCarousel.tsx b/src/features/player/components/ArtworkCarousel.tsx index 89aa21b..99b5239 100644 --- a/src/features/player/components/ArtworkCarousel.tsx +++ b/src/features/player/components/ArtworkCarousel.tsx @@ -276,7 +276,10 @@ function Slot({ track, width, size, offset }: SlotProps) { {content} From 55b16009a9efa2d77d944e75fb9112ac35ee3eb2 Mon Sep 17 00:00:00 2001 From: Yefee8 Date: Sun, 2 Aug 2026 23:48:50 +0300 Subject: [PATCH 13/13] docs: write down what the device actually showed, and correct what it did not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `docs/stats.md` said the repeat-listen behaviour was "verified on device". It had been, twice, and both reports were true about what they measured. Both were wrong about what they implied. Every track those sessions used 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 and therefore exactly the file you reach for when testing playback. The matrix was written down before it was run this time. On the Pixel_7 AVD: a played start to finish 1 event, play pass b looped 7 passes, 7 events, 27-30s apart pass c past the play mark, rewound to 0 2 events pass d skipped forward could not be driven e scrubbed back and forth could not be driven (d) and (e) need a forward seek and there is no way to perform one from automation: the scrubber is a Reanimated pan that neither `input swipe` nor a hand-built `input motionevent` sequence activates, and both `cmd media_session dispatch fast-forward` and `KEYCODE_MEDIA_FAST_FORWARD` left the position where it was. Both are covered by the harness, which drives the real `AudioEngine.seekTo`. So: 5/5 in the harness, 3/5 on hardware, and the three that ran on hardware are the three the harness cannot model — real audio timing, a real status stream, a real write per event. The gaps in (b) are the load-bearing number. 27–30 seconds 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. The Mi 9T could not be driven at all: `adb shell input` is still refused, and its notification shade was stuck open with no shell command able to collapse it. Everything recorded is the emulator, and the file says so. `docs/performance.md` also gains the notification results, including one that closes an old open question: `dumpsys media_session` reporting `state=NONE` with a stale title was the session being released and rebuilt on every track change, which is the same window that let the notification show "playing" for stopped audio. Fixing one made the other truthful. Co-Authored-By: Claude Opus 5 --- docs/performance.md | 90 +++++++++++++++++++++++++++++ docs/stats.md | 25 +++++++- src/features/player/QueueScreen.tsx | 5 +- 3 files changed, 117 insertions(+), 3 deletions(-) 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/stats.md b/docs/stats.md index 0b85d4b..646cb28 100644 --- a/docs/stats.md +++ b/docs/stats.md @@ -4,8 +4,10 @@ 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: counting, event recording, incremental rollups and the stats -> screen are implemented. The repeat-listen device check is recorded only after -> it has been run against the on-device database. +> 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. --- @@ -272,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/src/features/player/QueueScreen.tsx b/src/features/player/QueueScreen.tsx index c87c497..ef57c11 100644 --- a/src/features/player/QueueScreen.tsx +++ b/src/features/player/QueueScreen.tsx @@ -62,7 +62,10 @@ export function QueueScreen({ onClose }: QueueScreenProps) { ); 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. */ +