fix(trashbin): context menu label not updating#3744
Conversation
Replace `this.name` with direct `cntxMenu` reference in shouldAddContextMenu. `this` is no longer bound to the ContextMenu.Item in newer Spicetify versions. Fixes spicetify#3743
📝 WalkthroughWalkthroughUpdated context-menu labeling in the Trashbin extension: the label assignments inside Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Extensions/trashbin.js (1)
357-358:constis sufficient here;letis unnecessary.The variable
cntxMenuis never reassigned—only its.nameproperty is mutated. Property mutations are allowed onconstobjects. Usingconstbetter communicates intent that this binding won't change.Suggested change
- let cntxMenu = new Spicetify.ContextMenu.Item(THROW_TEXT, toggleThrow, shouldAddContextMenu, trashbinIcon); + const cntxMenu = new Spicetify.ContextMenu.Item(THROW_TEXT, toggleThrow, shouldAddContextMenu, trashbinIcon);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Extensions/trashbin.js` around lines 357 - 358, Change the binding for the context menu item from a mutable variable to an immutable one: replace the `let cntxMenu = new Spicetify.ContextMenu.Item(THROW_TEXT, toggleThrow, shouldAddContextMenu, trashbinIcon);` declaration with a `const` so the binding is not reassigned (the object’s properties like `.name` can still be mutated); keep the existing calls to `cntxMenu.register()` and all referenced symbols (`Spicetify.ContextMenu.Item`, `toggleThrow`, `shouldAddContextMenu`, `trashbinIcon`) unchanged.
🤖 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/trashbin.js`:
- Around line 357-358: Change the binding for the context menu item from a
mutable variable to an immutable one: replace the `let cntxMenu = new
Spicetify.ContextMenu.Item(THROW_TEXT, toggleThrow, shouldAddContextMenu,
trashbinIcon);` declaration with a `const` so the binding is not reassigned (the
object’s properties like `.name` can still be mutated); keep the existing calls
to `cntxMenu.register()` and all referenced symbols
(`Spicetify.ContextMenu.Item`, `toggleThrow`, `shouldAddContextMenu`,
`trashbinIcon`) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ffa8b402-de30-4f1f-b704-42d6815ddb9c
📒 Files selected for processing (1)
Extensions/trashbin.js
Extensions/trashbin.js
Outdated
| } | ||
|
|
||
| const cntxMenu = new Spicetify.ContextMenu.Item(THROW_TEXT, toggleThrow, shouldAddContextMenu, trashbinIcon); | ||
| let cntxMenu = new Spicetify.ContextMenu.Item(THROW_TEXT, toggleThrow, shouldAddContextMenu, trashbinIcon); |
There was a problem hiding this comment.
cntxMenu is an object.
| let cntxMenu = new Spicetify.ContextMenu.Item(THROW_TEXT, toggleThrow, shouldAddContextMenu, trashbinIcon); | |
| const cntxMenu = new Spicetify.ContextMenu.Item(THROW_TEXT, toggleThrow, shouldAddContextMenu, trashbinIcon); |
`shouldAddContextMenu` used `this.name` to update the context menu label, but `this` is no longer bound to the `ContextMenu.Item` in newer Spicetify versions. Referencing `cntxMenu` directly via closure fixes the stale label.
Fixes #3743
Summary by CodeRabbit