From 48a69754d670505592068ec14f3395006a2535fd Mon Sep 17 00:00:00 2001 From: Frank Stellmacher <171614930+Psychotoxical@users.noreply.github.com> Date: Mon, 18 May 2026 01:27:38 +0200 Subject: [PATCH] fix(virtual): apply scrollMargin to remaining useVirtualizer call-sites (#766) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(virtual): extract scrollMargin measurement into a reusable hook VirtualCardGrid carried its own useLayoutEffect + ResizeObserver block for the scroll-margin computation introduced in #764. The same pattern is needed on every other useVirtualizer site whose wrapper sits below other content; pull it out as useVirtualizerScrollMargin so each call-site is a single hook invocation instead of a 20-line duplicate. No behavior change for VirtualCardGrid — same scroll element, same deps, same observed nodes. * fix(virtual): apply scrollMargin to the remaining useVirtualizer call-sites #764 fixed the symptom for VirtualCardGrid (Albums / Artists grid via the shared component, Composers grid, "More by …" rail, etc.) but four more useVirtualizer call-sites have the same shape — wrapper sits below a sticky page header and TanStack would otherwise place rows starting at the scroll-element top, unmounting rows that are still in the viewport and at higher offsets refusing to render at all: - Artists grid view (Artists.tsx + ArtistsGridView.tsx) — under the sticky title + filter row + alphabet bar (~150–250 px depending on alphabet wrap). - Artists list view (Artists.tsx + ArtistsListView.tsx) — same header stack; flat letter/row stream means a noticeable jump on long libraries. - Composers list view (Composers.tsx) — identical header stack to Artists. - VirtualSongList — ~36 px sticky song-list header inside its own scroll container; the offset was small enough to be absorbed by overscan, but the pattern was the same so wire it up for consistency. Each call-site now feeds wrapRef + scroll-element lookup into useVirtualizerScrollMargin and forwards the value into useVirtualizer + the translateY of each rendered row. * docs(changelog): note PR #766 under Fixed in v1.46.0 --- CHANGELOG.md | 7 ++++ src/components/VirtualCardGrid.tsx | 38 +++++------------ src/components/VirtualSongList.tsx | 13 +++++- src/components/artists/ArtistsGridView.tsx | 5 ++- src/components/artists/ArtistsListView.tsx | 10 +++-- src/hooks/useVirtualizerScrollMargin.ts | 47 ++++++++++++++++++++++ src/pages/Artists.tsx | 26 +++++++++++- src/pages/Composers.tsx | 18 +++++++-- 8 files changed, 125 insertions(+), 39 deletions(-) create mode 100644 src/hooks/useVirtualizerScrollMargin.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 019dfe6a..c87353c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -688,6 +688,13 @@ Foundational work: faster reviews, narrower diffs, and a safety net under the pa * For local files and fully-cached HTTP tracks the Rust audio engine replays internally on the new device without any frontend round-trip. For partially-buffered HTTP streams and radio the existing frontend-restart path is kept, but resumes from the saved position via `seekFallbackVisualTarget` rather than from the beginning. * Root cause was a null-payload collision: `audio_set_device` was emitting a `()` (unit) payload that Tauri serialised to JSON `null`, triggering the new "Rust handled replay" guard in the frontend and silently preventing `playTrack` from being called. +### Virtualization — Artists, Composers and Tracks lists no longer drop rows on scroll + +**By [@Psychotoxical](https://github.com/Psychotoxical), PR [#766](https://github.com/Psychotoxical/psysonic/pull/766)** + +* Same scroll-margin bug as the one fixed by [#764](https://github.com/Psychotoxical/psysonic/pull/764) for the Album Detail "More by …" rail, on four more virtual lists: **Artists grid**, **Artists list**, **Composers list** and the **Tracks** virtual song browser. The virtual wrapper sat below the sticky page header but TanStack measured row positions from the scroll-element top — rows still on screen could unmount, and at larger header offsets the list refused to render at all. +* The measurement is now a shared `useVirtualizerScrollMargin` hook used by every virtual-list call-site (including the existing `VirtualCardGrid` fix from #764). + ## [1.45.0] - 2026-05-04 ## Added diff --git a/src/components/VirtualCardGrid.tsx b/src/components/VirtualCardGrid.tsx index e4231f76..0f06b41b 100644 --- a/src/components/VirtualCardGrid.tsx +++ b/src/components/VirtualCardGrid.tsx @@ -1,9 +1,10 @@ -import React, { useLayoutEffect, useRef, useState } from 'react'; +import React, { useRef } from 'react'; import { useVirtualizer } from '@tanstack/react-virtual'; import { APP_MAIN_SCROLL_VIEWPORT_ID } from '../constants/appScroll'; import { useElementClientHeightById } from '../hooks/useResizeClientHeight'; import { useCardGridMetrics } from '../hooks/useCardGridMetrics'; import { useRemeasureGridVirtualizer } from '../hooks/useRemeasureGridVirtualizer'; +import { useVirtualizerScrollMargin } from '../hooks/useVirtualizerScrollMargin'; import type { CardGridRowHeightVariant } from '../utils/cardGridLayout'; export type VirtualCardGridProps = { @@ -43,33 +44,14 @@ export function VirtualCardGrid({ const mainScrollViewportHeight = useElementClientHeightById(APP_MAIN_SCROLL_VIEWPORT_ID); const overscan = Math.max(2, Math.ceil(mainScrollViewportHeight / Math.max(1, rowHeightEst))); - // Vertical offset between the scroll element's top and this grid's top. - // react-virtual computes row positions relative to the scroll element, so - // without this margin a grid that sits below tall content (e.g. the - // "More by …" rail under a long tracklist) would have its first rows - // placed at negative positions and unmounted as out-of-viewport. - const [scrollMargin, setScrollMargin] = useState(0); - useLayoutEffect(() => { - if (disableVirtualization) return; - const wrap = wrapRef.current; - const scrollEl = document.getElementById(APP_MAIN_SCROLL_VIEWPORT_ID); - if (!wrap || !scrollEl) return; - const measure = () => { - const margin = - wrap.getBoundingClientRect().top - - scrollEl.getBoundingClientRect().top - + scrollEl.scrollTop; - setScrollMargin(prev => (Math.abs(prev - margin) < 1 ? prev : margin)); - }; - measure(); - const ro = new ResizeObserver(measure); - ro.observe(scrollEl); - // Anything above the grid resizing (tracklist growing, hero collapsing) - // shifts the grid down; observe the scroll content too so we re-measure. - const scrollContent = scrollEl.firstElementChild as Element | null; - if (scrollContent) ro.observe(scrollContent); - return () => ro.disconnect(); - }, [disableVirtualization, layoutSignal, virtualRowCount]); + const scrollMargin = useVirtualizerScrollMargin( + wrapRef, + () => document.getElementById(APP_MAIN_SCROLL_VIEWPORT_ID), + { + active: !disableVirtualization, + deps: [layoutSignal, virtualRowCount], + }, + ); const virtualizer = useVirtualizer({ count: disableVirtualization ? 0 : virtualRowCount, diff --git a/src/components/VirtualSongList.tsx b/src/components/VirtualSongList.tsx index 0c072b0e..2f835679 100644 --- a/src/components/VirtualSongList.tsx +++ b/src/components/VirtualSongList.tsx @@ -2,6 +2,7 @@ import { searchSongsPaged } from '../api/subsonicSearch'; import type { SubsonicSong } from '../api/subsonicTypes'; import React, { useCallback, useEffect, useRef, useState } from 'react'; import { useRefElementClientHeight } from '../hooks/useResizeClientHeight'; +import { useVirtualizerScrollMargin } from '../hooks/useVirtualizerScrollMargin'; import { Search as SearchIcon, X } from 'lucide-react'; import { useTranslation } from 'react-i18next'; import { useVirtualizer } from '@tanstack/react-virtual'; @@ -139,11 +140,19 @@ export default function VirtualSongList({ title, emptyBrowseText }: Props) { }; }, []); + const virtualWrapRef = useRef(null); + const scrollMargin = useVirtualizerScrollMargin( + virtualWrapRef, + () => scrollParentRef.current, + { active: true, deps: [songs.length] }, + ); + const virtualizer = useVirtualizer({ count: songs.length, getScrollElement: () => scrollParentRef.current, estimateSize: () => ROW_HEIGHT, overscan: songListOverscan, + scrollMargin, }); const totalSize = virtualizer.getTotalSize(); @@ -187,7 +196,7 @@ export default function VirtualSongList({ title, emptyBrowseText }: Props) { <>
-
+
{virtualizer.getVirtualItems().map(vi => { const song = songs[vi.index]; if (!song) return null; @@ -200,7 +209,7 @@ export default function VirtualSongList({ title, emptyBrowseText }: Props) { left: 0, width: '100%', height: ROW_HEIGHT, - transform: `translateY(${vi.start}px)`, + transform: `translateY(${vi.start - scrollMargin}px)`, }} > ; + scrollMargin: number; }; interface TileProps { @@ -115,7 +116,7 @@ export function ArtistsGridView({ const cols = Math.max(1, gridCols); if (virtualization) { - const { virtualizer } = virtualization; + const { virtualizer, scrollMargin } = virtualization; const rowCount = Math.ceil(visible.length / cols); return (
; + artistListWrapRef: React.RefObject; + artistListScrollMargin: number; selectionMode: boolean; selectedIds: Set; selectedArtists: SubsonicArtist[]; @@ -101,6 +103,8 @@ export function ArtistsListView({ letters, artistListFlatRows, artistListVirtualizer, + artistListWrapRef, + artistListScrollMargin, selectionMode, selectedIds, selectedArtists, @@ -133,7 +137,7 @@ export function ArtistsListView({ } return ( -
+

{row.letter}

@@ -169,7 +173,7 @@ export function ArtistsListView({ top: 0, left: 0, width: '100%', - transform: `translateY(${vi.start}px)`, + transform: `translateY(${vi.start - artistListScrollMargin}px)`, paddingBottom: row.isLastInLetter ? '1.5rem' : undefined, }} > diff --git a/src/hooks/useVirtualizerScrollMargin.ts b/src/hooks/useVirtualizerScrollMargin.ts new file mode 100644 index 00000000..8bb16e31 --- /dev/null +++ b/src/hooks/useVirtualizerScrollMargin.ts @@ -0,0 +1,47 @@ +import { useLayoutEffect, useRef, useState } from 'react'; +import type React from 'react'; + +/** + * Distance between a virtualizer's wrapper element and its scroll element. + * Pass as `scrollMargin` to `useVirtualizer`, and subtract from `vItem.start` + * when positioning rendered rows. Without it, `@tanstack/react-virtual` + * places rows relative to the scroll-element top — so a wrapper that sits + * below other content (sticky header, tracklist, hero) ends up with rows at + * the wrong Y, and rows still in the viewport may unmount. + * + * Re-measures via `ResizeObserver` on the scroll element and its first child + * so content growing above the wrapper updates the offset. + */ +export function useVirtualizerScrollMargin( + wrapRef: React.RefObject, + getScrollElement: () => HTMLElement | null, + options: { + active: boolean; + /** Caller-supplied dependencies that affect the wrapper's position. */ + deps: ReadonlyArray; + }, +): number { + const [scrollMargin, setScrollMargin] = useState(0); + const getterRef = useRef(getScrollElement); + getterRef.current = getScrollElement; + useLayoutEffect(() => { + if (!options.active) return; + const wrap = wrapRef.current; + const scrollEl = getterRef.current(); + if (!wrap || !scrollEl) return; + const measure = () => { + const margin = + wrap.getBoundingClientRect().top + - scrollEl.getBoundingClientRect().top + + scrollEl.scrollTop; + setScrollMargin(prev => (Math.abs(prev - margin) < 1 ? prev : margin)); + }; + measure(); + const ro = new ResizeObserver(measure); + ro.observe(scrollEl); + const scrollContent = scrollEl.firstElementChild as Element | null; + if (scrollContent) ro.observe(scrollContent); + return () => ro.disconnect(); + }, [options.active, wrapRef, ...options.deps]); + return scrollMargin; +} diff --git a/src/pages/Artists.tsx b/src/pages/Artists.tsx index 5a3d874f..e92256a7 100644 --- a/src/pages/Artists.tsx +++ b/src/pages/Artists.tsx @@ -12,6 +12,7 @@ import { APP_MAIN_SCROLL_VIEWPORT_ID } from '../constants/appScroll'; import { useElementClientHeightById } from '../hooks/useResizeClientHeight'; import { useCardGridMetrics } from '../hooks/useCardGridMetrics'; import { useRemeasureGridVirtualizer } from '../hooks/useRemeasureGridVirtualizer'; +import { useVirtualizerScrollMargin } from '../hooks/useVirtualizerScrollMargin'; import { usePerfProbeFlags } from '../utils/perf/perfFlags'; import { ALL_SENTINEL, @@ -99,6 +100,15 @@ export default function Artists() { Math.ceil(mainScrollViewportHeight / Math.max(1, artistGridRowHeightEst)), ); + const artistGridScrollMargin = useVirtualizerScrollMargin( + artistGridMeasureRef, + () => document.getElementById(APP_MAIN_SCROLL_VIEWPORT_ID), + { + active: !perfFlags.disableMainstageVirtualLists && viewMode === 'grid', + deps: [artistVirtualRowCount, artistGridCols], + }, + ); + const artistGridVirtualizer = useVirtualizer({ count: perfFlags.disableMainstageVirtualLists || viewMode !== 'grid' @@ -107,6 +117,7 @@ export default function Artists() { getScrollElement: () => document.getElementById(APP_MAIN_SCROLL_VIEWPORT_ID), estimateSize: () => artistGridRowHeightEst, overscan: artistGridOverscan, + scrollMargin: artistGridScrollMargin, }); useRemeasureGridVirtualizer(artistGridVirtualizer, { @@ -122,6 +133,16 @@ export default function Artists() { Math.ceil(mainScrollViewportHeight / ARTIST_LIST_ROW_EST), ); + const artistListWrapRef = useRef(null); + const artistListScrollMargin = useVirtualizerScrollMargin( + artistListWrapRef, + () => document.getElementById(APP_MAIN_SCROLL_VIEWPORT_ID), + { + active: !perfFlags.disableMainstageVirtualLists && viewMode === 'list', + deps: [artistListFlatRows.length], + }, + ); + const artistListVirtualizer = useVirtualizer({ count: perfFlags.disableMainstageVirtualLists || viewMode !== 'list' ? 0 : artistListFlatRows.length, @@ -140,6 +161,7 @@ export default function Artists() { return `artist:${row.artist.id}`; }, overscan: artistListOverscan, + scrollMargin: artistListScrollMargin, }); return ( @@ -228,7 +250,7 @@ export default function Artists() { virtualization={ perfFlags.disableMainstageVirtualLists ? null - : { virtualizer: artistGridVirtualizer } + : { virtualizer: artistGridVirtualizer, scrollMargin: artistGridScrollMargin } } selectionMode={selectionMode} selectedIds={selectedIds} @@ -248,6 +270,8 @@ export default function Artists() { letters={letters} artistListFlatRows={artistListFlatRows} artistListVirtualizer={artistListVirtualizer} + artistListWrapRef={artistListWrapRef} + artistListScrollMargin={artistListScrollMargin} selectionMode={selectionMode} selectedIds={selectedIds} selectedArtists={selectedArtists} diff --git a/src/pages/Composers.tsx b/src/pages/Composers.tsx index 3a2192dd..b9d7f90f 100644 --- a/src/pages/Composers.tsx +++ b/src/pages/Composers.tsx @@ -12,6 +12,7 @@ import { APP_MAIN_SCROLL_VIEWPORT_ID } from '../constants/appScroll'; import { useElementClientHeightById } from '../hooks/useResizeClientHeight'; import { usePerfProbeFlags } from '../utils/perf/perfFlags'; import { VirtualCardGrid } from '../components/VirtualCardGrid'; +import { useVirtualizerScrollMargin } from '../hooks/useVirtualizerScrollMargin'; const ALL_SENTINEL = 'ALL'; const ALPHABET = [ALL_SENTINEL, '#', ...'ABCDEFGHIJKLMNOPQRSTUVWXYZ'.split('')]; @@ -185,6 +186,16 @@ export default function Composers() { Math.ceil(mainScrollViewportHeight / COMPOSER_LIST_ROW_EST), ); + const composerListWrapRef = useRef(null); + const composerListScrollMargin = useVirtualizerScrollMargin( + composerListWrapRef, + () => document.getElementById(APP_MAIN_SCROLL_VIEWPORT_ID), + { + active: !perfFlags.disableMainstageVirtualLists && viewMode === 'list', + deps: [composerListFlatRows.length], + }, + ); + const composerListVirtualizer = useVirtualizer({ count: perfFlags.disableMainstageVirtualLists || viewMode !== 'list' ? 0 : composerListFlatRows.length, @@ -202,6 +213,7 @@ export default function Composers() { return `composer:${row.artist.id}`; }, overscan: composerListOverscan, + scrollMargin: composerListScrollMargin, }); if (loadError) { @@ -337,7 +349,7 @@ export default function Composers() { ))} ) : ( -
+

{row.letter}

@@ -373,7 +385,7 @@ export default function Composers() { top: 0, left: 0, width: '100%', - transform: `translateY(${vi.start}px)`, + transform: `translateY(${vi.start - composerListScrollMargin}px)`, paddingBottom: row.isLastInLetter ? '1.5rem' : undefined, }} >