From f4a804c356a70ccd83e2243f9da011fe34e9b9ae Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 20 May 2026 22:36:24 +0200 Subject: [PATCH 1/2] fix(library): stop popover clicks bubbling + clamp/flip portal placement Two follow-up issues caught in PR review: 1. React synthetic events still bubble through the React tree even when the popover is rendered through a portal. The container only stopped `onDoubleClick`, so clicking a playlist row also fired the album / artist tile `onClick` underneath and navigated away mid-pick. Stop `onClick` + `onMouseDown` at the popover root. 2. Portal placement used raw `rect.bottom + 4` / `rect.right - width` with no bounds checks. First-column trigger pushed the popover off the left edge; trigger near the viewport bottom clipped half the menu. Measure the popover via a second ResizeObserver, prefer below then flip above when below would clip, and clamp on both axes with an 8 px viewport margin. --- src/components/views/LibraryView.tsx | 67 +++++++++++++++++++++++----- 1 file changed, 57 insertions(+), 10 deletions(-) diff --git a/src/components/views/LibraryView.tsx b/src/components/views/LibraryView.tsx index c2805d3..dabf481 100644 --- a/src/components/views/LibraryView.tsx +++ b/src/components/views/LibraryView.tsx @@ -1360,12 +1360,15 @@ function AddToPlaylistPopover({ memberPlaylistIds, anchorEl, }: AddToPlaylistPopoverProps) { - // Portal mode: track the anchor's viewport rect so the popover follows - // it on scroll / resize / virtualization recycling. `null` rect = first - // render before the layout effect runs; we keep the popover invisible - // until we know where it goes so it never flashes at (0,0). + // Portal mode: track the anchor's viewport rect AND the popover's own + // height so we can flip / clamp against the viewport. `null` rect = + // first render before the layout effect runs; we keep the popover + // invisible until we know where it goes so it never flashes at (0,0). const POPOVER_WIDTH = 224; // matches `w-56` + const VIEWPORT_MARGIN = 8; + const popoverRef = useRef(null); const [rect, setRect] = useState(null); + const [popoverHeight, setPopoverHeight] = useState(0); useLayoutEffect(() => { if (!anchorEl) return; const update = () => setRect(anchorEl.getBoundingClientRect()); @@ -1380,22 +1383,66 @@ function AddToPlaylistPopover({ window.removeEventListener("resize", update); }; }, [anchorEl]); + // Measure the popover the first time it lays out and on resize so the + // flip-above check has a real height. Without this we'd default to 0 + // and never flip until the popover overflows. + useLayoutEffect(() => { + if (!anchorEl) return; + const el = popoverRef.current; + if (!el) return; + const measure = () => setPopoverHeight(el.offsetHeight); + measure(); + const ro = new ResizeObserver(measure); + ro.observe(el); + return () => ro.disconnect(); + }, [anchorEl, rect]); + + // Compute placement: prefer below, flip above when below would clip, + // then clamp horizontally so the first-column trigger doesn't push + // the popover off the left edge. + const placement = (() => { + if (!anchorEl || !rect) return null; + const vw = window.innerWidth; + const vh = window.innerHeight; + let top = rect.bottom + 4; + if ( + popoverHeight > 0 && + top + popoverHeight > vh - VIEWPORT_MARGIN && + rect.top - 4 - popoverHeight >= VIEWPORT_MARGIN + ) { + top = rect.top - 4 - popoverHeight; + } + top = Math.max( + VIEWPORT_MARGIN, + Math.min(top, vh - popoverHeight - VIEWPORT_MARGIN), + ); + let left = rect.right - POPOVER_WIDTH; + left = Math.max( + VIEWPORT_MARGIN, + Math.min(left, vw - POPOVER_WIDTH - VIEWPORT_MARGIN), + ); + return { top, left }; + })(); const inner = (
e.stopPropagation()} + onMouseDown={(e) => e.stopPropagation()} onDoubleClick={(e) => e.stopPropagation()} style={ anchorEl - ? rect + ? placement ? { position: "fixed", - // Anchor the right edge to the trigger's right edge, - // matching the in-flow `right-0` behaviour. Drop 4 px - // below the trigger to match `mt-1`. - top: rect.bottom + 4, - left: rect.right - POPOVER_WIDTH, + top: placement.top, + left: placement.left, width: POPOVER_WIDTH, } : { position: "fixed", visibility: "hidden" } From 58befa8bff7bbacb5658f20d105fb6169dbd5603 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 20 May 2026 22:44:37 +0200 Subject: [PATCH 2/2] fix(library): drop `rect` from popover-measure effect deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Qodo follow-up: the measurement `useLayoutEffect` depended on `rect`, which is updated on every scroll tick. Each tick tore down + remounted the ResizeObserver and forced a synchronous `offsetHeight` read, all to recompute a value that hadn't actually changed. Depend on `anchorEl` only — the ResizeObserver still catches real height changes (locale wrap, content reflow). --- src/components/views/LibraryView.tsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/components/views/LibraryView.tsx b/src/components/views/LibraryView.tsx index dabf481..653b4ce 100644 --- a/src/components/views/LibraryView.tsx +++ b/src/components/views/LibraryView.tsx @@ -1383,19 +1383,22 @@ function AddToPlaylistPopover({ window.removeEventListener("resize", update); }; }, [anchorEl]); - // Measure the popover the first time it lays out and on resize so the - // flip-above check has a real height. Without this we'd default to 0 - // and never flip until the popover overflows. + // Measure the popover the first time it lays out and on content + // resize so the flip-above check has a real height. We intentionally + // do NOT depend on `rect` — scroll updates `rect` many times per + // second, and re-running this effect would tear down the + // ResizeObserver and force a synchronous `offsetHeight` reflow each + // tick. The ResizeObserver already covers every real height change + // (translated label wrap, scrollable list growth, etc.). useLayoutEffect(() => { if (!anchorEl) return; const el = popoverRef.current; if (!el) return; - const measure = () => setPopoverHeight(el.offsetHeight); - measure(); - const ro = new ResizeObserver(measure); + setPopoverHeight(el.offsetHeight); + const ro = new ResizeObserver(() => setPopoverHeight(el.offsetHeight)); ro.observe(el); return () => ro.disconnect(); - }, [anchorEl, rect]); + }, [anchorEl]); // Compute placement: prefer below, flip above when below would clip, // then clamp horizontally so the first-column trigger doesn't push