From 8b4c9b00500cc813e0256194242a0fdc6607fc9e Mon Sep 17 00:00:00 2001 From: Sin Date: Wed, 24 Jun 2026 20:49:02 +0100 Subject: [PATCH] Fix list/tree view: select mode, parts visibility, song actions (#585) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix list/tree view: select mode, parts visibility, song actions Bring the v3 list/tree view to parity with the grid card: - Select mode now renders a per-row checkbox + selected-ring, preserves expanded artist groups across re-render, and a capture-phase guard makes a row/chip click select the song instead of starting playback. - Always-on favourite / save-for-later / overflow-menu cluster on each row, same actions as the grid card. Rebuild static/tailwind.min.css so the new utilities are compiled in - notably .sm:flex behind the arrangement chips' "hidden sm:flex" wrapper. Without it the chips (and #582's badges) render display:none on the Docker build, which serves the committed CSS; the desktop build looked fine only because it rebuilds Tailwind from source at bundle time. Signed-off-by: Sin * fix(v3): regenerate tailwind.min.css from source + add tree select tests + CHANGELOG The committed tailwind.min.css was over-built: 135,578 bytes / 1,428 selectors, with 294 selectors (accent-amber-400, bg-cyan-500, animate-spin, after:bg-gray-400, …) used in zero core source files — bloat from a local build scanning outside the repo's content globs. It would fail CI's rebuild-and-diff and violates the byte-stable rule in scripts/build-tailwind.sh. Regenerate via `scripts/build-tailwind.sh` (pinned tailwindcss@3.4.19): 111,491 bytes / 1,134 selectors, byte-identical to a clean rebuild, still containing the .sm\:flex fix plus every new tree class (ring-fb-primary, accent-fb-primary, pointer-events-none, …). Docker chips now render and CI stays green. Add tests/browser/v3-tree-select.spec.ts: - select mode keeps expanded artist groups open across the tree re-render (fails without loadTree's openArtists capture/restore) - clicking a row in select mode selects instead of playing Record the fix under CHANGELOG [Unreleased] -> Fixed. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Signed-off-by: Sin Co-authored-by: byrongamatos Co-authored-by: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + static/tailwind.min.css | 2 +- static/v3/songs.js | 44 ++++++++-- tests/browser/v3-tree-select.spec.ts | 118 +++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 tests/browser/v3-tree-select.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 76e6432..26234a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **v3 library: exact artist/album filters + scroll/page-depth restore** (feedBack#857). The v3 Songs toolbar gains Artist and Album dropdowns (Album populates from the selected artist and stays disabled until one is chosen), backed by new exact, case-insensitive (`COLLATE NOCASE`) `artist` / `album` query params threaded through `MetadataDB._build_where` → `query_page` / `query_artists` / `query_stats` and the `/api/library`, `/api/library/artists`, `/api/library/stats` endpoints (the free-text `q` search stays fuzzy and composes with the exact filters). The artist/album catalog is fetched independently of the active artist/album selection so the dropdowns always list the full set for the current provider/search. The toolbar is now sticky so filter controls stay reachable when browsing deep libraries, and returning from the player restores the previous scroll position **and** the loaded infinite-scroll page depth via a `sessionStorage` snapshot keyed by a filter/sort/view state hash (invalidated whenever those change, so a filter change still resets to the top). Tests: `tests/test_library_filters.py` (backend artist/album filters), `tests/js/v3_songs_scroll.test.js` (state-hash + snapshot helpers). ### Fixed +- **v3 list/tree view brought to parity with the grid: select mode, parts chips, and song actions — plus a stale-CSS Docker fix.** Re-lands a previously-reverted change. **Frontend (`static/v3/songs.js`):** entering select mode no longer collapses the tree — `loadTree()` now captures the expanded artist groups (`details[open]` keyed by `data-artist`) before the "Loading…" wipe and restores them on rebuild, so toggling select mode (which re-renders via `reload()`) keeps groups open and selection usable; tree rows gain a display-only checkbox + selection ring, the same fav / save-for-later / overflow-menu cluster as the grid card (always shown, all bound by `wireCards()`), and a capture-phase select guard mirroring the grid so clicking a row or arrangement chip in select mode selects instead of playing (`` headers sit outside `[data-fn]`, so native expand/collapse is untouched). **Docker fix (`static/tailwind.min.css`):** the committed Tailwind stylesheet was stale — `.sm\:flex` (and the other utilities behind #582's `hidden sm:flex` arrangement chips and the new action cluster) were never compiled in, so they rendered `display:none` on the Docker build (which serves the committed CSS as-is; Desktop rebuilds from source so it looked fine). Regenerated with the pinned `tailwindcss@3.4.19` via `scripts/build-tailwind.sh` so Docker matches Desktop and #582's chips render on every Docker deploy. Regression tests: `tests/browser/v3-tree-select.spec.ts`. - **Space bar now plays/pauses on the player screen even when a sidebar nav link or rail button has focus.** When any `' + + // Same fav / save-for-later / overflow-menu cluster as the grid + // card. Always shown (like the arrangement chips), not hover- + // revealed. wireCards() binds all three for any [data-fn]. + '
' + + '' + + '' + + '' + + '
' + ''); }).join('') + '').join('') + '').join(''); wireCards(host); } @@ -945,6 +964,21 @@ e.stopImmediatePropagation(); toggleSelect(card.getAttribute('data-fn'), card); }, true); + + // Same bulletproof guard for the list/tree view. Without it, clicking a + // song row (or its arrangement chip) in select mode falls through to the + // per-card play handler and starts playback instead of selecting. The + // group headers sit OUTSIDE any [data-fn], so closest() is null + // for them and their native expand/collapse is left untouched. + const treeEl = byId('v3-songs-tree'); + if (treeEl) treeEl.addEventListener('click', (e) => { + if (!state.selectMode) return; + const card = e.target.closest('[data-fn]'); + if (!card || !treeEl.contains(card)) return; + e.preventDefault(); + e.stopImmediatePropagation(); + toggleSelect(card.getAttribute('data-fn'), card); + }, true); const setView = (v) => { state.view = v; byId('v3-songs-grid-btn').className = 'px-3 py-2 text-sm ' + (v === 'grid' ? 'bg-fb-primary text-white' : 'text-fb-textDim'); diff --git a/tests/browser/v3-tree-select.spec.ts b/tests/browser/v3-tree-select.spec.ts new file mode 100644 index 0000000..f2eac4b --- /dev/null +++ b/tests/browser/v3-tree-select.spec.ts @@ -0,0 +1,118 @@ +import { test, expect } from '@playwright/test'; + +// Regression coverage for the list/tree view select-mode fix (PR #585, which +// re-lands a change that was reverted). The core bug: entering select mode +// re-renders the tree (setSelectMode -> reload -> loadTree), and the rebuild +// wiped every expanded
, collapsing the tree and making selection +// unusable. The fix captures the open artist groups before the wipe and +// restores them. We also cover: clicking a row in select mode selects instead +// of playing. +// +// Navigation uses programmatic element.click() rather than Playwright's +// actionability-gated click: this screen briefly re-renders its toolbar and +// the harness can show transient overlays, but element.click() still +// dispatches a real bubbling event through the capture-phase select handler. + +const ARTISTS = { + artists: [ + { + name: 'Alpha Band', + song_count: 2, + albums: [{ name: 'First Album', songs: [ + { filename: 'alpha/one.sloppak', title: 'Alpha One', artist: 'Alpha Band', album: 'First Album' }, + { filename: 'alpha/two.sloppak', title: 'Alpha Two', artist: 'Alpha Band', album: 'First Album' }, + ] }], + }, + { + name: 'Beta Crew', + song_count: 1, + albums: [{ name: 'Beta LP', songs: [ + { filename: 'beta/solo.sloppak', title: 'Beta Solo', artist: 'Beta Crew', album: 'Beta LP' }, + ] }], + }, + ], + total_artists: 2, +}; + +test.beforeEach(async ({ page }) => { + // Paged artists endpoint (used by both the tree and the artist catalog): + // page 0 returns data, later pages return empty so the paging loop ends. + await page.route('**/api/library/artists**', async route => { + const pageNum = Number(new URL(route.request().url()).searchParams.get('page') || '0'); + await route.fulfill({ json: pageNum === 0 ? ARTISTS : { artists: [], total_artists: 2 } }); + }); + await page.route('**/api/library/providers', route => route.fulfill({ json: { providers: [{ id: 'local', label: 'My Library' }] } })); + await page.route('**/api/library/tuning-names**', route => route.fulfill({ json: { tunings: [] } })); + await page.route('**/api/stats/best', route => route.fulfill({ json: {} })); + await page.route('**/api/library?**', route => route.fulfill({ json: { songs: [], total: 0, page: 0, size: 60 } })); +}); + +// Programmatic click — fires a real bubbling click through capture-phase +// handlers without Playwright's actionability gate. +async function clickSel(page, selector: string) { + await page.evaluate((s) => { + const el = document.querySelector(s) as HTMLElement | null; + if (!el) throw new Error('not found: ' + s); + el.click(); + }, selector); +} + +async function openTree(page) { + await page.goto('/'); + await page.waitForSelector('.screen.active', { timeout: 10000 }); + await page.evaluate(() => { + // @ts-ignore — record playback so an accidental row-click is detectable. + window.__played = 0; + // @ts-ignore + window.playSong = () => { window.__played++; return Promise.resolve(); }; + // @ts-ignore + window.showScreen('v3-songs'); + }); + await page.waitForSelector('#v3-songs-tree-btn', { state: 'attached', timeout: 8000 }); + await clickSel(page, '#v3-songs-tree-btn'); + await page.waitForSelector('#v3-songs-tree details', { state: 'attached', timeout: 8000 }); +} + +// Returns the
whose names the given artist. +function group(page, artist: string) { + return page.locator('#v3-songs-tree details', { has: page.locator('summary', { hasText: artist }) }); +} + +test('select mode keeps expanded artist groups open across the tree re-render (#585)', async ({ page }) => { + await openTree(page); + + // Expand Alpha (the precondition the bug used to destroy on re-render). + await page.evaluate(() => { + const d = [...document.querySelectorAll('#v3-songs-tree details')] + .find((el) => el.querySelector('summary')?.textContent?.includes('Alpha Band')) as HTMLDetailsElement; + d.open = true; + }); + await expect(group(page, 'Alpha Band')).toHaveAttribute('open', ''); + + // Enter select mode → triggers the full tree re-render. + await clickSel(page, '#v3-songs-select'); + await page.waitForSelector('#v3-songs-tree input[data-select]', { state: 'attached', timeout: 8000 }); + + // The bug: Alpha collapses after the rebuild. The fix restores it. + await expect(group(page, 'Alpha Band')).toHaveAttribute('open', ''); + // Beta was never opened — it must stay collapsed (no false restore). + await expect(group(page, 'Beta Crew')).not.toHaveAttribute('open', ''); +}); + +test('clicking a tree row in select mode selects it instead of playing (#585)', async ({ page }) => { + await openTree(page); + + await page.evaluate(() => { + const d = [...document.querySelectorAll('#v3-songs-tree details')] + .find((el) => el.querySelector('summary')?.textContent?.includes('Alpha Band')) as HTMLDetailsElement; + d.open = true; + }); + + await clickSel(page, '#v3-songs-select'); + await page.waitForSelector('#v3-songs-tree input[data-select]', { state: 'attached', timeout: 8000 }); + + await clickSel(page, '#v3-songs-tree [data-fn="alpha/one.sloppak"]'); + + await expect(page.locator('#v3-songs-tree [data-fn="alpha/one.sloppak"] input[data-select]')).toBeChecked(); + expect(await page.evaluate(() => (window as any).__played)).toBe(0); +});