Add gamepad/controller support - #1001
Merged
Merged
Conversation
Adds full gamepad/controller navigation and playback control, driven by requests from players who use fee[dB]ack on a TV/console setup and from wheelchair users for whom a controller is far more convenient than a keyboard + mouse. Confirmed working end-to-end on a Steam Deck across several rounds of on-device testing. - static/v3/gamepad.js: polls navigator.getGamepads() and dispatches synthetic keydown events (Arrow/Enter/Space/Escape) on the focused element (falling back to document), reusing the app's existing keyboard pipeline (static/js/shortcuts.js's scope-aware dispatcher, player shortcuts, text-field/modal guards) instead of a parallel action-mapping table. Only acts on gamepads reporting the W3C "standard" mapping — which is what Steam Input presents for the Deck's built-in controls, both in Gaming Mode and in Desktop Mode via a non-Steam shortcut — so button order is guaranteed correct and a non-standard/raw device safely no-ops instead of misfiring. Handles Steam Input's virtual-pad duplicates (a real controller plus 1-2 mirrored XInput slots) without spamming connect toasts or losing input when the live pad isn't at index 0. Xbox-style face button mapping: bottom face = Space (play/pause, and activates the focused control), right face = Escape (back), top face reveals the player screen's tool rail (focuses it into visibility via the existing CSS :focus-within rule). D-pad/stick repeat while held, mirroring OS keyboard auto-repeat. - static/v3/gamepad-nav.js: fills the one real gap in that reuse strategy — no screen but the song library grid had any arrow-key navigation, and Chromium doesn't run native Enter/Space button activation for untrusted synthetic events even when dispatched at the focused element. Gated entirely on `!e.isTrusted`, so it only ever reacts to gamepad-originated events and never touches real keyboard/mouse users: emulates Tab-order (the sidebar + active screen's real, already-focusable buttons/links) for Arrow keys, explicitly .click()s the focused element for Enter/Space, and gives Escape a consistent "go back" behavior — an existing in-screen back button if one's visible (reusing each screen's own drill-down logic for free), else the main menu. Every branch defers via `e.defaultPrevented` to any screen that already handles the key itself (the song grid, the player, settings), so nothing here overrides existing behavior. - static/v3/songs.js: adds real 2D d-pad/arrow-key navigation to the song library's virtualized grid (only a slice of the library is ever in the DOM), including fetching/scrolling off-screen rows into view and correcting for the sticky filter toolbar's occlusion. - static/v3/index.html: wires up the two new scripts.
Contributor
|
Warning Review limit reached
Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughChangesV3 now supports standard gamepad input through synthetic keyboard events, global focus traversal, Escape navigation, and cursor-based Songs grid navigation with lazy window loading and playback activation. V3 gamepad navigation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Gamepad
participant gamepadjs
participant SongsGrid
participant FocusNavigation
Gamepad->>gamepadjs: provide standard button or direction state
gamepadjs->>SongsGrid: dispatch synthetic Arrow or activation keydown
SongsGrid->>SongsGrid: move cursor, load window, or activate playback
gamepadjs->>FocusNavigation: dispatch synthetic navigation keydown
FocusNavigation->>FocusNavigation: traverse focus, activate control, or navigate back
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
static/v3/songs.js (1)
4092-4096: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the new helpers to camelCase.
Use names such as
gpCardEl,gpApplyHighlight,gpEnsureVisible,gpMove,gpActivate, andgpBlockedTarget.As per coding guidelines,
static/**/*.js: “Use camelCase for JavaScript functions.”Also applies to: 4109-4109, 4142-4162
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@static/v3/songs.js` around lines 4092 - 4096, Rename the newly added helper functions in the songs grid navigation flow from underscore-prefixed names to camelCase, including _gpCardEl, _gpApplyHighlight, _gpEnsureVisible, _gpMove, _gpActivate, and _gpBlockedTarget. Update every reference and invocation consistently while preserving their existing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@static/v3/gamepad-nav.js`:
- Around line 67-83: Update the Escape handling in the key listener to inspect
all matching back buttons rather than only the first querySelector result.
Select and click the first visible element matching [data-ap-back],
[data-albums-back], or `#v3-pl-back`; only call window.showScreen('v3-home') when
none of the matching buttons are visible.
In `@static/v3/gamepad.js`:
- Around line 133-139: Update anyLiveConnectedPad and the gamepad connection
handlers around the related lifecycle logic to consider only pads whose mapping
is "standard". Apply the same filter when announcing connect/disconnect events
and when deciding whether to keep the animation loop active, so non-standard raw
mirrors cannot suppress the supported virtual pad’s disconnect notification.
In `@static/v3/songs.js`:
- Around line 4170-4175: Update the keydown listener around songsActive and
state.view so grid cursor handling runs only for synthetic gamepad events,
excluding trusted physical keyboard events before preventDefault is called.
Preserve the existing activation and arrow-key filtering for eligible synthetic
events.
- Around line 4162-4172: Update _gpBlockedTarget to block INPUT, SELECT,
TEXTAREA, and BUTTON elements only when they are inside `#v3-songs`, while
preserving the existing contenteditable and visible-overlay blocking behavior.
Ensure a focused external launching button does not prevent subsequent Songs
gamepad navigation in the keydown handler.
- Around line 4142-4147: Update _gpMove so an unset cursor selects the first
card (index 0) before applying the movement delta, rather than using the delta
as the initial index. Preserve the existing clamping to the valid range and
no-op behavior when movement cannot change the current index.
---
Nitpick comments:
In `@static/v3/songs.js`:
- Around line 4092-4096: Rename the newly added helper functions in the songs
grid navigation flow from underscore-prefixed names to camelCase, including
_gpCardEl, _gpApplyHighlight, _gpEnsureVisible, _gpMove, _gpActivate, and
_gpBlockedTarget. Update every reference and invocation consistently while
preserving their existing behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 310fd650-360c-4024-b3a3-f3f35079ef9f
📒 Files selected for processing (4)
static/v3/gamepad-nav.jsstatic/v3/gamepad.jsstatic/v3/index.htmlstatic/v3/songs.js
Comment on lines
+4162
to
+4172
| function _gpBlockedTarget(el) { | ||
| if (!el) return false; | ||
| if (['INPUT', 'SELECT', 'TEXTAREA', 'BUTTON'].includes(el.tagName)) return true; | ||
| if (el.isContentEditable) return true; | ||
| if (el.closest && el.closest('[role="dialog"], .feedBack-modal, #lib-filter-drawer')) return true; | ||
| return false; | ||
| } | ||
|
|
||
| document.addEventListener('keydown', (e) => { | ||
| if (!songsActive() || state.view !== 'grid') return; | ||
| if (_gpBlockedTarget(document.activeElement)) return; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not let the launching button disable Songs navigation.
gamepad-nav.js activates controls using .click() but leaves focus on them. If a dashboard or navigation button opens Songs, Line 4172 blocks every subsequent gamepad key because that external button remains document.activeElement.
Restrict form/button blocking to controls inside #v3-songs, while continuing to block visible overlays.
Proposed fix
function _gpBlockedTarget(el) {
if (!el) return false;
- if (['INPUT', 'SELECT', 'TEXTAREA', 'BUTTON'].includes(el.tagName)) return true;
- if (el.isContentEditable) return true;
if (el.closest && el.closest('[role="dialog"], .feedBack-modal, `#lib-filter-drawer`')) return true;
+ if (!el.closest || !el.closest('`#v3-songs`')) return false;
+ if (['INPUT', 'SELECT', 'TEXTAREA', 'BUTTON'].includes(el.tagName)) return true;
+ if (el.isContentEditable) return true;
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function _gpBlockedTarget(el) { | |
| if (!el) return false; | |
| if (['INPUT', 'SELECT', 'TEXTAREA', 'BUTTON'].includes(el.tagName)) return true; | |
| if (el.isContentEditable) return true; | |
| if (el.closest && el.closest('[role="dialog"], .feedBack-modal, #lib-filter-drawer')) return true; | |
| return false; | |
| } | |
| document.addEventListener('keydown', (e) => { | |
| if (!songsActive() || state.view !== 'grid') return; | |
| if (_gpBlockedTarget(document.activeElement)) return; | |
| function _gpBlockedTarget(el) { | |
| if (!el) return false; | |
| if (el.closest && el.closest('[role="dialog"], .feedBack-modal, `#lib-filter-drawer`')) return true; | |
| if (!el.closest || !el.closest('`#v3-songs`')) return false; | |
| if (['INPUT', 'SELECT', 'TEXTAREA', 'BUTTON'].includes(el.tagName)) return true; | |
| if (el.isContentEditable) return true; | |
| return false; | |
| } | |
| document.addEventListener('keydown', (e) => { | |
| if (!songsActive() || state.view !== 'grid') return; | |
| if (_gpBlockedTarget(document.activeElement)) return; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@static/v3/songs.js` around lines 4162 - 4172, Update _gpBlockedTarget to
block INPUT, SELECT, TEXTAREA, and BUTTON elements only when they are inside
`#v3-songs`, while preserving the existing contenteditable and visible-overlay
blocking behavior. Ensure a focused external launching button does not prevent
subsequent Songs gamepad navigation in the keydown handler.
Comment on lines
+4170
to
+4175
| document.addEventListener('keydown', (e) => { | ||
| if (!songsActive() || state.view !== 'grid') return; | ||
| if (_gpBlockedTarget(document.activeElement)) return; | ||
| const isActivate = e.key === 'Enter' || e.key === ' ' || e.key === 'Spacebar'; | ||
| if (!isActivate && !['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown'].includes(e.key)) return; | ||
| e.preventDefault(); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restrict grid cursor handling to synthetic gamepad events.
This listener also intercepts trusted keyboard arrows, Enter, and Space and calls preventDefault(), contradicting the synthetic-only contract and changing physical keyboard behavior.
Proposed fix
document.addEventListener('keydown', (e) => {
+ if (e.isTrusted) return;
if (!songsActive() || state.view !== 'grid') return;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| document.addEventListener('keydown', (e) => { | |
| if (!songsActive() || state.view !== 'grid') return; | |
| if (_gpBlockedTarget(document.activeElement)) return; | |
| const isActivate = e.key === 'Enter' || e.key === ' ' || e.key === 'Spacebar'; | |
| if (!isActivate && !['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown'].includes(e.key)) return; | |
| e.preventDefault(); | |
| document.addEventListener('keydown', (e) => { | |
| if (e.isTrusted) return; | |
| if (!songsActive() || state.view !== 'grid') return; | |
| if (_gpBlockedTarget(document.activeElement)) return; | |
| const isActivate = e.key === 'Enter' || e.key === ' ' || e.key === 'Spacebar'; | |
| if (!isActivate && !['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown'].includes(e.key)) return; | |
| e.preventDefault(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@static/v3/songs.js` around lines 4170 - 4175, Update the keydown listener
around songsActive and state.view so grid cursor handling runs only for
synthetic gamepad events, excluding trusted physical keyboard events before
preventDefault is called. Preserve the existing activation and arrow-key
filtering for eligible synthetic events.
mhglover
force-pushed
the
feat/gamepad-support
branch
from
July 18, 2026 12:09
fe65d34 to
05afe7f
Compare
Rebuilt in a fresh clone (not the local working copy). Several plugin directories (audio_engine, plugin_manager, community_charts, etc.) are gitignored locally but present on disk from checking out plugin repos for local dev/testing — Tailwind's content scan picks them up regardless, so a rebuild against the contaminated local working copy bakes in extra utility classes that don't belong in the real, git-tracked build. A clean checkout reproduces CI's expected output exactly.
mhglover
force-pushed
the
feat/gamepad-support
branch
from
July 18, 2026 12:49
b06eb43 to
a58d475
Compare
document.querySelector on the combined [data-ap-back], [data-albums-back],
#v3-pl-back selector only ever inspects the first match in DOM order —
since screens stay in the DOM (hidden, not removed) when you navigate
away, a hidden back button from an unrelated screen could sort before
the one that's actually visible, incorrectly falling through to
showScreen('v3-home') instead of clicking it. Uses querySelectorAll +
find(visible) instead.
…rid 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.
- songs.js: CRLF-normalize 3-way merge (branch was LF vs main CRLF) - fix ring-fb-accent -> ring-fb-primary (accent was never a configured Tailwind color; primary is the grid's existing selection-ring class) - drop the unnecessary tailwind.min.css rebuild (no new generatable class) - strip left-in [gamepad] console.log debug statements Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
- gamepad.test.js (10): standard-mapping filter, Steam Input duplicate-slot dedup, disconnect masking, button edge-detection, d-pad/stick repeat timing, analog deadzone — driven via a fake navigator + manual rAF queue. - gamepad_nav.test.js (10): !isTrusted/defaultPrevented gating, arrow focus traversal + clamping, hidden-element skipping, Enter/Space click activation (not into text fields/body), Escape visible-back-button vs home fallback. songs.js grid nav is left to on-device coverage (async + windowed-DOM heavy). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What's included
static/v3/gamepad.js(new): pollsnavigator.getGamepads()and dispatches synthetic key events on the focused element, reusing the app's existing keyboard pipeline (shortcuts.js's scope-aware dispatcher, player shortcuts, text-field/modal guards) instead of a parallel action-mapping table. Only acts on gamepads reporting the W3C "standard" mapping, which is what Steam Input presents for the Deck's controls — button order is guaranteed correct, and anything else safely no-ops.static/v3/gamepad-nav.js(new): fills the one real gap — generic Tab-order emulation (sidebar + screen content) for Arrow keys, explicit.click()activation for Enter/Space (Chromium doesn't run native button activation for untrusted synthetic events), and a consistent Escape/B "go back" fallback. Everything here is gated on!e.isTrusted, so it only ever reacts to gamepad input and never touches real keyboard/mouse users, and defers viae.defaultPreventedto any screen that already handles a key itself.static/v3/songs.js: real 2D d-pad navigation for the song library's virtualized grid (only a slice of the library is ever in the DOM at once).static/v3/index.html: wires up the two new scripts.For Steam Deck users
Add the AppImage as a non-Steam shortcut in Desktop Mode (or launch from Gaming Mode) so Steam Input provides a standard virtual controller — this is how it's been tested.
Test plan
Summary by CodeRabbit
Note on gamepad.js vs. songs.js's keydown handling
gamepad-nav.js's generic listener is gated on!e.isTrusted— it only ever reactsto gamepad-originated synthetic events, by design, since every other screen already
had real Tab/click keyboard accessibility.
songs.js's grid-cursor listener is not gated the same way, on purpose: thev3-songsvirtualized grid had no keyboard navigation at all before this PR, forreal keyboard users or otherwise. That listener intentionally serves both gamepad and
real keyboard input as one real accessibility fix, not just a gamepad affordance.