mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-22 21:01:40 +00:00
fix(v3): decode stats:recorded filename so post-play score badge refreshes (#620)
PR #574 added a `stats:recorded` -> in-place accuracy-badge repaint so a just-earned score shows without restarting the app. But the repaint never matched a card, so the badge stayed stale until a full render() (app restart / search / re-enter the screen) -- exactly the "only updates after a restart" report. Root cause is a filename key-space mismatch. The event (like song:loading) carries the filename `encodeURIComponent`'d, because that is what playCard hands to playSong (the highway WS decodeURIComponent's it). Library cards, though, key on the DECODED localFilename (data-fn), and /api/stats/best is server-canonicalized to that same decoded key (server.py _canonical_song_filename). So repaintAccuracy's `data-fn !== key` check rejected every card and `state.accuracy[encoded]` was undefined. Decode the event filename back into the card / state.accuracy key space via a small `decFn` helper before marking dirty and repainting, fixing both the immediate repaint and the onV3SongsScreenEnter deferred path. decFn is idempotent for already-decoded names and falls back to the original on malformed input, so a real filename containing a literal '%' is never corrupted. Tests: tests/js/v3_songs_score_badge_refresh.test.js extracts the real decFn from the shipped source and proves the encoded event filename round-trips to the raw card key (incl. spaces and subfolder '/'). Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com>
This commit is contained in:
parent
fef870047b
commit
ee7bafbb47
@ -38,6 +38,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 song/lesson accuracy badges now refresh on the first return from a song — no restart needed.** PR #574 added a `stats:recorded` → in-place badge repaint, but the repaint never matched a card. The event (like `song:loading`) carries the filename **`encodeURIComponent`'d** — exactly as `playCard` hands it to `playSong` (the highway WS `decodeURIComponent`s it back) — whereas library cards key on the **decoded** `localFilename` (`data-fn`), and `/api/stats/best` is server-canonicalized to that same decoded key (`server.py` `_canonical_song_filename`). So `repaintAccuracy`'s `data-fn !== key` check rejected every card and `state.accuracy[encoded]` was `undefined`, leaving the just-earned badge stale until a full `render()` (app restart / search / re-enter the screen) — which is why it "came back after a restart." `static/v3/songs.js` now decodes the `stats:recorded` filename back into the card / `state.accuracy` key space via a small `decFn` helper before marking dirty and repainting (idempotent for already-decoded names; falls back to the original on malformed input so a real filename containing a literal `%` is never corrupted), so both the immediate repaint and the `onV3SongsScreenEnter` deferred path land on the right card. Tests: `tests/js/v3_songs_score_badge_refresh.test.js`.
|
||||
- **Escape now exits a song (and leaves Settings) even when a transport/rail control button holds keyboard focus.** Clicking a player control (Play / FF / RW / Restart) left that `<button>` focused, and `_shortcutDispatchBlocked()` in `static/app.js` treats any focused `INPUT/SELECT/TEXTAREA/BUTTON` as an "interactive control" and bails before the shortcut registry runs — so the player-scope `Escape → Back` shortcut never fired until the user clicked empty canvas to blur the control ("Escape in song not consistent"). Space already had a player-screen carve-out (#593) that let it fire through a focused control; Escape did not. Generalized that carve-out to Escape, scoped to the player **and** settings screens (both register an `Escape = Back` shortcut, and settings had the identical latent bug). The earlier guards are preserved and still win: text inputs are exempted first (Escape there clears/blurs the field), the Section Practice popover already claims Escape before the carve-out, and a true modal layered over the screen (`[role="dialog"][aria-modal="true"]` / `.feedBack-modal`) still traps Escape so it closes the modal rather than ejecting past it. Escape becomes a reliable, focus-independent "Back" — making it monotonic groundwork for an optional exit-confirm. Plugins that register a player-scope `Escape` shortcut benefit identically (they were broken the same way). Tests: `tests/browser/keyboard-shortcuts.spec.ts` (focused-button repro, text-input no-exit, no-escape-past-modal, Section Practice popover, settings twin-bug).
|
||||
- **The v3 "Up Next" pill can now be turned off — new "Show 'Up Next'" gameplay toggle (default ON).** The v0.3.0 player chrome's persistent upcoming-section pill (`#v3-upnext`, drawn by `static/v3/player-chrome.js`'s `updateUpNext()`) shipped with no off switch, so it always showed during playback whenever a section was upcoming — overlapping the top-right FPS HUD and ignoring the 3D-highway "Show 'Up Next' section card" checkbox (a *different*, in-canvas widget that was demoted to default-off precisely because this pill is the canonical readout). Users reading the pill as the same setting saw "disabled in settings but still there." Adds a real core toggle following the `autoplayExit` idiom: a client-only `showUpNext` `localStorage` pref (absence = enabled), a **Show "Up Next"** switch in the Gameplay settings tab (`static/v3/index.html`), reader/writer + `loadSettings()` hydration + a read-only `window.feedBack.showUpNext` getter in `static/app.js`, and a gate at the top of `updateUpNext()` that hides the pill when off. Disabling mid-playback hides it immediately; re-enabling re-shows it on the next chrome tick (~6 Hz). Added to `RESET_MAP.gameplay.local` in `static/v3/settings.js` so the Gameplay "Reset" restores the default-on state. Default ON = zero change for existing users. No Tailwind rebuild (plain markup + existing classes).
|
||||
- **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 (`<summary>` 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`.
|
||||
|
||||
@ -15,6 +15,20 @@
|
||||
const esc = (s) => String(s == null ? '' : s).replace(/[&<>"']/g, (c) => (
|
||||
{ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }[c]));
|
||||
const enc = encodeURIComponent;
|
||||
// Inverse of `enc` for matching a played song's filename back to a library
|
||||
// card. The `stats:recorded` event (like `song:loading`) carries the
|
||||
// filename exactly as it was handed to `playSong` — i.e. encodeURIComponent'd
|
||||
// (see `playCard`, and the highway WS which decodeURIComponent's it). But
|
||||
// cards key on the DECODED library filename (`cardKey` → `localFilename`),
|
||||
// and `/api/stats/best` is server-canonicalized to that same decoded key, so
|
||||
// an encoded filename matches no card and the post-play badge repaint silently
|
||||
// no-ops. Decode to land in the card/`state.accuracy` key space. Idempotent
|
||||
// for already-decoded names (no '%'); on malformed input falls back to the
|
||||
// original so a real filename containing a literal '%' is never corrupted.
|
||||
function decFn(fn) {
|
||||
if (typeof fn !== 'string' || fn.indexOf('%') === -1) return fn || '';
|
||||
try { return decodeURIComponent(fn); } catch (_) { return fn; }
|
||||
}
|
||||
|
||||
const SORTS = [
|
||||
['artist', 'Artist A–Z'], ['artist-desc', 'Artist Z–A'],
|
||||
@ -1173,7 +1187,13 @@
|
||||
// If the library is visible right now, repaint immediately; otherwise
|
||||
// mark it dirty and onV3SongsScreenEnter applies it on return.
|
||||
sm.on('stats:recorded', (e) => {
|
||||
const fn = e && e.detail && e.detail.filename;
|
||||
// Decode to the library-card key space — the event carries the
|
||||
// encodeURIComponent'd filename, but cards (data-fn) and
|
||||
// state.accuracy key on the decoded library filename. Without this
|
||||
// the repaint below (and applyScoreRefresh's repaintAccuracy) match
|
||||
// no card, so a just-earned score stays invisible until a full
|
||||
// render() (restart / search / re-enter), which is this bug.
|
||||
const fn = decFn(e && e.detail && e.detail.filename);
|
||||
if (!fn) return;
|
||||
_dirtyScores.add(fn);
|
||||
// Only repaint now if the library is the active screen; otherwise
|
||||
|
||||
99
tests/js/v3_songs_score_badge_refresh.test.js
Normal file
99
tests/js/v3_songs_score_badge_refresh.test.js
Normal file
@ -0,0 +1,99 @@
|
||||
// Regression guard for the post-play score-badge refresh bug
|
||||
// (#574 follow-up): after finishing a song, its accuracy badge on the
|
||||
// Songs screen stayed stale until a full re-render (app restart / search /
|
||||
// re-enter), even though stats-recorder fired `stats:recorded`.
|
||||
//
|
||||
// Root cause: `stats:recorded` (like `song:loading`) carries the filename
|
||||
// exactly as handed to playSong — encodeURIComponent'd (see playCard) — but
|
||||
// library cards key on the DECODED filename (data-fn = cardKey → localFilename)
|
||||
// and /api/stats/best is server-canonicalized to that same decoded key. So the
|
||||
// in-place repaint (repaintAccuracy) matched no card and silently no-oped.
|
||||
//
|
||||
// The fix is a `decFn` helper in static/v3/songs.js that decodes the event
|
||||
// filename back into the card / state.accuracy key space before matching. This
|
||||
// test extracts the REAL decFn from the shipped source (not a mirror) and proves
|
||||
// the encoded event filename round-trips to the raw card key.
|
||||
|
||||
'use strict';
|
||||
const { test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const vm = require('node:vm');
|
||||
|
||||
const SONGS_JS = path.join(__dirname, '..', '..', 'static', 'v3', 'songs.js');
|
||||
|
||||
// Brace-balanced extraction so nested braces / template strings survive.
|
||||
function extractFunctionSource(src, name) {
|
||||
const sig = `function ${name}`;
|
||||
const start = src.indexOf(sig);
|
||||
assert.ok(start !== -1, `function declaration '${name}' not found in songs.js`);
|
||||
const openBrace = src.indexOf('{', start);
|
||||
assert.ok(openBrace !== -1, `opening brace after '${name}' not found`);
|
||||
let depth = 1;
|
||||
let i = openBrace + 1;
|
||||
while (i < src.length && depth > 0) {
|
||||
const ch = src[i];
|
||||
if (ch === '{') depth++;
|
||||
else if (ch === '}') depth--;
|
||||
i++;
|
||||
}
|
||||
assert.ok(depth === 0, `unbalanced braces in function '${name}'`);
|
||||
return src.slice(start, i);
|
||||
}
|
||||
|
||||
function loadDecFn() {
|
||||
const src = fs.readFileSync(SONGS_JS, 'utf8');
|
||||
const fnSrc = extractFunctionSource(src, 'decFn');
|
||||
const sandbox = {};
|
||||
vm.createContext(sandbox);
|
||||
// decodeURIComponent is an intrinsic global in the fresh context.
|
||||
vm.runInContext(`${fnSrc}\nglobalThis.__decFn = decFn;`, sandbox);
|
||||
return sandbox.__decFn;
|
||||
}
|
||||
|
||||
const enc = encodeURIComponent; // exactly what playCard passes to playSong
|
||||
|
||||
// The on-disk library filenames from the bug report's screenshots, plus a
|
||||
// subfolder path (encodeURIComponent turns '/' into %2F too).
|
||||
const CARD_KEYS = [
|
||||
'Black Me Out.sloppak',
|
||||
'All In Now.sloppak',
|
||||
'Dogstar - All In Now.feedpak',
|
||||
'Subdir/Song (Live).sloppak',
|
||||
];
|
||||
|
||||
test('decFn decodes an encoded event filename back to the raw card key', () => {
|
||||
const decFn = loadDecFn();
|
||||
for (const key of CARD_KEYS) {
|
||||
const eventFilename = enc(key); // how stats:recorded carries it
|
||||
// Precondition: the encoded form does NOT equal the card key — this is
|
||||
// exactly why the un-decoded match failed and the badge stayed stale.
|
||||
assert.notEqual(eventFilename, key, `expected '${key}' to encode to something different`);
|
||||
// The fix: decoding lands back on the card / state.accuracy key.
|
||||
assert.equal(decFn(eventFilename), key, `decFn must recover the card key for '${key}'`);
|
||||
}
|
||||
});
|
||||
|
||||
test('decFn is idempotent for already-decoded filenames (no % present)', () => {
|
||||
const decFn = loadDecFn();
|
||||
for (const key of CARD_KEYS) {
|
||||
assert.equal(decFn(key), key, `decFn must leave the already-decoded '${key}' unchanged`);
|
||||
}
|
||||
});
|
||||
|
||||
test('decFn leaves a real literal-% filename intact rather than throwing', () => {
|
||||
const decFn = loadDecFn();
|
||||
// '%.sloppak' / '100%.sloppak' are malformed percent-escapes —
|
||||
// decodeURIComponent would throw; decFn must fall back to the original.
|
||||
for (const name of ['100%.sloppak', 'mix %.feedpak', '%zz.sloppak']) {
|
||||
assert.equal(decFn(name), name, `decFn must not corrupt/throw on '${name}'`);
|
||||
}
|
||||
});
|
||||
|
||||
test('decFn coerces non-string / empty input to an empty string', () => {
|
||||
const decFn = loadDecFn();
|
||||
assert.equal(decFn(null), '');
|
||||
assert.equal(decFn(undefined), '');
|
||||
assert.equal(decFn(''), '');
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user