From 1069182f410ebf4d83c167631fdb43da2224a460 Mon Sep 17 00:00:00 2001 From: Matthew Harris Glover Date: Sat, 18 Jul 2026 09:38:03 -0400 Subject: [PATCH] fix(gamepad): address CodeRabbit findings on connect/disconnect and grid nav MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - gamepad.js: anyLiveConnectedPad -> anyLiveStandardPad, filtering by mapping === 'standard' like firstLiveStandardPad already does, and applied at the top of the gamepadconnected handler too. A still- connected non-standard raw mirror could otherwise mask the real pad's disconnect (toast never fires, polling never stops). - songs.js _gpMove: an unset cursor now always seeds at index 0 before the first press, instead of applying that press's delta immediately (ArrowDown/Right previously skipped straight past row 0; Left/Up only looked right by accident of clamping). Matches the existing convention in shortcuts.js's legacy _handleLibArrowNav. - songs.js _gpBlockedTarget: form-control/button blocking now requires the element to be visible (offsetParent !== null), not just present. Screens stay in the DOM hidden (not removed) when you navigate away, so a real button focused on some other now-hidden screen could leave document.activeElement pointing at it and block all grid navigation indefinitely. (An el.closest('#v3-songs') scope was tried first and reverted — it fixed that case but broke blocking for the topbar search input, which lives outside #v3-songs's DOM subtree even while v3-songs is active; visibility is the distinction that actually matters, not DOM nesting.) Skipped two CodeRabbit suggestions, verified against current code: gating songs.js's grid keydown listener to synthetic-only events would regress the real keyboard accessibility this PR intentionally added (v3-songs' grid had none before); renaming the _gp* helpers to drop their underscore prefix would break from this codebase's own established module-private naming convention. Verified in-browser: first arrow press lands on index 0, stale hidden focus no longer blocks grid nav, the topbar search input still correctly blocks it, and normal nav resumes after blur. --- static/v3/gamepad.js | 18 +++++++++++++++--- static/v3/songs.js | 34 +++++++++++++++++++++++++++------- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/static/v3/gamepad.js b/static/v3/gamepad.js index 7295349..69fcb9e 100644 --- a/static/v3/gamepad.js +++ b/static/v3/gamepad.js @@ -130,10 +130,16 @@ return null; } - function anyLiveConnectedPad() { + // Same standard-mapping filter as firstLiveStandardPad — otherwise a + // still-connected non-standard raw mirror (or the real pad simply + // reporting a different mapping) can mask the actual pad's disconnect: + // the toast never fires and polling never stops, even though the pad + // this module can act on is gone. + function anyLiveStandardPad() { var pads = navigator.getGamepads ? navigator.getGamepads() : []; for (var i = 0; i < pads.length; i++) { - if (pads[i] && pads[i].connected) return true; + var p = pads[i]; + if (p && p.connected && p.mapping === 'standard') return true; } return false; } @@ -156,6 +162,12 @@ window.addEventListener('gamepadconnected', function (e) { var idx = e.gamepad && e.gamepad.index; console.log('[gamepad] connected', idx, e.gamepad && e.gamepad.id, 'mapping:', e.gamepad && e.gamepad.mapping); + // Non-standard slots (raw HID mirrors, or anything this module can't + // safely act on) are never tracked/toasted/polled for — only ever + // treat a standard-mapped pad as "a controller connected". Keeping a + // non-standard slot out of connectedIndices also keeps it out of + // anyLiveStandardPad's count, so it can't mask a real disconnect. + if (!e.gamepad || e.gamepad.mapping !== 'standard') return; if (connectedIndices[idx]) return; // already-announced slot re-firing (focus regain, etc.) // On the Deck, Steam Input mirrors a real pad with 1-2 virtual XInput // slots of its own (same physical button presses, extra indices) — only @@ -178,7 +190,7 @@ var idx = e.gamepad && e.gamepad.index; console.log('[gamepad] disconnected', idx, e.gamepad && e.gamepad.id); delete connectedIndices[idx]; - if (!anyLiveConnectedPad()) { + if (!anyLiveStandardPad()) { polling = false; notify('Controller disconnected', '🔌'); } diff --git a/static/v3/songs.js b/static/v3/songs.js index ca426e6..9cf6f2e 100644 --- a/static/v3/songs.js +++ b/static/v3/songs.js @@ -4141,12 +4141,19 @@ async function _gpMove(delta) { if (!state.total) return; - const base = _gpIdx == null ? 0 : _gpIdx; - const next = Math.max(0, Math.min(state.total - 1, base + delta)); - if (_gpIdx != null && next === _gpIdx) return; - _gpIdx = next; - await ensureWindow(Math.max(0, next - 1), Math.min(state.total, next + 2)); - await _gpEnsureVisible(next); + if (_gpIdx == null) { + // Nothing selected yet — land on the first card and stop, same as + // shortcuts.js's legacy _handleLibArrowNav does for an empty + // selection: the first press establishes a cursor, it doesn't also + // move it (Right/Down previously skipped straight past row 0). + _gpIdx = 0; + } else { + const next = Math.max(0, Math.min(state.total - 1, _gpIdx + delta)); + if (next === _gpIdx) return; + _gpIdx = next; + } + await ensureWindow(Math.max(0, _gpIdx - 1), Math.min(state.total, _gpIdx + 2)); + await _gpEnsureVisible(_gpIdx); _gpApplyHighlight(); } @@ -4159,9 +4166,22 @@ // controls, buttons, and dialog/drawer overlays — same intent as // shortcuts.js's _isInsideInteractiveControl, reimplemented locally since // this is a plain script (not an ES module) and can't import it. + // + // The form-control/button check requires the element to be VISIBLE, not + // merely present: screens stay in the DOM (hidden, not removed) when you + // navigate away, so a real