got-feedBack/feedBack
1
0
Fork 0
mirror of https://github.com/got-feedBack/feedBack.git synced 2026-08-11 03:09:57 +00:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

fix(nav): the library sometimes showed the legacy screen — map 'home' inside showScreen (#923)

Browse Source 
Testers: "randomly, when moving to the library from another menu option, the library shows the
old interface — never when a song ends."

━━━ WHAT WAS ACTUALLY HAPPENING ━━━

#home is the PRE-V3 library screen. The v3 shell replaced it with #v3-songs, and the mapping DID
exist — but only inside WRAPPERS on window.showScreen, and only for callers that go through
`window`. THREE independent parties monkey-patch it, each capturing whatever happens to be there
at the time:

    app.js publishes the raw function
      -> shell.js wraps it, adding the home -> v3-songs mapping
      -> the stems plugin wraps it AGAIN (src/main.js:1029), capturing the current value

Plugins load ASYNCHRONOUSLY. The chain links up in whatever order the race settles, and any
capture taken before shell.js installs — or any re-assignment after it — silently drops the
mapping. Hence "randomly".

AND THE INTERNAL CALLERS NEVER TOUCHED window.showScreen AT ALL. closeCurrentSong and the
Esc-from-settings shortcut call the IMPORTED showScreen, which no wrapper ever sees. Reproduced
in a browser: the unwrapped function with 'home' lands on the dead legacy screen EVERY time.

"Never when a song ends" is the tell, and it is what identified the mechanism: closeCurrentSong
resolves its target through _resolvePlayerOrigin(), which ALREADY applies this mapping. That one
path was fine — which is exactly why the bug looked random rather than total.

PRE-EXISTING, not a regression from the module carve: the onclick="showScreen('home')" links and
the wrapper-only mapping both date to 2026-06-22.

━━━ THE FIX ━━━

The guard lives inside showScreen now: ONE place, in the function every caller routes through,
instead of a chain of monkey-patches that must each remember. Wrapper order stops mattering, and
the module-internal callers are covered for the first time.

Verified in a browser: the raw, unwrapped showScreen('home') — which reproduced as #home — now
lands on #v3-songs, and cannot be undone by any wrapper order.

━━━ AND A [P1] I INTRODUCED, WHICH CODEX CAUGHT ━━━

My first cut mapped BOTH 'home' and 'v3-home', copied straight from _resolvePlayerOrigin.

That is correct THERE and wrong HERE. _resolvePlayerOrigin computes where to RETURN TO after a
song, and landing on the Songs list from the dashboard is the right behaviour. But #v3-home is
the v3 DASHBOARD — a real screen that the shell's Home nav, the onboarding tour and the dashboard
re-render listener all target. Redirecting it would have made Home unreachable.

A LEGACY ALIAS IS NOT THE SAME THING AS A RETURN TARGET. Only 'home' is mapped now, and a test
pins that: re-adding 'v3-home' to the guard fails it.

4 tests, bite-tested both ways.

node 1049, pytest 2425, ESLint 0, Codex 0.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Byron Gamatos
2026-07-12 16:05:50 +02:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 57e7db5c2a
commit f27d4f623c
2 changed files with 124 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
static/js/session.js
+39
View File
@@ -105,6 +105,45 @@ export let _settingsOriginScreen = 'home';
// ── Screen Navigation ─────────────────────────────────────────────────────
export async function showScreen(id) {
// ── 'home' is the LEGACY library screen. Always route it to the v3 Songs list. ──
//
// The v3 shell replaced #home with #v3-songs. That mapping DID exist — but only inside
// wrappers on `window.showScreen`, and only for callers that go through `window`:
//
// app.js publishes the raw fn -> shell.js wraps it (adding the mapping)
// -> the stems plugin wraps it AGAIN, capturing whatever
// happened to be there at the time
//
// Two ways that fails, and testers hit both:
//
// 1. ORDER. Three independent parties monkey-patch window.showScreen, each capturing the
// current value. Plugins load ASYNCHRONOUSLY, so the chain links up in whatever order
// the race settles — and any capture taken before shell.js installs, or any
// re-assignment after it, silently drops the mapping.
//
// 2. THE INTERNAL CALLERS NEVER TOUCHED window.showScreen AT ALL. closeCurrentSong and the
// Esc-from-settings shortcut call the IMPORTED showScreen directly, so no wrapper ever
// sees them. Verified in a browser: the unwrapped function with 'home' lands on the dead
// legacy screen every single time.
//
// Hence "randomly, when moving to the library from another menu option" — and "never when a
// song ends", because closeCurrentSong resolves its target through _resolvePlayerOrigin(),
// which already applies this mapping.
//
// So it lives HERE now: ONE guard in the function every caller routes through, rather than a
// chain of monkey-patches that must each remember.
//
// ONLY 'home'. NOT 'v3-home'. _resolvePlayerOrigin() maps BOTH — correctly, because it
// computes where to RETURN TO after a song, and coming back to the Songs list from the
// dashboard is the right behaviour. Copying that condition here was a [P1] (Codex caught it):
// #v3-home is the v3 DASHBOARD, a real screen the shell's Home nav, the onboarding tour and
// the dashboard re-render listener all target. Redirecting it would make Home unreachable.
//
// A legacy alias is not the same thing as a return target.
if (id === 'home' && document.getElementById('v3-songs')) {
id = 'v3-songs';
}
// Capture the previous screen before changing active classes
const prevScreenId = document.querySelector('.screen.active')?.id;
document.querySelectorAll('.screen').forEach(s => s.classList.remove('active'));