mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-21 20:31:21 +00:00
fix(gamepad): address CodeRabbit findings on connect/disconnect and grid nav
- 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.
This commit is contained in:
parent
346c0e1d3c
commit
1069182f41
@ -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', '🔌');
|
||||
}
|
||||
|
||||
@ -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 <button> focused on some OTHER now-hidden
|
||||
// screen (dashboard, etc.) can leave document.activeElement pointing at
|
||||
// it — an el.closest('#v3-songs') scope check would let that stale focus
|
||||
// through fine, but would ALSO wrongly stop blocking genuinely-focused
|
||||
// shared chrome like the topbar search input (#v3-search lives outside
|
||||
// #v3-songs's DOM subtree even while v3-songs is the active screen).
|
||||
// Visibility is the actual distinction that matters here, not DOM
|
||||
// nesting. The dialog/drawer-overlay and contentEditable checks stay as
|
||||
// they were — overlay-level concerns regardless of which screen sits
|
||||
// underneath.
|
||||
function _gpBlockedTarget(el) {
|
||||
if (!el) return false;
|
||||
if (['INPUT', 'SELECT', 'TEXTAREA', 'BUTTON'].includes(el.tagName)) return true;
|
||||
if (['INPUT', 'SELECT', 'TEXTAREA', 'BUTTON'].includes(el.tagName) && el.offsetParent !== null) return true;
|
||||
if (el.isContentEditable) return true;
|
||||
if (el.closest && el.closest('[role="dialog"], .feedBack-modal, #lib-filter-drawer')) return true;
|
||||
return false;
|
||||
|
||||
Loading…
Reference in New Issue
Block a user