diff --git a/AGENTS.md b/AGENTS.md index 9d692ae..d64534a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,7 +21,8 @@ call, it is wrong. This is the app's core promise and the reason it exists. ## Setup -Requirements: Node 22+, JDK 17 or 21, Android Studio with SDK Platform 35+, macOS or Linux. +Requirements: Node 22+, JDK 17 or 21, Android Studio with SDK Platform 36+, macOS or Linux. +(Expo SDK 57 compiles against 36; this said 35 until `app.sh` checked it and was wrong.) ### Environment (macOS) @@ -108,8 +109,10 @@ These are not preferences. Violating them is a bug. typed query functions. 4. **No business logic in component bodies.** Logic goes in `src/services/` (pure, testable) or `src/features/*/hooks/` (stateful orchestration). -5. **Settings live in MMKV. Data lives in SQLite.** Don't mix them. Zustand holds transient - player/UI state only, never anything that must survive a restart. +5. **Settings live in MMKV. Data lives in SQLite.** Don't mix them. Transient player/UI state + lives in the `AudioEngine` singleton and the other module-level stores, read from React + through `useSyncExternalStore` — never anything that must survive a restart. There is no + global state library; don't add one. 6. **Layers point downward.** `components → hooks → services → db`. Never the reverse. A service must not import a component. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..56873a1 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,88 @@ +# Contributing + +Read [AGENTS.md](AGENTS.md) first. It is the house style and it is binding — +this file is the process around it, not a summary of it. + +## The gate + +Nothing is done until all four pass: + +```bash +npm run lint +npm run typecheck +npm test +cd android && ./gradlew :audio-tags:testDebugUnitTest +``` + +Do not weaken a test to make it pass, and do not skip one. If a test is wrong, +fix the test and say in the commit message why it was wrong — that is useful +information, not an admission. + +## Commits + +Conventional commits: `feat:`, `fix:`, `refactor:`, `perf:`, `docs:`, `test:`, +`chore:`. Scope where it helps: `feat(shuffle): add discovery algorithm`. + +**One logical change per commit.** Never one commit per phase of work. + +The message body is the part that matters. It should say what changed and *why*, +and it is worth more than the diff — the diff already says what changed. In +particular: + +- If you measured something, put the numbers in. Before and after. +- If you were wrong about the cause before you found the real one, say so. The + next person will have the same wrong idea. +- If you decided not to do something, say what and why. + +Commit messages in this repository are read as the project's record. Several of +them are the only place a subtle decision is written down. + +## Claims + +- **A performance claim needs a measurement.** Before and after, with the device + and the conditions. `src/services/perf` exists for this; see + [docs/performance.md](docs/performance.md) for the method. +- **A UI change needs a device.** A screenshot or a specific observation — + "the toast clears the tab bar and has a dismiss control", not "should work". + The emulator is fine for behaviour; frame timing needs real hardware. +- **Do not claim something works without running it.** + +## Decisions + +Anything non-obvious gets a short ADR in `docs/adr/NNN-title.md`: context, +decision, consequences. Three paragraphs is plenty. Include the option you +rejected and why — an ADR that only argues for what was chosen is half a record. + +If a requirement turns out to be a bad idea once you are in the code, say so and +propose the alternative. Do not silently build something different, and do not +build something you know is wrong because it was asked for. + +## Documentation + +Docs are part of the work, not cleanup afterwards. A phase is not done until the +relevant `docs/*.md` is updated in the same change. + +Every exported component and service function gets a one-line JSDoc saying what +it does. Skip comments that restate the code; write the ones that explain why +the code is not the obvious thing. + +## Things that will fail review + +- A network call, analytics, crash reporting, or any SDK that phones home. +- A colour, spacing value, or user-facing string hardcoded in a component. +- A spacing class outside the scale. `src/theme/scale.test.ts` catches these, + and it exists because five of them shipped invisible. +- A string added to `en.json` but not `tr.json`, or the reverse. + `src/i18n/locales.test.ts` catches this. +- A component over 300 lines. +- `FlatList` where a list can grow. +- A hand-edit to `android/` or `ios/`, which are generated and git-ignored. +- Deleting a track row during rescan because a file is temporarily missing — + mark `is_missing = 1` instead, so playlists and history survive an unmounted + SD card. + +## Pull requests + +Say what changed, why, and how it was verified on a device. If something is +unverified, say that too — an honest gap is worth more than a confident guess, +and it tells the reviewer where to look. diff --git a/HANDOFF.md b/HANDOFF.md new file mode 100644 index 0000000..d8a45b3 --- /dev/null +++ b/HANDOFF.md @@ -0,0 +1,233 @@ +# Mufify — continue and finish + +Offline-only Android music player. React Native + Expo SDK 57, TypeScript strict. +Repo: `/Users/yefee/Desktop/projeler/mufify`, branch **`fix/performance-ux-stats`**. + +## Read first + +`AGENTS.md` (binding house style), then `README.md`, `CONTRIBUTING.md`, +`docs/performance.md` (every measurement taken so far), `docs/components.md`, +and all of `docs/adr/`. + +## Before anything else + +```bash +git status && git log --oneline -15 +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. +Do not build on red. + +## Do NOT branch from `main` + +`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. + +## State + +Everything in the last brief is done: performance (1a/1b/1c), all seven UI/UX +items, all four statistics items, the regression pass, and the release docs. +**68+ commits, unpushed.** No PR has been opened — nobody has asked for one. + +Read the commit messages rather than re-deriving. Several are the only place a +decision is written down. + +## Devices + +**Mi 9T `7a6f8791`** — the user's real phone, API 29, MIUI. Connected and +running the app right now (dark theme, Turkish, 521 tracks). + +- `adb install`, `am start`, `logcat` all work. +- **`adb shell input` and `pm grant` are blocked** (`SecurityException: + Injecting to another application requires INJECT_EVENTS`). Every tap needs a + human. **Batch them and ask once.** +- **MIUI's logcat rate limiter ("chatty") silently discards lines** under load. + If a measurement is missing, that is usually why — not a bug. Two per-render + counters were removed for exactly this reason; do not add more. +- Cold start is ~40 s here because it is a debug build pulling JS from Metro. + `adb -s 7a6f8791 reverse tcp:8081 tcp:8081` after every reconnect. + +**Pixel_7 AVD** — full automation, `input` works. Frame timing is worthless +(SwiftShader). Degrades badly after several hours up; restart it rather than +fighting flaky taps. + +## What is actually left + +### 1. Frame timing on the Mi 9T — still never obtained + +**The only item on this list that no amount of automation can close.** The AVD +renders through SwiftShader, so its frame numbers are worthless; MIUI blocks +`adb shell input`, so the phone cannot be scrolled without a human hand. It +needs both halves at once and there is no substitute. + +The user uninstalled the app from the phone on 2026-08-01, so this now needs a +reinstall first. Then, with the app on the Library tab: + +```bash +adb -s 7a6f8791 shell dumpsys gfxinfo dev.mufify.app reset +# human scrolls hard for ~10 seconds +adb -s 7a6f8791 shell dumpsys gfxinfo dev.mufify.app framestats +``` + +Note the package is `dev.mufify.app`, not `com.mufify` — querying the wrong one +returns "No process found", which reads exactly like the app having died. + +This is the only valid source for a 60 fps claim, and `docs/performance.md` +deliberately makes no frame-rate claim until it exists. Record it there. + +### 2. ~~Playlist chain, end to end~~ — DONE 2026-08-01 + +Verified on the Pixel_7 AVD in one pass, `stats_rollups` playlist rows included. +Full record in `docs/performance.md` under "Device verification". + +Two things worth carrying forward: + +- Every control has an `accessibilityLabel`, so drive the UI from + `uiautomator dump` by label rather than guessing coordinates. That is what + made this reproducible where the previous attempt was not. +- The reorder handle is `Gesture.Pan().activateAfterLongPress(120)`. A plain + `adb shell input swipe` **never activates it**; you need + `input motionevent DOWN` → hold → `MOVE`s → `UP`. + +Reading the database — **copy the `-wal` too.** WAL means recent writes are not +in the main file, and copying only `mufify.db` shows an empty `play_events`, +which looks exactly like the bug you would then go hunting: + +```bash +adb -s shell "run-as dev.mufify.app cat files/SQLite/mufify.db" > /tmp/m.db +adb -s shell "run-as dev.mufify.app cat files/SQLite/mufify.db-wal" > /tmp/m.db-wal +sqlite3 /tmp/m.db "SELECT entity_type, COUNT(*) FROM stats_rollups GROUP BY 1;" +``` + +### 3. ~~Repeat-listen seek-back~~ — DONE 2026-08-01 + +Two distinct `outcome=play` rows for one track from two seek-backs, rollups +agreeing. Recorded in `docs/performance.md`. + +### 4. ~~Release build~~ — DONE 2026-08-01 + +Installed and run for the first time, on the AVD rather than the phone, with +`adb reverse --remove-all` so it could not fall back to Metro. Boots clean, all +four tabs render, no `INTERNET` permission in the manifest. Details in +`docs/performance.md`. + +Still **not measured** — functional smoke test only. Every number in this repo +is from a debug build, which overstates JS cost. + +### 4b. A UX gap found while doing the above + +`MiniPlayer` is rendered only in `app/(tabs)/_layout.tsx`. Pushed stack screens +therefore have no transport control: start playback from a playlist detail +screen and there is no visible player, and no way to reach Now Playing without +going back to a tab. It follows from the routing structure rather than being a +defect, so it is a design call rather than a bug fix — but it is a real gap and +nobody has decided about it. + +### 5. One finding from the phone's database (the other is closed) + +Read on 2026-08-01 from the Mi 9T (`stats_rollups`, `tracks`). The codec finding +below was chased and closed the same day — it was never a defect. The remaining +one needs a human to press Scan. + +**Stage two is 446 of 521 short.** `last_scanned_at IS NULL` for 446 rows and +set for 75. That is consistent with a scan that was cancelled or a process that +was killed, and the design says it resumes from exactly there — the null column +*is* the queue. Worth confirming that pressing Scan resumes rather than +restarts, because it is a designed behaviour nobody has watched happen. + +**~~`codec` is null for all 521 rows~~ — RESOLVED, not a defect. Do not +reinvestigate.** It is `codecOf` in `src/services/scanner/trackMapping.ts` +working as designed: it returns null whenever the MIME subtype is already a +container name, because `audio/mpeg` otherwise renders the strip `MP3 · mpeg` +— the same fact spelled worse. `flac`, `mp4`, `mpeg` and `wav` are all in +`CONTAINER_NAMES`, so **a library of mainstream formats has a null codec on +every row, always.** + +The premise behind the alarm was wrong twice over: `codec` never comes from the +native reader, so "the reader returned one field and not the other" described a +path that does not exist; and the emulator's `FLAC · 44.1 kHz · 16-bit` is the +*container* column, which was being compared against the phone's *codec* +column. Pulled from the device to confirm — the 75 enriched rows read +`container=FLAC|M4A, codec=NULL, bitrate=143, sample_rate=44100, bit_depth=16, +channels=2`. The phone renders exactly what the emulator does. There is no API +29 versus 35 difference and nothing to fix. (`bit_depth` is null on the one M4A +row, also correct — AAC is lossy and has no bit depth.) + +`trackMapping.test.ts` now pins this with the device case named, so the next +reading of a null codec column resolves in one test file instead of a device +session. + +Note the library on that phone is **synthetic test files** (`perf-NNN`, album +`bulk`, no artist, no artwork), not the user's music. Scanning their real +library is the only way to tell these apart from a tagging artefact — and it is +also the only way `artists` and artwork get exercised at all, since 0 of 521 +current rows have either. + +### 6. Phase 10 leftovers + +Play Store listing copy (EN + TR), Data Safety answers, release AAB build +instructions, screenshots for the README. + +`docs/architecture.md` is **written** — startup ordering, the three places state +lives, the scan and listen flows, and the native boundary. Screenshots are the +one Phase 10 item that needs a device. + +## Traps already paid for — do not rediscover these + +- **`tailwind.config.js` overrides the spacing scale.** A class built from a + value outside it compiles to *nothing* — no warning, no size. Five shipped + invisible, including the swipe-to-queue reveal strip, whose icon had never + been seen. `src/theme/scale.test.ts` now fails on any such class. The same is + true of colours: there is no `danger` token, and `text-danger` silently does + nothing. +- **Axis-locking a pan gesture on the first `onUpdate` is wrong.** Both + translations are still `0`, and `Math.abs(0) >= Math.abs(0)` is true, so every + gesture classifies as horizontal and vertical ones are silently discarded. + Wait for ~6 px of real movement. +- **`router.push` on a control that is also reachable by gesture stacks two + screens.** The drag fires the handler while the underlying `Pressable` still + registers a press. Use `router.navigate`. Symptom: dismiss *and* the close + button both appear broken, because each correctly pops one of two. +- **The React Compiler's `react-hooks/immutability` rule rejects mutating a + Reanimated shared value captured by a hook.** Build gestures inline (as + `Scrubber` does) unless the component has many instances; `SwipeableRow` + memoizes because it has ~40 in a list and needs an eslint-disable for it. +- **NativeWind `className` only works on components it knows.** Register in + `src/theme/interop.ts` with `cssInterop`. +- **A virtualized list needs a bounded flex parent** — wrap in ``. +- **FlashList's default `drawDistance` is 250 px.** With 64 px rows that is under + four rows of buffer; a fling outruns it and leaves blank rows. It is 1200 now. +- **expo-router + React Compiler:** pass `(props) => ` to + `tabBar`, never the component. +- **`dumpsys media_session` lies about this app.** Use the engine's own stream. +- **Jest's `testMatch` treats any `*spec.ts` as a test.** Do not name a module that. +- **Property tests: seed with splitmix32, not an LCG.** Sequential LCG seeds give + correlated first draws. +- **MIUI marks call recordings `is_music=1`** — `MusicFilter` excludes recorder + folders by path. +- **`MediaMetadataRetriever` has no sample rate or bit depth below API 31.** + `AudioFormatReader` reads them from `MediaExtractor`. +- **New routes need Metro running** to regenerate `.expo/types/router.d.ts`. +- **Release builds ship no `INTERNET` permission** — `plugins/withOfflineOnly.js`. + Never undo this. +- **`android/` is generated by CNG and git-ignored.** Native config goes in + `app.json` or a config plugin. +- **The stress-library seeder does not survive a restart** — the launch sweep + retires tracks MediaStore cannot see, and synthetic rows have no files. Seed + and measure in the same session. + +## Working style + +Small verified increments, one logical change per commit, and a commit message +that explains what and why — the user reads those as the report. Measure before +claiming a performance fix, and put before/after numbers in. Verify UI on a +device and say what you actually saw. When you were wrong about a cause before +finding the real one, write that down; the next person will have the same wrong +idea. Do not stop at decision points — pick the most defensible option, record +it in an ADR, keep going. Say plainly what is verified and what is still owed. diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..e6c69f7 --- /dev/null +++ b/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Yefee8 + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/README.md b/README.md new file mode 100644 index 0000000..9206eae --- /dev/null +++ b/README.md @@ -0,0 +1,159 @@ +# Mufify + +An offline-only music player for Android. React Native, Expo SDK 57, TypeScript. + +It plays the files a streaming service will not: FLAC, ALAC, and anything else +already on the phone. It surfaces the technical truth of a file rather than +hiding it. And it does not have a network layer — not a disabled one, not one +behind a setting. Release builds ship without the `INTERNET` permission at all. + +## What it does + +- **Playback** of local files, lossless first-class, with background playback, + lock-screen controls and a persistent queue. +- **Five shuffle algorithms**, chosen in Settings, each explained where you + choose it. Not one shuffle behind a toggle — see [docs/shuffle.md](docs/shuffle.md). +- **Local playlists** with drag-reorder, a cover mosaic, and shuffle. +- **Listening statistics** computed on the device from your own history: top + tracks, artists, albums and playlists by week, month and year, with a Wrapped + summary. Nothing is uploaded because there is nowhere to upload it to. +- **Technical metadata surfaced**: bitrate, sample rate, bit depth, codec, file + size, on a monospaced spec strip. +- **Dark and light themes, Turkish and English**, both switchable. + +## The promise + +No network. No accounts. No telemetry. No analytics SDK. + +This is enforced rather than intended: `plugins/withOfflineOnly.js` strips the +`INTERNET` permission from the release manifest and restores it only for debug +builds, where Metro needs it. A change that introduces a network call does not +fail review — it fails to work. + +## Requirements + +- **Node 22+** +- **JDK 17 or 21** — Android Studio's bundled JBR is fine, no separate install +- **Android Studio** with **SDK Platform 36+** +- macOS or Linux + +`minSdkVersion` is 26. Raising it to 31 was considered and rejected: it costs +roughly a fifth of Android devices while deleting almost no code. See +[ADR 002](docs/adr/002-min-sdk-26.md). + +## Setup + +None of this is set by default, and every "it works on my machine" failure in +this project so far has been one of these three lines missing. Put them in +`~/.zshrc`: + +```bash +export JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home" +export ANDROID_HOME="$HOME/Library/Android/sdk" +export PATH="$JAVA_HOME/bin:$ANDROID_HOME/platform-tools:$ANDROID_HOME/emulator:$PATH" +``` + +Verify before blaming the code: + +```bash +java -version # 17 or 21 +adb devices # your device, "device" not "unauthorized" or "offline" +npx expo-doctor +``` + +Then: + +```bash +npm install +./app.sh # checks the environment, finds a device, starts the dev build +``` + +`app.sh` (and `app.bat` on Windows) only *checks* the environment — it will tell +you exactly what is missing and stop. It will not set `JAVA_HOME` or +`ANDROID_HOME` for you, because silently changing a developer's toolchain +environment is a worse failure than an error message. + +## Running + +```bash +npx expo start --dev-client # the normal one. JS/TS changes hot reload. +npm run lint +npm run typecheck +npm test +npm run db:generate # after a schema change +``` + +`npx expo run:android` is **only** for native changes: adding or removing a +native dependency, editing `app.json`, or changing a config plugin. It is a +ten-minute build, and reaching for it after a TypeScript edit is the most common +way to waste an afternoon here. + +Kotlin has its own tests: + +```bash +cd android && ./gradlew :audio-tags:testDebugUnitTest +``` + +### Installing on a device + +`adb install` is enough on most phones and on the emulator. On **MIUI / HyperOS** +(Xiaomi, Redmi, POCO) it fails with `INSTALL_FAILED_USER_RESTRICTED` regardless +of what developer options say. Push and install from the device instead: + +```bash +adb push android/app/build/outputs/apk/debug/app-debug.apk /sdcard/Download/ +``` + +MIUI also blocks `adb shell input` and `pm grant` with a `SecurityException`, so +automated UI testing on those devices is not possible — taps need a human. + +## Architecture + +``` +app/ routes only — read params, render a screen, nothing else +src/ + components/ui/ shared presentational components + features/ one directory per feature: screens, components, hooks + services/ pure logic — shuffle, stats, scanner, formatters + db/ schema, migrations, and the only place Drizzle is imported + theme/ design tokens, in exactly two files + i18n/ en.json and tr.json, kept in step by a test +modules/ local native modules (Kotlin) +``` + +Four rules that are bugs rather than preferences when violated: + +1. Only `src/services/audio/*` imports the audio library. +2. Only `src/db/queries/*` imports Drizzle or expo-sqlite. +3. No business logic in component bodies. +4. Layers point downward: `components → hooks → services → db`. + +[docs/architecture.md](docs/architecture.md) goes further: the startup ordering, +the two flows worth tracing, and why there is no global state library. + +## Documentation + +| | | +|---|---| +| [AGENTS.md](AGENTS.md) | the house style, binding on humans and agents alike | +| [docs/architecture.md](docs/architecture.md) | how the pieces fit, and where state lives | +| [docs/components.md](docs/components.md) | what each shared component is for | +| [docs/theming.md](docs/theming.md) | the token system, and how to add a colour | +| [docs/i18n.md](docs/i18n.md) | how to add a string and a language | +| [docs/database.md](docs/database.md) | schema, indexes, the play-counting rule | +| [docs/scanner.md](docs/scanner.md) | the two-stage scan and artwork cache | +| [docs/player.md](docs/player.md) | the audio engine and its Android gotchas | +| [docs/shuffle.md](docs/shuffle.md) | each algorithm in plain language | +| [docs/stats.md](docs/stats.md) | events, rollups, period keys, repeat detection | +| [docs/performance.md](docs/performance.md) | measurements, before and after | +| [docs/adr/](docs/adr/) | every non-obvious decision, with its reasoning | + +## Contributing + +See [CONTRIBUTING.md](CONTRIBUTING.md). The short version: read `AGENTS.md` +first, and `lint`, `typecheck` and `test` must all pass before a commit counts +as done. + +## Licence + +MIT. See [LICENSE](LICENSE). diff --git a/app.bat b/app.bat new file mode 100644 index 0000000..0a8ceeb --- /dev/null +++ b/app.bat @@ -0,0 +1,132 @@ +@echo off +REM One command to get Mufify running on Windows: check the toolchain, find a +REM device, start the dev server. +REM +REM This script *checks* and *reports*. It never sets JAVA_HOME, ANDROID_HOME or +REM PATH for you, and that is deliberate — silently rewriting a developer's +REM toolchain environment for the duration of one command produces a build that +REM works here and nowhere else, and the failure surfaces hours later somewhere +REM unrelated. When something is missing it says exactly what, and stops. + +setlocal enabledelayedexpansion +set PROBLEMS=0 + +echo Checking the toolchain + +REM --- Node --------------------------------------------------------------- +where node >nul 2>&1 +if errorlevel 1 ( + echo [X] Node is not installed. + echo Mufify needs Node 22 or newer: https://nodejs.org + set /a PROBLEMS+=1 +) else ( + for /f "tokens=*" %%v in ('node -p "process.versions.node.split('.')[0]"') do set NODE_MAJOR=%%v + if !NODE_MAJOR! LSS 22 ( + for /f "tokens=*" %%v in ('node -v') do echo [X] Node %%v is too old. Mufify needs 22 or newer. + set /a PROBLEMS+=1 + ) else ( + for /f "tokens=*" %%v in ('node -v') do echo [ok] Node %%v + ) +) + +REM --- Java --------------------------------------------------------------- +REM Checked through JAVA_HOME rather than whatever java is on PATH: Gradle uses +REM JAVA_HOME, so that is the one that decides whether a build works. +if "%JAVA_HOME%"=="" ( + echo [X] JAVA_HOME is not set. + echo Gradle reads JAVA_HOME, not the java on your PATH. + echo Android Studio's bundled JDK is fine. Set it to something like: + echo C:\Program Files\Android\Android Studio\jbr + set /a PROBLEMS+=1 +) else ( + if not exist "%JAVA_HOME%\bin\java.exe" ( + echo [X] JAVA_HOME points somewhere without a JDK: %JAVA_HOME% + echo There is no bin\java.exe under it. + set /a PROBLEMS+=1 + ) else ( + echo [ok] Java at %JAVA_HOME% + ) +) + +REM --- Android SDK -------------------------------------------------------- +if "%ANDROID_HOME%"=="" ( + echo [X] ANDROID_HOME is not set. + echo Set it to something like: %%LOCALAPPDATA%%\Android\Sdk + echo and add %%ANDROID_HOME%%\platform-tools to your PATH. + set /a PROBLEMS+=1 +) else ( + if not exist "%ANDROID_HOME%" ( + echo [X] ANDROID_HOME points at a directory that does not exist: %ANDROID_HOME% + set /a PROBLEMS+=1 + ) else ( + echo [ok] Android SDK at %ANDROID_HOME% + REM Point releases exist (android-36.1), so accept any android-36* too. + set SDK_OK=0 + if exist "%ANDROID_HOME%\platforms\android-36" set SDK_OK=1 + for /d %%d in ("%ANDROID_HOME%\platforms\android-3[6-9]*") do set SDK_OK=1 + if !SDK_OK!==0 ( + echo [X] SDK Platform 36 or newer is not installed. + echo Android Studio, Settings, Languages ^& Frameworks, Android SDK: + echo tick "Android API 36" and apply. + set /a PROBLEMS+=1 + ) else ( + echo [ok] SDK Platform 36 or newer + ) + ) +) + +where adb >nul 2>&1 +if errorlevel 1 ( + echo [X] adb is not on your PATH. + echo Add %%ANDROID_HOME%%\platform-tools to it. + set /a PROBLEMS+=1 +) else ( + echo [ok] adb found +) + +if %PROBLEMS% GTR 0 ( + echo. + echo %PROBLEMS% problem^(s^) above. Fix them and run this again. + exit /b 1 +) + +REM --- Dependencies ------------------------------------------------------- +if not exist node_modules ( + echo. + echo Installing dependencies + call npm install + if errorlevel 1 exit /b 1 +) else ( + echo [ok] Dependencies present +) + +REM --- Device ------------------------------------------------------------- +echo. +echo Looking for a device +set DEVICE_COUNT=0 +for /f "skip=1 tokens=1,2" %%a in ('adb devices') do ( + if "%%b"=="device" ( + set /a DEVICE_COUNT+=1 + REM Metro is reached over a reverse tunnel, re-established per connection. + adb -s %%a reverse tcp:8081 tcp:8081 >nul 2>&1 + ) +) + +if %DEVICE_COUNT%==0 ( + echo [!] No device or emulator connected. + echo Plug in a phone with USB debugging on, or start an emulator from + echo Android Studio's Device Manager. + echo Metro will start anyway; connect a device and it will pick it up. +) else ( + echo [ok] %DEVICE_COUNT% device^(s^) connected +) + +REM --- Go ----------------------------------------------------------------- +echo. +echo Starting Metro +echo If the app is not installed yet, run: npx expo run:android +echo That is a ten-minute native build and is only needed once, or after a +echo native dependency or app.json change. +echo. + +call npx expo start --dev-client diff --git a/app.sh b/app.sh new file mode 100755 index 0000000..9f0c64a --- /dev/null +++ b/app.sh @@ -0,0 +1,165 @@ +#!/usr/bin/env bash +# +# One command to get Mufify running: check the toolchain, find a device, start +# the dev server. +# +# This script *checks* and *reports*. It never sets JAVA_HOME, ANDROID_HOME or +# PATH for you, and that is deliberate — silently rewriting a developer's +# toolchain environment for the duration of one command produces a build that +# works here and nowhere else, and the failure surfaces hours later somewhere +# unrelated. When something is missing it says exactly what, and stops. + +set -euo pipefail + +readonly REQUIRED_NODE_MAJOR=22 +readonly REQUIRED_SDK_PLATFORM=36 + +# Only colour when writing to a terminal, so piping to a file stays readable. +if [ -t 1 ]; then + readonly BOLD=$'\033[1m' RED=$'\033[31m' YELLOW=$'\033[33m' GREEN=$'\033[32m' OFF=$'\033[0m' +else + readonly BOLD='' RED='' YELLOW='' GREEN='' OFF='' +fi + +problems=0 + +fail() { + printf '%s✗ %s%s\n' "$RED" "$1" "$OFF" >&2 + shift + for line in "$@"; do printf ' %s\n' "$line" >&2; done + problems=$((problems + 1)) +} + +ok() { printf '%s✓%s %s\n' "$GREEN" "$OFF" "$1"; } +warn() { printf '%s!%s %s\n' "$YELLOW" "$OFF" "$1"; } + +printf '%sChecking the toolchain%s\n' "$BOLD" "$OFF" + +# --- Node --------------------------------------------------------------- +if ! command -v node >/dev/null 2>&1; then + fail "Node is not installed." \ + "Mufify needs Node ${REQUIRED_NODE_MAJOR} or newer." \ + "https://nodejs.org, or: brew install node" +else + node_major=$(node -p 'process.versions.node.split(".")[0]') + if [ "$node_major" -lt "$REQUIRED_NODE_MAJOR" ]; then + fail "Node $(node -v) is too old." \ + "Mufify needs ${REQUIRED_NODE_MAJOR} or newer." + else + ok "Node $(node -v)" + fi +fi + +# --- Java --------------------------------------------------------------- +# Checked through JAVA_HOME rather than whatever `java` happens to be on PATH: +# Gradle uses JAVA_HOME, so that is the one that decides whether a build works. +if [ -z "${JAVA_HOME:-}" ]; then + fail "JAVA_HOME is not set." \ + "Gradle reads JAVA_HOME, not the java on your PATH." \ + "Android Studio's bundled JDK is fine. Add to ~/.zshrc:" \ + ' export JAVA_HOME="/Applications/Android Studio.app/Contents/jbr/Contents/Home"' +elif [ ! -x "$JAVA_HOME/bin/java" ]; then + fail "JAVA_HOME points somewhere without a JDK: $JAVA_HOME" \ + "There is no bin/java under it." +else + java_version=$("$JAVA_HOME/bin/java" -version 2>&1 | head -1 | sed 's/.*"\(.*\)".*/\1/') + java_major=${java_version%%.*} + if [ "$java_major" != "17" ] && [ "$java_major" != "21" ]; then + warn "Java $java_version — this project is built and tested against 17 and 21." + else + ok "Java $java_version" + fi +fi + +# --- Android SDK -------------------------------------------------------- +if [ -z "${ANDROID_HOME:-}" ]; then + fail "ANDROID_HOME is not set." \ + "Add to ~/.zshrc:" \ + ' export ANDROID_HOME="$HOME/Library/Android/sdk"' \ + ' export PATH="$ANDROID_HOME/platform-tools:$ANDROID_HOME/emulator:$PATH"' +elif [ ! -d "$ANDROID_HOME" ]; then + fail "ANDROID_HOME points at a directory that does not exist: $ANDROID_HOME" +else + ok "Android SDK at $ANDROID_HOME" + + # Any platform at or above the required one will do, and the installed + # directories carry point releases (android-36.1), so this compares the major + # number rather than looking for one exact directory name. + highest=$(ls "$ANDROID_HOME/platforms" 2>/dev/null \ + | sed -n 's/^android-\([0-9][0-9]*\).*/\1/p' | sort -n | tail -1) + + if [ -z "$highest" ]; then + fail "No SDK platform is installed." \ + "Android Studio → Settings → Languages & Frameworks → Android SDK," \ + "tick 'Android API $REQUIRED_SDK_PLATFORM' and apply." + elif [ "$highest" -lt "$REQUIRED_SDK_PLATFORM" ]; then + fail "SDK Platform $REQUIRED_SDK_PLATFORM or newer is required; highest installed is $highest." \ + "Android Studio → Settings → Languages & Frameworks → Android SDK," \ + "tick 'Android API $REQUIRED_SDK_PLATFORM' and apply." + else + ok "SDK Platform $highest" + fi +fi + +if ! command -v adb >/dev/null 2>&1; then + fail "adb is not on your PATH." \ + 'Add: export PATH="$ANDROID_HOME/platform-tools:$PATH"' +else + ok "adb $(adb version 2>/dev/null | head -1 | awk '{print $NF}')" +fi + +if [ "$problems" -gt 0 ]; then + printf '\n%s%d problem(s) above. Fix them and run this again.%s\n' "$RED" "$problems" "$OFF" >&2 + exit 1 +fi + +# --- Dependencies ------------------------------------------------------- +# node_modules older than the lockfile means someone pulled a dependency change. +if [ ! -d node_modules ] || [ package-lock.json -nt node_modules ]; then + printf '\n%sInstalling dependencies%s\n' "$BOLD" "$OFF" + npm install +else + ok "Dependencies up to date" +fi + +# --- Device ------------------------------------------------------------- +printf '\n%sLooking for a device%s\n' "$BOLD" "$OFF" + +devices=$(adb devices | awk 'NR>1 && $2=="device" {print $1}') + +if [ -z "$devices" ]; then + warn "No device or emulator connected." + echo + echo " Plug in a phone with USB debugging on, or start an emulator:" + if command -v emulator >/dev/null 2>&1; then + avds=$(emulator -list-avds 2>/dev/null || true) + if [ -n "$avds" ]; then + echo "$avds" | sed 's/^/ emulator -avd /' + echo + echo " On a machine short of RAM, headless survives where windowed does not:" + echo " emulator -avd -no-snapshot -no-boot-anim -no-window -gpu swiftshader_indirect -memory 2048" + else + echo " (no AVDs found — create one in Android Studio's Device Manager)" + fi + fi + echo + echo " Metro will start anyway; connect a device and it will pick it up." +else + count=$(printf '%s\n' "$devices" | wc -l | tr -d ' ') + ok "$count device(s): $(printf '%s' "$devices" | tr '\n' ' ')" + + # Metro is reached over a reverse tunnel, which has to be re-established for + # every device on every connection. + for device in $devices; do + adb -s "$device" reverse tcp:8081 tcp:8081 >/dev/null 2>&1 || true + done +fi + +# --- Go ----------------------------------------------------------------- +printf '\n%sStarting Metro%s\n' "$BOLD" "$OFF" +echo " If the app is not installed yet, run: npx expo run:android" +echo " That is a ten-minute native build and is only needed once, or after a" +echo " native dependency or app.json change." +echo + +exec npx expo start --dev-client diff --git a/app/(tabs)/_layout.tsx b/app/(tabs)/_layout.tsx index 3d5c4f0..f95ca72 100644 --- a/app/(tabs)/_layout.tsx +++ b/app/(tabs)/_layout.tsx @@ -8,6 +8,7 @@ import { BarChart3, Disc3, ListMusic, SlidersHorizontal } from 'lucide-react-nat import { useTranslation } from 'react-i18next'; import { View } from 'react-native'; +import { Toaster } from '@/components/ui/Toaster'; import { MiniPlayer } from '@/features/player/components/MiniPlayer'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { useTheme } from '@/theme/useTheme'; @@ -23,6 +24,16 @@ function TabBarWithPlayer(props: BottomTabBarProps) { useLifecycleTrace('TabBar'); 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 + an offset large enough to clear both the mini player and the tab bar — + and those heights are not design-system spacing values, so expressing + them would have meant either an arbitrary class (which the Tailwind + config correctly compiles to nothing) or a magic number in a style prop. + Stacking solves it with neither. + */} + diff --git a/app/_layout.tsx b/app/_layout.tsx index eaefe9a..10b4db4 100644 --- a/app/_layout.tsx +++ b/app/_layout.tsx @@ -63,6 +63,7 @@ export default function RootLayout() { + ); } diff --git a/app/collection/[kind]/[id].tsx b/app/collection/[kind]/[id].tsx new file mode 100644 index 0000000..9110396 --- /dev/null +++ b/app/collection/[kind]/[id].tsx @@ -0,0 +1,13 @@ +import { useLocalSearchParams } from 'expo-router'; + +import { CollectionDetailScreen } from '@/features/library/CollectionDetailScreen'; + +/** One artist or one album. Route files read params and render, nothing else. */ +export default function CollectionRoute() { + const { kind, id } = useLocalSearchParams<{ kind: string; id: string }>(); + const numericId = Number(id); + + if (!Number.isFinite(numericId) || (kind !== 'artist' && kind !== 'album')) return null; + + return ; +} diff --git a/docs/01-TECH-STACK.md b/docs/01-TECH-STACK.md index 8b961e3..a0133ac 100644 --- a/docs/01-TECH-STACK.md +++ b/docs/01-TECH-STACK.md @@ -30,7 +30,7 @@ These are the four choices that shape everything else. Read §2 before locking t | 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. | | Key-value | **`react-native-mmkv`** | Settings, theme, last queue snapshot. Synchronous → no theme flash on boot. | -| State | **Zustand** | Player/UI state only. Persisted data lives in SQLite, not in a store. | +| State | **`useSyncExternalStore`** against module singletons | No state library. Player/UI state only, held by `AudioEngine` and friends. Persisted data lives in SQLite, not in a store. | | Lists | **`@shopify/flash-list`** | Required — a 10k-track library will not survive `FlatList`. | | Images | **`expo-image`** | Built-in memory + disk cache, `recyclingKey`, blurhash placeholders. | | Animation | **`react-native-reanimated`** + **`react-native-gesture-handler`** | Mini-player → full-player transition, swipe-to-queue. | diff --git a/docs/adr/010-scanning-is-user-initiated.md b/docs/adr/010-scanning-is-user-initiated.md new file mode 100644 index 0000000..377ff56 --- /dev/null +++ b/docs/adr/010-scanning-is-user-initiated.md @@ -0,0 +1,77 @@ +# 010 — Scanning is user-initiated + +## Context + +Until now the library scan started itself. `useScan` fired a MediaStore sweep +from an effect shortly after the Library screen mounted, gated only on the audio +permission being granted. The user never asked for it and was never told it was +happening. + +That produced the report this ADR follows from: *"automatic detection freezes +the whole app."* Two separate problems were hiding behind each other. + +The first was a real performance defect and is fixed independently: stage one +wrote rows with five awaited queries per track inside a loop over a 500-row page, +holding the JS thread for 859ms at a time. That is fixed in `saveEnumerated`, +measured at 20–31ms per block afterwards, and would have been worth fixing +whether or not the scan was automatic. + +The second is not a performance problem at all. Even at 20ms blocks, an +unannounced scan is the app deciding on the user's behalf to read every audio +file on the device, immediately, on launch. This app's entire proposition is that +it does not do things behind your back. Reading the whole filesystem quietly is a +strange thing for it to make an exception for. + +There is also no good moment for it to be automatic. A scan is cheap when the +library has not changed and expensive when it has, and the app cannot tell which +it is facing without doing the expensive part first. + +## Decision + +**Nothing scans unless the user presses something.** + +- The automatic launch sweep is removed. There is no code path that begins a + MediaStore enumeration without a press. +- A **Scan** button lives permanently in the Library header — not only in the + empty state, because a user who copies an album across next month needs to + reach it without emptying their library first. +- Pressing it opens a confirmation that says what will happen: every indexed + audio file will be read, a large library takes a while, it can be stopped, and + whatever was found is kept. No progress estimate is promised, because + MediaStore does not report a count until it has been asked. +- The scan banner's **Stop** is wired to the scanner's existing cancellation, and + cancellation is checked at the top of every batch in both stages. +- The two manual routes are untouched and remain independent: the folder picker + and pull-to-refresh both go through the same pipeline and neither depends on + the sweep. + +The empty state now offers **Scan** as its action rather than the folder picker, +because scanning is the answer for most people and picking a folder is the answer +for the ones MediaStore fails. + +## Consequences + +A first-run user sees an empty library with an explicit invitation instead of a +library that fills itself. That is one extra tap, and it buys an app that never +reads the user's files without being asked. + +Launch is cheaper, but not because of anything clever: an already-scanned library +is already in SQLite, and the list paints from the database with no MediaStore +involvement at all. The sweep was never needed to show the library — only to +notice changes to it. + +The cost is that new files are not discovered on their own. A user who adds music +and does not scan will not see it. Pull-to-refresh and the always-present button +are the mitigation, and the empty state and the folder picker both remain as +routes in. This is a deliberate trade: silent staleness is a smaller failure than +silent work. + +One thing this does not change: `retireUnseen` still only runs after a +*complete* enumeration, so a cancelled scan never retires anything. Stopping a +scan halfway cannot make tracks disappear. + +## References + +- `docs/performance.md` — the before/after numbers for the freeze itself. +- ADR 007 — why picked folders go through MediaStore rather than a tree walk. +- ADR 008 — why the permission is asked for rather than assumed. diff --git a/docs/adr/011-repeat-listen-detection.md b/docs/adr/011-repeat-listen-detection.md new file mode 100644 index 0000000..561fb43 --- /dev/null +++ b/docs/adr/011-repeat-listen-detection.md @@ -0,0 +1,94 @@ +# 011 — A repeated track is a second listen + +## Context + +ADR 005 settled what counts as a play: `min(30s, duration × 0.5)` of actual +playback, with `skip` below `duration × 0.2` and `partial` in between. **That +rule is not reopened here and none of its numbers change.** + +It answers a different question from the one this ADR is about. ADR 005 decides +whether *a* listen counted. It says nothing about where one listen ends and the +next begins, because until now that was never in doubt: the engine closed a +listen when the loaded track changed, and a track only changed when playback +moved on. + +Repeat-one breaks that assumption completely. A song looped all afternoon never +changes the loaded track, so it produced exactly one `play_event`. The same is +true of dragging the scrubber back to the start and listening again. The +statistics were not wrong about what counted — they were wrong about how many +times it happened, which for anyone who loops their favourites is the more +visible error. + +## Decision + +**A track that starts over, having already earned a play, ends its listen and +begins a new one.** + +Two conditions, checked on every status tick against the previous one: + +1. The current listen has already accumulated at least the ADR 005 play + threshold. A listen that has not counted yet has nothing worth banking. +2. The position jumped backwards to at or below **25% of the track**. + +Both a loop to zero and a manual drag to the beginning satisfy the second +condition. A nudge back over the last chorus does not. + +The rule lives in `src/services/stats/repeatListen.ts` as a pure function, and +its thresholds are pinned by tests — this decides whether a listen is counted +once or twice, so a quiet change to either condition silently rewrites the +user's history. + +### Why the first condition + +Without it, scrubbing around inside the first thirty seconds of a track would +shatter one listen into a dozen fragments, each too short to count as anything. +A real play would be recorded as a pile of skips. Requiring the listen to have +already counted means the only thing a rewind can do is *add* a listen, never +subtract one. + +### Why 25% + +The number has to separate two things that look identical from the outside — +"start it again" and "go back a bit" — and the only signal available is how far +back the position went. + +A tighter bound would count a scrub back over the final chorus as a replay. A +looser one would miss a genuine restart on a track someone had nearly finished. +A quarter means the listener has given up at least three quarters of their +progress, which is a decision rather than an adjustment. + +### What is deliberately not required + +The new listen does **not** have to pass the play threshold for the boundary to +fire. The boundary fires on the rewind; the new listen is then classified by the +ordinary ADR 005 rule when *it* ends. + +This falls out better than the alternative. Someone who rewinds and then leaves +gets a `play` for what they heard and a `skip` for what they abandoned — both +true — rather than having the abandoned fragment silently merged into the +completed play. + +## Consequences + +A track looped three times produces three `play_events` and three increments of +`play_count`, which is what a listener means when they say they played something +three times. + +`startedAt` is reset to the moment the new listen begins, not left at the +original. Period keys derive from when a listen started, so a loop that runs +across midnight puts its two halves in the right days. + +Existing history is unaffected. This changes how future listens are segmented +and rewrites nothing already recorded, so the counts for a track someone looped +last week stay as they were. + +One cost worth naming: the engine now keeps `lastPositionMs` between status +ticks, which is playback state existing solely for a statistics feature. It is +one integer and the alternative — having the recorder subscribe to position at +2 Hz and reconstruct the sequence itself — would be worse in every way. + +## References + +- ADR 005 — the play/skip/partial thresholds, unchanged by this. +- `docs/stats.md` — the recording pipeline, with a section on this. +- `src/services/stats/repeatListen.test.ts` — the pinned thresholds. diff --git a/docs/adr/012-artist-and-album-shelves.md b/docs/adr/012-artist-and-album-shelves.md new file mode 100644 index 0000000..70c907f --- /dev/null +++ b/docs/adr/012-artist-and-album-shelves.md @@ -0,0 +1,80 @@ +# 012 — Artist and album shelves, and no genre shelf + +## Context + +The library had one face: an alphabetical list of every track. The tech stack +doc calls for "Tracks / Albums / Artists / Genres" segments, and until now only +the first existed. Nothing in the schema was missing — `artists` and `albums` +have been populated by the scanner since Phase 2 — so this was a query and a +screen, not a data problem. + +## Decision + +**Three segments: Tracks, Artists, Albums.** Artists and albums render as a +two-column grid of square cards; tapping one opens a detail screen with its +tracks, a Play button and a Shuffle button. + +The detail screen reuses `LibraryTracks` wholesale rather than growing a second, +thinner track list. A track on an album screen therefore has exactly the same +verbs as a track in the library — swipe to queue, long-press for the sheet, +multi-select — and there is one place to change them. Two lists would have +drifted within a release. + +Playing from a shelf attributes the listen to that artist or album via +`QueueSource`, which is what puts rows under those entity types in +`stats_rollups`. + +### No genre shelf + +The fourth segment is deliberately absent. + +Genre reaches us from MediaStore, and on real libraries it is close to useless: +ripped files frequently carry none, files from different sources disagree on +spelling and case ("Hip-Hop", "hip hop", "HipHop" are three genres), and a large +fraction of any collection lands in one bucket called Unknown. A shelf whose +biggest card is "Unknown" and whose next four are spellings of the same word is +not a way to find music — it is a way to discover that your tags are a mess. + +The data is still there and still indexed. If a genre view earns its place +later, it needs normalisation — case folding, separator handling, an alias table +— and that is a feature with a design, not a fourth `useLiveQuery`. + +### Search stays on tracks + +The search field is hidden on the artist and album segments. Filtering a grid of +a few dozen cards is not the problem search solves, and a box that silently does +nothing to the current view is worse than no box. + +## Consequences + +`LibraryScreen` had reached exactly the 300-line limit `AGENTS.md` sets, so it +was split before anything was added: it now owns the *library* — scanning, +searching, which view is showing — and `LibraryTracks` owns *tracks*. The split +was forced by a line count and is the right boundary regardless. + +Album covers come from a track, not from `albums.artwork_path`. That column +exists in the schema and the scanner never fills it, because artwork is +extracted per file. The stats queries resolve it with a correlated subquery +bounded by the result limit; the card queries use `min(artwork_path)` over the +group, which is arbitrary but stable — and stable matters more than which, +because a card whose cover changes between renders looks broken. + +Both card queries `INNER JOIN` tracks rather than starting from `artists`. The +artist table gains a row the first time a name is seen and nothing ever removes +one, so a left join would list artists with zero tracks. What is on the device is +what the library shows. + +## Postscript: a class that compiled to nothing + +Building this surfaced a bug worth recording because it had already shipped in +several places. `tailwind.config.js` **overrides** the spacing scale, so a class +built from a value outside it produces no CSS — no warning, no error, no size. + +`h-32 w-32` on the detail cover meant the artwork drew at zero by zero and was +simply absent. The same class was already doing the same thing to the playlist +mosaic. `w-24` had left the swipe-to-queue reveal strip with no width, so its +icon had never been visible, and `h-7 w-7` had done it to a checkbox. + +`AGENTS.md` names this trap and it caught us anyway, because the failure is +silent. `src/theme/scale.test.ts` now fails on any spacing class outside the +scale, which is the only thing that turns silence into noise. diff --git a/docs/architecture.md b/docs/architecture.md new file mode 100644 index 0000000..9b643ce --- /dev/null +++ b/docs/architecture.md @@ -0,0 +1,154 @@ +# Architecture + +How Mufify is put together, and why it is put together that way. The per-topic +documents go deep on individual subsystems — this one is the map that says +which subsystem you want and how they meet. + +Mufify is an offline-only Android music player: React Native on Expo SDK 57, +TypeScript in strict mode, SQLite for everything persistent, and two small +Kotlin modules for the things the JS side cannot see. Release builds ship +without the `INTERNET` permission, which is not a setting but a structural +guarantee — `plugins/withOfflineOnly.js` strips it, and +[ADR 009](adr/009-expo-audio-and-our-own-queue.md) records how both merged +manifests were generated to verify that the release one really does go without +it. + +## The shape + +``` +app/ routes only — read params, render a screen, nothing else +src/ + components/ui/ shared presentational components + features/ one directory per feature: screens, components, hooks + services/ pure logic — shuffle, stats, scanner, formatters + db/ schema, migrations, and the only place Drizzle is imported + theme/ design tokens, in exactly two files + i18n/ en.json and tr.json, kept in step by a test +modules/ local native modules (Kotlin) +``` + +Four rules. Each is a bug rather than a preference when violated, because each +one has a failure that follows from breaking it: + +1. **Only `src/services/audio/*` imports the audio library.** `AudioEngine.ts` + is the sole file that names an `expo-audio` symbol. RNTP v4 is frozen and + pre-New-Architecture and RNTP v5 is commercially licensed while this app + ships MIT, so the engine choice may genuinely have to be revisited. Behind + a facade that is a one-file change. +2. **Only `src/db/queries/*` imports Drizzle or expo-sqlite.** A component that + builds its own query is a component that cannot be tested without a + database, and a schema change becomes a repo-wide search. +3. **No business logic in component bodies.** If it can be decided without + rendering, it belongs in `services/` where a plain Jest test reaches it. This + is why the shuffle algorithms, period keys and repeat-listen detection have + real test coverage: none of them needs a device. +4. **Layers point downward:** `components → hooks → services → db`. Nothing + below reaches up. + +## Where state actually lives + +This is the part that surprises people, because there is no global state +library. State lives in three places, chosen by lifetime: + +**SQLite — everything persistent.** Tracks, playlists, play events, rollups. +Opened once in [client.ts](../src/db/client.ts) with WAL, `synchronous=NORMAL`, +foreign keys on, and `enableChangeListener: true`. That last flag is what makes +Drizzle's `useLiveQuery` react to writes, so lists refresh themselves and no +screen needs manual invalidation. WAL is why the library stays scrollable while +a scan writes — and why pulling the database off a device without the `-wal` +file shows an empty `play_events`. + +**The `AudioEngine` singleton — playback.** Deliberately not a hook and not +React state: playback outlives every screen, which is the entire point of +background audio, so it cannot be owned by a component that unmounts when the +user opens Settings. Screens subscribe through `useSyncExternalStore` in +[src/features/player/hooks](../src/features/player/hooks) — the right primitive +for a store that lives outside React, and one that gets tearing right during +concurrent renders where a hand-rolled subscription does not. Note that +`usePlayback` re-renders twice a second while anything plays, because the +engine reports position on a 500 ms interval; `usePlaybackPhase` and +`useCurrentTrack` exist so that only Now Playing pays that cost. + +**MMKV — settings and theme.** Synchronous reads, which is the whole reason it +is here rather than AsyncStorage: `applyStoredTheme()` and `initI18n()` run at +module scope in [app/_layout.tsx](../app/_layout.tsx), before the first frame, +so the app never paints the wrong theme or the wrong language and then corrects +itself. + +## Startup, in order + +The root layout's ordering is load-bearing rather than incidental: + +1. `applyStoredTheme()` and `initI18n()` at module scope — synchronous MMKV + reads, before anything paints. +2. `registerComponentInterop()` — NativeWind's `className` only works on + components registered with `cssInterop`. A component registered late has + already painted itself unstyled. +3. Splash is held (`preventAutoHideAsync`) until **both** fonts and migrations + settle. Rendering earlier means a frame in the fallback font, or a screen + querying a schema that has not been migrated yet. +4. `startListenRecording()` — for the life of the app, not the life of a + screen. +5. `GestureHandlerRootView` wraps the tree. Gesture-handler throws rather than + silently ignoring gestures without it. + +Migrations are generated by `npm run db:generate` and committed, never derived +at runtime, so a build always knows exactly which schema it expects. + +## Two flows worth tracing + +**Scanning** is two stages on purpose, and the split is what makes it +resumable. `enumerateLibrary` walks MediaStore and writes cheap rows; +`enrichLibrary` opens each file for tags, artwork and format. `last_scanned_at` +is null until stage two has run, so **the null column is the queue** — a +cancelled or killed scan resumes from exactly where it stopped, with no +separate progress state to keep honest. `needsRescan` compares size and mtime, +which is what makes rescanning an untouched library near-instant. Details in +[scanner.md](scanner.md). + +**A listen becoming a statistic** crosses a boundary deliberately. The engine +reports that a listen finished and knows nothing about `play_events`, rollups +or week-start preferences; [listenRecorder.ts](../src/features/player/listenRecorder.ts) +does that wiring. Playback stays testable without a database, and the layer +direction stays pointing down. A failed write is logged in development and +swallowed in production — losing one statistics row is a smaller harm than +interrupting playback to complain about it. Details in [stats.md](stats.md). + +## The native boundary + +Two local Expo modules, both Kotlin, both Android-only: + +- **`audio-tags`** — MediaStore enumeration, tag reading, artwork extraction + and audio format. It exists because `MediaMetadataRetriever` reports no + sample rate and no bit depth below API 31, so a hi-res library on an Android + 10 phone lost exactly the fields this app exists to show; + `AudioFormatReader` reads them from `MediaExtractor`, which has carried them + since API 16. `SpecMath.kt` holds the arithmetic with no Android import at + all, so it runs as a plain JVM unit test — no device, no emulator. +- **`audio-focus`** — becoming-noisy events (headphones unplugged). + +`android/` is generated by CNG and git-ignored. Native configuration goes in +`app.json` or a config plugin, never into the generated directory. + +## Adding something + +- **A screen** → a route in `app/` that only reads params and renders, plus the + real component under `src/features//`. New routes need Metro running + to regenerate `.expo/types/router.d.ts`. +- **A query** → `src/db/queries/`, nowhere else. +- **A decision you had to think about** → an ADR in [adr/](adr/). Twelve of them + exist; several are the only written record of why an obvious-looking + alternative was rejected. +- **A colour or a spacing value** → [theming.md](theming.md) first. + `tailwind.config.js` overrides the spacing scale, and a class built from a + value outside it compiles to *nothing* — no warning, no size. `scale.test.ts` + now fails on any such class, which is the only reason it is safe to guess. + +## Further reading + +[AGENTS.md](../AGENTS.md) is binding house style and comes before all of these. +Then [database.md](database.md), [scanner.md](scanner.md), +[player.md](player.md), [shuffle.md](shuffle.md), [stats.md](stats.md), +[theming.md](theming.md), [i18n.md](i18n.md), +[components.md](components.md), [performance.md](performance.md), and +[adr/](adr/). diff --git a/docs/components.md b/docs/components.md new file mode 100644 index 0000000..837d915 --- /dev/null +++ b/docs/components.md @@ -0,0 +1,116 @@ +# Components + +What each component is for, and the ones with a reason worth knowing. + +Every exported component already carries a JSDoc saying what it does; this is +the map, not a duplicate of it. Where a component has a non-obvious constraint, +it is repeated here because that is the thing a reader needs before touching it. + +## Rules that shape all of them + +- **Under 300 lines.** Over means it should be two components. Enforced in + review, not aspirational — `LibraryScreen` was split at exactly 300. +- **One component per file**, file named for the component. +- **NativeWind only.** No `StyleSheet.create`, no inline style objects, except + for Reanimated animated styles and native components NativeWind cannot reach + into (the platform `Switch`). +- **Semantic classes only** — `bg-surface`, `text-muted`, `border-subtle`. The + Tailwind config *overrides* rather than extends the scales, so anything + outside the design system compiles to nothing at all. `src/theme/scale.test.ts` + fails on spacing values outside the scale, because five such classes shipped + invisible before it existed. +- **Every screen has its empty, loading and error state** in the same commit as + the happy path. + +--- + +## `components/ui` — shared + +| Component | What it is for | +|---|---| +| `Screen` | The standard frame: safe area, surface, display-face title. | +| `EmptyState` | Icon, one line, and the way out. Picks one of several phrasings per mount so the app does not read like a recording. | +| `ErrorState` | What failed in one plain sentence, and the retry. Never a raw error string. | +| `Skeleton` | One placeholder block, sized by the caller. Pulses via a Reanimated worklet; stops dead under reduce-motion. | +| `SkeletonRows` | A list's worth, shaped like real rows so nothing jumps when data lands. Hidden from screen readers. | +| `SkeletonCards` | The same for the artist and album grids. Mirrors `CollectionGrid`'s two-column layout exactly. | +| `SegmentedControl` | Two to four short, self-evident choices on one line. `perRow` wraps when there are more. | +| `OptionList` | One choice per row with a sentence explaining it. The right control when the names need explaining — which is why shuffle uses it and theme does not. | +| `SettingGroup` / `SettingRow` / `SettingSwitch` | A titled block, a labelled row with a description line, and an on/off row. Every setting gets a description; a list of bare names makes the user guess. | +| `ActionSheet` | A sheet of actions for one thing. Closes before running the action, so the sheet never lingers while a track loads. | +| `ConfirmDialog` | Asks before something slow or irreversible. The confirm button names the action — never "OK". | +| `SwipeableRow` | Reveals one action when dragged left. **Transient by design**: it never stays open, because this lives in a recycling list and a row holding open state gets recycled with it. | +| `Toaster` | Where transient confirmations appear. Reads a module-level store, so a toast re-renders this and nothing else. Sits above the mini player in the tab bar stack. | +| `ProgressBar` | A determinate bar. Used by the scan banner. | + +--- + +## `features/library` + +| 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`. | +| `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. | +| `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. | +| `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`. | + +--- + +## `features/player` + +| Component | What it is for | +|---|---| +| `PlayerScreen` | Now Playing. A modal route: somewhere you go from something and dismiss. | +| `ArtworkCarousel` | Three slots — previous, current, next — all mounted, so a neighbour slides in already decoded. Commits on distance **or** velocity. Rubber-bands at the ends of the queue. | +| `MiniPlayer` | The persistent transport strip. Subscribes to phase and track only, never position: doing otherwise reconciled it 20 times per ten seconds of playback. | +| `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. | + +--- + +## `features/playlists` + +| Component | What it is for | +|---|---| +| `PlaylistsScreen` / `PlaylistRow` | The list, and one playlist in 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. | + +--- + +## `features/stats` + +| Component | What it is for | +|---|---| +| `StatsScreen` | Reads `stats_rollups` only. Aggregating `play_events` here would be a scan over the whole history on every tab switch, growing forever. | +| `Wrapped` | The period in one card, leading with the listening time. Deliberately not a gradient, a collage, or a share sheet — a screenshot is already the share mechanism. | +| `StatTotals` | The three headline tiles. | +| `TopList` | A ranked list. Every row carries both the count and the duration, because they disagree constantly. Renders nothing when empty. | + +--- + +## `features/settings` + +| Component | What it is for | +|---|---| +| `SettingsScreen` | Every setting, each with a line saying what it does. | +| `ScanFolderList` | Folders the user added by hand, on top of what MediaStore indexes. | +| `DevTools` | The stress-library seeder and query timer. `__DEV__` only, and its strings are deliberately not translated — none of it ships. | diff --git a/docs/performance.md b/docs/performance.md index 3bd891d..e54c499 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -143,6 +143,64 @@ rows. Included because it is the only way to reproduce any of the above. --- +## Transitions, modals and lists + +The complaint was that screen transitions, the drawer and modal openings feel +slow. Taken as a checklist, with a measurement or a source-level fact for each. + +### Is anything animating on the JS thread? + +No. Every animation in the app goes through Reanimated worklets: `Scrubber`, +`MiniPlayer`, `MiniProgress`, `ArtworkCarousel`, `SwipeableRow`, +`ReorderableEntry`, `Skeleton` and `Toaster`. There is no `Animated` import from +`react-native`, no `LayoutAnimation`, and no `setInterval` or +`requestAnimationFrame` driving a visual anywhere in `src` or `app`. + +Screen transitions themselves are react-navigation's, which run natively through +`react-native-screens`. + +### Are list rows memoized with stable callbacks? + +Yes, and it is measured rather than asserted. Counting `LibraryRow` body +executions on the Pixel_7 AVD: + +| | Before | After | +|---|---|---| +| Rows reconciled per checkbox tap | **47** | **1** | + +The fix is in `46d07a3`; the short version is that four separate things were +handing the list a new identity on every render — the selection object, the +screen's callbacks, `selection.has` per row, and an unmemoized `Gesture.Pan()`. + +### Does opening a modal re-render what is underneath? + +No. Long-pressing a row to open the action sheet reconciles **1** of the 47 +visible rows — the one that was pressed. The sheet's state lives in +`LibraryTracks`, and `TrackList`'s `renderItem` does not depend on it, so +FlashList never calls it again. + +### Is there an over-broad context? + +There is no React context in the app at all. Everything shared between screens — +playback state, the queue, toasts — is a module-level store read through +`useSyncExternalStore`, so a change notifies only the components that subscribed +to it. The toast store is the clearest case: a toast confirming a swipe +re-renders `Toaster` and nothing else, which is why it can appear over a list +without touching it. + +### expo-image + +All nine call sites pass both `cachePolicy` and `recyclingKey`. + +### A silent layout bug found on the way + +`tailwind.config.js` overrides the spacing scale, so a class built from a value +outside it compiles to nothing — no warning, no size. Five had already shipped +invisible, including the swipe-to-queue reveal strip, whose icon had therefore +never been seen by anyone. `src/theme/scale.test.ts` now fails on any such class. + +--- + ## Still owed - **Frame timing on the Mi 9T.** `adb shell dumpsys gfxinfo dev.mufify.app @@ -154,4 +212,96 @@ rows. Included because it is the only way to reproduce any of the above. dev-mode React, no precompiled bytecode for lazily loaded modules. Debug overstates JS cost substantially, so the cold-start figure in particular should be re-taken against a release build before it is treated as what a user - experiences. + experiences. The release build now *runs* (see above) but has not been + measured — functional smoke test only. +- **Artwork and artists at scale.** Both emulator and phone libraries are + synthetic files with no artist and, apart from two, no artwork. The artist + shelf and the artwork cache have never met a real library. + +--- + +## Regression pass + +Run after the three critical sections closed, on the Pixel_7 AVD, with both +gates green — `lint`, `typecheck`, 292 JS tests across 20 suites, and +`:audio-tags:testDebugUnitTest` forced with `--rerun-tasks`. + +| Area | Result | +|---|---| +| Theme, light ↔ dark | Both render correctly on every screen. Dark uses its own lighter indigo and dark-on-accent text, as the tokens require. | +| Language, en ↔ tr | Every string translated, tabs included. No raw keys, no English left in the Turkish build. | +| Library, Playlists, Stats, Settings | All four render clean. No JS error or warning in logcat across the sweep. | +| Scanning | Button, confirmation, skeleton, cancel wiring. An unchanged rescan measures 20 ms and 1 ms for its two pages. | +| Playback | Play, carousel, mini player, swipe up and down, queue attribution. | +| Shuffle | Selection persists and `play_events.shuffle_algorithm` records it. | +| Statistics | Wrapped, tiles and all four ranked lists, with covers and durations. | + +~~**Not exercised end to end:** the playlist create → add tracks → play flow.~~ +Closed on 2026-08-01 — see below. + +--- + +## Device verification, 2026-08-01 + +Pixel_7 AVD, API 35, rebooted first because the previous session's flaky taps +were an emulator that had been up nine hours rather than a real fault. Every +control in this app carries an `accessibilityLabel`, so the whole pass was +driven from `uiautomator dump` by label instead of guessed pixel coordinates — +which is what made it reproducible where the earlier attempt was not. + +**The playlist chain, end to end.** Create → name → add 3 tracks → drag-reorder +→ play, in one pass. The reorder needed a real `motionevent` DOWN / hold / +MOVE / UP sequence, because `ReorderableEntry` uses +`Gesture.Pan().activateAfterLongPress(120)` and a plain `input swipe` never +activates it. Moving perf-001 from position 2 to 0 renumbered all three rows +correctly in `playlist_tracks`. + +`QueueSource` reaches the database: `play_events` recorded `source_type=playlist, +source_id=1, ms_played=6089, completed=1`, and `stats_rollups` gained three +matching rows — `week 2026-W31`, `month 2026-08`, `year 2026`, each +`playlist/1, play_count=1, ms_played=6089`. That path was unit-covered but had +never been seen on a device. + +**Repeat-listen seek-back.** MUSE - Cryogen, 5:10, threshold 30 s. Played to +3:00, dragged the scrubber to the start, played again, dragged back a second +time. Two distinct qualifying rows for the same track — `ms_played=211171` and +`ms_played=144145`, both `outcome=play` — which is exactly what +`isRewindToRestart` exists to produce. `track_stats` and all three rollup +periods agree at 4 plays / 464455 ms. + +**Background audio**, incidentally: playback continued through the app being +backgrounded to the launcher and was still at `state=PLAYING` on return. The +engine outliving every screen is the reason, and this is the first time it has +been watched. + +**The spec strip on a real MP3** reads `MP3 · 44.1 kHz · 138 kbps · Stereo · +5.1 MB`. Two things confirmed there: no codec field, because `codecOf` returns +null when the subtype is already the container name; and 138 kbps, which is +`SpecMath.bitrateKbps` computing from size and duration rather than trusting +the retriever's reported 32 — the exact file its doc comment describes. + +**One UX gap found.** `MiniPlayer` is rendered only in `app/(tabs)/_layout.tsx`, +so pushed stack screens have no transport control. Start playback from a +playlist detail screen and there is no visible player and no route to Now +Playing without going back to a tab. It follows from the routing structure +rather than being a defect, but it is a real gap. + +--- + +## Release build, first run ever + +`app-release.apk`, 133 MB universal, installed on the Pixel_7 AVD with +`adb reverse --remove-all` first, so nothing could quietly fall back to Metro. + +- **No `INTERNET` permission**, confirmed with `aapt2 dump permissions` before + installing. `ACCESS_NETWORK_STATE` is present and grants no network access + without it, as ADR 009 records. +- Boots standalone, no crash, no red box, no `Unable to load script`. +- All four tabs render under Hermes and minification: Library's empty state, + Playlists, Stats with its Week/Month/Year segments, and Settings with theme, + language including `Türkçe`, and all five shuffle algorithms with their + descriptions. Lucide SVG icons and all three fonts load. + +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. diff --git a/docs/stats.md b/docs/stats.md index 5e91850..699ba14 100644 --- a/docs/stats.md +++ b/docs/stats.md @@ -60,10 +60,66 @@ Non-positive duration or `ms_played` is `partial`. Settled in `docs/adr/005-play-skip-partial.md`. Not reopening it: every rollup will depend on it. -### Not yet implemented +--- + +## Repeated listens + +`src/services/stats/repeatListen.ts`, decided in +`docs/adr/011-repeat-listen-detection.md`. A layer **on top of** the counting +rule above, which is unchanged. + +The rule above answers whether *a* listen counted. It says nothing about where +one listen ends and the next begins, because until playback existed that was +never in doubt: the engine closed a listen when the loaded track changed. + +Repeat-one breaks that completely. A song looped all afternoon never changes the +loaded track, so it produced exactly one `play_event`. So did dragging the +scrubber back and listening again. The counts were not wrong about what +qualified — they were wrong about how many times it happened, which for anyone +who loops their favourites is the more visible error. + +### The rule + +A listen ends and a new one begins when **both** hold, checked on each status +tick against the previous one: + +1. The current listen has already earned a play — at least `min(30s, duration × + 0.5)` of playback. +2. The position jumped **backwards to at or below 25% of the track**. + +A loop to zero and a manual drag to the start both satisfy the second. A nudge +back over the last chorus does not. + +### Why each condition + +**The earned-a-play requirement** is what stops seeking from shredding history. +Without it, scrubbing around inside the first thirty seconds would split one +listen into a dozen fragments, each too short to count as anything — turning a +real play into a pile of skips. With it, a rewind can only ever *add* a listen. + +**25%** has to separate two things that look identical from outside: "start it +again" and "go back a bit". The only available signal is how far back the +position went. Tighter would count a scrub over the final chorus as a replay; +looser would miss a genuine restart on a nearly-finished track. A quarter means +at least three quarters of the progress was given up, which is a decision rather +than an adjustment. + +### What the boundary deliberately does not check + +That the *new* listen also passes the play threshold. The boundary fires on the +rewind; the new listen is then classified by the ordinary rule when it ends. + +This falls out better than the alternative. Rewind and then leave, and you get a +`play` for what you heard plus a `skip` for what you abandoned — both true — +rather than the abandoned fragment silently merging into the completed play. + +`startedAt` resets to the moment the new listen begins. Period keys come from +when a listen started, so a loop running across midnight puts its halves in the +right days. -Seeking backwards must not create a second event. That is a recorder concern -and lands with playback in Phase 3. +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. --- diff --git a/eslint.config.js b/eslint.config.js index ca4d480..aca4ab3 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -27,7 +27,10 @@ const RESTRICTED_IMPORTS = [LAYER_DIRECTION, DATABASE_BOUNDARY, AUDIO_BOUNDARY]; module.exports = defineConfig([ expoConfig, { - ignores: ['dist/*', 'android/*', 'ios/*', '.expo/*'], + // `.claude/worktrees/*` holds full checkouts of other branches. Linting + // them reports the root-level src/ exemptions as violations, because the + // path-based rules below only match src/ at the repo root. + ignores: ['dist/*', 'android/*', 'ios/*', '.expo/*', '.claude/**'], }, { files: ['**/*.test.ts', '**/*.test.tsx', 'jest.setup.js'], diff --git a/jest.config.js b/jest.config.js index 11b33c2..bd66324 100644 --- a/jest.config.js +++ b/jest.config.js @@ -5,6 +5,10 @@ module.exports = { transformIgnorePatterns: [ 'node_modules/(?!((jest-)?react-native|@react-native(-community)?)|expo(nent)?|@expo(nent)?/.*|@expo-google-fonts/.*|react-navigation|@react-navigation/.*|@sentry/react-native|native-base|react-native-svg|nativewind|react-native-css-interop)', ], + // Overriding this drops the default, so /node_modules/ is repeated here. + // `.claude/worktrees/*` holds full checkouts of other branches; without + // this, every suite there is collected a second time. + testPathIgnorePatterns: ['/node_modules/', '/\\.claude/'], // AGENTS.md: real coverage on services/, not on components. collectCoverageFrom: ['src/services/**/*.ts', 'src/utils/**/*.ts'], }; diff --git a/package-lock.json b/package-lock.json index de02cf0..281e0cf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42,8 +42,7 @@ "react-native-safe-area-context": "~5.7.0", "react-native-screens": "~4.26.0", "react-native-svg": "15.15.4", - "react-native-worklets": "0.10.1", - "zustand": "^5.0.14" + "react-native-worklets": "0.10.1" }, "devDependencies": { "@testing-library/react-native": "~14.0.1", @@ -18144,35 +18143,6 @@ "peerDependencies": { "zod": "^3.25.0 || ^4.0.0" } - }, - "node_modules/zustand": { - "version": "5.0.14", - "resolved": "https://registry.npmjs.org/zustand/-/zustand-5.0.14.tgz", - "integrity": "sha512-/8tAspM5LMPr28b3fwLYrtdj77ECpfZviaP75CMTnwO8ISyaE4GDIG/9rDDYq/cH9D2Xw2A2RXglLInmVBQB/g==", - "license": "MIT", - "engines": { - "node": ">=12.20.0" - }, - "peerDependencies": { - "@types/react": ">=18.0.0", - "immer": ">=9.0.6", - "react": ">=18.0.0", - "use-sync-external-store": ">=1.2.0" - }, - "peerDependenciesMeta": { - "@types/react": { - "optional": true - }, - "immer": { - "optional": true - }, - "react": { - "optional": true - }, - "use-sync-external-store": { - "optional": true - } - } } } } diff --git a/package.json b/package.json index b187a39..935bc7f 100644 --- a/package.json +++ b/package.json @@ -47,8 +47,7 @@ "react-native-safe-area-context": "~5.7.0", "react-native-screens": "~4.26.0", "react-native-svg": "15.15.4", - "react-native-worklets": "0.10.1", - "zustand": "^5.0.14" + "react-native-worklets": "0.10.1" }, "devDependencies": { "@testing-library/react-native": "~14.0.1", diff --git a/src/components/ui/ActionSheet.tsx b/src/components/ui/ActionSheet.tsx index 4dc17eb..f7e0d6d 100644 --- a/src/components/ui/ActionSheet.tsx +++ b/src/components/ui/ActionSheet.tsx @@ -58,7 +58,7 @@ export function ActionSheet({ {/* Absorbs taps so pressing the sheet does not dismiss it. */} diff --git a/src/components/ui/ConfirmDialog.tsx b/src/components/ui/ConfirmDialog.tsx new file mode 100644 index 0000000..ca2a0ab --- /dev/null +++ b/src/components/ui/ConfirmDialog.tsx @@ -0,0 +1,88 @@ +import { useTranslation } from 'react-i18next'; +import { Modal, Pressable, Text, View } from 'react-native'; + +export interface ConfirmDialogProps { + visible: boolean; + /** Already translated. */ + title: string; + /** Already translated. What is about to happen, in one or two plain sentences. */ + body: string; + /** Already translated. Names the action rather than saying "OK". */ + confirmLabel: string; + /** Draws the confirm in red. For anything that destroys data. */ + destructive?: boolean; + onConfirm: () => void; + onCancel: () => void; +} + +/** + * Ask before doing something slow or irreversible. + * + * A `Modal` rather than `Alert.alert`, for the same reason `NamePlaylistDialog` + * is: the platform dialog cannot be themed, and a system-grey box in the middle + * of a dark hi-fi panel looks like a different application interrupted. + * + * The confirm button says what it will do — "Scan", "Delete history" — never + * "OK". A dialog whose buttons are "OK" and "Cancel" makes the user re-read the + * body to find out which one is safe. + */ +export function ConfirmDialog({ + visible, + title, + body, + confirmLabel, + destructive = false, + onConfirm, + onCancel, +}: ConfirmDialogProps) { + const { t } = useTranslation(); + + return ( + + + + {title} + {body} + + + + {t('common.cancel')} + + + + + {confirmLabel} + + + + + + + ); +} + +function absorb(): void { + // Intentionally empty: the press is absorbed rather than handled. +} diff --git a/src/components/ui/OptionList.tsx b/src/components/ui/OptionList.tsx new file mode 100644 index 0000000..7dca458 --- /dev/null +++ b/src/components/ui/OptionList.tsx @@ -0,0 +1,85 @@ +import { Check } from 'lucide-react-native'; +import { Pressable, Text, View } from 'react-native'; + +import { useThemeColors } from '@/theme/useTheme'; + +export interface Option { + value: T; + /** Already translated. */ + label: string; + /** Already translated. One line saying what this choice actually does. */ + description: string; +} + +export interface OptionListProps { + options: readonly Option[]; + value: T; + onChange: (value: T) => void; + /** Translated group label, for screen readers. */ + accessibilityLabel: string; +} + +/** + * One choice per row, each with a sentence explaining it. + * + * The alternative — a segmented control — is right when the options are + * self-explanatory and wrong the moment they are not. "Discovery" and + * "Favourites" are names, not descriptions, and the entire argument for offering + * five shuffle algorithms is that a user can tell them apart. A control that + * cannot fit the explanation is the wrong control. + * + * It also solves the layout problem honestly rather than by wrapping: five + * segments across a phone gave each about a fifth of the width, and Turkish runs + * 10–20% longer than English. A column has as much room as it needs in any + * language and at any font scale. + * + * The tick marks the selection rather than a filled background: with a + * description under every row, filling the selected one would put body text on + * an indigo panel and cost the contrast the tokens guarantee. + */ +export function OptionList({ + options, + value, + onChange, + accessibilityLabel, +}: OptionListProps) { + const colors = useThemeColors(); + + return ( + + {options.map((option) => { + const selected = option.value === value; + return ( + onChange(option.value)} + accessibilityRole="radio" + accessibilityState={{ selected }} + accessibilityLabel={option.label} + accessibilityHint={option.description} + className="min-h-11 flex-row items-start gap-3 py-2" + > + {/* Fixed-width gutter, so every title starts on the same column + whether or not it is the selected one. */} + + {selected ? : null} + + + + + {option.label} + + {option.description} + + + ); + })} + + ); +} diff --git a/src/components/ui/SettingRow.tsx b/src/components/ui/SettingRow.tsx index e69ffa6..43467bc 100644 --- a/src/components/ui/SettingRow.tsx +++ b/src/components/ui/SettingRow.tsx @@ -10,22 +10,55 @@ export interface SettingRowProps { /** Already translated. */ label: string; /** The current value, spelled out. Sits right, in the muted tone. */ - value: string; - /** The control itself — a segmented control, a switch, a link. */ - children: ReactNode; + value?: string; + /** + * One line saying what this setting does. Already translated. + * + * Not optional by preference — every row should have one. It is typed as + * optional only because a row whose control is itself self-describing (an + * `OptionList`, which explains each choice) would otherwise say the same thing + * twice. + */ + description?: string; + /** The control itself — an option list, a switch, a link. */ + children?: ReactNode; } -/** Label and current value on one line, with the control underneath. */ -export function SettingRow({ icon: Icon, label, value, children }: SettingRowProps) { +/** + * Label, current value, an explanation, and the control underneath. + * + * The explanation is the point of this component. A settings screen that lists + * only names makes the user guess, and the guesses are wrong in exactly the + * places that matter — nobody knows what "Discovery" does from the word alone. + */ +export function SettingRow({ + icon: Icon, + label, + value, + description, + children, +}: SettingRowProps) { const colors = useThemeColors(); return ( - - - - {label} - {value} + + + {/* Nudged down so the icon sits on the label's centre line rather than + the top of a block that may be two lines tall. */} + + + + + + {label} + {description ? ( + {description} + ) : null} + + + {value ? {value} : null} + {children} ); diff --git a/src/components/ui/SettingSwitch.tsx b/src/components/ui/SettingSwitch.tsx new file mode 100644 index 0000000..30e3f09 --- /dev/null +++ b/src/components/ui/SettingSwitch.tsx @@ -0,0 +1,58 @@ +import type { LucideIcon } from 'lucide-react-native'; +import { Switch, Text, View } from 'react-native'; + +import { useThemeColors } from '@/theme/useTheme'; + +export interface SettingSwitchProps { + icon: LucideIcon; + /** Already translated. */ + label: string; + /** Already translated. Says what turning it on actually changes. */ + description: string; + value: boolean; + onChange: (value: boolean) => void; +} + +/** + * A setting that is on or off. + * + * The platform `Switch` rather than a custom control: it is the one widget + * users recognise instantly, it already answers to the system font scale and to + * TalkBack, and reimplementing it would trade all of that for a marginally + * better fit with the panel. + * + * Its colours are props rather than classes — `Switch` is a native component and + * NativeWind cannot reach inside it — so they come from the same tokens the + * classes do, via `useThemeColors`. + */ +export function SettingSwitch({ + icon: Icon, + label, + description, + value, + onChange, +}: SettingSwitchProps) { + const colors = useThemeColors(); + + return ( + + + + + + + {label} + {description} + + + + + ); +} diff --git a/src/components/ui/Skeleton.tsx b/src/components/ui/Skeleton.tsx new file mode 100644 index 0000000..9092e46 --- /dev/null +++ b/src/components/ui/Skeleton.tsx @@ -0,0 +1,125 @@ +import { useEffect } from 'react'; +import { View } from 'react-native'; +import Animated, { + useAnimatedStyle, + useSharedValue, + withRepeat, + withTiming, +} from 'react-native-reanimated'; + +import { useReducedMotion } from '@/theme/useReducedMotion'; + +/** The pulse's quietest and loudest points. Deliberately a narrow range. */ +const DIM = 0.45; +const BRIGHT = 1; +/** One breath. Slow enough to read as waiting, not as a spinner. */ +const PULSE_MS = 900; + +export interface SkeletonProps { + /** + * Tailwind classes for the block's size and shape. + * + * Passed rather than derived, because a skeleton's whole job is to be the + * exact shape of the thing that will replace it — only the caller knows that. + */ + className: string; +} + +/** + * One placeholder block. + * + * `bg-surface-elevated` and nothing else. The tokens have no dedicated skeleton + * colour and do not need one: a skeleton is an empty panel, and the panel value + * is already the one step up from the surface that says "something goes here". + * + * The pulse is opacity in a Reanimated worklet, so it never touches the JS + * thread — a loading indicator that competes with the work it is indicating is + * worse than no indicator. It also stops entirely under reduce-motion, where a + * looping animation is exactly what the setting is there to prevent. + */ +export function Skeleton({ className }: SkeletonProps) { + const opacity = useSharedValue(BRIGHT); + const reducedMotion = useReducedMotion(); + + useEffect(() => { + if (reducedMotion) { + opacity.value = DIM; + return; + } + opacity.value = withRepeat(withTiming(DIM, { duration: PULSE_MS }), -1, true); + }, [reducedMotion, opacity]); + + const style = useAnimatedStyle(() => ({ opacity: opacity.value })); + + return ; +} + +export interface SkeletonRowsProps { + /** Enough to fill a screen. More would only animate off-frame. */ + rows?: number; + /** Matches the real row's height class, so nothing jumps when data lands. */ + rowClassName?: string; + /** Drawn on the left: artwork, a checkbox, a rank. */ + leading?: 'square' | 'none'; +} + +/** + * A list's worth of placeholder rows. + * + * The States rule asks for skeletons shaped like the content rather than a + * centred spinner, specifically so the layout does not jump when data lands. + * Hidden from accessibility entirely — a screen reader announcing eight + * identical empty rows is worse than silence, and the screen it is standing in + * for will announce itself when it arrives. + */ +export function SkeletonRows({ + rows = 8, + rowClassName = 'h-16', + leading = 'square', +}: SkeletonRowsProps) { + return ( + + {Array.from({ length: rows }, (_, index) => ( + + {leading === 'square' ? : null} + + + {/* Two widths, alternating, so it reads as a list rather than a grid. */} + + + + + + + ))} + + ); +} + +/** + * A card grid's worth of placeholders, for the album and artist shelves. + * + * Two per row, matching `AlbumCard`'s layout, so the shelf does not reflow when + * the real cards arrive. + */ +export function SkeletonCards({ count = 4 }: { count?: number }) { + return ( + + {Array.from({ length: count }, (_, index) => ( + // Half-width cells with padding, matching `CollectionGrid` exactly, so + // the real cards land where the placeholders were. + + + + + + ))} + + ); +} diff --git a/src/components/ui/SwipeableRow.tsx b/src/components/ui/SwipeableRow.tsx index 780fab5..f0165b7 100644 --- a/src/components/ui/SwipeableRow.tsx +++ b/src/components/ui/SwipeableRow.tsx @@ -1,5 +1,5 @@ import type { LucideIcon } from 'lucide-react-native'; -import type { ReactNode } from 'react'; +import { useMemo, type ReactNode } from 'react'; import { View } from 'react-native'; import { Gesture, GestureDetector } from 'react-native-gesture-handler'; import Animated, { @@ -59,25 +59,45 @@ export function SwipeableRow({ /** Grows as the row nears the commit point, so the icon fades in with it. */ const progress = useSharedValue(0); - const pan = Gesture.Pan() - .activeOffsetX([-ACTIVATION_SLOP, ACTIVATION_SLOP]) - .failOffsetY([-ACTIVATION_SLOP, ACTIVATION_SLOP]) - .onUpdate((event) => { - // Left only. Dragging right does nothing, rather than doing this action - // in reverse — one gesture, one meaning. - const travelled = Math.min(0, event.translationX); - const past = Math.max(0, -travelled - COMMIT_DISTANCE); + /* + * Memoized, because a `Gesture.Pan()` is not free. + * + * Building one allocates a handler and `GestureDetector` re-attaches it when + * the object identity changes. This is inside a virtualized list, so an + * unmemoized gesture meant rebuilding and re-attaching one per visible row on + * every parent render — roughly forty at a time. `onSwipe` is the only + * dependency, and callers pass a stable one. + */ + const pan = useMemo( + () => + Gesture.Pan() + .activeOffsetX([-ACTIVATION_SLOP, ACTIVATION_SLOP]) + .failOffsetY([-ACTIVATION_SLOP, ACTIVATION_SLOP]) + .onUpdate((event) => { + // Left only. Dragging right does nothing, rather than doing this + // action in reverse — one gesture, one meaning. + const travelled = Math.min(0, event.translationX); + const past = Math.max(0, -travelled - COMMIT_DISTANCE); - offset.value = travelled + past * (1 - OVERSHOOT_RATIO); - progress.value = Math.min(1, -travelled / COMMIT_DISTANCE); - }) - .onEnd((event) => { - if (event.translationX <= -COMMIT_DISTANCE) runOnJS(onSwipe)(); - }) - .onFinalize(() => { - offset.value = withSpring(0, { damping: 22, stiffness: 240 }); - progress.value = withTiming(0, { duration: 150 }); - }); + offset.value = travelled + past * (1 - OVERSHOOT_RATIO); + progress.value = Math.min(1, -travelled / COMMIT_DISTANCE); + }) + .onEnd((event) => { + if (event.translationX <= -COMMIT_DISTANCE) runOnJS(onSwipe)(); + }) + .onFinalize(() => { + offset.value = withSpring(0, { damping: 22, stiffness: 240 }); + progress.value = withTiming(0, { duration: 150 }); + }), + /* + * `onSwipe` only. The two shared values are deliberately absent: a + * `useSharedValue` handle never changes identity, and listing one as a + * dependency of a hook that then writes to it is what the compiler's + * immutability rule rejects — correctly, for ordinary values. + */ + // eslint-disable-next-line react-hooks/exhaustive-deps -- shared values are stable handles + [onSwipe], + ); const rowStyle = useAnimatedStyle(() => ({ transform: [{ translateX: offset.value }] })); /* @@ -93,7 +113,7 @@ export function SwipeableRow({ diff --git a/src/components/ui/Toaster.tsx b/src/components/ui/Toaster.tsx new file mode 100644 index 0000000..d7b8eaa --- /dev/null +++ b/src/components/ui/Toaster.tsx @@ -0,0 +1,64 @@ +import { X } from 'lucide-react-native'; +import { useSyncExternalStore } from 'react'; +import { useTranslation } from 'react-i18next'; +import { Pressable, Text, View } from 'react-native'; +import Animated, { FadeInDown, FadeOutDown } from 'react-native-reanimated'; + +import { dismissToast, getToast, subscribeToast } from '@/services/toast'; +import { useReducedMotion } from '@/theme/useReducedMotion'; +import { useThemeColors } from '@/theme/useTheme'; + +/** + * Where transient confirmations appear. + * + * Mounted once, directly above the mini player in the tab bar stack. It is the + * only subscriber to the toast store, so a toast re-renders this and nothing + * else — a message confirming a swipe must not cost a re-render of the list that + * was swiped. + * + * Non-blocking in the strict sense: it occupies no space and takes no touches + * except on the pill itself. `position: absolute` keeps it out of the tab bar's + * layout so the bar does not shift when a toast appears, and `box-none` lets + * everything except the pill fall through to the list underneath. + */ +export function Toaster() { + const { t } = useTranslation(); + const colors = useThemeColors(); + const reducedMotion = useReducedMotion(); + const toast = useSyncExternalStore(subscribeToast, getToast); + + if (toast === null) return null; + + return ( + + + + {toast.message} + + + {/* Dismissible, per the requirement that it never gets in the way. */} + + + + + + ); +} diff --git a/src/db/queries/scanning.ts b/src/db/queries/scanning.ts index 5826ec1..365f370 100644 Binary files a/src/db/queries/scanning.ts and b/src/db/queries/scanning.ts differ diff --git a/src/db/queries/stats.ts b/src/db/queries/stats.ts index f5a656b..8310c55 100644 --- a/src/db/queries/stats.ts +++ b/src/db/queries/stats.ts @@ -4,7 +4,7 @@ import { useLiveQuery } from 'drizzle-orm/expo-sqlite'; import type { PeriodType } from '@/services/stats/rollups'; import { db } from '../client'; -import { artists, statsRollups, tracks } from '../schema'; +import { albums, artists, playlists, statsRollups, tracks } from '../schema'; /** * Everything the statistics screens read. @@ -21,6 +21,8 @@ export interface TopEntry { subtitle: string | null; playCount: number; msPlayed: number; + /** Bare path into the artwork cache. Null when there is nothing to show. */ + artworkPath: string | null; } export interface PeriodTotals { @@ -30,6 +32,48 @@ export interface PeriodTotals { trackCount: number; } +/** The rollup rows for one period and entity type, ranked. Shared `where`. */ +function rankedRollups(periodType: PeriodType, periodKey: string, entityType: string) { + return and( + eq(statsRollups.periodType, periodType), + eq(statsRollups.periodKey, periodKey), + eq(statsRollups.entityType, entityType), + ); +} + +/** + * Any album's cover, taken from a track that has one. + * + * `albums.artwork_path` exists in the schema and the scanner never fills it — + * artwork is extracted per file, so the cover lives on the tracks. A correlated + * subquery rather than a join because it must return exactly one row per album + * and a join would multiply the rollup row by every track on the record. + * + * Bounded by the `limit` on the outer query, so this runs ten times, not once + * per album in the library. + */ +const albumCover = sql`( + SELECT t.artwork_path FROM tracks t + WHERE t.album_id = ${albums.id} AND t.artwork_path IS NOT NULL + LIMIT 1 +)`; + +/** The same, for an artist. */ +const artistCover = sql`( + SELECT t.artwork_path FROM tracks t + WHERE t.artist_id = ${artists.id} AND t.artwork_path IS NOT NULL + LIMIT 1 +)`; + +/** The same, for a playlist: the first track in it that has a cover. */ +const playlistCover = sql`( + SELECT t.artwork_path FROM playlist_tracks pt + JOIN tracks t ON t.id = pt.track_id + WHERE pt.playlist_id = ${playlists.id} AND t.artwork_path IS NOT NULL + ORDER BY pt.position + LIMIT 1 +)`; + /** Top tracks for a period, most played first. */ export function useTopTracks(periodType: PeriodType, periodKey: string, limit = 10) { const query = db @@ -39,17 +83,12 @@ export function useTopTracks(periodType: PeriodType, periodKey: string, limit = subtitle: artists.name, playCount: statsRollups.playCount, msPlayed: statsRollups.msPlayed, + artworkPath: tracks.artworkPath, }) .from(statsRollups) .innerJoin(tracks, eq(tracks.id, statsRollups.entityId)) .leftJoin(artists, eq(artists.id, tracks.artistId)) - .where( - and( - eq(statsRollups.periodType, periodType), - eq(statsRollups.periodKey, periodKey), - eq(statsRollups.entityType, 'track'), - ), - ) + .where(rankedRollups(periodType, periodKey, 'track')) .orderBy(desc(statsRollups.playCount), desc(statsRollups.msPlayed)) .limit(limit); @@ -66,16 +105,60 @@ export function useTopArtists(periodType: PeriodType, periodKey: string, limit = subtitle: sql`null`, playCount: statsRollups.playCount, msPlayed: statsRollups.msPlayed, + artworkPath: artistCover, }) .from(statsRollups) .innerJoin(artists, eq(artists.id, statsRollups.entityId)) - .where( - and( - eq(statsRollups.periodType, periodType), - eq(statsRollups.periodKey, periodKey), - eq(statsRollups.entityType, 'artist'), - ), - ) + .where(rankedRollups(periodType, periodKey, 'artist')) + .orderBy(desc(statsRollups.playCount), desc(statsRollups.msPlayed)) + .limit(limit); + + const { data } = useLiveQuery(query, [periodType, periodKey, limit]); + return data; +} + +/** Top albums for a period. */ +export function useTopAlbums(periodType: PeriodType, periodKey: string, limit = 10) { + const query = db + .select({ + id: albums.id, + title: albums.name, + subtitle: artists.name, + playCount: statsRollups.playCount, + msPlayed: statsRollups.msPlayed, + artworkPath: albumCover, + }) + .from(statsRollups) + .innerJoin(albums, eq(albums.id, statsRollups.entityId)) + .leftJoin(artists, eq(artists.id, albums.artistId)) + .where(rankedRollups(periodType, periodKey, 'album')) + .orderBy(desc(statsRollups.playCount), desc(statsRollups.msPlayed)) + .limit(limit); + + const { data } = useLiveQuery(query, [periodType, periodKey, limit]); + return data; +} + +/** + * Top playlists for a period. + * + * Empty until something is played *from* a playlist — the rollup is keyed on + * where the queue came from, not on whether the track happens to be in one. + * See `QueueSource` in `src/services/audio/types.ts`. + */ +export function useTopPlaylists(periodType: PeriodType, periodKey: string, limit = 10) { + const query = db + .select({ + id: playlists.id, + title: playlists.name, + subtitle: sql`null`, + playCount: statsRollups.playCount, + msPlayed: statsRollups.msPlayed, + artworkPath: playlistCover, + }) + .from(statsRollups) + .innerJoin(playlists, eq(playlists.id, statsRollups.entityId)) + .where(rankedRollups(periodType, periodKey, 'playlist')) .orderBy(desc(statsRollups.playCount), desc(statsRollups.msPlayed)) .limit(limit); @@ -98,13 +181,7 @@ export function usePeriodTotals(periodType: PeriodType, periodKey: string): Peri trackCount: sql`count(*)`, }) .from(statsRollups) - .where( - and( - eq(statsRollups.periodType, periodType), - eq(statsRollups.periodKey, periodKey), - eq(statsRollups.entityType, 'track'), - ), - ); + .where(rankedRollups(periodType, periodKey, 'track')); const { data } = useLiveQuery(query, [periodType, periodKey]); return data[0] ?? { playCount: 0, msPlayed: 0, trackCount: 0 }; diff --git a/src/db/queries/tracks.ts b/src/db/queries/tracks.ts index 932d583..745a6bf 100644 --- a/src/db/queries/tracks.ts +++ b/src/db/queries/tracks.ts @@ -131,10 +131,14 @@ export function useTracks(search = ''): { tracks: TrackListItem[]; isLoading: bo const { data, updatedAt } = useLiveQuery(query, [term]); - // Temporary instrumentation for the tab-switch investigation. - perf.count('useTracks.render'); + /* + * Time to first rows, which is the number the cold-start investigation turned + * on. The per-render counter that used to sit here was removed: on the Mi 9T + * it fired often enough that MIUI's logcat rate limiter discarded this + * measurement, and a probe that hides the thing it is measuring is worse than + * no probe. + */ useEffect(() => { - perf.count('useTracks.subscribe'); perf.mark('useTracks.firstRows'); }, [term]); useEffect(() => { @@ -258,3 +262,95 @@ export async function markMissing(trackIds: number[]): Promise { .set({ isMissing: 1 }) .where(sql`${tracks.id} IN ${trackIds}`); } + +/** An artist or album as a card: cover, name, and how much of it there is. */ +export interface CollectionCard { + id: number; + name: string; + /** For an album, its artist. Null for an artist card. */ + subtitle: string | null; + trackCount: number; + artworkPath: string | null; +} + +/** + * Every artist that has at least one present track. + * + * `innerJoin` rather than a left join from `artists`: the table accumulates a + * row the first time a name is seen and nothing ever removes one, so an artist + * whose only album has been deleted would otherwise sit in the list showing + * zero tracks. What is on the device is what the library should show. + * + * The cover is `min(artwork_path)`, which is an arbitrary-but-stable choice of + * one of the artist's covers. Stable matters more than which: a card whose + * artwork changes between renders looks broken. + */ +export function useArtistCards(): CollectionCard[] { + const query = db + .select({ + id: artists.id, + name: artists.name, + subtitle: sql`null`, + trackCount: count(tracks.id), + artworkPath: sql`min(${tracks.artworkPath})`, + }) + .from(artists) + .innerJoin(tracks, and(eq(tracks.artistId, artists.id), eq(tracks.isMissing, 0))) + .groupBy(artists.id) + .orderBy(asc(sql`${artists.sortName} COLLATE NOCASE`)); + + const { data } = useLiveQuery(query); + return useThrottledData(data); +} + +/** Every album that has at least one present track. */ +export function useAlbumCards(): CollectionCard[] { + const query = db + .select({ + id: albums.id, + name: albums.name, + subtitle: artists.name, + trackCount: count(tracks.id), + artworkPath: sql`min(${tracks.artworkPath})`, + }) + .from(albums) + .innerJoin(tracks, and(eq(tracks.albumId, albums.id), eq(tracks.isMissing, 0))) + .leftJoin(artists, eq(artists.id, albums.artistId)) + .groupBy(albums.id) + .orderBy(asc(sql`${albums.name} COLLATE NOCASE`)); + + const { data } = useLiveQuery(query); + return useThrottledData(data); +} + +/** + * The tracks of one artist or album, ready to play. + * + * Ordered by disc and track number where they exist, falling back to title — + * an album played in alphabetical order is not the album. Nulls sort last so a + * partially-tagged record still opens with the tracks that know where they go. + */ +export function useCollectionTracks(kind: 'artist' | 'album', id: number): TrackListItem[] { + const query = db + .select(listSelection) + .from(tracks) + .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), + kind === 'artist' ? eq(tracks.artistId, id) : eq(tracks.albumId, id), + ), + ) + .orderBy( + asc(sql`${tracks.discNo} IS NULL`), + asc(tracks.discNo), + asc(sql`${tracks.trackNo} IS NULL`), + asc(tracks.trackNo), + asc(sql`${tracks.title} COLLATE NOCASE`), + ); + + const { data } = useLiveQuery(query, [kind, id]); + return useThrottledData(data); +} diff --git a/src/features/library/CollectionDetailScreen.tsx b/src/features/library/CollectionDetailScreen.tsx new file mode 100644 index 0000000..6768f7d --- /dev/null +++ b/src/features/library/CollectionDetailScreen.tsx @@ -0,0 +1,145 @@ +import { useRouter } from 'expo-router'; +import { ChevronLeft, Play, Shuffle } from 'lucide-react-native'; +import { useCallback, useMemo } from 'react'; +import { useTranslation } from 'react-i18next'; +import { Pressable, Text, View } from 'react-native'; +import { SafeAreaView } from 'react-native-safe-area-context'; + +import { Skeleton } from '@/components/ui/Skeleton'; +import { useAlbumCards, useArtistCards, useCollectionTracks } from '@/db/queries/tracks'; +import { AudioEngine } from '@/services/audio/AudioEngine'; +import type { QueueSource } from '@/services/audio/types'; +import { getShuffleAlgorithm } from '@/services/settings'; +import { useThemeColors } from '@/theme/useTheme'; + +import { toPlayable } from '../player/toPlayable'; +import { CollectionHeader } from './components/CollectionHeader'; +import { LibraryTracks } from './LibraryTracks'; + +export interface CollectionDetailScreenProps { + kind: 'artist' | 'album'; + id: number; +} + +/** + * Everything by one artist, or everything on one album. + * + * Reuses `LibraryTracks` wholesale, so a track here has exactly the same verbs + * as a track in the library — swipe to queue, long-press for the sheet, + * multi-select. A second, thinner track list for this screen would have drifted + * from the first within a release. + * + * Playing from here attributes the listen to the artist or album rather than to + * the library, which is what puts rows under those entity types in + * `stats_rollups` — see `QueueSource`. + */ +export function CollectionDetailScreen({ kind, id }: CollectionDetailScreenProps) { + const { t } = useTranslation(); + const colors = useThemeColors(); + const router = useRouter(); + + const tracks = useCollectionTracks(kind, id); + + /* + * The card comes from the same query the grid used, so the header shows + * exactly what the user tapped. Cheap: both lists are already live and in + * memory for the library screen behind this one. + */ + const artists = useArtistCards(); + const albums = useAlbumCards(); + const card = (kind === 'artist' ? artists : albums).find((entry) => entry.id === id); + + const source = useMemo(() => ({ type: kind, id }), [kind, id]); + + const play = useCallback(() => { + if (tracks.length > 0) void AudioEngine.setQueue(tracks.map(toPlayable), 0, source); + }, [tracks, source]); + + const shuffle = useCallback(async () => { + if (tracks.length === 0) return; + // Queue first, shuffle second: the engine keeps the unshuffled order, so + // turning shuffle off later restores the album's real running order. + await AudioEngine.setQueue(tracks.map(toPlayable), 0, source); + await AudioEngine.setShuffled(true, getShuffleAlgorithm()); + }, [tracks, source]); + + const goBack = useCallback(() => router.back(), [router]); + + return ( + + + + + + + + {card ? ( + + ) : ( + // The card list is live and lands a frame or two after the tracks do. + + + + + + + + )} + + {tracks.length > 0 ? ( + + + + + {t('playlists.playAll')} + + + + void shuffle()} + accessibilityRole="button" + accessibilityLabel={t('playlists.shuffleAll')} + className="min-h-11 flex-1 flex-row items-center justify-center gap-2 rounded-sm border border-subtle px-4" + > + + + {t('playlists.shuffleAll')} + + + + ) : null} + + + + ); +} + +function noop(): void { + // This screen cannot scan or refresh; the library above it does both. +} diff --git a/src/features/library/LibraryScreen.tsx b/src/features/library/LibraryScreen.tsx index b8f4a1d..781bb45 100644 --- a/src/features/library/LibraryScreen.tsx +++ b/src/features/library/LibraryScreen.tsx @@ -1,32 +1,30 @@ -import { Music, SearchX } from 'lucide-react-native'; +import { Disc3, User } from 'lucide-react-native'; import { useCallback, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { Linking, View } from 'react-native'; -import { EmptyState } from '@/components/ui/EmptyState'; +import { ConfirmDialog } from '@/components/ui/ConfirmDialog'; import { ErrorState } from '@/components/ui/ErrorState'; import { Screen } from '@/components/ui/Screen'; -import type { TrackListItem } from '@/db/queries/tracks'; -import { useMessages } from '@/i18n'; +import { SegmentedControl, type SegmentedControlOption } from '@/components/ui/SegmentedControl'; +import { SkeletonCards } from '@/components/ui/Skeleton'; +import { useAlbumCards, useArtistCards } from '@/db/queries/tracks'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { isPermissionError } from '@/services/scanner/permission'; -import { AddToPlaylistSheet } from '../playlists/components/AddToPlaylistSheet'; -import { useCurrentTrack, usePlaybackControls } from '../player/hooks/usePlayback'; -import { toPlayable } from '../player/toPlayable'; +import { CollectionGrid } from './components/CollectionGrid'; import { LibraryHeader } from './components/LibraryHeader'; import { ScanBanner } from './components/ScanBanner'; import { SearchField } from './components/SearchField'; -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 { LibraryTracks } from './LibraryTracks'; +import { useCollectionRouting } from './hooks/useCollectionRouting'; import { useDebounced } from './hooks/useDebounced'; import { useTracks } from './hooks/useLibrary'; import { useScan } from './hooks/useScan'; -import { useSelection } from './hooks/useSelection'; -import { useTrackActions } from './hooks/useTrackActions'; + +/** Which face of the library is on screen. */ +const VIEWS = ['tracks', 'artists', 'albums'] as const; +type LibraryView = (typeof VIEWS)[number]; /** Opens this app's page in system settings, where the permission switch is. */ function openAppSettings(): void { @@ -34,147 +32,80 @@ function openAppSettings(): void { } /** - * The library: every present track, with the scan that fills it. + * The library, in three views: tracks, artists, albums. + * + * This screen owns the *library* — scanning, searching, which view is showing — + * and delegates each view to a component that owns the things in it. That split + * happened because the tracks view alone had reached the 300-line limit; adding + * two more views to the same file was not an option, and the boundary it forced + * is the right one anyway. * - * This screen decides *what* happens to a track — play it, queue it, select it, - * describe it. `TrackList` decides how a row is drawn, and the sheets decide how - * a choice is offered. The four states — loading, scanning, failed, empty — are - * all here, per the States rule, and the scan banner sits above the list rather - * than replacing it so the user can keep scrolling. + * Genres are absent. The tech stack doc lists them alongside artists and albums, + * and MediaStore's genre tagging is unreliable enough on real libraries that a + * genre shelf is mostly one bucket called "Unknown" — see + * `docs/adr/012-artist-and-album-shelves.md`. */ export function LibraryScreen() { useLifecycleTrace('LibraryScreen'); - const { t, i18n } = useTranslation(); - const messages = useMessages('library.empty'); + const { t } = useTranslation(); + const [view, setView] = useState('tracks'); const [search, setSearch] = useState(''); // The field stays instant; only the query waits. const { tracks, isLoading } = useTracks(useDebounced(search)); - /* - * The launch sweep waits for the list to be on screen. Firing it on mount put - * a MediaStore enumeration on the JS thread alongside the first library query - * — see `useScan` and docs/performance.md. - */ - const { progress, isScanning, isRefreshing, scanLibrary, addFolder, rescan, cancel } = - useScan(!isLoading); + const { progress, isScanning, isRefreshing, scanLibrary, addFolder, rescan, cancel } = useScan(); - const { playFrom } = usePlaybackControls(); - const currentTrack = useCurrentTrack(); - const selection = useSelection(); - const { addToQueue, playNext, toggleFavorite } = useTrackActions(); + const artists = useArtistCards(); + const albums = useAlbumCards(); + const { openArtist, openAlbum } = useCollectionRouting(); - /** Which track's action sheet is open. */ - const [actionTarget, setActionTarget] = useState(null); - /** Which track's info sheet is open. */ - const [infoTarget, setInfoTarget] = useState(null); - /** Tracks queued for "add to playlist". Empty means the sheet is closed. */ - const [playlistTargets, setPlaylistTargets] = useState([]); + /** True while the scan confirmation is on screen. */ + const [confirmingScan, setConfirmingScan] = useState(false); + const askToScan = useCallback(() => setConfirmingScan(true), []); const hasFailed = !isScanning && progress.phase === 'failed'; const permissionFailed = isPermissionError(progress.error); const permissionBlocked = progress.error === 'permission-blocked'; - const find = useCallback( - (id: number) => tracks.find((track) => track.id === id) ?? null, - [tracks], - ); + const viewOptions: SegmentedControlOption[] = VIEWS.map((value) => ({ + value, + label: t(`library.view.${value}`), + })); /* - * 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. + * Skeleton while the query is in flight *and* while a scan has not yet + * produced a first row. + * + * The second case is the fix for a contradiction: with an empty library and a + * scan running, the list showed its empty state — "No music found yet. Scan + * the device" with a Scan button — directly underneath a banner reporting a + * scan in progress. Two parts of one screen disagreeing about whether anything + * was happening. */ - const onPress = useCallback( - (id: number) => { - if (selection.isActive) { - selection.toggle(id); - return; - } - const index = tracks.findIndex((track) => track.id === id); - if (index === -1) return; - playFrom(tracks.map(toPlayable), index); - }, - [tracks, playFrom, selection], - ); - - 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 (selection.isActive) { - selection.toggle(id); - return; - } - setActionTarget(find(id)); - }, - [selection, find], - ); - - const onSwipeToQueue = useCallback( - (id: number) => { - const track = find(id); - if (track) addToQueue([track]); - }, - [find, addToQueue], - ); - - const onAction = useCallback( - (action: TrackAction) => { - const track = actionTarget; - if (!track) return; - - switch (action) { - case 'playNext': - playNext([track]); - return; - case 'addToQueue': - addToQueue([track]); - return; - case 'addToPlaylist': - setPlaylistTargets([track.id]); - return; - case 'favorite': - toggleFavorite(track); - return; - case 'select': - selection.begin(track.id); - return; - case 'info': - setInfoTarget(track); - return; - } - }, - [actionTarget, playNext, addToQueue, toggleFavorite, selection], - ); - - 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 onSelectionPlaylist = useCallback(() => { - setPlaylistTargets(selection.ids); - }, [selection.ids]); - - const closePlaylistSheet = useCallback(() => { - setPlaylistTargets([]); - selection.clear(); - }, [selection]); + const waiting = isLoading || (isScanning && tracks.length === 0); return ( setView('tracks')} /> - + + + + + {/* Search filters tracks only. An artist grid of two cards does not need + a search box, and hiding it makes that obvious rather than puzzling. */} + {view === 'tracks' ? : null} {isScanning ? : null} @@ -201,62 +132,53 @@ export function LibraryScreen() { ) : null} {/* - The list owns whatever height is left, explicitly. Without a bounded flex - parent a virtualized list keeps the height it first measured, so mounting - the scan banner above it shrank the space without shrinking the list — - which is where the blank band above the first row during "Reading tags…" - came from. + Every view owns whatever height is left, explicitly. Without a bounded + flex parent a virtualized list keeps the height it first measured, so + mounting the scan banner above it shrinks the space without shrinking the + list — which is where the blank band above the first row came from. */} - - {isLoading ? ( - - ) : ( - - ) : ( - - ) - } - /> - )} - - - {selection.isActive ? ( - selection.toggleAll(tracks.map((track) => track.id))} - onAddToQueue={onSelectionQueue} - onAddToPlaylist={onSelectionPlaylist} - onCancel={selection.clear} + {view === 'tracks' ? ( + - ) : null} + ) : ( + + {waiting ? ( + + ) : ( + + )} + + )} - setActionTarget(null)} + {/* + Scanning reads every audio file on the device, so it says so before it + starts. There is no progress estimate to offer — MediaStore does not + report a count until it has been asked — so the copy promises a duration + proportional to the library rather than a number it cannot know. + */} + { + setConfirmingScan(false); + void scanLibrary(); + }} + onCancel={() => setConfirmingScan(false)} /> - setInfoTarget(null)} /> - ); } diff --git a/src/features/library/LibraryTracks.tsx b/src/features/library/LibraryTracks.tsx new file mode 100644 index 0000000..73116b1 --- /dev/null +++ b/src/features/library/LibraryTracks.tsx @@ -0,0 +1,221 @@ +import { Music, SearchX } from 'lucide-react-native'; +import { useCallback, useMemo, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { View } from 'react-native'; + +import { EmptyState } from '@/components/ui/EmptyState'; +import type { TrackListItem } from '@/db/queries/tracks'; +import { useMessages } from '@/i18n'; + +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 { + tracks: TrackListItem[]; + /** Skeleton instead of a list while true. */ + isLoading: boolean; + isRefreshing: boolean; + onRefresh: () => void; + /** The active search term, so "no results" can quote it. */ + search: string; + /** Suppresses the empty state — the screen is already showing an error. */ + suppressEmpty: boolean; + /** Offered by the empty state when the library has never been filled. */ + onScan: () => void; +} + +/** + * The track list and everything you can do to a track. + * + * 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 + * screen above owns the *library*: scanning, searching, and which view is on + * screen. + */ +export function LibraryTracks({ + tracks, + isLoading, + isRefreshing, + onRefresh, + search, + suppressEmpty, + onScan, +}: LibraryTracksProps) { + const { t, i18n } = useTranslation(); + const messages = useMessages('library.empty'); + + 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(); + + /** Which track's action sheet is open. */ + const [actionTarget, setActionTarget] = useState(null); + /** Which track's info sheet is open. */ + const [infoTarget, setInfoTarget] = useState(null); + /** Tracks queued for "add to playlist". Empty means the sheet is closed. */ + const [playlistTargets, setPlaylistTargets] = useState([]); + + const find = useCallback( + (id: number) => tracks.find((track) => track.id === id) ?? null, + [tracks], + ); + + /* + * 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. + */ + const onPress = useCallback( + (id: number) => { + if (isSelecting) { + toggleSelected(id); + return; + } + const index = tracks.findIndex((track) => track.id === id); + if (index === -1) return; + playFrom(tracks.map(toPlayable), index); + }, + [tracks, playFrom, isSelecting, toggleSelected], + ); + + 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 onSwipeToQueue = useCallback( + (id: number) => { + const track = find(id); + if (track) addToQueue([track]); + }, + [find, addToQueue], + ); + + const onAction = useCallback( + (action: TrackAction) => { + const track = actionTarget; + if (!track) return; + + switch (action) { + case 'playNext': + playNext([track]); + return; + case 'addToQueue': + addToQueue([track]); + return; + case 'addToPlaylist': + setPlaylistTargets([track.id]); + return; + case 'favorite': + toggleFavorite(track); + return; + case 'select': + selection.begin(track.id); + return; + case 'info': + setInfoTarget(track); + return; + } + }, + [actionTarget, playNext, addToQueue, toggleFavorite, selection], + ); + + 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 ( + <> + + {isLoading ? ( + + ) : ( + + ) : ( + + ) + } + /> + )} + + + {isSelecting ? ( + selection.toggleAll(tracks.map((track) => track.id))} + onAddToQueue={onSelectionQueue} + onAddToPlaylist={() => setPlaylistTargets(selection.ids)} + onCancel={selection.clear} + /> + ) : null} + + setActionTarget(null)} + /> + setInfoTarget(null)} /> + + + ); +} diff --git a/src/features/library/components/CollectionCard.tsx b/src/features/library/components/CollectionCard.tsx new file mode 100644 index 0000000..67953ca --- /dev/null +++ b/src/features/library/components/CollectionCard.tsx @@ -0,0 +1,87 @@ +import { Image } from 'expo-image'; +import type { LucideIcon } from 'lucide-react-native'; +import { memo, useCallback } from 'react'; +import { useTranslation } from 'react-i18next'; +import { Pressable, Text, View } from 'react-native'; + +import type { CollectionCard as Card } from '@/db/queries/tracks'; +import { useThemeColors } from '@/theme/useTheme'; + +export interface CollectionCardProps { + card: Card; + /** Drawn when there is no cover. A disc for albums, a person for artists. */ + icon: LucideIcon; + onPress: (id: number) => void; +} + +/** + * One artist or album: cover, name, and how much of it there is. + * + * A square rather than a row, because a grid of covers is the one place this + * app lets artwork carry a screen — the design direction says album art is the + * only saturated colour in most views and should be allowed to. Track lists + * stay as rows; this is the shelf. + * + * `rounded-xs` on the cover, matching every other piece of list artwork. The + * radius scale reserves `md` for cards and sheets, and using it here would make + * the covers rounder than the panel they sit on. + */ +const CollectionCardComponent = function CollectionCard({ + card, + icon: Icon, + onPress, +}: CollectionCardProps) { + const { t } = useTranslation(); + const colors = useThemeColors(); + const handlePress = useCallback(() => onPress(card.id), [onPress, card.id]); + + const subtitle = card.subtitle ?? t('library.trackCount', { count: card.trackCount }); + + return ( + + {card.artworkPath ? ( + + ) : ( + + + + )} + + + + {card.name} + + + {subtitle} + + + + ); +}; + +function isSameCard(previous: CollectionCardProps, next: CollectionCardProps): boolean { + return ( + previous.onPress === next.onPress && + previous.icon === next.icon && + previous.card.id === next.card.id && + previous.card.name === next.card.name && + previous.card.subtitle === next.card.subtitle && + previous.card.trackCount === next.card.trackCount && + previous.card.artworkPath === next.card.artworkPath + ); +} + +export const CollectionCard = memo(CollectionCardComponent, isSameCard); diff --git a/src/features/library/components/CollectionGrid.tsx b/src/features/library/components/CollectionGrid.tsx new file mode 100644 index 0000000..74bb40d --- /dev/null +++ b/src/features/library/components/CollectionGrid.tsx @@ -0,0 +1,60 @@ +import { FlashList, type ListRenderItem } from '@shopify/flash-list'; +import type { LucideIcon } from 'lucide-react-native'; +import type { ReactElement } from 'react'; +import { useCallback } from 'react'; +import { View } from 'react-native'; + +import type { CollectionCard as Card } from '@/db/queries/tracks'; + +import { CollectionCard } from './CollectionCard'; + +/** Cards per row. Two keeps the covers big enough to recognise on a phone. */ +const COLUMNS = 2; + +export interface CollectionGridProps { + cards: readonly Card[]; + icon: LucideIcon; + onPress: (id: number) => void; + empty: ReactElement | null; +} + +/** + * A grid of artist or album cards. + * + * FlashList with `numColumns`, not a wrapping flex row: an artist grid is as + * long as the library is wide, and the performance rule puts every long list on + * FlashList without exception. Three thousand albums laid out in a ScrollView + * would mount three thousand images. + * + * No `overrideItemLayout` here, unlike the track list. Card height depends on + * the width the grid is given — the cover is square — so a hardcoded size would + * be wrong on the first rotation. Cards are uniform, so FlashList measures one + * and reuses it. + */ +export function CollectionGrid({ cards, icon, onPress, empty }: CollectionGridProps) { + const renderItem = useCallback>( + ({ item }) => ( + // Gutter as padding on the cell rather than a gap on the list: FlashList + // sizes cells itself, and a gap would be applied outside that measurement. + + + + ), + [icon, onPress], + ); + + return ( + + ); +} + +function keyExtractor(card: Card): string { + return String(card.id); +} diff --git a/src/features/library/components/CollectionHeader.tsx b/src/features/library/components/CollectionHeader.tsx new file mode 100644 index 0000000..85ef258 --- /dev/null +++ b/src/features/library/components/CollectionHeader.tsx @@ -0,0 +1,61 @@ +import { Image } from 'expo-image'; +import { Disc3, User } from 'lucide-react-native'; +import { useTranslation } from 'react-i18next'; +import { Text, View } from 'react-native'; + +import { useThemeColors } from '@/theme/useTheme'; + +export interface CollectionHeaderProps { + kind: 'artist' | 'album'; + name: string; + /** The album's artist. Null for an artist. */ + subtitle: string | null; + trackCount: number; + artworkPath: string | null; +} + +/** Cover, name and size for an artist or album detail screen. */ +export function CollectionHeader({ + kind, + name, + subtitle, + trackCount, + artworkPath, +}: CollectionHeaderProps) { + const { t } = useTranslation(); + const colors = useThemeColors(); + const Icon = kind === 'artist' ? User : Disc3; + + return ( + + {artworkPath ? ( + + ) : ( + + + + )} + + + + {name} + + {subtitle ? ( + + {subtitle} + + ) : null} + + {t('library.trackCount', { count: trackCount })} + + + + ); +} diff --git a/src/features/library/components/LibraryHeader.tsx b/src/features/library/components/LibraryHeader.tsx index 1c9c979..3b2665f 100644 --- a/src/features/library/components/LibraryHeader.tsx +++ b/src/features/library/components/LibraryHeader.tsx @@ -1,4 +1,4 @@ -import { CheckSquare, Plus } from 'lucide-react-native'; +import { CheckSquare, FolderPlus, RefreshCw } from 'lucide-react-native'; import { useTranslation } from 'react-i18next'; import { Pressable, Text, View } from 'react-native'; @@ -9,28 +9,39 @@ export interface LibraryHeaderProps { count: number; /** Hides the count while scanning, when it is still climbing. */ isScanning: boolean; - onAddMusic: () => void; + /** Sweep MediaStore. Confirmed first — it reads every audio file on the device. */ + onScan: () => void; + /** Open the system folder picker. */ + onAddFolder: () => void; onStartSelecting: () => void; } /** - * The count, and the two things you can do to the whole library. + * The count, and the three 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 * nothing — see the performance rules. + * + * Scanning has its own button here rather than happening on launch. It is the + * only control in the app that reads every audio file on the device, so it is + * the one control that has to be pressed rather than assumed — see + * `docs/adr/010-scanning-is-user-initiated.md`. Having it always present, rather + * 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, - onAddMusic, + onScan, + onAddFolder, onStartSelecting, }: LibraryHeaderProps) { const { t } = useTranslation(); const colors = useThemeColors(); return ( - + {isScanning ? '' : t('library.trackCount', { count })} @@ -45,22 +56,29 @@ export function LibraryHeader({ accessibilityState={{ disabled: count === 0 }} className="min-h-11 min-w-11 items-center justify-center" > - + + + + + - + - {t('library.addMusic')} + {t('library.scan')} diff --git a/src/features/library/components/LibraryRow.tsx b/src/features/library/components/LibraryRow.tsx new file mode 100644 index 0000000..448ab27 --- /dev/null +++ b/src/features/library/components/LibraryRow.tsx @@ -0,0 +1,90 @@ +import { ListEnd } from 'lucide-react-native'; +import { memo, useCallback, useMemo } from 'react'; + +import { SwipeableRow } from '@/components/ui/SwipeableRow'; +import type { TrackListItem } from '@/db/queries/tracks'; + +import { TrackRow } from './TrackRow'; + +export interface LibraryRowProps { + track: TrackListItem; + locale: string; + /** Stable. Selection state is passed as booleans, never as an object. */ + onPress: (id: number) => 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; +} + +/** + * One library row, with its gesture. + * + * Exists so `renderItem` can stay a one-liner that passes only primitives and + * stable callbacks. The version this replaced built the whole tree inline, + * 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. + */ +const LibraryRowComponent = function LibraryRow({ + track, + locale, + onPress, + onLongPress, + onSwipeToQueue, + isSelecting, + isSelected, + isCurrent, + swipeLabel, +}: LibraryRowProps) { + const handleSwipe = useCallback(() => onSwipeToQueue(track.id), [onSwipeToQueue, track.id]); + + const row = useMemo( + () => ( + + ), + [track, locale, onPress, onLongPress, isSelecting, isSelected, 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} + + ); +}; + +function isSameRow(previous: LibraryRowProps, next: LibraryRowProps): boolean { + return ( + previous.track === next.track && + previous.locale === next.locale && + 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 + ); +} + +export const LibraryRow = memo(LibraryRowComponent, isSameRow); diff --git a/src/features/library/components/ScanBanner.tsx b/src/features/library/components/ScanBanner.tsx index 1e95cf0..7802715 100644 --- a/src/features/library/components/ScanBanner.tsx +++ b/src/features/library/components/ScanBanner.tsx @@ -21,6 +21,17 @@ export function ScanBanner({ progress, onCancel }: ScanBannerProps) { const { t } = useTranslation(); const ratio = progress.total > 0 ? progress.processed / progress.total : 0; + /* + * Both stages report a total only once they have counted, and counting is + * itself a query that takes a moment on a large library. Until then the + * honest thing is to say nothing rather than "0 / 0", which reads as a scan + * that found nothing rather than one that has not looked yet. + * + * The label and the Stop button appear immediately either way, so the banner + * still confirms the press the instant it happens. + */ + const hasTotal = progress.total > 0; + const label = progress.phase === 'enumerating' ? t('library.scanning.enumerating') @@ -44,9 +55,11 @@ export function ScanBanner({ progress, onCancel }: ScanBannerProps) { - - {progress.processed} / {progress.total} - + {hasTotal ? ( + + {progress.processed} / {progress.total} + + ) : null} ); } diff --git a/src/features/library/components/TrackInfoSheet.tsx b/src/features/library/components/TrackInfoSheet.tsx index 80f9255..f274fee 100644 --- a/src/features/library/components/TrackInfoSheet.tsx +++ b/src/features/library/components/TrackInfoSheet.tsx @@ -61,7 +61,7 @@ export function TrackInfoSheet({ track, onClose }: TrackInfoSheetProps) { {rows.map(([label, value]) => ( - {label} + {label} {/* Mono for every technical value, so the column aligns. */} {value} diff --git a/src/features/library/components/TrackList.tsx b/src/features/library/components/TrackList.tsx index 6ffe72e..1c57b8d 100644 --- a/src/features/library/components/TrackList.tsx +++ b/src/features/library/components/TrackList.tsx @@ -1,16 +1,13 @@ import { FlashList, type ListRenderItem } from '@shopify/flash-list'; -import { ListEnd } from 'lucide-react-native'; import type { ReactElement } from 'react'; import { useCallback } from 'react'; import { useTranslation } from 'react-i18next'; import { RefreshControl } from 'react-native'; -import { SwipeableRow } from '@/components/ui/SwipeableRow'; import type { TrackListItem } from '@/db/queries/tracks'; import { useThemeColors } from '@/theme/useTheme'; -import type { Selection } from '../hooks/useSelection'; -import { TrackRow } from './TrackRow'; +import { LibraryRow } from './LibraryRow'; /** * Every row is exactly this tall, from `h-16` on `TrackRow`. @@ -38,7 +35,16 @@ const DRAW_DISTANCE = 1_200; export interface TrackListProps { tracks: TrackListItem[]; locale: string; - selection: Selection; + /** 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. */ onPress: (id: number) => void; /** Opens the action sheet, or starts selection. */ @@ -63,7 +69,8 @@ export interface TrackListProps { export function TrackList({ tracks, locale, - selection, + isSelecting, + selectedIds, onPress, onLongPress, onSwipeToQueue, @@ -75,38 +82,34 @@ export function TrackList({ const { t } = useTranslation(); const colors = useThemeColors(); + const swipeLabel = t('selection.addToQueue'); + const renderItem = useCallback>( ({ item }) => { - const row = ( - ); - - /* - * 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 (selection.isActive) return row; - - return ( - onSwipeToQueue(item.id)} - icon={ListEnd} - accessibilityLabel={t('selection.addToQueue')} - > - {row} - - ); }, - [locale, onPress, onLongPress, onSwipeToQueue, selection, currentTrackId, t], + [ + locale, + onPress, + onLongPress, + onSwipeToQueue, + isSelecting, + selectedIds, + currentTrackId, + swipeLabel, + ], ); return ( diff --git a/src/features/library/components/TrackListSkeleton.tsx b/src/features/library/components/TrackListSkeleton.tsx index ffe6c2d..98d33ae 100644 --- a/src/features/library/components/TrackListSkeleton.tsx +++ b/src/features/library/components/TrackListSkeleton.tsx @@ -1,4 +1,4 @@ -import { View } from 'react-native'; +import { SkeletonRows } from '@/components/ui/Skeleton'; export interface TrackListSkeletonProps { /** Enough to fill a screen. More would only animate off-frame. */ @@ -6,33 +6,12 @@ export interface TrackListSkeletonProps { } /** - * Placeholder rows in the shape of real ones. + * Placeholder rows in the shape of library rows. * - * The States rule asks for skeletons that match the content rather than a - * centred spinner, specifically so the layout does not jump when data lands: - * these are the same 64px row with the same 40px artwork square in the same - * place, so the only thing that changes is that the grey blocks become text. + * A named wrapper rather than `SkeletonRows` inline at the call sites, because + * the row geometry — 64px tall, 40px artwork square — has to stay in step with + * `TrackRow`, and one place to change it is the point. */ export function TrackListSkeleton({ rows = 8 }: TrackListSkeletonProps) { - return ( - - {Array.from({ length: rows }, (_, index) => ( - - - - {/* Two widths, alternating, so it reads as a list rather than a grid. */} - - - - - - ))} - - ); + return ; } diff --git a/src/features/library/hooks/useCollectionRouting.ts b/src/features/library/hooks/useCollectionRouting.ts new file mode 100644 index 0000000..74016ee --- /dev/null +++ b/src/features/library/hooks/useCollectionRouting.ts @@ -0,0 +1,30 @@ +import { useRouter } from 'expo-router'; +import { useCallback, useMemo } from 'react'; + +export interface CollectionRouting { + openArtist: (id: number) => void; + openAlbum: (id: number) => void; +} + +/** + * Navigation into an artist or album. + * + * A hook rather than two inline arrows, so `CollectionGrid` receives callbacks + * that are stable across renders — the cards are memoized on prop identity and + * a fresh closure per render would defeat that for every visible cover. + */ +export function useCollectionRouting(): CollectionRouting { + const router = useRouter(); + + const openArtist = useCallback( + (id: number) => router.navigate(`/collection/artist/${id}`), + [router], + ); + + const openAlbum = useCallback( + (id: number) => router.navigate(`/collection/album/${id}`), + [router], + ); + + return useMemo(() => ({ openArtist, openAlbum }), [openArtist, openAlbum]); +} diff --git a/src/features/library/hooks/useScan.ts b/src/features/library/hooks/useScan.ts index f5c5228..0e7db13 100644 --- a/src/features/library/hooks/useScan.ts +++ b/src/features/library/hooks/useScan.ts @@ -1,9 +1,10 @@ import AudioTags from 'audio-tags'; import { Directory, Paths } from 'expo-file-system'; -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { useCallback, useMemo, useRef, useState } from 'react'; import { addScanFolder, + countUnenriched, listScanFolders, listUnenrichedUris, retireUnseen, @@ -11,6 +12,7 @@ import { saveEnumerated, } from '@/db/queries/scanning'; import { permissionErrorFor } from '@/services/scanner/permission'; +import { getIgnoreShortFiles } from '@/services/settings'; import { isPickerDismissal } from '@/services/scanner/pickerError'; import { PRIMARY_VOLUME_ROOT, treeUriToPath } from '@/services/scanner/treeUri'; import { @@ -23,6 +25,9 @@ import { const IDLE: ScanProgress = { phase: 'idle', total: 0, processed: 0 }; +/** What "short" means when the setting is on. Thirty seconds, as the copy says. */ +const SHORT_FILE_MS = 30_000; + /** * Whether a scan may proceed, and if so whether this is the first time the app * has ever been allowed to read the library. @@ -46,12 +51,16 @@ export interface UseScanResult { /** * True only for a scan the user pulled for. * - * The refresh spinner is a response to a gesture, so it must not appear for - * the automatic launch sweep — that already has the scan banner, and showing + * The refresh spinner is a response to a gesture, so it must not appear for a + * scan started from the button — that already has the scan banner, and showing * both puts a spinner and a progress bar on screen for the same work. */ isRefreshing: boolean; - /** MediaStore sweep. Safe to call on every launch. */ + /** + * Sweep MediaStore. Asks for the audio permission first if it does not have + * it. Only ever called from an explicit user action — see the note in the + * body about there being no automatic sweep. + */ scanLibrary: () => Promise; /** Opens the system folder picker, then scans what was chosen. */ addFolder: () => Promise; @@ -67,18 +76,14 @@ export interface UseScanResult { /** * Drives the two-stage scan and exposes its progress. * - * `ready` gates the automatic launch sweep: pass false until the library has - * painted. The manual entry points ignore it — a user pressing "Add music" has - * asked for the work and should not wait on anything. - * - * Both entry points — the automatic MediaStore sweep and the manual folder - * pick — go through the same pipeline. Manual adding is a first-class way to - * fill the library, not a fallback for when the automatic scan disappoints: - * MediaStore does not index files the system scanner has not seen, folders - * with a `.nomedia`, or some SD card layouts, and this audience keeps music - * in exactly those places. + * All three entry points — the scan button, the folder picker and pull to + * refresh — go through the same pipeline. Manual adding is a first-class way to + * fill the library, not a fallback for when the sweep disappoints: MediaStore + * does not index files the system scanner has not seen, folders with a + * `.nomedia`, or some SD card layouts, and this audience keeps music in exactly + * those places. */ -export function useScan(ready: boolean): UseScanResult { +export function useScan(): UseScanResult { const [progress, setProgress] = useState(IDLE); const [pulled, setPulled] = useState(false); const cancelled = useRef(false); @@ -91,6 +96,7 @@ export function useScan(ready: boolean): UseScanResult { saveEnumerated, saveEnriched, listUnenrichedUris, + countUnenriched, retireUnseen, // Hand the frame back between batches so scrolling never stutters. // `requestIdleCallback` rather than InteractionManager, which RN 0.86 @@ -106,7 +112,17 @@ export function useScan(ready: boolean): UseScanResult { const run = useCallback(async () => { cancelled.current = false; const controller = { isCancelled: () => cancelled.current }; - const options = { ...DEFAULT_SCAN_OPTIONS, artworkDirectory: artworkDirectory() }; + + /* + * Read at scan time, not captured at mount: the switch is in Settings and + * the scan is started from the Library, so the value can legitimately change + * between this hook mounting and the user pressing Scan. + */ + const options = { + ...DEFAULT_SCAN_OPTIONS, + minDurationMs: getIgnoreShortFiles() ? SHORT_FILE_MS : DEFAULT_SCAN_OPTIONS.minDurationMs, + artworkDirectory: artworkDirectory(), + }; const enumerated = await enumerateLibrary(ports, options, setProgress, controller); if (enumerated.phase !== 'done') return; @@ -168,14 +184,12 @@ export function useScan(ready: boolean): UseScanResult { } 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, the automatic sweep that - * runs at launch has never had it, so the library has never actually - * been read. Cancelling the folder picker would then leave a permitted - * app sitting on an empty library until the next cold start. Adding a - * folder is a way to *add* to the library, never the only way to fill - * it. + * 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; @@ -228,43 +242,24 @@ export function useScan(ready: boolean): UseScanResult { }, []); /* - * The automatic sweep. Starts itself once per app session, and only after the - * library it is filling has something on screen. - * - * `ready` is what makes that true, and it was worth measuring. This used to - * be `requestIdleCallback(…, { timeout: 1_000 })` on mount, which meant the - * sweep fired a second after mount whether or not the thread had ever gone - * idle — landing on top of the very first library query. Time from the query - * subscribing to its rows arriving, cold start: + * There is deliberately no automatic sweep. * - * Pixel_7 AVD, 528 tracks 210ms - * Mi 9T, 521 tracks 1608ms + * It used to start itself as soon as the permission was granted, which is how + * a user's first launch became "the app froze". Two separate problems were + * hiding behind each other: stage one held the JS thread for 859ms per page + * (fixed in `saveEnumerated`), and nothing the user did had asked for the work + * in the first place, so there was no moment where waiting felt earned. * - * The query itself is 14–15ms on the emulator, measured five times in a row - * with nothing else running. So nearly all of that was contention, and on the - * slower real device it was a second and a half of skeleton for work that - * takes fifteen milliseconds. + * Scanning is now something the user presses, and the button says what it will + * cost before it starts. That is a real behaviour change, not only a + * performance one: a library scan reads every audio file on the device, and + * doing that unannounced on launch is the sort of thing this app exists not to + * do. See docs/adr/010-scanning-is-user-initiated.md. * - * Because the scan is incremental, an unchanged library still costs one - * MediaStore count and nothing else. A missing permission is not surfaced - * here: the automatic path stays quiet and the empty state does the asking. + * The launch cost of *not* scanning is zero, and a library that has already + * been scanned is already in SQLite — the list paints from the database with + * no MediaStore involvement at all. */ - const swept = useRef(false); - useEffect(() => { - if (!ready || swept.current) return; - swept.current = true; - - const handle = requestIdleCallback( - () => { - void AudioTags.hasAudioPermission().then((granted) => { - if (granted) void run(); - }); - }, - { timeout: 1_000 }, - ); - - return () => cancelIdleCallback(handle); - }, [ready, run]); const isScanning = progress.phase === 'enumerating' || progress.phase === 'enriching'; diff --git a/src/features/library/hooks/useSelection.ts b/src/features/library/hooks/useSelection.ts index 6c8ab2f..90fca62 100644 --- a/src/features/library/hooks/useSelection.ts +++ b/src/features/library/hooks/useSelection.ts @@ -77,5 +77,21 @@ export function useSelection(): Selection { setIds([]); }, []); - return { isActive, ids, has, toggle, activate, begin, toggleAll, clear }; + /* + * 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 e9af739..1165075 100644 --- a/src/features/library/hooks/useTrackActions.ts +++ b/src/features/library/hooks/useTrackActions.ts @@ -1,9 +1,11 @@ import { useCallback } from 'react'; +import { useTranslation } from 'react-i18next'; import type { TrackListItem } from '@/db/queries/tracks'; import { setFavorite } from '@/db/queries/tracks'; import { AudioEngine } from '@/services/audio/AudioEngine'; import { commitFeedback, rejectFeedback } from '@/services/haptics'; +import { showToast } from '@/services/toast'; import { toPlayable } from '../../player/toPlayable'; @@ -27,30 +29,50 @@ export interface TrackActions { * 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 * than saying no. + * + * Each one also raises a toast. A swipe that queues a track has no visible + * result — the queue is on another screen — so without a message the gesture is + * indistinguishable from a scroll that did nothing. The haptic alone is not + * enough: it is invisible, and it is off for anyone who turned it off. */ export function useTrackActions(): TrackActions { - const addToQueue = useCallback((tracks: TrackListItem[]) => { - if (tracks.length === 0) { - rejectFeedback(); - return; - } - commitFeedback(); - void AudioEngine.enqueue(tracks.map(toPlayable)); - }, []); - - const playNext = useCallback((tracks: TrackListItem[]) => { - if (tracks.length === 0) { - rejectFeedback(); - return; - } - commitFeedback(); - void AudioEngine.playNext(tracks.map(toPlayable)); - }, []); - - const toggleFavorite = useCallback((track: TrackListItem) => { - commitFeedback(); - void setFavorite(track.id, !track.isFavorite); - }, []); + const { t } = useTranslation(); + + const addToQueue = useCallback( + (tracks: TrackListItem[]) => { + if (tracks.length === 0) { + rejectFeedback(); + return; + } + commitFeedback(); + void AudioEngine.enqueue(tracks.map(toPlayable)); + showToast(t('toast.queued', { count: tracks.length })); + }, + [t], + ); + + const playNext = useCallback( + (tracks: TrackListItem[]) => { + if (tracks.length === 0) { + rejectFeedback(); + return; + } + commitFeedback(); + void AudioEngine.playNext(tracks.map(toPlayable)); + showToast(t('toast.playingNext', { count: tracks.length })); + }, + [t], + ); + + const toggleFavorite = useCallback( + (track: TrackListItem) => { + commitFeedback(); + const next = !track.isFavorite; + void setFavorite(track.id, next); + showToast(next ? t('toast.favorited') : t('toast.unfavorited')); + }, + [t], + ); return { addToQueue, playNext, toggleFavorite }; } diff --git a/src/features/player/PlayerScreen.tsx b/src/features/player/PlayerScreen.tsx index 11187c2..4174c49 100644 --- a/src/features/player/PlayerScreen.tsx +++ b/src/features/player/PlayerScreen.tsx @@ -1,4 +1,3 @@ -import { Image } from 'expo-image'; import { useRouter } from 'expo-router'; import { ChevronDown, @@ -25,11 +24,12 @@ import type { RepeatMode } from '@/services/audio/types'; import { formatDuration } from '@/services/format/duration'; import { useThemeColors } from '@/theme/useTheme'; +import { ArtworkCarousel } from './components/ArtworkCarousel'; import { FavoriteButton } from './components/FavoriteButton'; import { Scrubber } from './components/Scrubber'; import { SpecStrip } from './components/SpecStrip'; -import { TransportSwipe } from './components/TransportSwipe'; import { usePlayback, usePlaybackControls } from './hooks/usePlayback'; +import { useQueueNeighbours } from './hooks/useQueueNeighbours'; /** * Now Playing. @@ -44,11 +44,14 @@ export function PlayerScreen() { const { phase, track, positionMs, durationMs, error } = usePlayback(); const { toggle, toggleShuffle, next, previous, seekTo } = usePlaybackControls(); + const neighbours = useQueueNeighbours(); const [repeat, setRepeatState] = useState(() => AudioEngine.getRepeat()); const [shuffled, setShuffledState] = useState(() => AudioEngine.isShuffled()); const close = useCallback(() => router.back(), [router]); - const openQueue = useCallback(() => router.push('/queue'), [router]); + // `navigate` rather than `push`, for the same reason as the player itself: + // a double press must not leave two identical screens on the stack. + const openQueue = useCallback(() => router.navigate('/queue'), [router]); const onShufflePress = useCallback(() => { toggleShuffle(); @@ -73,39 +76,30 @@ export function PlayerScreen() { const isPlaying = phase === 'playing'; const isLoading = phase === 'loading'; const algorithm = getShuffleAlgorithm(); - const artworkUri = track.artworkPath ? `file://${track.artworkPath}` : null; const RepeatIcon = repeat === 'one' ? Repeat1 : Repeat; return (
- + {/* The artwork carries the gestures, not the whole screen: the scrubber - below it owns a pan of its own, and two competing pans on one surface + below owns a pan of its own, and two competing pans on one surface means a scrub that sometimes dismisses the screen instead. Down dismisses, which is what the brief asks for and what Android's - modal presentation does not give on its own. + modal presentation does not give on its own. Sideways moves through the + queue with the neighbouring covers already on screen. */} - - - {artworkUri ? ( - - ) : ( - - )} - - - - + + + {track.title} @@ -119,12 +113,12 @@ export function PlayerScreen() { {phase === 'error' ? ( - + {t('player.error')} {error ? ` ${error}` : ''} ) : ( - + )} - + = Math.abs(0)` is true — so every + * gesture locked to horizontal and vertical ones were silently discarded. A + * downward drag simply sprang back. Waiting for real movement is the fix. + */ +const AXIS_LOCK_SLOP = 6; +/** One spring for every snap, so a flick and a drag settle identically. */ +const SPRING = { damping: 22, stiffness: 190, mass: 0.6 } as const; + +export interface ArtworkCarouselProps { + neighbours: QueueNeighbours; + onNext: () => void; + onPrevious: () => void; + /** Swipe down on the artwork dismisses the player. */ + onDismiss: () => void; +} + +/** + * The Now Playing artwork, as a carousel of three. + * + * Previous, current and next are all mounted side by side and the whole strip + * translates under the finger, so the neighbour is a real decoded image sliding + * in rather than a blank square that fills in after the transition. That is the + * difference between this and the version it replaces, which moved one image + * and swapped its source on release. + * + * Release is decided by distance **or** velocity: a lazy drag past 28% of the + * screen commits, and so does a flick over 500px/s however short. Committing on + * distance alone is what makes a carousel feel unresponsive to people who flick; + * committing on velocity alone strands people who drag slowly. + * + * At the ends of the queue the strip still moves, but at a quarter rate and it + * always springs back. Refusing to move at all reads as a dropped gesture; a + * rubber band says "there is nothing here" in the language the gesture is + * already speaking. + * + * The commit is optimistic in exactly one respect: `translateX` snaps to the + * neighbour's slot and is reset to centre by the engine's queue update that + * follows. Both are driven from the same spring, so the seam is not visible. + */ +export function ArtworkCarousel({ + neighbours, + onNext, + onPrevious, + onDismiss, +}: ArtworkCarouselProps) { + const { width } = 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); + + const { previous, current, next } = neighbours; + const hasPrevious = previous !== null; + const hasNext = next !== null; + + /* + * Built inline rather than memoized, matching `Scrubber`. + * + * Memoizing it is what `SwipeableRow` does, and there it is worth it — that + * one has forty live instances in a list. This has exactly one, so rebuilding + * the gesture on the rare render costs nothing measurable, and doing it inline + * keeps the shared values out of a hook's closure. The React Compiler's + * immutability rule rejects mutating a value captured by a hook, which is + * correct for ordinary values and unavoidable friction for Reanimated. + */ + const pan = Gesture.Pan() + .onBegin(() => { + axis.value = 0; + }) + .onUpdate((event) => { + if (axis.value === 0) { + const dx = Math.abs(event.translationX); + const dy = Math.abs(event.translationY); + // 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) { + // Down only. Dragging up from the player has no meaning. + offsetY.value = Math.max(0, event.translationY); + return; + } + + const wanted = event.translationX; + // Resist where there is nothing to reveal: dragging right at the start of + // the queue, or left at the end. + const blocked = (wanted > 0 && !hasPrevious) || (wanted < 0 && !hasNext); + offsetX.value = blocked ? wanted * RUBBER_BAND : wanted; + }) + .onEnd((event) => { + if (axis.value === 2) { + if (event.translationY > DISMISS_DISTANCE || event.velocityY > DISMISS_VELOCITY) { + runOnJS(onDismiss)(); + } + return; + } + + const far = Math.abs(event.translationX) > width * DISTANCE_THRESHOLD; + const fast = Math.abs(event.velocityX) > VELOCITY_THRESHOLD; + + if (far || fast) { + // Left reveals the next track; right reveals the previous one. + if (event.translationX < 0 && hasNext) { + offsetX.value = withSpring(-width, SPRING, (finished) => { + if (!finished) return; + /* + * Snap back to centre in the same frame the engine is told to + * advance. The queue update re-fills the slots a moment later, so + * leaving the strip parked one slot over would show the right track + * in the wrong position. + */ + offsetX.value = 0; + runOnJS(onNext)(); + }); + return; + } + if (event.translationX > 0 && hasPrevious) { + offsetX.value = withSpring(width, SPRING, (finished) => { + if (!finished) return; + offsetX.value = 0; + runOnJS(onPrevious)(); + }); + return; + } + } + + // Did not commit, or committed against the end of the queue: spring home. + offsetX.value = withSpring(0, SPRING); + }) + .onFinalize(() => { + axis.value = 0; + offsetY.value = withSpring(0, SPRING); + }); + + const stripStyle = useAnimatedStyle(() => ({ + transform: [{ translateX: offsetX.value }, { translateY: offsetY.value }], + })); + + return ( + + {/* Clips the neighbours to the visible slot. */} + + + + + + + + + ); +} + +interface SlotProps { + track: PlayableTrack | null; + width: number; + /** Where this slot sits relative to the centre one. */ + offset: number; +} + +/** + * One square in the strip. + * + * 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. + */ +function Slot({ track, width, offset }: SlotProps) { + const { t } = useTranslation(); + const colors = useThemeColors(); + + const artworkUri = track?.artworkPath ? `file://${track.artworkPath}` : null; + const isCentre = offset === 0; + + const style = useMemo( + () => ({ width, left: offset, position: 'absolute' as const, top: 0, bottom: 0 }), + [width, offset], + ); + + 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} + + ); + } + + return ( + + {content} + + ); +} diff --git a/src/features/player/components/MiniPlayer.tsx b/src/features/player/components/MiniPlayer.tsx index 9727a38..b759bc0 100644 --- a/src/features/player/components/MiniPlayer.tsx +++ b/src/features/player/components/MiniPlayer.tsx @@ -1,16 +1,46 @@ import { Image } from 'expo-image'; import { useRouter } from 'expo-router'; -import { Music, Pause, Play, SkipForward } from 'lucide-react-native'; +import { Music, Pause, Play, SkipBack, SkipForward } from 'lucide-react-native'; import { useCallback } from 'react'; import { useTranslation } from 'react-i18next'; import { Pressable, Text, View } from 'react-native'; +import { Gesture, GestureDetector } from 'react-native-gesture-handler'; +import Animated, { + runOnJS, + useAnimatedStyle, + useSharedValue, + withSpring, +} from 'react-native-reanimated'; +import { tapFeedback } from '@/services/haptics'; import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; +import { useReducedMotion } from '@/theme/useReducedMotion'; import { useThemeColors } from '@/theme/useTheme'; import { useCurrentTrack, usePlaybackControls, usePlaybackPhase } from '../hooks/usePlayback'; import { MiniProgress } from './MiniProgress'; -import { TransportSwipe } from './TransportSwipe'; + +/** 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. */ +const SKIP_DISTANCE = 56; +/** Below this the pan never claims the touch, so the buttons still work. */ +const ACTIVATION_SLOP = 10; +/** + * Movement needed before the gesture commits to an axis. + * + * Not cosmetic. Deciding on the first `onUpdate` compares two translations that + * are both still zero, and `Math.abs(0) >= Math.abs(0)` is true — so every + * gesture locked to horizontal and vertical ones were silently discarded. A + * downward drag simply sprang back. Waiting for real movement is the fix. + */ +const AXIS_LOCK_SLOP = 6; +/** How far the strip follows the finger. Damped, so it reads as resistance. */ +const FOLLOW_RATIO = 0.4; + +const SPRING = { damping: 20, stiffness: 220 } as const; /** * The persistent transport strip above the tab bar. @@ -18,31 +48,115 @@ import { TransportSwipe } from './TransportSwipe'; * Renders nothing at all when idle rather than sitting there empty — a dead * strip on a fresh install is clutter, and the tab bar should meet the list * until there is something playing. + * + * Three ways to open the player, because the strip is 64px tall and precision is + * not always available: tap the artwork and title, drag the strip upwards, or + * 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() { useLifecycleTrace('MiniPlayer'); const { t } = useTranslation(); const colors = useThemeColors(); const router = useRouter(); + const reducedMotion = useReducedMotion(); + /* * Phase and track, never position. * * Measured on the Pixel_7 AVD over ten seconds of playback: subscribing to * the whole engine state re-rendered this component 20 times — exactly the - * engine's 2 Hz status interval — and 0 times after the split. `BottomTabBar` - * next door was never affected either way, which is worth writing down - * because it was the thing this change was first blamed on: React re-renders - * the component whose store changed and its children, not its siblings. - * - * Twenty reconciliations of an `expo-image` and three Pressables per ten - * seconds, forever, for a strip whose text has not changed. Position belongs - * to `MiniProgress`, which is one animated view and re-renders never. + * engine's 2 Hz status interval — and 0 times after the split. Twenty + * reconciliations of an `expo-image` and four Pressables per ten seconds, for + * a strip whose text has not changed. Position belongs to `MiniProgress`, + * which is one animated view and re-renders never. */ const phase = usePlaybackPhase(); const track = useCurrentTrack(); const { toggle, next, previous } = usePlaybackControls(); - const openPlayer = useCallback(() => router.push('/player'), [router]); + /* + * `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]); + + const offsetX = useSharedValue(0); + const offsetY = useSharedValue(0); + const axis = useSharedValue(0); + + /* + * The gesture is built here rather than in a shared wrapper, and that is the + * fix for "swiping the mini player up does not open the player". + * + * The previous version put a generic pan on a container whose children are all + * Pressables, with `activeOffsetX` and `activeOffsetY` both set. Two axes of + * activation on a surface made entirely of touch targets meant the pan + * routinely lost the race to a child's press responder, so the swipe worked + * sometimes and looked broken the rest of the time. + * + * This one activates on vertical movement only (`activeOffsetY`) and treats + * horizontal travel as a secondary read once it already owns the gesture, so + * it never competes with a tap and never needs to. + * + * Built inline rather than memoized, like `Scrubber` and unlike + * `SwipeableRow`: there is exactly one mini player, so a rebuild per render + * costs nothing, and keeping the shared values out of a hook's closure avoids + * the React Compiler's immutability rule — which is right about ordinary + * values and simply does not model Reanimated. + */ + const pan = Gesture.Pan() + .activeOffsetY([-ACTIVATION_SLOP, ACTIVATION_SLOP]) + .onBegin(() => { + axis.value = 0; + }) + .onUpdate((event) => { + if (axis.value === 0) { + const dx = Math.abs(event.translationX); + const dy = Math.abs(event.translationY); + // 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 === 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; + }) + .onEnd((event) => { + if (axis.value === 1) { + if (event.translationX <= -SKIP_DISTANCE) runOnJS(next)(); + else if (event.translationX >= SKIP_DISTANCE) runOnJS(previous)(); + return; + } + + // Distance or velocity, so a short flick opens it as readily as a + // deliberate drag. + const far = event.translationY <= -OPEN_DISTANCE; + const fast = event.velocityY <= -OPEN_VELOCITY; + if (far || fast) runOnJS(openPlayer)(); + }) + .onFinalize(() => { + axis.value = 0; + offsetX.value = withSpring(0, SPRING); + offsetY.value = withSpring(0, SPRING); + }); + + const followStyle = useAnimatedStyle(() => ({ + transform: [{ translateX: offsetX.value }, { translateY: offsetY.value }], + })); if (phase === 'idle' || track === null) return null; @@ -53,76 +167,88 @@ export function MiniPlayer() { {/* A hairline of progress rather than a scrub bar. The mini player says - where you are; seeking is the full player's job, and a 2px target is - not a control anyone can hit. + where you are; seeking is the full player's job, and a 2px target is not + a control anyone can hit. - Outside the swipe wrapper: the bar reports position and should not slide - around with the strip that reports the track. + Outside the gesture wrapper: the bar reports position and should not + slide around with the strip that reports the track. */} - {/* - Swipe up to open, sideways to change track. The strip is small and its - two icon buttons are the only precise targets on it, so the gesture is - how most people will actually drive it. - */} - - - - {artworkUri ? ( - - ) : ( - - - - )} - - - - {track.title} - - {track.artistName ? ( - - {track.artistName} + + + + + {artworkUri ? ( + + ) : ( + + + + )} + + + + {track.title} - ) : null} - - - - - {isPlaying ? ( - - ) : ( - - )} - - - - - - - + {track.artistName ? ( + + {track.artistName} + + ) : null} + + + + {/* + Previous belongs here as much as next does. Skipping back is the + commonest correction there is — you skip one too many and want it + back — and having only "next" made the strip a one-way control. + */} + + + + + + {isPlaying ? ( + + ) : ( + + )} + + + + + + + + ); } diff --git a/src/features/player/components/TransportSwipe.tsx b/src/features/player/components/TransportSwipe.tsx deleted file mode 100644 index d892be4..0000000 --- a/src/features/player/components/TransportSwipe.tsx +++ /dev/null @@ -1,112 +0,0 @@ -import type { ReactNode } from 'react'; -import { Gesture, GestureDetector } from 'react-native-gesture-handler'; -import Animated, { - runOnJS, - useAnimatedStyle, - useSharedValue, - withSpring, -} from 'react-native-reanimated'; - -import { tapFeedback } from '@/services/haptics'; -import { useReducedMotion } from '@/theme/useReducedMotion'; - -/** How far a finger must travel before the gesture counts as a swipe. */ -const COMMIT_DISTANCE = 60; -/** Below this the pan never activates, so taps and scrolls are unaffected. */ -const ACTIVATION_SLOP = 12; -/** How far the content follows the finger. Damped, so it reads as resistance. */ -const FOLLOW_RATIO = 0.35; - -export interface TransportSwipeProps { - children: ReactNode; - /** Swipe right — the direction that goes back, as it does in a book. */ - onSwipeRight?: () => void; - onSwipeLeft?: () => void; - onSwipeUp?: () => void; - onSwipeDown?: () => void; -} - -/** - * Swipes for the transport, with the content following the finger. - * - * Wraps the mini player and the Now Playing artwork. Horizontal changes track, - * vertical opens or dismisses — the gestures everyone already has in their - * hands from every other player, which is the entire argument for them. - * - * The axis is decided once, at the point the pan activates, and held for the - * rest of the gesture. Deciding per frame lets a diagonal drag flip between - * "change track" and "dismiss" mid-swipe, so the user gets whichever one their - * finger happened to be favouring when they let go. - * - * Distance decides, not velocity. A flick and a slow drag of the same length - * mean the same thing, and velocity thresholds are the reason swipe controls - * feel unreliable to people who do not flick. - */ -export function TransportSwipe({ - children, - onSwipeRight, - onSwipeLeft, - onSwipeUp, - onSwipeDown, -}: TransportSwipeProps) { - const offsetX = useSharedValue(0); - const offsetY = useSharedValue(0); - /** 0 undecided, 1 horizontal, 2 vertical. Set once per gesture. */ - const axis = useSharedValue(0); - const reducedMotion = useReducedMotion(); - - const pan = Gesture.Pan() - .activeOffsetX([-ACTIVATION_SLOP, ACTIVATION_SLOP]) - .activeOffsetY([-ACTIVATION_SLOP, ACTIVATION_SLOP]) - .onBegin(() => { - axis.value = 0; - }) - .onUpdate((event) => { - if (axis.value === 0) { - axis.value = Math.abs(event.translationX) >= Math.abs(event.translationY) ? 1 : 2; - } - - if (reducedMotion) return; - - if (axis.value === 1) offsetX.value = event.translationX * FOLLOW_RATIO; - else offsetY.value = event.translationY * FOLLOW_RATIO; - }) - .onEnd((event) => { - const horizontal = axis.value === 1; - const travelled = horizontal ? event.translationX : event.translationY; - - if (Math.abs(travelled) >= COMMIT_DISTANCE) { - if (horizontal) runOnJS(fire)(travelled > 0 ? onSwipeRight : onSwipeLeft); - else runOnJS(fire)(travelled > 0 ? onSwipeDown : onSwipeUp); - } - }) - .onFinalize(() => { - // Always springs home. The committed action replaces the content or the - // screen; leaving the view displaced would show the next track offset. - offsetX.value = withSpring(0, { damping: 20, stiffness: 200 }); - offsetY.value = withSpring(0, { damping: 20, stiffness: 200 }); - axis.value = 0; - }); - - const followStyle = useAnimatedStyle(() => ({ - transform: [{ translateX: offsetX.value }, { translateY: offsetY.value }], - })); - - return ( - - {children} - - ); -} - -/** - * Run a handler if there is one, with feedback. - * - * A swipe in a direction nothing is bound to stays silent rather than buzzing: - * confirming an action that did not happen is worse than no confirmation. - */ -function fire(handler: (() => void) | undefined): void { - if (!handler) return; - tapFeedback(); - handler(); -} diff --git a/src/features/player/hooks/useQueueNeighbours.ts b/src/features/player/hooks/useQueueNeighbours.ts new file mode 100644 index 0000000..e1bb463 --- /dev/null +++ b/src/features/player/hooks/useQueueNeighbours.ts @@ -0,0 +1,50 @@ +import { useMemo, useSyncExternalStore } from 'react'; + +import { AudioEngine, type QueueSnapshot } from '@/services/audio/AudioEngine'; +import type { PlayableTrack } from '@/services/audio/types'; + +export interface QueueNeighbours { + previous: PlayableTrack | null; + current: PlayableTrack | null; + next: PlayableTrack | null; +} + +/** + * What is playing, and what sits either side of it in the queue. + * + * The carousel needs all three mounted at once. That is the whole point of it: + * the neighbouring artwork is already decoded and on screen just off the edge, + * so dragging sideways reveals a real image instead of a blank square that + * fills in a moment later. + * + * Subscribes to the engine's queue rather than its playback state — state is + * emitted twice a second for the position, and re-deriving this at 2 Hz would + * hand the carousel a new object on every tick. + * + * Null at either end rather than wrapping. Repeat-all does wrap, but the queue + * screen and the transport both treat the ends as ends, and a carousel that + * silently loops from the last track back to the first would be showing the + * user something the skip button would not do. + */ +export function useQueueNeighbours(): QueueNeighbours { + const snapshot = useSyncExternalStore(subscribe, getSnapshot); + + return useMemo(() => { + const { tracks, index } = snapshot; + if (index < 0) return { previous: null, current: null, next: null }; + + return { + previous: tracks[index - 1] ?? null, + current: tracks[index] ?? null, + next: tracks[index + 1] ?? null, + }; + }, [snapshot]); +} + +function subscribe(onChange: () => void): () => void { + return AudioEngine.subscribeQueue(onChange); +} + +function getSnapshot(): QueueSnapshot { + return AudioEngine.getQueueSnapshot(); +} diff --git a/src/features/player/listenRecorder.ts b/src/features/player/listenRecorder.ts index cd3fcdf..b613adc 100644 --- a/src/features/player/listenRecorder.ts +++ b/src/features/player/listenRecorder.ts @@ -24,7 +24,9 @@ export function startListenRecording(): () => void { durationMs: listen.track.durationMs, msPlayed: listen.msPlayed, startedAt: listen.startedAt, - sourceType: 'library', + sourceType: listen.source.type, + sourceId: listen.source.id, + shuffleAlgorithm: listen.shuffleAlgorithm ?? undefined, completed: listen.completed, }, getWeekStart(), diff --git a/src/features/playlists/PlaylistDetailScreen.tsx b/src/features/playlists/PlaylistDetailScreen.tsx index d3341a4..28e3ae0 100644 --- a/src/features/playlists/PlaylistDetailScreen.tsx +++ b/src/features/playlists/PlaylistDetailScreen.tsx @@ -1,7 +1,7 @@ import { FlashList, type ListRenderItem } from '@shopify/flash-list'; import { useRouter } from 'expo-router'; import { ListMusic } from 'lucide-react-native'; -import { useCallback, useState } from 'react'; +import { useCallback, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { View } from 'react-native'; import { SafeAreaView } from 'react-native-safe-area-context'; @@ -19,10 +19,9 @@ import { } from '@/db/queries/playlists'; import { useMessages } from '@/i18n'; import { AudioEngine } from '@/services/audio/AudioEngine'; -import type { PlayableTrack } from '@/services/audio/types'; +import type { PlayableTrack, QueueSource } from '@/services/audio/types'; import { getShuffleAlgorithm } from '@/services/settings'; -import { usePlaybackControls } from '../player/hooks/usePlayback'; import { AddTracksSheet } from './components/AddTracksSheet'; import { NamePlaylistDialog } from './components/NamePlaylistDialog'; import { PlaylistDetailHeader } from './components/PlaylistDetailHeader'; @@ -41,14 +40,24 @@ export function PlaylistDetailScreen({ playlistId }: PlaylistDetailScreenProps) const entries = usePlaylistEntries(playlistId); const playlist = usePlaylists().find((entry) => entry.id === playlistId); - const { playFrom } = usePlaybackControls(); const [renaming, setRenaming] = useState(false); const [adding, setAdding] = useState(false); + /* + * Every entry point here declares the playlist 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], + ); + const playAll = useCallback(() => { - if (entries.length > 0) playFrom(entries.map(toPlayableEntry), 0); - }, [entries, playFrom]); + if (entries.length > 0) void AudioEngine.setQueue(entries.map(toPlayableEntry), 0, source); + }, [entries, source]); /* * Shuffle uses whichever algorithm Settings has selected, read at press time. @@ -59,16 +68,16 @@ export function PlaylistDetailScreen({ playlistId }: PlaylistDetailScreenProps) */ const shuffleAll = useCallback(async () => { if (entries.length === 0) return; - await AudioEngine.setQueue(entries.map(toPlayableEntry), 0); + await AudioEngine.setQueue(entries.map(toPlayableEntry), 0, source); await AudioEngine.setShuffled(true, getShuffleAlgorithm()); - }, [entries]); + }, [entries, source]); const playAt = useCallback( (position: number) => { const index = entries.findIndex((entry) => entry.position === position); - if (index !== -1) playFrom(entries.map(toPlayableEntry), index); + if (index !== -1) void AudioEngine.setQueue(entries.map(toPlayableEntry), index, source); }, - [entries, playFrom], + [entries, source], ); const remove = useCallback( diff --git a/src/features/playlists/components/AddToPlaylistSheet.tsx b/src/features/playlists/components/AddToPlaylistSheet.tsx index 4a8d1dc..8d100fb 100644 --- a/src/features/playlists/components/AddToPlaylistSheet.tsx +++ b/src/features/playlists/components/AddToPlaylistSheet.tsx @@ -71,7 +71,7 @@ export function AddToPlaylistSheet({ trackIds, onClose }: AddToPlaylistSheetProp > {trackIds.length > 1 diff --git a/src/features/playlists/components/AddTracksSheet.tsx b/src/features/playlists/components/AddTracksSheet.tsx index ab8ea5f..b695f5b 100644 --- a/src/features/playlists/components/AddTracksSheet.tsx +++ b/src/features/playlists/components/AddTracksSheet.tsx @@ -149,8 +149,8 @@ function PickRow({ track, isPicked, onToggle }: PickRowProps) { {isPicked ? : null} diff --git a/src/features/playlists/components/PlaylistMosaic.tsx b/src/features/playlists/components/PlaylistMosaic.tsx index fe53407..ed34444 100644 --- a/src/features/playlists/components/PlaylistMosaic.tsx +++ b/src/features/playlists/components/PlaylistMosaic.tsx @@ -28,7 +28,13 @@ export interface PlaylistMosaicProps { */ export function PlaylistMosaic({ covers, size = 'sm' }: PlaylistMosaicProps) { const colors = useThemeColors(); - const box = size === 'lg' ? 'h-32 w-32' : 'h-12 w-12'; + /* + * A fraction rather than a fixed size for the large variant. The obvious + * 128px square is not on the spacing scale, so it compiled to nothing and the + * mosaic drew at zero by zero — see `src/theme/scale.test.ts`, which now fails + * on any such class. + */ + const box = size === 'lg' ? 'aspect-square w-1/3' : 'h-12 w-12'; if (covers.length === 0) { return ( diff --git a/src/features/settings/SettingsScreen.tsx b/src/features/settings/SettingsScreen.tsx index 98faec6..a49d823 100644 --- a/src/features/settings/SettingsScreen.tsx +++ b/src/features/settings/SettingsScreen.tsx @@ -1,17 +1,33 @@ -import { Languages, Monitor, Moon, Shuffle, Sun, type LucideIcon } from 'lucide-react-native'; +import { + Clock, + Languages, + Monitor, + Moon, + Shuffle, + Sun, + Vibrate, + type LucideIcon, +} from 'lucide-react-native'; import { useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { ScrollView, Text } from 'react-native'; +import { ScrollView } from 'react-native'; +import { OptionList, type Option } from '@/components/ui/OptionList'; import { Screen } from '@/components/ui/Screen'; import { SegmentedControl, type SegmentedControlOption } from '@/components/ui/SegmentedControl'; import { SettingGroup } from '@/components/ui/SettingGroup'; import { SettingRow } from '@/components/ui/SettingRow'; +import { SettingSwitch } from '@/components/ui/SettingSwitch'; import { changeLanguage } from '@/i18n'; +import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { + getHapticsEnabled, + getIgnoreShortFiles, getLanguagePreference, getShuffleAlgorithm, LANGUAGE_PREFERENCES, + setHapticsEnabled, + setIgnoreShortFiles, setShuffleAlgorithm, THEME_PREFERENCES, type LanguagePreference, @@ -19,7 +35,6 @@ import { } from '@/services/settings'; import { SHUFFLE_ALGORITHMS, type ShuffleAlgorithm } from '@/services/shuffle'; import { useTheme } from '@/theme/useTheme'; -import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { DevTools } from './components/DevTools'; import { ScanFolderList } from './components/ScanFolderList'; @@ -30,12 +45,27 @@ const THEME_ICONS: Record = { dark: Moon, }; +/** + * Every setting, each one explained. + * + * The rule this screen follows: a row names a setting, and a line under it says + * what the setting does. A list of bare names makes the user guess, and the + * guesses are wrong exactly where it matters — nobody knows what "Discovery" + * means from the word. + * + * Theme and language stay as segmented controls: three and three short, + * self-evident options each. Shuffle is a column, because five names that need + * explaining is a different problem — see `OptionList`. + */ export function SettingsScreen() { useLifecycleTrace('SettingsScreen'); const { t } = useTranslation(); const { preference: theme, setPreference: setTheme } = useTheme(); + const [language, setLanguage] = useState(getLanguagePreference); const [shuffle, setShuffle] = useState(getShuffleAlgorithm); + const [haptics, setHaptics] = useState(getHapticsEnabled); + const [ignoreShort, setIgnoreShort] = useState(getIgnoreShortFiles); const themeOptions: SegmentedControlOption[] = THEME_PREFERENCES.map( (value) => ({ @@ -49,9 +79,11 @@ export function SettingsScreen() { (value) => ({ value, label: t(`settings.language.${value}`) }), ); - const shuffleOptions: SegmentedControlOption[] = SHUFFLE_ALGORITHMS.map( - (value) => ({ value, label: t(`settings.shuffle.${value}`) }), - ); + const shuffleOptions: Option[] = SHUFFLE_ALGORITHMS.map((value) => ({ + value, + label: t(`settings.shuffle.${value}`), + description: t(`settings.shuffle.${value}Hint`), + })); function onShuffleChange(next: ShuffleAlgorithm) { setShuffle(next); @@ -63,6 +95,16 @@ export function SettingsScreen() { changeLanguage(next); } + function onHapticsChange(next: boolean) { + setHaptics(next); + setHapticsEnabled(next); + } + + function onIgnoreShortChange(next: boolean) { + setIgnoreShort(next); + setIgnoreShortFiles(next); + } + return ( @@ -71,6 +113,7 @@ export function SettingsScreen() { icon={THEME_ICONS[theme]} label={t('settings.appearance.theme')} value={t(`settings.appearance.${theme}`)} + description={t('settings.appearance.description')} > + {/* No `description` on the row: the list explains every option, and + saying it twice is worse than saying it once. */} - - {/* Says what each one does — the names alone do not, and the whole - point of offering five is that the user can tell them apart. */} - - {t(`settings.shuffle.${shuffle}Hint`)} - + + + {/* + No "resume on launch" switch yet. The preference exists in the store, + but nothing persists or restores a queue, so the control would have + been a switch that changes nothing — worse than an absent feature, + because it claims one. It goes in when the queue does. + */} + + + diff --git a/src/features/stats/StatsScreen.tsx b/src/features/stats/StatsScreen.tsx index ba1f11d..460392e 100644 --- a/src/features/stats/StatsScreen.tsx +++ b/src/features/stats/StatsScreen.tsx @@ -1,4 +1,4 @@ -import { BarChart3 } from 'lucide-react-native'; +import { BarChart3, Disc3, ListMusic, Music, User } from 'lucide-react-native'; import { useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { ScrollView, View } from 'react-native'; @@ -6,15 +6,22 @@ import { ScrollView, View } from 'react-native'; import { EmptyState } from '@/components/ui/EmptyState'; import { Screen } from '@/components/ui/Screen'; import { SegmentedControl, type SegmentedControlOption } from '@/components/ui/SegmentedControl'; -import { usePeriodTotals, useTopArtists, useTopTracks } from '@/db/queries/stats'; +import { + usePeriodTotals, + useTopAlbums, + useTopArtists, + useTopPlaylists, + useTopTracks, +} from '@/db/queries/stats'; import { useMessages } from '@/i18n'; +import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { getWeekStart } from '@/services/settings'; import { periodKeys } from '@/services/stats/periodKeys'; import { PERIOD_TYPES, type PeriodType } from '@/services/stats/rollups'; -import { useLifecycleTrace } from '@/services/perf/useLifecycleTrace'; import { StatTotals } from './components/StatTotals'; import { TopList } from './components/TopList'; +import { Wrapped } from './components/Wrapped'; /** * Listening statistics, computed on-device from the user's own history. @@ -22,6 +29,10 @@ import { TopList } from './components/TopList'; * Reads `stats_rollups` only. Aggregating `play_events` here would be a scan * over the whole listening history on every tab switch, growing forever — see * `docs/stats.md`. + * + * The Wrapped card leads, then the tiles, then the four ranked lists. That order + * is deliberate: the summary answers the question people open this tab for, and + * everything below it is there for whoever wants to keep reading. */ export function StatsScreen() { useLifecycleTrace('StatsScreen'); @@ -36,6 +47,8 @@ export function StatsScreen() { const totals = usePeriodTotals(period, periodKey); const topTracks = useTopTracks(period, periodKey); const topArtists = useTopArtists(period, periodKey); + const topAlbums = useTopAlbums(period, periodKey); + const topPlaylists = useTopPlaylists(period, periodKey); const periodOptions: SegmentedControlOption[] = PERIOD_TYPES.map((value) => ({ value, @@ -57,9 +70,19 @@ export function StatsScreen() { {hasData ? ( + + - - + + + + + ) : ( /* diff --git a/src/features/stats/components/TopList.tsx b/src/features/stats/components/TopList.tsx index 19647ed..db076e6 100644 --- a/src/features/stats/components/TopList.tsx +++ b/src/features/stats/components/TopList.tsx @@ -1,23 +1,38 @@ +import { Image } from 'expo-image'; +import type { LucideIcon } from 'lucide-react-native'; import { useTranslation } from 'react-i18next'; import { Text, View } from 'react-native'; import type { TopEntry } from '@/db/queries/stats'; +import { formatListeningTime } from '@/services/format/listeningTime'; +import { useThemeColors } from '@/theme/useTheme'; export interface TopListProps { /** Already translated. */ title: string; entries: readonly TopEntry[]; + /** Drawn when an entry has no cover. Says what kind of thing this list holds. */ + icon: LucideIcon; } /** - * A ranked list — top tracks, top artists. + * A ranked list — top tracks, artists, albums or playlists. * * Renders nothing when empty rather than an empty card: on a fresh week the * screen already says there is no listening yet, and a second "nothing here" - * underneath it is noise. + * underneath it is noise. Top playlists is empty for most people most of the + * time, and a permanent empty card for it would be worse than its absence. + * + * Every row carries both numbers. The play count answers "how often" and the + * listening time answers "how much", and they disagree constantly — a + * three-minute song played twice beats a forty-minute mix played once on one + * measure and loses badly on the other. Showing only the count was hiding half + * of what `stats_rollups` already knew. */ -export function TopList({ title, entries }: TopListProps) { +export function TopList({ title, entries, icon: Icon }: TopListProps) { const { t, i18n } = useTranslation(); + const colors = useThemeColors(); + if (entries.length === 0) return null; const leader = entries[0]?.playCount ?? 0; @@ -26,26 +41,38 @@ export function TopList({ title, entries }: TopListProps) { {title} - + {entries.map((entry, index) => ( - - {index + 1} + + {/* Mono, so the ranks line up as a column. */} + {index + 1} + + {entry.artworkPath ? ( + + ) : ( + + + + )} {entry.title} - {entry.subtitle ? ( - - {entry.subtitle} - - ) : null} + {/* - A bar relative to the leader. Cheaper to read than a number - and it makes "one track dominated the week" visible at a - glance, which is the whole appeal of a Wrapped-style summary. + A bar relative to the leader. Cheaper to read than a number and + it makes "one track dominated the week" visible at a glance, + which is the whole appeal of a Wrapped-style summary. */} - + 0 ? (entry.playCount / leader) * 100 : 0}%` }} @@ -53,12 +80,18 @@ export function TopList({ title, entries }: TopListProps) { - - {t('stats.playCount', { - count: entry.playCount, - formatted: new Intl.NumberFormat(i18n.language).format(entry.playCount), - })} - + {/* Both numbers, right-aligned so the column reads down. */} + + + {t('stats.playCount', { + count: entry.playCount, + formatted: new Intl.NumberFormat(i18n.language).format(entry.playCount), + })} + + + {formatListeningTime(entry.msPlayed, i18n.language)} + + ))} diff --git a/src/features/stats/components/Wrapped.tsx b/src/features/stats/components/Wrapped.tsx new file mode 100644 index 0000000..a32e40e --- /dev/null +++ b/src/features/stats/components/Wrapped.tsx @@ -0,0 +1,89 @@ +import { useTranslation } from 'react-i18next'; +import { Text, View } from 'react-native'; + +import type { PeriodTotals, TopEntry } from '@/db/queries/stats'; +import { formatListeningTime } from '@/services/format/listeningTime'; +import type { PeriodType } from '@/services/stats/rollups'; + +export interface WrappedProps { + period: PeriodType; + totals: PeriodTotals; + topTrack: TopEntry | undefined; + topArtist: TopEntry | undefined; +} + +/** + * The period in one card. + * + * The brief asks for a summary "worth screenshotting", and the thing that makes + * one worth screenshotting is a single sentence a person would actually repeat. + * Nobody says "my top-ten had a Gini coefficient of 0.4"; they say "I listened + * to four hours and it was mostly one album". + * + * So it leads with the listening time in the display face at a size nothing else + * on the screen uses, and follows with two facts underneath. Everything more + * granular lives in the ranked lists below it, which is the right place for + * detail — this is the headline. + * + * Deliberately not a gradient, a collage, or a share sheet. The design direction + * rules out the first two by name, and the third would need an outward-facing + * intent in an app whose whole promise is that nothing leaves the device. A + * screenshot is already the share mechanism, and it needs no permission. + */ +export function Wrapped({ period, totals, topTrack, topArtist }: WrappedProps) { + const { t, i18n } = useTranslation(); + + const time = formatListeningTime(totals.msPlayed, i18n.language); + const number = (value: number) => new Intl.NumberFormat(i18n.language).format(value); + + return ( + + + + {t(`stats.wrapped.${period}`)} + + {/* + The one number the card exists for, in the display face. Indigo, which + the design direction reserves for what is active or important — and on + this screen nothing else competes for it. + */} + {time} + + {t('stats.wrapped.across', { + plays: number(totals.playCount), + tracks: number(totals.trackCount), + count: totals.trackCount, + })} + + + + {/* Absent rather than blank when a period has no clear leader yet. */} + {topTrack || topArtist ? ( + + {topTrack ? ( + + ) : null} + {topArtist ? ( + + ) : null} + + ) : null} + + ); +} + +interface FactProps { + label: string; + value: string; +} + +function Fact({ label, value }: FactProps) { + return ( + + {label} + + {value} + + + ); +} diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index 4fa0dfc..3d4380a 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -8,14 +8,11 @@ "library": { "title": "Library", "empty": [ - "No music found. Choose a folder to scan.", - "Nothing here yet. Point it at a folder and it fills up.", - "Your library is one folder away.", - "Empty for now. Tell it where the music lives." + "No music found yet. Scan the device, or point Mufify at a folder.", + "Nothing here yet. One scan and it fills up.", + "Your library is one scan away.", + "Empty for now. Tell it where the music lives, or let it look." ], - "emptyAction": "Add music", - "addMusic": "Add music", - "rescan": "Rescan", "trackCount_one": "{{count}} track", "trackCount_other": "{{count}} tracks", "scanning": { @@ -33,7 +30,19 @@ "search": "Search", "searchPlaceholder": "Title, artist, album", "clearSearch": "Clear search", - "noResults": "Nothing matches “{{term}}”." + "noResults": "Nothing matches “{{term}}”.", + "scan": "Scan", + "addFolder": "Add a folder", + "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." + }, + "view": { + "label": "Library view", + "tracks": "Tracks", + "artists": "Artists", + "albums": "Albums" + } }, "playlists": { "title": "Playlists", @@ -86,7 +95,18 @@ "topTracks": "Top tracks", "topArtists": "Top artists", "playCount_one": "{{formatted}} play", - "playCount_other": "{{formatted}} plays" + "playCount_other": "{{formatted}} plays", + "topAlbums": "Top albums", + "topPlaylists": "Top playlists", + "wrapped": { + "week": "This week", + "month": "This month", + "year": "This year", + "across_one": "{{plays}} plays across {{tracks}} track", + "across_other": "{{plays}} plays across {{tracks}} tracks", + "mostPlayed": "Most played", + "mostHeard": "Most heard" + } }, "settings": { "title": "Settings", @@ -95,20 +115,24 @@ "theme": "Theme", "system": "System", "light": "Light", - "dark": "Dark" + "dark": "Dark", + "description": "Dark is the one this app was designed in. System follows your phone." }, "language": { "title": "Language", "label": "App language", "system": "System", "en": "English", - "tr": "Türkçe" + "tr": "Türkçe", + "description": "Turkish and English. System follows your phone." }, "folders": { "title": "Music folders", "empty": "No folders added. Mufify still scans everything Android has indexed.", "remove": "Remove {{folder}}", - "removeNote": "Removing a folder stops re-indexing it. Your tracks stay." + "removeNote": "Removing a folder stops re-indexing it. Your tracks stay.", + "ignoreShort": "Ignore short files", + "ignoreShortHint": "Skips anything under 30 seconds. Off by default: interludes and album segues are real tracks." }, "shuffle": { "title": "Shuffle", @@ -118,11 +142,16 @@ "discovery": "Discovery", "favorites": "Favourites", "album": "By album", - "pureHint": "Truly random. Clusters, because randomness does.", - "balancedHint": "Spreads each artist across the queue.", - "discoveryHint": "Favours tracks you have played least.", - "favoritesHint": "Favours what you play most, and what you have hearted.", - "albumHint": "Shuffles the albums, never the album. Each one plays in order." + "pureHint": "Truly random. It will sometimes play the same artist twice in a row, because that is what random does.", + "balancedHint": "Spreads each artist across the queue, so no two of their tracks land together.", + "discoveryHint": "Favours what you have played least. An unplayed track comes up twice as often as one you have heard once.", + "favoritesHint": "The opposite of Discovery: favours what you play most, and pushes anything you have hearted further up.", + "albumHint": "Shuffles the albums, not the tracks. Each album plays through in order — for classical and anything with a running order." + }, + "playback": { + "title": "Playback", + "haptics": "Haptics", + "hapticsHint": "A short buzz on play, pause, queueing and reordering." } }, "player": { @@ -192,5 +221,15 @@ "channels": "Channels", "fileSize": "File size" } + }, + "toast": { + "queued_one": "Added to queue", + "queued_other": "{{count}} tracks added to queue", + "playingNext_one": "Playing next", + "playingNext_other": "{{count}} tracks playing next", + "favorited": "Added to favourites", + "unfavorited": "Removed from favourites", + "addedToPlaylist_one": "Added to the playlist", + "addedToPlaylist_other": "{{count}} tracks added to the playlist" } } diff --git a/src/i18n/locales/tr.json b/src/i18n/locales/tr.json index 5419a12..56897da 100644 --- a/src/i18n/locales/tr.json +++ b/src/i18n/locales/tr.json @@ -8,14 +8,11 @@ "library": { "title": "Kitaplık", "empty": [ - "Henüz müzik yok. Taranacak bir klasör seç.", - "Burası şimdilik boş. Bir klasör göster, gerisi kendiliğinden gelir.", - "Kitaplığın tek bir klasör uzağında.", - "Müziğin nerede duruyorsa, oraya bak diyelim." + "Henüz müzik yok. Cihazı tara ya da bir klasör göster.", + "Burası şimdilik boş. Bir tarama yeter.", + "Kitaplığın tek bir tarama uzağında.", + "Şimdilik boş. Müziğin nerede olduğunu söyle ya da bulmasına izin ver." ], - "emptyAction": "Müzik ekle", - "addMusic": "Müzik ekle", - "rescan": "Yeniden tara", "trackCount_one": "{{count}} parça", "trackCount_other": "{{count}} parça", "scanning": { @@ -33,7 +30,19 @@ "search": "Ara", "searchPlaceholder": "Parça, sanatçı, albüm", "clearSearch": "Aramayı temizle", - "noResults": "“{{term}}” ile eşleşen bir şey yok." + "noResults": "“{{term}}” ile eşleşen bir şey yok.", + "scan": "Tara", + "addFolder": "Klasör ekle", + "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." + }, + "view": { + "label": "Kitaplık görünümü", + "tracks": "Parçalar", + "artists": "Sanatçılar", + "albums": "Albümler" + } }, "playlists": { "title": "Listeler", @@ -86,7 +95,18 @@ "topTracks": "En çok çalınan parçalar", "topArtists": "En çok dinlenen sanatçılar", "playCount_one": "{{formatted}} çalma", - "playCount_other": "{{formatted}} çalma" + "playCount_other": "{{formatted}} çalma", + "topAlbums": "En çok dinlenen albümler", + "topPlaylists": "En çok dinlenen listeler", + "wrapped": { + "week": "Bu hafta", + "month": "Bu ay", + "year": "Bu yıl", + "across_one": "{{tracks}} parçada {{plays}} çalma", + "across_other": "{{tracks}} parçada {{plays}} çalma", + "mostPlayed": "En çok çalınan", + "mostHeard": "En çok dinlenen" + } }, "settings": { "title": "Ayarlar", @@ -95,20 +115,24 @@ "theme": "Tema", "system": "Sistem", "light": "Açık", - "dark": "Koyu" + "dark": "Koyu", + "description": "Uygulama koyu tema için tasarlandı. Sistem, telefonunu takip eder." }, "language": { "title": "Dil", "label": "Uygulama dili", "system": "Sistem", "en": "English", - "tr": "Türkçe" + "tr": "Türkçe", + "description": "Türkçe ve İngilizce. Sistem, telefonunu takip eder." }, "folders": { "title": "Müzik klasörleri", "empty": "Eklenmiş klasör yok. Mufify yine de Android’in indekslediği her şeyi tarar.", "remove": "{{folder}} klasörünü kaldır", - "removeNote": "Klasörü kaldırmak yeniden taranmasını durdurur. Parçaların kalır." + "removeNote": "Klasörü kaldırmak yeniden taranmasını durdurur. Parçaların kalır.", + "ignoreShort": "Kısa dosyaları yok say", + "ignoreShortHint": "30 saniyeden kısa olanları atlar. Varsayılan olarak kapalı: ara parçalar ve geçişler de birer parçadır." }, "shuffle": { "title": "Karıştırma", @@ -118,11 +142,16 @@ "discovery": "Keşif", "favorites": "Favoriler", "album": "Albüme göre", - "pureHint": "Gerçekten rastgele. Kümelenir, çünkü rastgelelik böyledir.", - "balancedHint": "Her sanatçıyı kuyruğa yayar.", - "discoveryHint": "En az çaldığın parçaları öne alır.", - "favoritesHint": "En çok çaldıklarını ve favorilediklerini öne alır.", - "albumHint": "Albümleri karıştırır, albümün içini değil. Her biri sırasıyla çalar." + "pureHint": "Gerçekten rastgele. Aynı sanatçıyı arka arkaya çalabilir, çünkü rastgelelik böyledir.", + "balancedHint": "Her sanatçıyı kuyruğa yayar; aynı sanatçıdan iki parça art arda gelmez.", + "discoveryHint": "En az çaldıklarını öne alır. Hiç çalınmamış bir parça, bir kez dinlenmişin iki katı sıklıkta gelir.", + "favoritesHint": "Keşif’in tersi: en çok çaldıklarını öne alır, favorilediklerini daha da yukarı taşır.", + "albumHint": "Parçaları değil albümleri karıştırır. Her albüm kendi sırasıyla çalar — klasik ve sıralı dinlenen her şey için." + }, + "playback": { + "title": "Çalma", + "haptics": "Titreşim", + "hapticsHint": "Çalma, duraklatma, sıraya ekleme ve sıralama değişiminde kısa bir titreşim." } }, "player": { @@ -192,5 +221,15 @@ "channels": "Kanal", "fileSize": "Dosya boyutu" } + }, + "toast": { + "queued_one": "Sıraya eklendi", + "queued_other": "{{count}} parça sıraya eklendi", + "playingNext_one": "Sıradaki olarak eklendi", + "playingNext_other": "{{count}} parça sıradaki olarak eklendi", + "favorited": "Favorilere eklendi", + "unfavorited": "Favorilerden çıkarıldı", + "addedToPlaylist_one": "Listeye eklendi", + "addedToPlaylist_other": "{{count}} parça listeye eklendi" } } diff --git a/src/services/audio/AudioEngine.ts b/src/services/audio/AudioEngine.ts index 5287509..d11a233 100644 --- a/src/services/audio/AudioEngine.ts +++ b/src/services/audio/AudioEngine.ts @@ -8,13 +8,16 @@ import { } from 'expo-audio'; import { shuffleTracks, type ShuffleAlgorithm } from '@/services/shuffle'; +import { isRewindToRestart } from '@/services/stats/repeatListen'; import { isPlayable, nextIndex, playNextIndex, previousIndex, shiftForInsert } from './queue'; import { IDLE_PLAYBACK, + LIBRARY_SOURCE, type FinishedListen, type PlayableTrack, type PlaybackState, + type QueueSource, type RepeatMode, } from './types'; @@ -62,6 +65,15 @@ class Engine { */ private sourceQueue: PlayableTrack[] = []; private shuffled = false; + + /* + * Where the current queue came from, and which shuffle reordered it. Both are + * attributes of the *queue*, not of a track, so they belong here rather than + * on `PlayableTrack` — the same track played from a playlist and from the + * library is two different listens with two different attributions. + */ + private source: QueueSource = LIBRARY_SOURCE; + private shuffleAlgorithm: ShuffleAlgorithm | null = null; private state: PlaybackState = IDLE_PLAYBACK; private listeners = new Set(); private configured = false; @@ -88,6 +100,16 @@ class Engine { private playedMs = 0; private lastTickAt: number | null = null; + /** + * Position at the previous status tick, for spotting a rewind. + * + * A listen used to end only when the loaded track changed, so a track on + * repeat-one recorded one play no matter how many times it went round. This is + * what lets the engine notice the track starting over without the track + * changing. See `src/services/stats/repeatListen.ts`. + */ + private lastPositionMs = 0; + /** * Set when a track has been handed to the player but is not open yet. * @@ -171,11 +193,23 @@ class Engine { for (const listener of this.listeners) listener(this.state); } - /** Replace the queue and start at `startIndex`. */ - async setQueue(tracks: PlayableTrack[], startIndex: number): Promise { + /** + * Replace the queue and start at `startIndex`. + * + * `source` defaults to the library because that is where most queues come + * from, and defaulting it means a caller that forgets attributes a listen + * plausibly rather than crashing. Playlist playback passes its own. + */ + async setQueue( + tracks: PlayableTrack[], + startIndex: number, + source: QueueSource = LIBRARY_SOURCE, + ): Promise { this.sourceQueue = tracks; this.queue = tracks; this.shuffled = false; + this.shuffleAlgorithm = null; + this.source = source; if (!isPlayable(startIndex, tracks.length)) { await this.stop(); @@ -259,6 +293,7 @@ class Engine { async setShuffled(shuffled: boolean, algorithm: ShuffleAlgorithm): Promise { const current = this.queue[this.index] ?? null; this.shuffled = shuffled; + this.shuffleAlgorithm = shuffled ? algorithm : null; if (!shuffled) { this.queue = this.sourceQueue; @@ -292,7 +327,14 @@ class Engine { const startedAt = this.startedAt; if (track !== null && startedAt !== null && this.playedMs > 0) { - this.reportListen?.({ track, msPlayed: Math.round(this.playedMs), startedAt, completed }); + this.reportListen?.({ + track, + msPlayed: Math.round(this.playedMs), + startedAt, + completed, + source: this.source, + shuffleAlgorithm: this.shuffleAlgorithm, + }); } this.startedAt = null; @@ -300,6 +342,19 @@ class Engine { this.lastTickAt = null; } + /** + * Bank the listen so far and open a new one, same track still loaded. + * + * Distinct from `flushListen` in one respect that matters: `startedAt` is set + * to now rather than cleared, because the next listen has already begun. Period + * keys come from when a listen *started*, so a loop that crosses midnight puts + * its two halves in the right days. + */ + private beginNextCycle(): void { + this.flushListen(true); + this.startedAt = new Date(); + } + /** Fold the time since the last tick into the running total. */ private accumulate(): void { if (this.lastTickAt === null) return; @@ -316,6 +371,7 @@ class Engine { this.startedAt = new Date(); this.index = index; + this.lastPositionMs = 0; this.emitQueue(); this.emit({ phase: 'loading', track, positionMs: 0, durationMs: track.durationMs }); @@ -379,6 +435,8 @@ class Engine { // The pending start from `loadIndex`, now that the file may be open. if (status.isLoaded) this.startIfReady(); + const positionMs = Math.round(status.currentTime * 1000); + // Clock the interval that just elapsed before anything else changes. if (status.playing) { this.accumulate(); @@ -391,15 +449,37 @@ class Engine { // once, unlike `currentTime >= duration`, which fires on every tick after. if (status.didJustFinish) { this.flushListen(true); + this.lastPositionMs = 0; void this.advance(false); return; } + /* + * The same track started over — looped, or dragged back to the beginning. + * Close the listen and open another, so a song on repeat is counted as many + * times as it is actually heard. + * + * Checked before the state is emitted, so `lastPositionMs` is still the + * previous tick's value when the comparison happens. + */ + if ( + isRewindToRestart({ + previousPositionMs: this.lastPositionMs, + positionMs, + durationMs: this.state.durationMs, + msPlayedInCycle: this.playedMs, + }) + ) { + this.beginNextCycle(); + } + + this.lastPositionMs = positionMs; + if (this.state.phase === 'error') return; this.emit({ phase: status.playing ? 'playing' : this.state.phase === 'loading' ? 'loading' : 'paused', - positionMs: Math.round(status.currentTime * 1000), + positionMs, // expo-audio reports -1 or 0 before the file is open; the scanner's // figure is the better answer until then. durationMs: diff --git a/src/services/audio/types.ts b/src/services/audio/types.ts index 7846aaa..59e07f1 100644 --- a/src/services/audio/types.ts +++ b/src/services/audio/types.ts @@ -55,6 +55,24 @@ export interface PlaybackState { export type RepeatMode = 'off' | 'all' | 'one'; +/** + * Where a queue came from. + * + * Carried so a finished listen can be attributed. Without it every play looks + * like it came from the library, and `stats_rollups` — which has an entity type + * for playlists — never gains a single playlist row. The statistics screen can + * then show top tracks and top artists but never top playlists, and nothing + * about that failure is visible: the query returns no rows, which looks exactly + * like a user who has not played any playlists. + */ +export interface QueueSource { + type: 'library' | 'album' | 'artist' | 'playlist' | 'queue'; + /** The playlist, album or artist id. Absent for the library. */ + id?: number; +} + +export const LIBRARY_SOURCE: QueueSource = { type: 'library' }; + /** * One finished listen, handed out when a track stops being the current one. * @@ -70,6 +88,16 @@ export interface FinishedListen { startedAt: Date; /** True when it reached its end rather than being skipped or replaced. */ completed: boolean; + /** Where the queue this played from came from. */ + source: QueueSource; + /** + * Which shuffle algorithm was running, or null when playing in order. + * + * `play_events` has had this column since Phase 1 and nothing ever wrote to + * it, so the question it exists to answer — which shuffle produces listens + * people finish — was unanswerable. + */ + shuffleAlgorithm: string | null; } export const IDLE_PLAYBACK: PlaybackState = { diff --git a/src/services/format/listeningTime.test.ts b/src/services/format/listeningTime.test.ts index 5403c14..5510e83 100644 --- a/src/services/format/listeningTime.test.ts +++ b/src/services/format/listeningTime.test.ts @@ -42,3 +42,28 @@ describe('formatListeningTime', () => { expect(formatListeningTime(90 * 60_000, 'tr')).toBe('1h 30m'); }); }); + +describe('formatListeningTime under a minute', () => { + it('reports seconds rather than a useless zero', () => { + /* + * The per-row totals on the statistics screen exposed this: a handful of + * six-second tracks all read "0m", which tells the reader nothing and looks + * like a value that failed to load. + */ + expect(formatListeningTime(42_000, 'en')).toBe('42s'); + }); + + it('rounds seconds down, like the minutes above', () => { + expect(formatListeningTime(6_900, 'en')).toBe('6s'); + }); + + it('says 0s for nothing at all, not an empty string', () => { + expect(formatListeningTime(0, 'en')).toBe('0s'); + expect(formatListeningTime(-1, 'en')).toBe('0s'); + }); + + it('switches to minutes the moment there is one', () => { + expect(formatListeningTime(59_999, 'en')).toBe('59s'); + expect(formatListeningTime(60_000, 'en')).toBe('1m'); + }); +}); diff --git a/src/services/format/listeningTime.ts b/src/services/format/listeningTime.ts index fee9c7d..2c3ed1a 100644 --- a/src/services/format/listeningTime.ts +++ b/src/services/format/listeningTime.ts @@ -24,16 +24,29 @@ export function listeningTimeParts(milliseconds: number): ListeningTimeParts { } /** - * `4h 37m`, or `37m` under an hour. + * `4h 37m`, or `37m` under an hour, or `42s` under a minute. * - * The `h`/`m` suffixes are deliberately not localised: they are the same in + * The seconds case exists because the per-row totals on the statistics screen + * exposed it: a handful of short tracks all read "0m", which tells the reader + * nothing at all and looks like a value that failed to load. Under a minute the + * only informative unit is seconds. + * + * It stops there. `4h 37m 12s` is not how anyone reports listening time, and + * once there are minutes to show the seconds are noise. + * + * The `h`/`m`/`s` suffixes are deliberately not localised: they are the same in * both shipped locales, and a translated unit would need plural rules for a * string nobody reads as a sentence. If a third locale disagrees, this becomes - * two `t()` keys and the function returns parts instead. + * three `t()` keys and the function returns parts instead. */ export function formatListeningTime(milliseconds: number, locale: string): string { const { hours, minutes } = listeningTimeParts(milliseconds); const format = (value: number) => new Intl.NumberFormat(locale).format(value); - return hours > 0 ? `${format(hours)}h ${format(minutes)}m` : `${format(minutes)}m`; + if (hours > 0) return `${format(hours)}h ${format(minutes)}m`; + if (minutes > 0) return `${format(minutes)}m`; + + // Rounded down, like the minutes above: never round a listen up into a lie. + const seconds = Math.max(0, Math.floor((Number.isFinite(milliseconds) ? milliseconds : 0) / 1000)); + return `${format(seconds)}s`; } diff --git a/src/services/scanner/scanner.test.ts b/src/services/scanner/scanner.test.ts index a0230c7..d2f9926 100644 --- a/src/services/scanner/scanner.test.ts +++ b/src/services/scanner/scanner.test.ts @@ -87,6 +87,7 @@ function harness(library: MediaStoreTrack[], pending: string[] = []): Harness { enriched.push(...rows); }, listUnenrichedUris: async (limit) => queue.splice(0, limit), + countUnenriched: async () => queue.length, yieldToUi: async () => { state.yields += 1; }, @@ -196,6 +197,37 @@ describe('enrichLibrary', () => { expect(result.processed).toBe(9); }); + it('reports a denominator the bar can actually fill', async () => { + /* + * Stage two used to set `total` to however many it had already processed, + * which makes the ratio permanently 1 — the bar was full from the first + * batch and the label read "N / N" throughout. A progress bar that is + * always finished is worse than no progress bar. + */ + const test = harness([], [...pending]); + const seen: { processed: number; total: number }[] = []; + + await enrichLibrary(test.ports, options, ({ processed, total }) => { + seen.push({ processed, total }); + }); + + // Known before the first report, so no frame ever shows "0 / 0". + expect(seen.every((step) => step.total === 9)).toBe(true); + expect(seen.map((step) => step.processed)).toEqual([0, 4, 8, 9, 9]); + }); + + it('never claims more done than there is to do', async () => { + // A resumed scan starts partway through, so the count taken at the start + // can be beaten by what the loop actually drains. + const test = harness([], [...pending]); + test.ports.countUnenriched = async () => 2; + + const result = await enrichLibrary(test.ports, options, () => {}); + + expect(result.processed).toBe(9); + expect(result.total).toBeGreaterThanOrEqual(result.processed); + }); + it('writes each batch before starting the next', async () => { // A scan killed halfway must keep what it already did. const test = harness([], [...pending]); diff --git a/src/services/scanner/scanner.ts b/src/services/scanner/scanner.ts index 7f62e28..d019ac4 100644 --- a/src/services/scanner/scanner.ts +++ b/src/services/scanner/scanner.ts @@ -61,6 +61,16 @@ export interface ScannerPorts { /** URIs that stage two has not reached yet. */ listUnenrichedUris(limit: number): Promise; + /** + * How many rows stage two still has to open. + * + * Asked once, at the start of the stage, so the progress bar has a + * denominator. Without it stage two reported `total` as however many it had + * already done, which makes the ratio permanently 1 and the bar permanently + * full — a progress bar that is always finished is worse than none. + */ + countUnenriched(): Promise; + /** * Mark every present track the sweep did not see as missing. * @@ -153,9 +163,17 @@ export async function enrichLibrary( controller: ScanController = NEVER_CANCELLED, ): Promise { let progress: ScanProgress = { phase: 'enriching', total: 0, processed: 0 }; - onProgress(progress); try { + /* + * Counted before the first report, not after. Rows only leave the queue as + * this stage writes them, so the denominator is fixed from here and the bar + * fills honestly — and emitting once beforehand would have put a single + * frame of "0 / 0" on screen, which is the bug this exists to fix. + */ + progress = { ...progress, total: await ports.countUnenriched() }; + onProgress(progress); + let processed = 0; for (;;) { @@ -178,13 +196,16 @@ export async function enrichLibrary( await ports.saveEnriched(rows); processed += uris.length; - progress = { ...progress, processed, total: processed }; + // `total` can be beaten by reality: a file that would not open is still + // counted as processed, and a scan resumed after a crash starts partway. + // Never report more done than there is to do. + progress = { ...progress, processed, total: Math.max(progress.total, processed) }; onProgress(progress); await ports.yieldToUi(); } - return finish({ ...progress, processed, total: processed }, 'done', onProgress); + return finish({ ...progress, processed }, 'done', onProgress); } catch (error) { return fail(progress, error, onProgress); } diff --git a/src/services/scanner/trackMapping.test.ts b/src/services/scanner/trackMapping.test.ts index 8b5cb43..9330f44 100644 --- a/src/services/scanner/trackMapping.test.ts +++ b/src/services/scanner/trackMapping.test.ts @@ -241,6 +241,20 @@ describe('codecOf', () => { expect(codecOf('audio/mpeg')).toBeNull(); expect(codecOf('audio/flac')).toBeNull(); expect(codecOf('audio/x-wav')).toBeNull(); + expect(codecOf('audio/mp4')).toBeNull(); + }); + + it('is null for every format on the user Mi 9T, which is not a defect', () => { + // Read from the device on 2026-08-01: `codec` null on all 521 rows while + // `container` and the rest of the spec were populated. That was written up + // as an unexplained finding twice. It is this function working: a library + // of mainstream formats has a null codec on every row by design, because + // the MIME subtype is already the container name. A non-null codec needs a + // subtype outside CONTAINER_NAMES, which a FLAC/MP3/M4A library never has. + for (const mime of ['audio/flac', 'audio/mp4', 'audio/mpeg']) { + expect(codecOf(mime)).toBeNull(); + expect(containerOf(mime)).not.toBeNull(); + } }); it('keeps a subtype nothing is known about', () => { diff --git a/src/services/stats/repeatListen.test.ts b/src/services/stats/repeatListen.test.ts new file mode 100644 index 0000000..6fa8322 --- /dev/null +++ b/src/services/stats/repeatListen.test.ts @@ -0,0 +1,180 @@ +import { playThresholdMs } from './playCounting'; +import { isRewindToRestart, REWIND_FRACTION } from './repeatListen'; + +/** + * The thresholds are pinned here on purpose. + * + * This rule decides whether a listen is counted once or twice, so a quiet + * change to either condition silently rewrites the user's history. Every test + * below names the behaviour it is protecting rather than the number. + */ + +/** Four minutes: long enough that the play threshold is the flat 30 seconds. */ +const LONG = 240_000; +/** Six seconds, like the short test files. Threshold is half of it. */ +const SHORT = 6_000; + +function check(over: Partial[0]> = {}) { + return isRewindToRestart({ + previousPositionMs: 120_000, + positionMs: 0, + durationMs: LONG, + msPlayedInCycle: playThresholdMs(LONG), + ...over, + }); +} + +describe('isRewindToRestart', () => { + it('starts a new listen when a counted track loops back to zero', () => { + // The case this whole rule exists for: repeat-one all afternoon used to be + // recorded as a single play. + expect(check({ positionMs: 0 })).toBe(true); + }); + + it('starts a new listen when the user drags back to the beginning', () => { + expect(check({ previousPositionMs: 200_000, positionMs: 4_000 })).toBe(true); + }); + + it('does not start one before the current listen has counted', () => { + /* + * Without this, scrubbing around inside the first thirty seconds would + * shatter one listen into a dozen too short to count — turning a real play + * into a pile of skips. + */ + expect(check({ msPlayedInCycle: playThresholdMs(LONG) - 1 })).toBe(false); + }); + + it('does not treat a small step backwards as a restart', () => { + // Going back over the last chorus is an adjustment, not a replay. + expect(check({ previousPositionMs: 200_000, positionMs: 185_000 })).toBe(false); + }); + + it('does not fire while playback moves forward', () => { + expect(check({ previousPositionMs: 60_000, positionMs: 60_500 })).toBe(false); + }); + + it('does not fire when the position has not moved', () => { + // A paused track keeps reporting the same position. It is not restarting. + expect(check({ previousPositionMs: 90_000, positionMs: 90_000 })).toBe(false); + }); + + it('draws the line exactly at a quarter of the track', () => { + const boundary = LONG * REWIND_FRACTION; + expect(check({ previousPositionMs: 200_000, positionMs: boundary })).toBe(true); + expect(check({ previousPositionMs: 200_000, positionMs: boundary + 1 })).toBe(false); + }); + + it('works on a track short enough that the threshold is half its length', () => { + // A six-second file counts as played after three seconds, so looping it + // has to produce a second listen just as a four-minute one does. + expect( + isRewindToRestart({ + previousPositionMs: 5_800, + positionMs: 0, + durationMs: SHORT, + msPlayedInCycle: playThresholdMs(SHORT), + }), + ).toBe(true); + }); + + it('refuses to guess when the duration is unknown', () => { + // Duration is zero until the file is open. Every fraction of zero is zero, + // so without this guard a track would "restart" on its first status tick. + expect(check({ durationMs: 0 })).toBe(false); + }); + + it('counts a rewind-then-leave as its own short listen rather than nothing', () => { + /* + * The boundary fires on the rewind, not on the second listen reaching the + * threshold. That is deliberate: the new listen is classified by the same + * rule as any other when it ends, so abandoning it records a skip — which + * is what happened — instead of silently merging into the previous play. + */ + expect(check({ previousPositionMs: 239_000, positionMs: 0 })).toBe(true); + }); +}); + +/** + * The rule applied to a run of status ticks, the way the engine applies it. + * + * `isRewindToRestart` is a single decision; what matters to a user is how many + * listens a *session* produces. This walks realistic tick sequences and counts + * the boundaries, which is the behaviour the feature was asked for — and it is + * deterministic, unlike watching a six-second file loop on an emulator. + */ +function countBoundaries(positions: readonly number[], durationMs: number): number { + let previous = 0; + let msPlayedInCycle = 0; + let boundaries = 0; + + for (const positionMs of positions) { + if ( + isRewindToRestart({ + previousPositionMs: previous, + positionMs, + durationMs, + msPlayedInCycle, + }) + ) { + boundaries += 1; + msPlayedInCycle = 0; + } else { + /* + * Credit only forward movement. The engine accumulates wall-clock time + * spent playing, which for ordinary playback is the position delta — and + * a backwards jump adds nothing, because seeking is not listening. An + * earlier version of this harness advanced a fixed amount per tick, which + * quietly made every position jump free and produced a failure that + * looked like a bug in the rule. + */ + msPlayedInCycle += Math.max(0, positionMs - previous); + } + previous = positionMs; + } + + return boundaries; +} + +/** Ticks for one pass through a track, then back to zero. */ +function loop(durationMs: number, tickMs = 500): number[] { + const ticks: number[] = []; + for (let at = tickMs; at < durationMs; at += tickMs) ticks.push(at); + ticks.push(0); + return ticks; +} + +describe('over a run of status ticks', () => { + it('turns three loops of one track into three extra listens', () => { + // The headline case from the brief: a song looped three times is three + // plays, not one. + const ticks = [...loop(SHORT), ...loop(SHORT), ...loop(SHORT)]; + expect(countBoundaries(ticks, SHORT)).toBe(3); + }); + + it('counts a manual drag back to the start once', () => { + // Play well past the threshold, then drag the scrubber to the beginning. + const ticks = [30_000, 60_000, 90_000, 0, 500, 1_000]; + expect(countBoundaries(ticks, LONG)).toBe(1); + }); + + it('ignores scrubbing around before the listen has counted', () => { + /* + * Someone hunting for the right moment in the first few seconds. Every one + * of these is a backwards jump below the quarter mark, and none of them may + * split the listen — otherwise a real play is recorded as a pile of skips. + */ + const ticks = [2_000, 500, 4_000, 1_000, 6_000, 2_000, 8_000]; + expect(countBoundaries(ticks, LONG)).toBe(0); + }); + + it('ignores repeated short rewinds late in a counted track', () => { + // Replaying the last chorus four times is one listen, not five. + const ticks = [30_000, 200_000, 180_000, 200_000, 180_000, 200_000, 180_000]; + expect(countBoundaries(ticks, LONG)).toBe(0); + }); + + it('does not split a track played straight through', () => { + const ticks = loop(LONG, 10_000).slice(0, -1); + expect(countBoundaries(ticks, LONG)).toBe(0); + }); +}); diff --git a/src/services/stats/repeatListen.ts b/src/services/stats/repeatListen.ts new file mode 100644 index 0000000..f36e6db --- /dev/null +++ b/src/services/stats/repeatListen.ts @@ -0,0 +1,74 @@ +import { playThresholdMs } from './playCounting'; + +/** + * When playing the same track again counts as a second listen. + * + * A layer *on top of* the play/skip/partial rule, not a change to it. ADR 005 + * decides whether a listen counted; this decides where one listen ends and the + * next begins while the track never changes. Looping a song five times is five + * listens, and until now it was one — the engine only closed a listen when the + * loaded track changed, so a repeat-one all afternoon recorded a single play. + * + * See `docs/adr/011-repeat-listen-detection.md`. + */ + +/** + * How far back the position must jump to start a new listen. + * + * A quarter of the track. The number has to separate two things that look + * identical from the outside — "start it again" and "go back a bit" — and the + * only signal available is how far back the position went. + * + * A tighter bound would count a scrub back over the last chorus as a replay; a + * looser one would miss a genuine restart on a track someone had nearly + * finished. A quarter means the listener has given up at least three quarters + * of their progress, which is a decision rather than an adjustment. + */ +export const REWIND_FRACTION = 0.25; + +export interface RewindCheck { + /** Position on the previous status tick. */ + previousPositionMs: number; + /** Position now. */ + positionMs: number; + durationMs: number; + /** Playback time accumulated since this listen began. */ + msPlayedInCycle: number; +} + +/** + * True when the current listen should be banked and a fresh one started. + * + * Two conditions, and both matter: + * + * **The current listen must already have earned a play.** Without this, seeking + * around inside the first thirty seconds of a track would shatter one listen + * into a dozen, each too short to count as anything — turning a real play into + * a pile of skips. A listen that has not yet counted has nothing worth banking. + * + * **The position must have jumped back past the rewind mark.** Both a loop to + * zero and a manual drag to the start satisfy it; nudging back a few seconds + * does not. + * + * Note what this deliberately does *not* require: that the new listen goes on to + * pass the play threshold too. It does not need to be checked here, because the + * new listen is classified by the same rule as every other one when it ends. If + * the user rewinds and then leaves, the second listen is recorded as a skip, + * which is what happened. + */ +export function isRewindToRestart({ + previousPositionMs, + positionMs, + durationMs, + msPlayedInCycle, +}: RewindCheck): boolean { + if (durationMs <= 0) return false; + + // Nothing worth banking yet. + if (msPlayedInCycle < playThresholdMs(durationMs)) return false; + + // Forward, or standing still. Ordinary playback. + if (positionMs >= previousPositionMs) return false; + + return positionMs <= durationMs * REWIND_FRACTION; +} diff --git a/src/services/toast/index.ts b/src/services/toast/index.ts new file mode 100644 index 0000000..45f234b --- /dev/null +++ b/src/services/toast/index.ts @@ -0,0 +1,75 @@ +/** + * Transient confirmations. + * + * A module-level store rather than a React context, for the same reason the + * audio engine is one: a context provider high in the tree re-renders everything + * under it whenever a toast appears, and the whole point of a toast is that it + * does not disturb what you were doing. Only `` subscribes. + * + * Toasts say what *happened*, never what is happening — "Added to queue", not + * "Adding…". Anything slow enough to need a progress indicator needs a real one, + * not a message that disappears. + */ + +export interface Toast { + /** Monotonic, so a repeated message still re-triggers the animation. */ + id: number; + /** Already translated. One short sentence. */ + message: string; +} + +type Listener = (toast: Toast | null) => void; + +/** Long enough to read a short sentence, short enough not to linger. */ +const VISIBLE_MS = 2_600; + +let current: Toast | null = null; +let nextId = 1; +let timer: ReturnType | null = null; +const listeners = new Set(); + +function emit(): void { + for (const listener of listeners) listener(current); +} + +/** + * Show a message. + * + * A new toast replaces the one on screen rather than queueing behind it. + * Queueing means the fifth swipe is still being confirmed ten seconds later, + * long after the user has moved on — the most recent action is the only one + * still worth reporting. + */ +export function showToast(message: string): void { + if (timer !== null) clearTimeout(timer); + + current = { id: nextId++, message }; + emit(); + + timer = setTimeout(() => { + timer = null; + current = null; + emit(); + }, VISIBLE_MS); +} + +/** Dismiss whatever is showing. The user swiped or tapped it away. */ +export function dismissToast(): void { + if (timer !== null) { + clearTimeout(timer); + timer = null; + } + current = null; + emit(); +} + +export function subscribeToast(listener: Listener): () => void { + listeners.add(listener); + return () => { + listeners.delete(listener); + }; +} + +export function getToast(): Toast | null { + return current; +} diff --git a/src/theme/scale.test.ts b/src/theme/scale.test.ts new file mode 100644 index 0000000..2953db5 --- /dev/null +++ b/src/theme/scale.test.ts @@ -0,0 +1,112 @@ +import { readFileSync, readdirSync, statSync } from 'node:fs'; +import { join } from 'node:path'; + +import { SPACING } from './tokens'; + +/** + * Every spacing-derived class must exist in the scale. + * + * `tailwind.config.js` **overrides** the spacing scale rather than extending it, + * so a class built from a value that is not in it produces no CSS at all. There + * is no warning, no error, and no visual hint — the element simply has no size. + * + * This is not hypothetical. Every one of these shipped and was invisible: + * + * - `h-32 w-32` on the artist and album detail cover, so the header drew at zero + * by zero and the artwork never appeared. + * - the same on the playlist mosaic at `size="lg"`. + * - `w-24` on the swipe-to-queue reveal track, so the action strip behind a + * swiped row had no width and its icon was never seen. + * - `h-7 w-7` on the track picker's checkbox. + * - `max-h-96` on two bottom sheets. + * + * `AGENTS.md` names this trap and it caught us anyway, because the failure mode + * is silence. A test is the only thing that turns it into noise. + * + * Fractions (`w-1/2`), `full`, `auto`, `px` and arbitrary values are all left + * alone: those come from Tailwind's own scales, which are not overridden. Only + * bare numbers are checked. + */ + +const SOURCE_ROOTS = ['src', 'app']; +const SOURCE_EXTENSIONS = ['.ts', '.tsx']; + +/** Utilities whose numeric values come from `theme.spacing`. */ +const SPACED = [ + 'w', + 'h', + 'min-w', + 'min-h', + 'max-w', + 'max-h', + 'p', + 'px', + 'py', + 'pt', + 'pb', + 'pl', + 'pr', + 'm', + 'mx', + 'my', + 'mt', + 'mb', + 'ml', + 'mr', + 'gap', + 'gap-x', + 'gap-y', + 'top', + 'bottom', + 'left', + 'right', + 'inset', + 'inset-x', + 'inset-y', + 'size', +]; + +const ALLOWED = new Set(Object.keys(SPACING)); +const PATTERN = new RegExp(`\\b(${SPACED.join('|')})-(\\d+)\\b`, 'g'); + +function sourceFiles(directory: string): string[] { + return readdirSync(directory).flatMap((entry) => { + const path = join(directory, entry); + if (statSync(path).isDirectory()) return sourceFiles(path); + return SOURCE_EXTENSIONS.some((extension) => path.endsWith(extension)) ? [path] : []; + }); +} + +describe('spacing scale', () => { + it('has no class built from a value outside the scale', () => { + const offenders: string[] = []; + + for (const root of SOURCE_ROOTS) { + for (const file of sourceFiles(root)) { + // Skip this file, whose whole job is to contain the examples. + if (file.endsWith('scale.test.ts')) continue; + + const source = readFileSync(file, 'utf8'); + for (const [match, , value] of source.matchAll(PATTERN)) { + if (value !== undefined && !ALLOWED.has(value)) { + offenders.push(`${file}: ${match}`); + } + } + } + } + + expect(offenders).toEqual([]); + }); + + it('keeps the TypeScript scale and the Tailwind config in step', () => { + // `tokens.test.ts` guards the colours the same way. This is the spacing + // half: the two files are synchronised by hand, and the config is the one + // that decides whether a class compiles. + const config = readFileSync('tailwind.config.js', 'utf8'); + const spacingBlock = config.slice(config.indexOf('spacing: {'), config.indexOf('borderRadius')); + + for (const [key, value] of Object.entries(SPACING)) { + expect(spacingBlock).toContain(`${key}: '${value}px'`); + } + }); +});