fix(library): hoist track row above siblings while + popover is open#74
Merged
Merged
Conversation
Each virtualized row is `position: absolute` without a z-index, so the "Add to a playlist" popover's `z-50` couldn't escape its own row's stacking context. Sibling rows rendered after it in DOM order painted on top, making the popover look transparent (showing the duration / heart of underlying rows) and swallowing every click on the menu items. Bump the row's z-index while its popover is open so it stacks above the following rows. Closes #72
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require review |
📝 WalkthroughWalkthroughCette pull request corrige un problème d'ordre d'empilage visuel dans la table des morceaux. Un zIndex conditionnel est ajouté à la ligne virtualisée pour que le popover « Ajouter à la playlist » reste visible et cliquable au-dessus des lignes rendues après, résolvant le bug où le popover était masqué ou inaccessible. ChangesAffichage du popover « Ajouter à la playlist »
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 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 docstrings
🧪 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 |
github-actions
Bot
added
scope: frontend
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/views/LibraryView.tsx (1)
1546-1567: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAppliquer le même fix aux grilles Albums et Artistes pour résoudre le z-index des popovers.
Les lignes virtualisées dans
AlbumGrid(ligne 1557) etArtistList(ligne 1826) utilisenttransform: translateY()sanszIndex, créant des contextes d'empilement qui piègent les popovers « Ajouter à la playlist » rendus à l'intérieur. AjouterzIndex: isMenuOpen ? 20 : undefinedaux styles de chaque ligne virtualisée permet aux popovers d'échapper à ce contexte.🤖 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 `@src/components/views/LibraryView.tsx` around lines 1546 - 1567, The virtualized row container created in the AlbumGrid (the map over virtualizer.getVirtualItems() that returns the div with transform: `translateY(${row.start - scrollMargin}px)`) is creating a new stacking context and trapping popovers; update that div's inline style to include a conditional zIndex (e.g., zIndex: isMenuOpen ? 20 : undefined) so when a card menu/popover inside (renderAlbumCard) is open it can escape the stacking context; apply the same change to the equivalent virtualized row in ArtistList so both Albums and Artists grids use the conditional zIndex when their item menu is open.
🤖 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.
Outside diff comments:
In `@src/components/views/LibraryView.tsx`:
- Around line 1546-1567: The virtualized row container created in the AlbumGrid
(the map over virtualizer.getVirtualItems() that returns the div with transform:
`translateY(${row.start - scrollMargin}px)`) is creating a new stacking context
and trapping popovers; update that div's inline style to include a conditional
zIndex (e.g., zIndex: isMenuOpen ? 20 : undefined) so when a card menu/popover
inside (renderAlbumCard) is open it can escape the stacking context; apply the
same change to the equivalent virtualized row in ArtistList so both Albums and
Artists grids use the conditional zIndex when their item menu is open.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c905966-a675-4d81-bdcd-f71aab3cdc98
📒 Files selected for processing (1)
src/components/views/LibraryView.tsx
4 tasks
InstaZDLL
added a commit
that referenced
this pull request
May 20, 2026
…ver is open (#75) Follow-up to PR #74. Same stacking-context trap as TrackTable: every virtualized row in AlbumGrid and ArtistList is `position: absolute` without a z-index, so the `+` popover's `z-50` couldn't escape its row and the rows rendered after it in DOM order painted on top — same visual transparency + click-swallowing symptoms. Bump the row's z-index when one of its cards owns the open popover.
6 tasks
InstaZDLL
added a commit
that referenced
this pull request
May 20, 2026
Follow-up to #75. The z-index fix wasn't enough in practice: the virtualizer rows use `transform: translateY()`, which creates a stacking context, and the popover's `top-full right-0` resolved to "below the entire tile" rather than "below the + button". Some ancestor (likely the page scroller's own transform / will-change set by tanstack-virtual) was clipping or restacking the popover so the following row's avatar/cover still bled through. Switch the popover to a `createPortal(..., document.body)` render path. Each `+` button registers its DOM node in a `Map<id, HTMLButtonElement>` ref; the popover takes that node as `anchorEl` and positions itself via `getBoundingClientRect` + scroll/resize listeners. Now anchored right under the trigger button (not below the full tile), and lives at the document root so stacking contexts no longer apply. TrackTable keeps its current in-flow popover — its z-index fix from #74 works because the row layout is a single flex line, not a CSS grid of multiple cards.
5 tasks
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
+button on a track row in the Songs view appeared semi-transparent and didn't accept clicks.TrackTableisposition: absolutewithout az-index. The popover's ownz-50is scoped to its row's stacking context, so it could not climb above sibling rows rendered later in DOM order. Those rows painted on top of the popover, which simultaneously made it look transparent (the underlying row's duration / heart leaking through) and stole every click on a menu item.z-indexto20while its popover is open, so it stacks above every following row.Test plan
+button: popover is fully opaque, no duration/heart from rows below leaking through.Summary by CodeRabbit