[fix] fixing the laggy playlist render and scrolling#8
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR overhauls playlist rendering to support virtualization for large track lists, adds shuffle/repeat/menu controls to the bottom player, and updates documentation with a more conversational tone. The changes introduce TanStack virtual-core integration, concurrent duration resolution, centralized state and event handling, and refactored table styling for fixed-layout virtual rendering. ChangesBottom Player & Documentation
Playlist Virtualization & Rendering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@README.md`:
- Line 97: Fix the spelling mistake in the README sentence "Audio normalization
(or loudness nomalization something i'm not an audio engineer)." by replacing
"nomalization" with "normalization" and ensure the sentence reads correctly
(e.g., "Audio normalization (or loudness normalization, something I'm not an
audio engineer)."); update capitalization/apostrophe as needed for consistency.
- Around line 5-9: The README paragraph for "Blueberry Music Player" has
multiple grammar and capitalization errors; update the text to use correct
capitalization and grammar: change "Support" to "Supports", "VanillaJs" to
"Vanilla JS", both instances of "Window Media Player" to "Windows Media Player",
all lowercase "i" to "I", change "is i didn't know" to "is that I didn't know"
(or rephrase to "I didn't realize"), and replace "critics" with "criticism" or
"feedback"; also tidy the sentence about playlists (e.g., "I don't like how
Windows Media Player playlists work, and I like how Spotify playlists work, so I
combined those ideas") and rephrase the RAM/Tauri sentence more formally.
In `@ui/components/bottom-player/player.html`:
- Around line 58-60: The button with id "TrackMenuBtn" is inconsistent with the
other camelCase IDs and lacks an aria-label; rename the id to "trackMenuBtn"
(matching peers like trackShuffleBtn, trackRepeatBtn, playPauseBtn) and add an
appropriate aria-label attribute (e.g., aria-label="Track menu") to the <button>
element to restore naming consistency and screen-reader accessibility.
In `@ui/pages/playlist/playlist-virtualizer.js`:
- Around line 37-39: Remove the private/manual lifecycle usage: delete calls to
virtualizer._willUpdate() and the assignment of virtualizer._didMount()’s return
value to virtualizer.destroy; instead rely on the public API and call
virtualizer.destroy() when you need to tear down the virtualizer. Specifically,
remove references to virtualizer._willUpdate and virtualizer._didMount and
ensure any teardown code invokes the public virtualizer.destroy() method (and
use public update/measure APIs if you need to trigger re-renders) rather than
assigning a private cleanup function.
- Around line 9-16: The dynamic import in playlist-virtualizer.js uses a brittle
deep path; change the import target used by virtualCorePromise to the package
entry '`@tanstack/virtual-core`' (i.e. virtualCorePromise ||=
import('`@tanstack/virtual-core`')), and ensure the package is moved from
devDependencies to dependencies in package.json so it is available at runtime;
also stop calling private internals on the Virtualizer instance (remove uses of
virtualizer._willUpdate(), virtualizer._didMount() and replacing
virtualizer.destroy) and instead rely on the public lifecycle methods
(virtualizer.destroy()) and the library’s supported measurement/init APIs for
setup/cleanup.
In `@ui/pages/playlist/playlist.js`:
- Around line 274-281: renderTotalDuration() and per-cell updates can write
stale state after teardown because cleanup() doesn't invalidate
totalDurationRunId or cancel durationCellsRaf; update cleanup() to increment or
change totalDurationRunId to invalidate any in-flight
resolveTrackDurationsLimited onResolved callbacks, call
cancelAnimationFrame(durationCellsRaf) (and null it) to stop pending RAF work,
and ensure scheduleDurationCellUpdate and any other callbacks (the onResolved
passed into resolveTrackDurationsLimited) continue to compare the captured runId
against totalDurationRunId before mutating state.
🪄 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
Run ID: ae931999-819a-4557-8854-a1b1f7169aeb
📒 Files selected for processing (7)
README.mdui/components/bottom-player/player.cssui/components/bottom-player/player.htmlui/pages/playlist/playlist-row-renderer.jsui/pages/playlist/playlist-virtualizer.jsui/pages/playlist/playlist.cssui/pages/playlist/playlist.js
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
1 participant
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 by CodeRabbit
New Features
Style
Documentation