fix: update loopyLoop to handle Spotify DOM restructuring#3724
fix: update loopyLoop to handle Spotify DOM restructuring#3724rxri merged 4 commits intospicetify:mainfrom
loopyLoop to handle Spotify DOM restructuring#3724Conversation
📝 WalkthroughWalkthroughRewrote Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
Pull request overview
Updates the Loopy loop Spicetify extension to keep working with Spotify’s updated playback bar DOM and recent React component context requirements by switching to structural DOM traversal and a non-React context menu.
Changes:
- Replace the progress bar element lookup with traversal via
.playback-progressbar-containerandinput[type="range"]. - Replace
ReactComponent.MenuItemrendering with plain HTML menu items. - Remove the
asyncIIFE /Spicetify.Events.webpackLoadedwait previously used for React rendering.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Extensions/loopyLoop.js (2)
80-89: Consider adding accessibility attributes for keyboard navigation.The plain DOM menu items work but may lack accessibility features that the React
MenuItemcomponent provides (e.g.,role,tabindex, keyboard event handling). For consistency with Spotify's UX:♿ Suggested accessibility improvements
function createMenuItem(title, callback) { const item = document.createElement("li"); item.classList.add("main-contextMenu-menuItemButton"); + item.setAttribute("role", "menuitem"); + item.setAttribute("tabindex", "-1"); item.textContent = title; item.onclick = () => { contextMenu.hidden = true; callback?.(); }; + item.onkeydown = (e) => { + if (e.key === "Enter" || e.key === " ") { + contextMenu.hidden = true; + callback?.(); + } + }; return item; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Extensions/loopyLoop.js` around lines 80 - 89, The createMenuItem function creates plain <li> elements that lack accessibility attributes and keyboard handling; update createMenuItem to set ARIA/role and focusability (e.g., set role="menuitem" and tabindex="0" on the created item), add keyboard event handling for Enter/Space to invoke the callback (mirror the existing item.onclick behavior), and ensure hiding the contextMenu and calling callback via the same handler; reference createMenuItem, contextMenu, and the "main-contextMenu-menuItemButton" class when making these changes.
12-12: Fragile DOM traversal assumption.The selector
rangeInput?.closest("label")?.nextElementSiblingassumes a specific DOM structure where the progress bar is the next sibling of a label containing the range input. This structural dependency could break if Spotify reorganizes the DOM again (which is the very reason this PR exists).Consider adding a fallback query or a more robust selector strategy, and/or adding a comment documenting the expected structure for future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Extensions/loopyLoop.js` at line 12, The DOM traversal using rangeInput?.closest("label")?.nextElementSibling (variable bar) is brittle; change it to try more robust fallbacks: first attempt the existing closest("label") path, then if that yields no valid progress element, search upward for a common parent (e.g., closest container element) and run a querySelector for known progress/progress-bar selectors or elements (e.g., [role="progressbar"], .progress, .progress-bar, svg.progress) and validate the found element before using it; add a short code comment next to the rangeInput/bar logic documenting the expected DOM relationship and why the fallback queries exist so future maintainers understand the intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Extensions/loopyLoop.js`:
- Around line 80-89: The createMenuItem function creates plain <li> elements
that lack accessibility attributes and keyboard handling; update createMenuItem
to set ARIA/role and focusability (e.g., set role="menuitem" and tabindex="0" on
the created item), add keyboard event handling for Enter/Space to invoke the
callback (mirror the existing item.onclick behavior), and ensure hiding the
contextMenu and calling callback via the same handler; reference createMenuItem,
contextMenu, and the "main-contextMenu-menuItemButton" class when making these
changes.
- Line 12: The DOM traversal using
rangeInput?.closest("label")?.nextElementSibling (variable bar) is brittle;
change it to try more robust fallbacks: first attempt the existing
closest("label") path, then if that yields no valid progress element, search
upward for a common parent (e.g., closest container element) and run a
querySelector for known progress/progress-bar selectors or elements (e.g.,
[role="progressbar"], .progress, .progress-bar, svg.progress) and validate the
found element before using it; add a short code comment next to the
rangeInput/bar logic documenting the expected DOM relationship and why the
fallback queries exist so future maintainers understand the intent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 66dcab70-0807-455c-a85d-60a2dfe2bb0e
📒 Files selected for processing (1)
Extensions/loopyLoop.js
yuvibirdi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Extensions/loopyLoop.js`:
- Around line 8-16: The init loop in LoopyLoop currently only waits for DOM
elements and can still attach Spicetify.Player listeners too early; update
LoopyLoop so it also waits for Spicetify to be ready by checking for
Spicetify?.Events?.webpackLoaded (or listening for that event) before finishing
init and before any call to Spicetify.Player.addEventListener; e.g., if
Spicetify.Events.webpackLoaded hasn’t fired, register a one-time handler to
continue initialization (or poll Spicetify?.Player existence) and only attach
player event listeners after that readiness check to avoid race conditions with
the wrapper that makes the API safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa2a58b0-a1da-4db5-bde6-6718bf6d8a72
📒 Files selected for processing (1)
Extensions/loopyLoop.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Extensions/loopyLoop.js`:
- Around line 80-92: In createMenuItem, the class
main-contextMenu-menuItemButton is applied to the li but the interactive element
is the inner button, so move the class to the button element: remove
classList.add on the li and instead add
classList.add("main-contextMenu-menuItemButton") to the button, keep the li
role="menuitem" and append the button as before, and ensure existing button
event handlers (onclick) remain attached so hover/focus CSS selectors in
Themes/SpicetifyDefault/user.css (e.g., .main-contextMenu-menuItemButton:hover
and :focus) will match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a2688863-0ac3-4066-b827-e821798b2426
📒 Files selected for processing (1)
Extensions/loopyLoop.js
yuvibirdi
changed the title
yuvibirdi
changed the title
rxri
changed the title
loopyLoop to handle Spotify DOM restructuring
.playback-bar .progress-barno longer matches, Spotify restructured the playback bar DOM with new hashed classes not present in the CSS map.playback-progressbar-containerandinput[type="range"]ReactDOM.render(MenuItem)with plain HTML menu items since,MenuItemnow requires React Router context (StableUseNavigateProvider) which isn't available when rendering into a detached divasync/webpackLoadeddependency since React components are no longer usedSummary by CodeRabbit
Refactor
Bug Fixes