Skip to content

Conversation

@ianz56
Copy link
Contributor

@ianz56 ianz56 commented Jan 19, 2026

try to fix the crash when pressing the lyrics button, and fix the playbar button
Issue: #3679

Summary by CodeRabbit

  • Bug Fixes
    • Improved playbar button initialization resilience with automatic retry mechanism to handle delayed platform availability
    • Enhanced navigation stability and state handling when accessing the lyrics view
    • Refined playbar button icon styling for consistent visual presentation

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 19, 2026

📝 Walkthrough

Walkthrough

This PR refactors the PlaybarButton initialization to use a polling mechanism that waits for Spicetify.Platform.History availability, defers button creation until History is ready, patches history.push to handle lyrics-plus navigation with cinemaState support, and updates the button's icon wrapper CSS class.

Changes

Cohort / File(s) Summary
Playbar Button Initialization Refactor
CustomApps/lyrics-plus/PlaybarButton.js
Replaced synchronous History guard with checkHistory polling (~30s cap), deferred button creation to separate init() function, added history.push patching (converts string paths to objects with cinemaState: null), introduced history._lyricsPlusPatched flag to prevent duplicate patching, restored event listeners for button registration/deregistration and history-based active state updates.
Icon Wrapper Styling
jsHelper/spicetifyWrapper.js
Changed button icon wrapper CSS class from generic responsive classes (Wrapper-sm-only, Wrapper-small-only) to specific class (e-91000-button__icon-wrapper).

Sequence Diagram

sequenceDiagram
    participant App as App Initialization
    participant Check as checkHistory<br/>(Polling)
    participant Init as init()<br/>(Deferred)
    participant History as Spicetify<br/>Platform.History
    participant Storage as localStorage
    participant Listeners as Event Listeners

    App->>Check: Start polling for History availability
    loop Until History found (max ~30s)
        Check->>History: Check if History exists
        alt History available
            Check->>Init: Proceed to initialization
        else History unavailable
            Check->>Check: Wait & retry
        end
    end
    
    Init->>History: Patch history.push (add cinemaState)
    Init->>History: Mark as patched (_lyricsPlusPatched flag)
    Init->>Storage: Check playbar button localStorage flag
    Init->>Init: Create playbar button
    Init->>Listeners: Register button via setPlaybarButton
    Init->>History: Listen for pathname changes
    Init->>History: Listen for lyrics-plus events
    Listeners->>Listeners: Toggle button based on events
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Suggested reviewers

  • rxri

Poem

🐰 A button once eager, now learns to wait,
For History arrives—let's not tempt fate!
With polling and patching, cinematic dreams flow,
The lyrics glow brighter, the passive row show. 🎵

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix in the changeset: preventing a crash when clicking the lyrics button, which aligns with the primary objective of the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rxri
Copy link
Member

rxri commented Jan 19, 2026

No. This is something we need to fix in the spicetify since this crash also happens on normal custom app link. There already is a regex for that which must be updated to work on 1.2.81

@rxri rxri closed this Jan 19, 2026
@ianz56 ianz56 deleted the playbar-fix branch January 19, 2026 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants