From a9852c18a0798bb0c99606ac4c671d8605da6a85 Mon Sep 17 00:00:00 2001 From: Logan Date: Mon, 17 Aug 2026 10:15:01 -0400 Subject: [PATCH 1/4] docs: the device answered both open questions, and neither as expected Both faults reported from the phone are now measured rather than inferred, with the installed build and current main compared on the same device. "The controls are off screen" was literal and already fixed: the installed build predates B2 phase 2, so its player bar still carried the seek bar and volume at 424px and the transport ran past the right edge. Current main measures no horizontal overflow and the controls at 200..380 inside 424, on the phone's own engine. "No icons" was my own screenshot: taken six seconds after a cold start, before the icon fetches landed. On the settled app every icon paints, and the earlier black `fill` was the svg root rather than the path that carries `fill="currentColor"`. Two conclusions from one misread node, both corrected. Chrome 113's missing Popover API does not break the menus, which was the standing worry: a long-press opens the real panel with seven items, positioned and painted -- so long-press is now verified on hardware over a 1,744-track library, not just in a browser at a phone-shaped viewport. What the device does add is a measurement for phase 4: the track list's columns fit the host exactly and are simply too many for 424px. --- .planning/NOTES.md | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/.planning/NOTES.md b/.planning/NOTES.md index bc5b1e3..2d6e233 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3208,3 +3208,52 @@ probes, and that the hardware back button no longer kills the app (the `.dev` build carries the history fix; pid survived a BACK press). Unverified: what happened to the icons and the transport controls, which is where this resumes. + +## What the device actually said, with both builds side by side (2026-08-17) + +The phone inspectable and awake, the same Light Phone III running two +builds of this app in turn. This closes both questions the previous entry +left open, and **neither answer was the one the symptom suggested**. + +**"The playback controls are off screen" was true, literal, and already +fixed.** The installed build is from B2 **phase 1** — it carries +`bottom-nav` and no `now-playing-view`, which dates it between 57bfbdf +and 1b05dde. Settled (30 s after launch, not 6), its player bar shows +art, title, favourite, shuffle, prev — and stops. Play/pause, next, +repeat and queue are past the right edge, because at 424 px the bar was +still carrying the seek bar and volume that **phase 2 moved into +`now-playing-view`**. On the current build, on the same phone and the +same engine, `document.body.scrollWidth` equals `clientWidth` (424) and +`player-controls` measures 200..380 inside 424. So the fix was already +on main, unreleased, and the device is what proved it rather than +argued it. + +**"No icons" was an artefact of my own screenshot.** A `wa-icon` on the +device has `path` computed fill `rgb(255,212,59)` and paints; the first +capture was six seconds after a cold start, before the icon fetches had +landed. Two corrections in two entries from the same misreading: measure +the node that paints, and let the app settle before believing a picture. + +**Chrome 113's missing Popover API does not break the menus.** This was +the leading worry and it is unfounded: a long-press on a row opens the +real panel at (212,145), 162x193, `visibility: visible`, seven +`role=menuitem`s, all seven inside the panel and clear of the player bar +— confirmed by screenshot as well as by measurement. Web Awesome's +`showPopover?.()` is an optional call and `wa-popup` positions itself, +so the attribute being inert costs nothing. **Long-press itself works on +real hardware**, over a real 1,744-track library, which is the phase 3 +verification the browser tier could only approximate. + +**The one genuine fault the device adds is phase 4's.** `track-list` at +424 px computes `--grid-cols: 24px 102px 101px 101px 80px` — which fits +the host exactly, so nothing overflows — but "Duration" does not fit in +80 px and neither does most content. The columns are not too wide; there +are simply too many of them for a phone, which is what phase 4 already +says. It is now a measurement rather than a prediction. + +Two operational notes. The debug sibling scanned the phone's real music +and its data directory is **414 MB**, so it is worth uninstalling when +done (`adb uninstall app.yellowjacket.dev` — the sibling id is exactly +what makes that safe). And `am start` does not reliably take focus while +another app is foreground: check `topResumedActivity` before trusting a +screenshot, or you will read someone else's app. From 2c78b5820756b761adf19576c1673ed8edc3042e Mon Sep 17 00:00:00 2001 From: Logan Date: Mon, 17 Aug 2026 10:36:29 -0400 Subject: [PATCH 2/4] feat(ui): the track list a phone can read B2 phase 4, and the last of it. Measured on the device: at 424 CSS px the four configured columns fit the row *exactly* -- `--grid-cols` came out `24px 102px 101px 101px 80px` -- and not one of them fit its content, with "Duration" too narrow for its own header. The columns were never too wide; there were too many of them. So a phone draws `titleArtist` (the title with the artist under it, across the row's whole width) plus the duration, and drops the column headers and the resize handles, which are a click-to-sort and a drag with no touch equivalent. It is a **column set, not a second row template**: the row, its delegated events, the selection semantics, the playing marker and the virtualizer never learn anything changed, because from their side only the number of columns did. Three rules come with it. The row height is in two places (`PHONE_ROW_HEIGHT` and the CSS rule) and must agree, since the virtualizer positions rows from that number and a taller row overlaps its neighbour. What is drawn and what can be sorted are different questions, so the sort list is built from `configuredColumns` -- a phone has no headers either, and building it from the drawn columns would leave it able to sort by title and duration alone. And a phone's column widths are neither loaded nor saved. That third rule is the bug the device found with the arrangement already passing five component tests and five e2e specs at the phone's own viewport. `loadColumnWidths` is keyed by column *id* and fills a gap with `MIN_COLUMN_WIDTH`, so the stacked column -- which nothing can ever have saved a width for -- came out at 148px beside a duration column of 236. The mirror image was worse and unreachable from a phone at all: saving would have written those widths back under the same ids, replacing the width the user dragged on a desktop. The specs asserted shape, and the fault depended on what `localStorage` held for a different column set; the unit test now carries that map as a fixture. Verified: 809 component tests, 112 e2e specs, and on the phone at 424x439 -- `24px 304px 80px`, 52px rows, no truncation, no overflow. One full e2e run of three saw an unrelated autotag keypress spec flake and pass on retry. --- .planning/NOTES.md | 44 +++++ .../pending/016-android-feature-parity.md | 19 ++- CLAUDE.md | 22 +++ e2e/specs/phone-track-list.spec.ts | 137 +++++++++++++++ frontend/src/components/track-list/columns.ts | 54 +++++- .../src/components/track-list/track-list.ts | 155 +++++++++++++++-- .../test/components/track-list-phone.test.ts | 161 ++++++++++++++++++ 7 files changed, 569 insertions(+), 23 deletions(-) create mode 100644 e2e/specs/phone-track-list.spec.ts create mode 100644 frontend/test/components/track-list-phone.test.ts diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 2d6e233..9079bb2 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3257,3 +3257,47 @@ done (`adb uninstall app.yellowjacket.dev` — the sibling id is exactly what makes that safe). And `am start` does not reliably take focus while another app is foreground: check `topResumedActivity` before trusting a screenshot, or you will read someone else's app. + +## The phone track list, and the bug a viewport could not have found (2026-08-17) + +B2 phase 4. A phone draws `titleArtist` — the title with the artist +under it — plus the duration, and drops the column headers and the +resize handles. It is a **column set, not a second row template**: the +row, its delegated events, the selection semantics, the playing marker +and the virtualizer never learn that anything changed, because from +their side only the number of columns did. + +Three rules, each one a way it breaks otherwise. The row height is in +two places (`PHONE_ROW_HEIGHT` and the CSS) and they must agree, since +the virtualizer positions rows from that number. What is *drawn* and +what can be *sorted* are separate questions — the sort list is built +from `configuredColumns`, or a phone with no headers could sort by +nothing but title and duration. And a phone's widths are neither loaded +nor saved. + +**That last one is the finding, and it came from the device.** With the +arrangement passing five component tests and five e2e specs at +424x439, the phone showed `24px 148px 236px`: the duration column with +55% of the row. `loadColumnWidths` is keyed by column *id* and fills a +gap with `MIN_COLUMN_WIDTH`, so the stacked column — which nothing can +ever have saved a width for, there being no handles to drag — came out +at the minimum while `trackLength` inherited a width saved for a +four-column desktop row. The mirror image is worse and was never +reachable from a phone at all: `saveColumnWidths` would have written the +computed phone widths back under the same ids, replacing the width the +user dragged on a desktop. + +**Why every browser test missed it.** The specs assert the *shape* — how +many grid tracks, no header, no overflow, the title's share of the row — +and the width bug depends on what is in `localStorage` for a *different* +column set. dev-headless's seed happened to hold widths that split the +other way, so the same assertion passed in the browser and failed on the +phone. The unit test now carries the desktop map as a fixture, which is +the reproduction the browser needed to have. + +Two tooling notes worth keeping. `playwright-cli` holds its page across +a `make dev-headless` restart, so a probe after a rebuild can be +answering for the *old* bundle — it reported the desktop layout at 424 px +until the page was reopened. And wireless adb dropped twice more mid- +session when the screen slept; USB for anything longer than a few +probes. diff --git a/.planning/plans/pending/016-android-feature-parity.md b/.planning/plans/pending/016-android-feature-parity.md index 4e499d6..9b43dac 100644 --- a/.planning/plans/pending/016-android-feature-parity.md +++ b/.planning/plans/pending/016-android-feature-parity.md @@ -315,8 +315,8 @@ places had to agree — `abiFilters`, the Makefile's `android:package` anchor is what stops it also matching the fat APK's line. Adding the ABI back, if modernc ever fixes `Xlstat64`, is those same three edits. -**B2, the desktop shell.** Scope decided (below); **phases 1, 2 and 3 -are done.** +**B2, the desktop shell.** Scope decided (below); **all four phases are +done.** - *Phase 1, the shell.* Below 600px the sidebar column is gone, `` is the primary navigation, and the shell fits 320px @@ -338,8 +338,19 @@ are done.** cannot dispatch a trusted event and that path would otherwise be the only uncovered one. -What is left of B2 is the track list, whose resizable columns are a -pointer feature with no touch equivalent. Not started. +- *Phase 4, the track list.* A phone draws `titleArtist` (title over + artist) plus the duration, and drops the column headers and the resize + handles — a column set rather than a second row template, so the row + and everything delegated on it is unchanged. Verified at the device's + own 424x439: `24px 304px 80px`, 52 px rows, no truncation, no + overflow. The device also found the bug in it, which no browser + viewport would have: saved *desktop* column widths reached the phone + through an id-keyed store and gave the duration column 55% of the row. + +**B2 is complete.** What is left in this plan is B3 (tag writing, which +needs a device), B4 (the catalog download on a metered connection), and +the standing question of the Light Phone's Chrome 113 — which so far has +cost nothing: menus, dialogs and long-press all work on it. **B3/B4** are unchanged, and B3 is now *possible* where it was not: with all-files access, `tagwriter` can write in place. diff --git a/CLAUDE.md b/CLAUDE.md index a71ce1a..4c617df 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1536,6 +1536,28 @@ by the three places that need them (the default widths, the normaliser, and the resize handles' positions), because they were written out separately and that is how they came to disagree. +**A phone draws one column of two lines, and that is a column set +rather than a second row template.** Measured on the device: at 424 px +the four configured columns fit the row *exactly* (`--grid-cols` came +out `24px 102px 101px 101px 80px`) and not one of them fit its content +— "Duration" did not fit its own header. The columns were never too +wide; there were too many of them. `PHONE_COLUMN_IDS` is `titleArtist` +(title over artist, sharing the row's whole width) plus the duration, so +the row, the delegated events, the selection semantics, the playing +marker and the virtualizer are all untouched: from their side only the +number of columns changed. Three rules come with it. **The row height +lives in two places and they must agree** — `PHONE_ROW_HEIGHT` and the +CSS rule — because the virtualizer positions rows from that number, so a +taller row overlaps its neighbour. **What is drawn and what can be +sorted are different questions**: the page header's sort list is built +from `configuredColumns`, or a phone (which has no column headers +either) could sort by nothing but title and duration. And **a phone's +widths are neither loaded nor saved**: `loadColumnWidths` is keyed by +column *id* and fills a gap with the minimum, so the stacked column — +which nothing can ever have saved a width for — came out at 148 px +beside a duration column of 236, and saving would have replaced the +width the user dragged on a desktop for the same id. + **The default columns are declared twice and must agree.** `tracklist.DefaultColumns` is what a fresh install persists; `DEFAULT_COLUMN_IDS` in `track-list/columns.ts` is what the list draws diff --git a/e2e/specs/phone-track-list.spec.ts b/e2e/specs/phone-track-list.spec.ts new file mode 100644 index 0000000..f80d2bc --- /dev/null +++ b/e2e/specs/phone-track-list.spec.ts @@ -0,0 +1,137 @@ +import { test, expect } from '../support/fixtures.js'; + +/** + * The track list on a phone (plan 016 B2 phase 4). + * + * The component tier pins the arrangement; this pins it in the real + * shell, at the viewport of the device the work was measured on — 424 x + * 439, a Light Phone III — because the fault it fixes was invisible to + * every assertion the app had. The columns *fit*: `--grid-cols` summed + * to exactly the host width, nothing overflowed, and every column was + * still unreadable. Only a measurement of what a cell can hold, or a + * screenshot, shows that. + */ +type Page = import('@playwright/test').Page; + +/** The phone this was built against, in CSS pixels. */ +const DEVICE = { width: 424, height: 439 }; + +/** A common small phone, as the shell specs use. */ +const PHONE = { width: 390, height: 844 }; + +const list = (page: Page) => page.locator('track-list'); + +/** The row's grid tracks and the widest text a cell can show. */ +const rowGeometry = (page: Page) => + page.evaluate(() => { + const sr = document.querySelector('track-list')?.shadowRoot; + const row = sr?.querySelector('.track-row'); + + if (!row) return null; + + const title = row.querySelector('.stacked-title'); + const sub = row.querySelector('.stacked-sub'); + + return { + tracks: getComputedStyle(row) + .gridTemplateColumns.split(/\s+/) + .filter(Boolean).length, + rowHeight: Math.round(row.getBoundingClientRect().height), + headerRow: !!sr?.querySelector('.header-row'), + handles: sr?.querySelectorAll('.col-resize-handle').length ?? 0, + titleWidth: title ? Math.round(title.getBoundingClientRect().width) : 0, + // A truncated cell is the fault; a cell wider than its text is fine. + titleTruncated: title ? title.scrollWidth > title.clientWidth + 1 : null, + subText: sub?.textContent?.trim() ?? null, + }; + }); + +test.describe('the track list on a phone', () => { + test.beforeEach(async ({ app }) => { + await app.setViewportSize(DEVICE); + await app.getByTestId('tab-tracks').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'tracks', + ); + await expect(list(app).first()).toBeVisible(); + }); + + test.afterEach(async ({ app }) => { + await app.setViewportSize({ width: 1440, height: 900 }); + }); + + test('stacks the title over the artist and drops the pointer affordances', async ({ + app, + }) => { + const geo = await rowGeometry(app); + + expect(geo).not.toBeNull(); + // Favourite + one stacked column + duration. + expect(geo?.tracks).toBe(3); + expect(geo?.headerRow).toBe(false); + expect(geo?.handles).toBe(0); + expect(geo?.subText).toBeTruthy(); + + // The row height has to match the virtualizer's item size, or rows + // overlap; 52 is that number. + expect(geo?.rowHeight).toBe(52); + }); + + test('gives the title most of the row instead of a quarter of it', async ({ + app, + }) => { + const geo = await rowGeometry(app); + + // Four columns at this width gave a title ~102px. The measurement + // that matters is the share of the row, not the pixel count. + expect(geo?.titleWidth ?? 0).toBeGreaterThan(DEVICE.width * 0.55); + }); + + test('needs no sideways scrolling, and neither does the shell', async ({ + app, + }) => { + const overflow = await app.evaluate(() => ({ + body: [document.body.scrollWidth, document.body.clientWidth], + list: (() => { + const sr = document.querySelector('track-list')?.shadowRoot; + const row = sr?.querySelector('.track-row'); + + return row ? [row.scrollWidth, row.clientWidth] : null; + })(), + })); + + expect(overflow.body[0]).toBe(overflow.body[1]); + expect(overflow.list?.[0]).toBe(overflow.list?.[1]); + }); + + test('keeps the sorts a phone has no headers to reach', async ({ app }) => { + // With no column headers, the page header's sort control is the only + // route to sort-by-artist — so it must still offer the columns the + // phone does not draw. + const ids = await app.evaluate(() => { + const header = document + .querySelector('track-list') + ?.shadowRoot?.querySelector('page-header') as + | (Element & { sortOptions?: { id: string }[] }) + | null; + + return (header?.sortOptions ?? []).map((o) => o.id); + }); + + expect(ids).toContain('artistName'); + expect(ids).toContain('album'); + }); + + test('is the desktop list again above the breakpoint', async ({ app }) => { + await app.setViewportSize(PHONE); + await expect.poll(async () => (await rowGeometry(app))?.tracks).toBe(3); + + await app.setViewportSize({ width: 1024, height: 800 }); + + // The same element, re-laid-out: this is one component with two + // column sets, not two components. + await expect.poll(async () => (await rowGeometry(app))?.headerRow).toBe(true); + await expect.poll(async () => (await rowGeometry(app))?.tracks).toBe(5); + }); +}); diff --git a/frontend/src/components/track-list/columns.ts b/frontend/src/components/track-list/columns.ts index 8504b60..059863a 100644 --- a/frontend/src/components/track-list/columns.ts +++ b/frontend/src/components/track-list/columns.ts @@ -9,6 +9,8 @@ import { import { formatMilliseconds } from '@utils/time'; import { html, nothing } from 'lit'; +import { highlightText } from './search-ranking'; + /** Compares two strings using locale-aware ordering. */ const compareStr = ( a: string, @@ -36,8 +38,17 @@ export interface ColumnDef { defaultWidth: string; /** Text alignment. Defaults to left. */ align?: 'left' | 'right'; - /** Optional custom render function returning an HTML template. */ - renderCell?: (track: library.Track) => unknown; + /** + * Optional custom render function returning an HTML template. + * + * `term` is the active search term, for a cell that wants to + * highlight its own text: the default path applies `highlightText` + * to `accessor`'s value, and a cell that renders itself has to do + * that itself or silently lose the highlight. Only `titleArtist` + * needs it, which is why it is optional rather than a second + * required parameter on all of them. + */ + renderCell?: (track: library.Track, term?: string) => unknown; /** * Comparison function for sorting two tracks by this column. * Returns negative if a < b, positive if a > b, zero if equal. @@ -85,6 +96,27 @@ export const COLUMN_DEFS: Record = { />`; }, }, + titleArtist: { + id: 'titleArtist', + // Named for what it sorts by, since that is the only place the + // label is user-visible: the phone has no column headers, and + // the page header's sort list is built from the *configured* + // columns rather than the drawn ones. + label: 'Track Name', + accessor: (t) => t.TrackName, + defaultWidth: '1fr', + comparator: (a, b) => compareStr(a.TrackName, b.TrackName), + renderCell: (t, term) => html` +
+ ${term ? highlightText(t.TrackName, term) : t.TrackName} + ${term ? highlightText(t.ArtistName, term) : t.ArtistName} +
+ `, + }, trackName: { id: 'trackName', label: 'Track Name', @@ -249,6 +281,24 @@ export const CORE_SEARCH_COLUMN_IDS: string[] = [ 'album', ]; +/** + * The one column a phone shows, and it is two lines. + * + * At 424 CSS px -- the width of the phone this was measured on -- four + * columns fit the row exactly and none of them fits its *content*: + * `--grid-cols` came out `24px 102px 101px 101px 80px`, so "Duration" + * did not fit its own header and a title had ~20 characters. The + * columns were never too wide; there were too many of them. + * + * So the phone gets the title with the artist under it, which is the + * shape every phone music list has, and the full row width to put them + * in. It is a *column definition* rather than a second row template on + * purpose: the row, the delegated events, the selection semantics, the + * playing marker and the virtualizer all keep working, because from + * their side nothing has changed except how many columns there are. + */ +export const PHONE_COLUMN_IDS: string[] = ['titleArtist', 'trackLength']; + /** * Default column IDs. Album is in them (H-15): without it, the three * `Tideline / Aurora Fields / 00:06` rows in this app's own fixture diff --git a/frontend/src/components/track-list/track-list.ts b/frontend/src/components/track-list/track-list.ts index 04fac93..d6a81aa 100644 --- a/frontend/src/components/track-list/track-list.ts +++ b/frontend/src/components/track-list/track-list.ts @@ -30,6 +30,7 @@ import { LibraryController } from '@store/controllers/library-controller'; import { COLUMN_DEFS, DEFAULT_COLUMN_IDS, + PHONE_COLUMN_IDS, } from './columns'; import type { ColumnDef } from './columns'; import { classMap } from 'lit/directives/class-map.js'; @@ -90,6 +91,18 @@ const ROW_PADDING_X = 8; const ROW_CHROME_WIDTH = FAV_COL_WIDTH + ROW_PADDING_X * 2; +/** + * Row heights, in the same relationship as the widths above: the number + * is read by the CSS *and* by the virtualizer's layout, so they cannot + * disagree. A phone row is two lines (title over artist). + */ +const ROW_HEIGHT = 33; +const PHONE_ROW_HEIGHT = 52; + +/** The shell's phone breakpoint, as `index.css` and every component + * stylesheet spells it. */ +const PHONE_QUERY = '(max-width: 599px)'; + // Inline SVG paths for favorite icons — eliminates wa-icon shadow DOM // overhead (30-50 shadow roots during scroll). Font Awesome 6 paths. const FAV_ICONS = { @@ -175,19 +188,34 @@ export class TrackList * Resolved column definitions for the currently configured * column IDs. Falls back to defaults for any unknown ID. */ - private get activeColumns(): ColumnDef[] { + /** + * The columns the user has chosen — what a desktop draws, and what + * *anything* may be sorted by. + * + * This is deliberately separate from `activeColumns`: "which columns + * are drawn" and "what can I sort by" are different questions, and + * the phone is exactly where they diverge. Building the sort list + * from the drawn columns would silently take sort-by-artist and + * sort-by-album away from the phone, which has no other route to + * them since it has no column headers either. + */ + private get configuredColumns(): ColumnDef[] { const ids = this.trackListCtrl.columnIds; + const chosen = !ids || ids.length === 0 ? DEFAULT_COLUMN_IDS : ids; - if (!ids || ids.length === 0) { - return DEFAULT_COLUMN_IDS - .map((id) => COLUMN_DEFS[id]) - .filter( - (d): d is ColumnDef => - d !== undefined, - ); - } + return chosen + .map((id) => COLUMN_DEFS[id]) + .filter( + (d): d is ColumnDef => + d !== undefined, + ); + } - return ids + /** The columns actually drawn: two stacked lines on a phone. */ + private get activeColumns(): ColumnDef[] { + if (!this.phone) return this.configuredColumns; + + return PHONE_COLUMN_IDS .map((id) => COLUMN_DEFS[id]) .filter( (d): d is ColumnDef => @@ -374,9 +402,42 @@ export class TrackList // doesn't need to measure items. Without this hint, the default 100px // estimate causes constant scroll error correction (scrollTo() calls) // that produce visible jumping/skipping during scroll. + /** + * The virtualizer's item size and the CSS row height are the same + * number in two places, and they must agree: the layout positions + * rows from this figure, so a row that is really taller overlaps its + * neighbour and a shorter one leaves a gap. Both come from here. + */ private flowLayout = flow({ - _itemSize: { width: 100, height: 33 }, + _itemSize: { width: 100, height: ROW_HEIGHT }, } as Parameters[0]); + + private phoneFlowLayout = flow({ + _itemSize: { width: 100, height: PHONE_ROW_HEIGHT }, + } as Parameters[0]); + + private get rowLayout(): Parameters[0] { + return this.phone ? this.phoneFlowLayout : this.flowLayout; + } + + /** + * Phone width, from the shell's own breakpoint. + * + * A media query *inside* a shadow root is answered by the viewport, + * which is what lets every other component state what it drops at + * phone width in its own stylesheet. This list cannot: its grid is + * computed in JS from the host width, so the same threshold has to + * be readable from JS as well. One breakpoint, two expressions of + * it, and the reason is written here rather than inferred. + */ + @state() + private phone = matchMedia(PHONE_QUERY).matches; + + private phoneQuery = matchMedia(PHONE_QUERY); + + private onPhoneChange = (e: MediaQueryListEvent): void => { + this.phone = e.matches; + }; private hasRestoredScroll = false; private scrollSaveRAFId: number | null = null; @@ -543,6 +604,20 @@ export class TrackList } private initColumnWidths() { + // A phone's widths are never the saved ones. `loadColumnWidths` + // is keyed by column *id* and fills a gap with + // `MIN_COLUMN_WIDTH`, so the phone's stacked column -- which + // nothing has ever saved a width for, there being no handles to + // drag -- came out at the minimum while the duration column + // inherited a width saved for a four-column desktop row. Found + // on the device: `24px 148px 236px`, the duration column with + // 55% of a phone's row. + if (this.phone) { + this.computeDefaultWidths(); + + return; + } + const saved = this.loadColumnWidths(); const cols = this.activeColumns; @@ -673,6 +748,13 @@ export class TrackList } private saveColumnWidths() { + // And a phone's widths are never *saved*: they are computed from + // a column set the user did not choose, and writing them would + // overwrite the width they dragged for the same column on a + // desktop. Nothing on a phone can resize a column anyway, so + // this is only reachable by a window crossing the breakpoint. + if (this.phone) return; + try { const cols = this.activeColumns; @@ -1045,6 +1127,35 @@ export class TrackList contain: strict; } + /* A phone row is two lines, and this height must equal + PHONE_ROW_HEIGHT: the virtualizer positions rows from that number, + so a taller row overlaps its neighbour and a shorter one gaps. */ + @media (max-width: 599px) { + .track-row { + height: 52px; + } + } + + .stacked { + display: flex; + flex-direction: column; + justify-content: center; + gap: 2px; + min-width: 0; + } + + .stacked-title, + .stacked-sub { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + + .stacked-sub { + font-size: var(--yj-text-xs); + color: var(--yj-text-secondary, #b3b3b3); + } + .track-row > * { min-width: 0; } @@ -1171,9 +1282,17 @@ export class TrackList ); this.resizeObserver.observe(this); + + // Connection, not the view lifecycle: this only sets state, so + // it is harmless (and wanted) while the list is off screen -- a + // rotation on another view must not leave this one laid out for + // the wrong width when the user comes back to it. + this.phoneQuery.addEventListener('change', this.onPhoneChange); } override disconnectedCallback() { + this.phoneQuery.removeEventListener('change', this.onPhoneChange); + // Remove delegated event handlers from virtualizer. const virt = this.virtualizer; if (virt) { @@ -2017,7 +2136,7 @@ export class TrackList ${cols.map((col) => { - const customCell = col.renderCell?.(track); + const customCell = col.renderCell?.(track, term); if (customCell !== undefined && customCell !== nothing) { return html`
${customCell}
`; } @@ -2065,7 +2184,7 @@ export class TrackList private renderPageHeader() { const options: SortOption[] = [ { id: '', label: 'Default' }, - ...this.activeColumns + ...this.configuredColumns .filter((c) => c.comparator) .map((c) => ({ id: c.id, label: c.label })), ]; @@ -2115,7 +2234,7 @@ export class TrackList aria-busy=${this.loadingTracks} @keydown=${this.onListKeydown} > -
+ ${this.phone ? nothing : html`
${cols.map( (col) => html` @@ -2149,7 +2268,7 @@ export class TrackList
`, )} -
+ `} ${visibleTracks.length === 0 ? html`

No tracks match your search. @@ -2160,12 +2279,14 @@ export class TrackList .items=${visibleTracks} .renderItem=${this.renderTrackRow} .keyFunction=${(track: library.Track) => track.FilePath} - .layout=${this.flowLayout} + .layout=${this.rowLayout} > `} +

- ${this.colBoundaryPositions.map( + ${(this.phone ? [] : this.colBoundaryPositions).map( (pos, i) => html`
({ + FilePath: `/music/track-${i}.mp3`, + TrackName: `Track ${i}`, + ArtistName: `Artist ${i}`, + Album: 'An Album', + Duration: 180 + i, +})) as never[]; + +const real = window.matchMedia.bind(window); + +/** Force the shell's phone breakpoint on or off. */ +function stubPhone(phone: boolean): void { + window.matchMedia = ((q: string) => + q.includes('max-width: 599px') + ? { + matches: phone, + media: q, + addEventListener() {}, + removeEventListener() {}, + } + : real(q)) as typeof window.matchMedia; +} + +async function mount(phone: boolean): Promise { + stubPhone(phone); + + // Narrow, so a desktop layout at this width would be the cramped one + // the plan describes rather than a comfortable one. + const el = await fixture('track-list', { + externalTracks: TRACKS, + }); + + el.style.width = '424px'; + el.style.height = '400px'; + await el.updateComplete; + + return el; +} + +afterEach(() => { + window.matchMedia = real; + localStorage.removeItem('track-list-column-widths'); +}); + +describe('the track list at phone width', () => { + it('draws the title with the artist under it, and the duration', async () => { + const el = await mount(true); + const row = shadow(el, '.track-row'); + + expect(row).not.toBeNull(); + expect(shadow(el, '.stacked-title')?.textContent?.trim()).toBe('Track 0'); + expect(shadow(el, '.stacked-sub')?.textContent?.trim()).toBe('Artist 0'); + + // Two drawn columns plus the favourite: three grid tracks, not five. + const tracks = getComputedStyle(row as Element) + .gridTemplateColumns.split(/\s+/) + .filter(Boolean); + + expect(tracks).toHaveLength(3); + }); + + it('drops the column headers and the resize handles', async () => { + const el = await mount(true); + + // Both are pointer affordances: a header is where a click sorts and + // a handle is where a drag resizes, and a phone can do neither. + expect(shadow(el, '.header-row')).toBeNull(); + expect(shadowAll(el, '.col-resize-handle')).toHaveLength(0); + }); + + it('keeps every sort the desktop offers', async () => { + const el = await mount(true); + const header = shadow(el, 'page-header') as + | (Element & { sortOptions?: { id: string }[] }) + | null; + + // The regression this guards: building the sort list from the + // *drawn* columns would leave a phone able to sort by title and + // duration only, with no column headers to reach the rest by. + const ids = (header?.sortOptions ?? []).map((o) => o.id); + + expect(ids).toContain('artistName'); + expect(ids).toContain('album'); + }); + + it('still marks the row a screen reader has to understand', async () => { + const el = await mount(true); + const row = shadow(el, '.track-row'); + + // The row is the same row: only the cells inside it changed, which + // is the entire argument for doing this as a column set. + expect(row?.getAttribute('role')).toBe('row'); + expect(row?.getAttribute('aria-selected')).toBe('false'); + expect(row?.getAttribute('data-testid')).toBe('track-row'); + expect(shadowAll(el, '[role="gridcell"]').length).toBeGreaterThan(0); + }); + + it('ignores widths saved for the desktop, and does not overwrite them', async () => { + // The bug the device found, in the fixture that reproduces it. + // `loadColumnWidths` is keyed by column *id* and fills a gap with + // MIN_COLUMN_WIDTH, so the phone's stacked column -- which nothing + // can ever have saved a width for -- came out at the minimum while + // the duration column inherited a width dragged on a wide window: + // measured `24px 148px 236px` on a 424px phone. + const desktop = { trackName: 300, artistName: 200, album: 200, trackLength: 236 }; + + localStorage.setItem('track-list-column-widths', JSON.stringify(desktop)); + + const el = await mount(true); + const tracks = getComputedStyle(shadow(el, '.track-row') as Element) + .gridTemplateColumns.split(/\s+/) + .filter(Boolean) + .map((t) => Math.round(parseFloat(t))); + + // Favourite, then the stacked column, then the duration -- and the + // stacked one is the widest thing in the row. + expect(tracks[1]).toBeGreaterThan(tracks[2] ?? 0); + + // And the desktop's own widths survive being on a phone: writing the + // computed phone widths back would silently replace the width the + // user dragged for the same column id. + expect(JSON.parse(localStorage.getItem('track-list-column-widths') ?? '{}')) + .toMatchObject(desktop); + }); + + it('leaves the desktop alone', async () => { + const el = await mount(false); + const row = shadow(el, '.track-row'); + + expect(shadow(el, '.header-row')).not.toBeNull(); + expect(shadow(el, '.stacked-title')).toBeNull(); + + const tracks = getComputedStyle(row as Element) + .gridTemplateColumns.split(/\s+/) + .filter(Boolean); + + expect(tracks).toHaveLength(5); + }); +}); From de2b324e20a9e516e13298751ceba832b845d82e Mon Sep 17 00:00:00 2001 From: Logan Date: Mon, 17 Aug 2026 10:48:00 -0400 Subject: [PATCH 3/4] feat(explore): refuse 0.6 GB on someone's mobile data Plan 016 B4. The catalog artifact is about 0.6 GB and the app fetched it with no awareness of the connection: on a desktop that is a minute of bandwidth, on a phone it can be a month's allowance. It is now skipped on a cellular connection unless `AllowMeteredCatalogDownload` is on, with the toggle in Settings' Search Index section, where the text explaining what the catalog is already lives. The file layout is dictated by the cgo rule rather than by taste. `explore` is imported by `cmd/indexbuild`, which builds with CGO_ENABLED=0 and must not link Wails, so `netpolicy.go` holds the policy and the JSON parsing -- tested on every platform -- and the single platform call is a closure injected from `app.go`, which already names `application` legitimately. Three rules in it are load-bearing. An unknown answer is not a metered one: only mobile answers at all, and treating silence as metered would have disabled the download for every desktop user in the world. Cellular is the only signal available, because the runtime reports `wifi|cellular|ethernet|none` and no metered flag -- so a metered Wi-Fi cannot be detected and is not refused, which is documented rather than implied. And the gate runs before the first status write, so declining is a no-op instead of a job in the indicator and an error tier to dismiss. Two corrections to the plan while implementing it: the portable API is `application.Mobile.NetworkJSON()`, not `application.Android`'s, which exists only under the `android` build tag; and the permission is read at the moment a download would start, so enabling it takes effect on the next attempt rather than the next launch. --- .planning/NOTES.md | 31 ++++ .../pending/016-android-feature-parity.md | 14 +- CLAUDE.md | 25 +++ backend/app.go | 18 ++ backend/config/config.go | 45 +++++ backend/config/general.go | 6 + backend/explore/artifactbuild.go | 14 ++ backend/explore/netpolicy.go | 137 +++++++++++++++ backend/explore/netpolicy_test.go | 158 ++++++++++++++++++ backend/explore/searchindex.go | 5 + .../yellowjacket/backend/config/config.ts | 19 +++ .../src/components/config-page/config-page.ts | 63 ++++++- 12 files changed, 527 insertions(+), 8 deletions(-) create mode 100644 backend/explore/netpolicy.go create mode 100644 backend/explore/netpolicy_test.go diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 9079bb2..c8781c4 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3301,3 +3301,34 @@ answering for the *old* bundle — it reported the desktop layout at 424 px until the page was reopened. And wireless adb dropped twice more mid- session when the screen slept; USB for anything longer than a few probes. + +## The catalog download now asks about the connection (2026-08-17) + +Plan 016 B4. ~0.6 GB had no network awareness at all; it is skipped on a +cellular connection unless the user says otherwise +(`AllowMeteredCatalogDownload`, default false, toggle in Settings' Search +Index section). + +**The shape is dictated by the cgo rule, not by taste.** `explore` is +imported by `cmd/indexbuild`, which builds with `CGO_ENABLED=0` and must +not link Wails, so `netpolicy.go` holds the policy and the JSON parsing — +tested on every platform — while the one platform call is a closure +injected from `app.go`, where naming `application` is already legitimate. + +Four things measured or corrected in the doing: + +- **The portable name is `application.Mobile`, not `application.Android`** + (which the plan and `CLAUDE.md` both named). `Android` exists only + under the `android` build tag; `Mobile`'s desktop implementation is a + stub whose `NetworkJSON()` returns `""`. +- **The runtime reports no metered flag.** `{"connected":bool, + "type":"wifi|cellular|ethernet|none"}` is all there is, so cellular is + the signal and a metered *Wi-Fi* — a phone hotspot, a hotel — cannot be + detected. Android itself knows (`NET_CAPABILITY_NOT_METERED`) and the + runtime does not pass it on. Documented gap, not an oversight. +- **An unknown answer must not read as metered.** Every desktop answers + `""`, so the obvious defensive default would have disabled the catalog + download for every desktop user in the world. +- **The gate belongs before the first status write.** Declining is a + no-op — no job in the indicator, no error tier to dismiss — which is + what makes the refusal safe to have on by default. diff --git a/.planning/plans/pending/016-android-feature-parity.md b/.planning/plans/pending/016-android-feature-parity.md index 9b43dac..4a3fbf2 100644 --- a/.planning/plans/pending/016-android-feature-parity.md +++ b/.planning/plans/pending/016-android-feature-parity.md @@ -347,8 +347,18 @@ done.** viewport would have: saved *desktop* column widths reached the phone through an id-keyed store and gave the duration column 55% of the row. -**B2 is complete.** What is left in this plan is B3 (tag writing, which -needs a device), B4 (the catalog download on a metered connection), and +**B2 and B4 are complete.** B4 is `backend/explore/netpolicy.go`: the +catalog download is skipped on a cellular connection unless +`AllowMeteredCatalogDownload` is on, with the toggle in Settings' Search +Index section. The policy and the JSON parsing are in `explore` (tested +on every platform) and only the platform call is injected from `app.go`, +because `cmd/indexbuild` imports `explore` and must not link Wails. Two +things the plan got slightly wrong: the portable API is +`application.Mobile.NetworkJSON()` rather than `Android`'s, and it +reports no metered flag — so cellular is the signal and a metered Wi-Fi +cannot be seen. + +What is left in this plan is B3 (tag writing, which needs a device) and the standing question of the Light Phone's Chrome 113 — which so far has cost nothing: menus, dialogs and long-press all work on it. diff --git a/CLAUDE.md b/CLAUDE.md index 4c617df..4c8d04d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -508,6 +508,31 @@ selected as a literal `0`. Adding the column to the importer's SELECT list without that is how a published artifact — which nobody can re-cut retroactively — starts failing with `no such column`. +**A 0.6 GB download asks about the connection first.** `explore`'s +catalog artifact had no network awareness at all, which on a phone is a +month's data allowance spent without being asked (plan 016 B4). +`netpolicy.go` is the gate, and its shape is dictated by one constraint: +`explore` is imported by `cmd/indexbuild`, which is built with +`CGO_ENABLED=0` and must not link Wails — so the *policy* and the +*parsing* live here and are tested on every platform, while the platform +call is a closure injected from `app.go`. It is +`application.Mobile.NetworkJSON()`, not `application.Android`'s: the +latter exists only under the `android` build tag, and `Mobile`'s desktop +implementation is a stub returning `""`. + +Three rules in it are load-bearing. **An unknown answer is not a metered +one** — only mobile answers at all, so treating silence as metered would +refuse the download on every desktop. **Cellular is the only signal +available**: the runtime reports `wifi|cellular|ethernet|none` and no +metered flag, so a metered *Wi-Fi* (a hotspot, a hotel) cannot be +detected and is not refused, which is a documented gap rather than an +oversight. And **the gate runs before anything is staged**, so declining +is a no-op rather than a job in the indicator and a status the user has +to dismiss. The permission (`AllowMeteredCatalogDownload`, default +false, so an existing config is careful without a migration) is read at +the moment a download would start, so turning it on takes effect on the +next attempt rather than the next launch. + **Background work yields, and says so in the context.** The post-scan backfills share MusicBrainz's rate limiters with every page the user can open, and both were FIFO — so a thousand-artist enrichment put an diff --git a/backend/app.go b/backend/app.go index 9a25996..376f0ae 100644 --- a/backend/app.go +++ b/backend/app.go @@ -191,6 +191,24 @@ func NewYellowJacketApp( yjApp.library.SetJobRegistry(yjApp.jobs) yjApp.explore.SetJobRegistry(yjApp.jobs) + // Whether this connection is one to spend ~0.6 GB of catalog on + // (plan 016 B4). The probe is injected from here because `explore` is + // imported by `cmd/indexbuild`, which must not link Wails: naming + // `application` there is what `TestIndexToolsDoNotImportWails` + // forbids. + // + // `application.Mobile`, not `application.Android`: the latter exists + // only under the `android` build tag, while `Mobile` is the portable + // name whose desktop implementation is a stub returning "" — which + // parses to "unknown" and refuses nothing. Plan 016 named the tagged + // one; this is the same call by the name every build has. + yjApp.explore.SetNetworkPolicy( + func() explore.Network { + return explore.ParseNetworkJSON(application.Mobile.NetworkJSON()) + }, + yjApp.appConfig.GetAllowMeteredCatalogDownload, + ) + // Let the release prefetch skip albums the user already owns in // full — those open with no catalog call at all, so warming their // tracklists spends the most expensive request in the app on diff --git a/backend/config/config.go b/backend/config/config.go index c178ccb..f0ff93b 100644 --- a/backend/config/config.go +++ b/backend/config/config.go @@ -620,6 +620,51 @@ func (c *Config) SetQueueFallback(mode string) error { return nil } +// GetAllowMeteredCatalogDownload reports whether the ~0.6 GB Explore +// catalog may be fetched on a metered connection. +func (c *Config) GetAllowMeteredCatalogDownload() bool { + if c.General == nil { + return false + } + + return c.General.AllowMeteredCatalogDownload +} + +// SetAllowMeteredCatalogDownload saves the metered-download permission. +// +// There is nothing to validate and nothing to restart: the policy is +// read at the moment a download would start, so turning it on takes +// effect on the next attempt rather than needing this launch to be over. +func (c *Config) SetAllowMeteredCatalogDownload(allow bool) error { + if c.General == nil { + c.General = &GeneralConfig{} + c.General.ApplyDefaults() + } + + c.General.AllowMeteredCatalogDownload = allow + + if err := c.Save(); err != nil { + return fmt.Errorf( + "could not save config: %w", err, + ) + } + + events.Emit( + c.ctx, + events.GeneralConfigChanged, + map[string]any{ + "AllowMeteredCatalogDownload": allow, + }, + ) + + c.logger.Info( + "metered catalog download permission updated", + "allow", allow, + ) + + return nil +} + // GetTrackListColumns returns the configured track-list columns. func (c *Config) GetTrackListColumns() []tracklist.Column { if c.TrackList == nil { diff --git a/backend/config/general.go b/backend/config/general.go index 641c3ab..895b391 100644 --- a/backend/config/general.go +++ b/backend/config/general.go @@ -48,6 +48,12 @@ var errUnknownQueueFallback = errors.New("unknown queue fallback") type GeneralConfig struct { DefaultPage DefaultPage `toml:"DefaultPage"` QueueFallback QueueFallback `toml:"QueueFallback"` + // AllowMeteredCatalogDownload permits the ~0.6 GB Explore catalog to + // be fetched on a connection the platform calls cellular. It defaults + // to false, which is the whole point: the zero value is the safe one, + // so an existing config with no such key refuses by default rather + // than needing a migration to become careful. + AllowMeteredCatalogDownload bool `toml:"AllowMeteredCatalogDownload"` } // ApplyDefaults fills zero-value fields with sensible defaults. diff --git a/backend/explore/artifactbuild.go b/backend/explore/artifactbuild.go index b592367..df52097 100644 --- a/backend/explore/artifactbuild.go +++ b/backend/explore/artifactbuild.go @@ -27,6 +27,20 @@ var artifactStageNames = [...]string{ // failure path is non-fatal by design: the caller falls back, and a // fresh install with no network still gets its own library in Explore. func (si *SearchIndex) tryCoreArtifact(ctx context.Context) error { + // Before anything is staged: ~0.6 GB is not a download to start on + // someone's cellular allowance without being asked (plan 016 B4). + // This is checked first so no job appears and no status changes -- + // declining is a no-op, not a failure the user has to dismiss. + if si.netPolicy.refuses() { + si.logIndexJob( + jobs.LevelInfo, + "Skipping the catalog download on a metered connection. "+ + "Enable it in Settings to download anyway.", + ) + + return ErrMeteredNetwork + } + si.mu.Lock() si.buildStatus = IndexStatus{ Building: true, diff --git a/backend/explore/netpolicy.go b/backend/explore/netpolicy.go new file mode 100644 index 0000000..72d2da0 --- /dev/null +++ b/backend/explore/netpolicy.go @@ -0,0 +1,137 @@ +package explore + +import ( + "encoding/json" + "errors" + "strings" + "sync" +) + +// Whether the catalog artifact may be downloaded on this connection +// (plan 016 B4). +// +// The artifact is ~0.6 GB. On a desktop that is a minute of someone +// else's bandwidth; on a phone it can be a month's allowance, and the +// app had no awareness of the difference at all. +// +// Three decisions shape this file. +// +// **The policy lives here and the platform call does not.** `explore` is +// imported by `cmd/indexbuild`, which is built with `CGO_ENABLED=0` in a +// plain Go container, so naming `application` here would break the one +// job that must not fail (see `TestIndexToolsDoNotImportWails`). What is +// injected is a closure; what is *tested* is the parsing and the +// decision, on every platform. +// +// **An unknown answer is not a metered one.** Only mobile answers this +// question — the desktop stub returns an empty string — so a policy that +// treated silence as "metered" would refuse the download on every +// desktop in the world. Silence means "no reason to refuse". +// +// **Cellular is the signal, and it is the only one available.** Wails +// reports `{"connected":bool,"type":"wifi|cellular|ethernet|none"}` and +// no metered flag, so a metered *wifi* — a phone hotspot, a hotel — is +// invisible to us and will not be refused. That is a known gap rather +// than an oversight: Android knows (`NET_CAPABILITY_NOT_METERED`) and +// the runtime does not pass it on. + +// ErrMeteredNetwork is returned instead of downloading the catalog when +// the connection looks metered and the user has not opted in. Every +// failure path in `tryCoreArtifact` is already non-fatal, so this +// behaves like any other reason the artifact is not available yet. +var ErrMeteredNetwork = errors.New( + "explore: catalog download declined on a metered connection", +) + +// Network is what the platform can say about the connection. +type Network struct { + // Known is false when nothing answered — every desktop, and any + // mobile build whose bridge is not up yet. + Known bool + // Connected reports a usable connection of any kind. + Connected bool + // Metered reports a connection the user is plausibly paying for by + // the byte. See the note above on what this cannot see. + Metered bool +} + +// NetworkProbe answers "what kind of connection is this", or an unknown +// Network when the platform does not say. +type NetworkProbe func() Network + +// ParseNetworkJSON reads the runtime's network payload. +// +// Anything unparseable is `Known: false` rather than an error: this +// decides whether to *skip* an optional download, and a malformed +// payload is not a reason to refuse one. +func ParseNetworkJSON(payload string) Network { + var raw struct { + Connected bool `json:"connected"` + Type string `json:"type"` + } + + if strings.TrimSpace(payload) == "" { + return Network{} + } + + if err := json.Unmarshal([]byte(payload), &raw); err != nil { + return Network{} + } + + return Network{ + Known: true, + Connected: raw.Connected, + Metered: strings.EqualFold(raw.Type, "cellular"), + } +} + +// networkPolicy is the injected half: how to ask, and whether the user +// has said yes anyway. +type networkPolicy struct { + mu sync.RWMutex + probe NetworkProbe + allowMetered func() bool +} + +func (p *networkPolicy) set(probe NetworkProbe, allowMetered func() bool) { + p.mu.Lock() + defer p.mu.Unlock() + + p.probe = probe + p.allowMetered = allowMetered +} + +// refuses reports whether a large optional download should be skipped. +func (p *networkPolicy) refuses() bool { + p.mu.RLock() + probe, allow := p.probe, p.allowMetered + p.mu.RUnlock() + + if probe == nil { + return false + } + + if allow != nil && allow() { + return false + } + + state := probe() + + return state.Known && state.Metered +} + +// SetNetworkPolicy wires how the catalog download decides whether this +// connection is one to spend 0.6 GB on. Both arguments may be nil, which +// is the desktop's answer: never refuse. +// +//wails:ignore // internal wiring, not part of the app's IPC surface. +func (si *SearchIndex) SetNetworkPolicy(probe NetworkProbe, allowMetered func() bool) { + si.netPolicy.set(probe, allowMetered) +} + +// SetNetworkPolicy wires the metered-connection policy into the index. +// +//wails:ignore // internal wiring, not part of the app's IPC surface. +func (e *Service) SetNetworkPolicy(probe NetworkProbe, allowMetered func() bool) { + e.index.SetNetworkPolicy(probe, allowMetered) +} diff --git a/backend/explore/netpolicy_test.go b/backend/explore/netpolicy_test.go new file mode 100644 index 0000000..008394a --- /dev/null +++ b/backend/explore/netpolicy_test.go @@ -0,0 +1,158 @@ +package explore + +import ( + "errors" + "testing" +) + +// The catalog is ~0.6 GB and the decision not to fetch it is the only +// part of plan 016 B4 that can be tested anywhere but on a phone: the +// platform call is a one-line closure injected from app.go, and +// everything that decides anything is here. + +func TestParseNetworkJSON(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + payload string + want Network + }{{ + name: "cellular is metered", + payload: `{"connected":true,"type":"cellular"}`, + want: Network{Known: true, Connected: true, Metered: true}, + }, { + name: "wifi is not", + payload: `{"connected":true,"type":"wifi"}`, + want: Network{Known: true, Connected: true}, + }, { + name: "ethernet is not", + payload: `{"connected":true,"type":"ethernet"}`, + want: Network{Known: true, Connected: true}, + }, { + name: "the case is the platform's business, not ours", + payload: `{"connected":true,"type":"Cellular"}`, + want: Network{Known: true, Connected: true, Metered: true}, + }, { + name: "offline is known and unmetered", + payload: `{"connected":false,"type":"none"}`, + want: Network{Known: true}, + }, { + // The desktop stub. This is the case that must not read as + // "metered": every desktop in the world answers this way. + name: "an empty payload is unknown", + payload: "", + want: Network{}, + }, { + name: "so is a malformed one", + payload: `{"connected":`, + want: Network{}, + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + if got := ParseNetworkJSON(tt.payload); got != tt.want { + t.Errorf("ParseNetworkJSON(%q) = %+v, want %+v", tt.payload, got, tt.want) + } + }) + } +} + +func TestNetworkPolicyRefuses(t *testing.T) { + t.Parallel() + + cellular := func() Network { + return Network{Known: true, Connected: true, Metered: true} + } + wifi := func() Network { return Network{Known: true, Connected: true} } + unknown := func() Network { return Network{} } + yes := func() bool { return true } + no := func() bool { return false } + + tests := []struct { + name string + probe NetworkProbe + allowMetered func() bool + want bool + }{{ + name: "no probe wired refuses nothing", + probe: nil, + want: false, + }, { + name: "an unknown connection refuses nothing", + probe: unknown, + want: false, + }, { + name: "wifi refuses nothing", + probe: wifi, + want: false, + }, { + name: "cellular refuses by default", + probe: cellular, + want: true, + }, { + name: "cellular with no permission refuses", + probe: cellular, + allowMetered: no, + want: true, + }, { + name: "cellular the user opted into does not", + probe: cellular, + allowMetered: yes, + want: false, + }, { + // The permission is read at decision time rather than captured, + // so turning it on takes effect on the next attempt instead of + // the next launch. + name: "permission is asked, not remembered", + probe: cellular, + allowMetered: yes, + want: false, + }} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var p networkPolicy + + p.set(tt.probe, tt.allowMetered) + + if got := p.refuses(); got != tt.want { + t.Errorf("refuses() = %v, want %v", got, tt.want) + } + }) + } +} + +// The gate has to come before anything is staged: a declined download is +// a no-op, not a job in the indicator or a status the user must dismiss. +func TestTryCoreArtifactDeclinesMeteredWithoutStaging(t *testing.T) { + t.Parallel() + + si := &SearchIndex{} + + si.SetNetworkPolicy( + func() Network { return Network{Known: true, Connected: true, Metered: true} }, + nil, + ) + + err := si.tryCoreArtifact(t.Context()) + + if !errors.Is(err, ErrMeteredNetwork) { + t.Fatalf("tryCoreArtifact() error = %v, want ErrMeteredNetwork", err) + } + + // Nothing announced itself: no build status, no tiers, no job. A + // SearchIndex with no database would panic on any of the work below + // the gate, which is itself part of the assertion. + if si.buildStatus.Building { + t.Error("declining a metered download still reported a build in progress") + } + + if len(si.buildStatus.Tiers) != 0 { + t.Errorf("declining staged %d tiers, want none", len(si.buildStatus.Tiers)) + } +} diff --git a/backend/explore/searchindex.go b/backend/explore/searchindex.go index 384bfd5..d7a0448 100644 --- a/backend/explore/searchindex.go +++ b/backend/explore/searchindex.go @@ -212,6 +212,11 @@ type SearchIndex struct { cancel context.CancelFunc done chan struct{} + // netPolicy decides whether this connection is one to spend ~0.6 GB + // of catalog on. Its own lock: it is written once at startup and read + // from the build goroutine (netpolicy.go). + netPolicy networkPolicy + mu sync.RWMutex ready bool diff --git a/frontend/bindings/yellowjacket/backend/config/config.ts b/frontend/bindings/yellowjacket/backend/config/config.ts index 75f97fa..ddd06ac 100644 --- a/frontend/bindings/yellowjacket/backend/config/config.ts +++ b/frontend/bindings/yellowjacket/backend/config/config.ts @@ -17,6 +17,14 @@ import * as download$0 from "../download/models.js"; // @ts-ignore: Unused imports import * as tracklist$0 from "../tracklist/models.js"; +/** + * GetAllowMeteredCatalogDownload reports whether the ~0.6 GB Explore + * catalog may be fetched on a metered connection. + */ +export function GetAllowMeteredCatalogDownload(): $CancellablePromise { + return $Call.ByID(2258585139); +} + /** * GetDefaultPage returns the view the app opens to on launch. */ @@ -127,6 +135,17 @@ export function Save(): $CancellablePromise { return $Call.ByID(1988945736); } +/** + * SetAllowMeteredCatalogDownload saves the metered-download permission. + * + * There is nothing to validate and nothing to restart: the policy is + * read at the moment a download would start, so turning it on takes + * effect on the next attempt rather than needing this launch to be over. + */ +export function SetAllowMeteredCatalogDownload(allow: boolean): $CancellablePromise { + return $Call.ByID(192700351, allow); +} + /** * SetDefaultPage validates and saves a new launch page. */ diff --git a/frontend/src/components/config-page/config-page.ts b/frontend/src/components/config-page/config-page.ts index cf7d9b9..c0850a0 100644 --- a/frontend/src/components/config-page/config-page.ts +++ b/frontend/src/components/config-page/config-page.ts @@ -17,6 +17,8 @@ import { SetDefaultPage, GetQueueFallback, SetQueueFallback, + GetAllowMeteredCatalogDownload, + SetAllowMeteredCatalogDownload, } from '@go/config/config.js'; import { GetIndexStatus } from '@go/explore/service.js'; import { notificationStore } from '@store/notification-store'; @@ -75,6 +77,9 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { // --- Now Playing state --- @state() private scrollMode = 'hover'; + /** Whether the ~0.6 GB catalog may be fetched on mobile data. */ + @state() private allowMeteredCatalogDownload = false; + // --- Favorites state --- @state() private playlists: playlist.Summary[] = []; @@ -888,17 +893,20 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) { private async loadLibraries(): Promise { try { - const [libs, mode, defaultPage, queueFallback] = await Promise.all([ - GetAllLibrariesWithTrackCounts(), - GetScanConcurrency(), - GetDefaultPage(), - GetQueueFallback(), - ]); + const [libs, mode, defaultPage, queueFallback, allowMetered] = + await Promise.all([ + GetAllLibrariesWithTrackCounts(), + GetScanConcurrency(), + GetDefaultPage(), + GetQueueFallback(), + GetAllowMeteredCatalogDownload(), + ]); this.libraries = libs ?? []; this.concurrencyMode = mode; this.defaultPage = defaultPage; this.queueFallback = queueFallback; + this.allowMeteredCatalogDownload = allowMetered; } catch (err) { console.error( @@ -1500,10 +1508,53 @@ export class ConfigPage extends ViewLifecycleMixin(LitElement) {
` : html`
Loading status…
`}
+ + `; } + /** + * The catalog download's one permission (plan 016 B4). + * + * It is in this section rather than General because it is about + * *this* download and nothing else, and because the section already + * explains what the catalog is — the toggle would be unreadable + * beside "Default page". + */ + private handleAllowMeteredChange = ( + e: CustomEvent, + ): void => { + const allow = Boolean(e.detail.value); + const previous = this.allowMeteredCatalogDownload; + + this.allowMeteredCatalogDownload = allow; + + void SetAllowMeteredCatalogDownload(allow).catch((err: unknown) => { + console.error('failed to save metered download permission', err); + // The visible state reverted, so this is the Transient case: + // a small action the user can simply repeat. + this.allowMeteredCatalogDownload = previous; + notificationStore.transient({ + key: 'metered-catalog-setting', + title: 'Setting not saved', + text: describeError(err, 'That setting could not be saved.'), + }); + }); + }; + private tierIcon(state: string): string { switch (state) { case 'complete': From d0250a213324b86323c2f779d82c1a60d327b588 Mon Sep 17 00:00:00 2001 From: Logan Date: Mon, 17 Aug 2026 10:51:23 -0400 Subject: [PATCH 4/4] docs: confirm the phone track list on the phone The arrangement and the width fix, measured on the device with the build installed rather than at the same viewport in a browser: `24px 304px 80px`, 52px rows, no header, the title untruncated, no overflow. Same numbers both places, which is why both were measured. --- .planning/NOTES.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.planning/NOTES.md b/.planning/NOTES.md index c8781c4..3bad75d 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -3295,6 +3295,12 @@ other way, so the same assertion passed in the browser and failed on the phone. The unit test now carries the desktop map as a fixture, which is the reproduction the browser needed to have. +**Confirmed on the phone afterwards**, with the fix installed: +`24px 304px 80px`, 52 px rows, no header row, the title 298 px and not +truncated, `body.scrollWidth == clientWidth`. The same numbers the +browser gives at that viewport, which is the point of having measured +both. + Two tooling notes worth keeping. `playwright-cli` holds its page across a `make dev-headless` restart, so a probe after a rebuild can be answering for the *old* bundle — it reported the desktop layout at 424 px