mirror of
https://github.com/kilyabin/psysonic.git
synced 2026-07-21 22:15:40 +00:00
fix(virtual): apply scrollMargin to remaining useVirtualizer call-sites (#766)
* 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
This commit is contained in:
committed by
GitHub
parent
d3fc5c91fc
commit
48a69754d6
@@ -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
|
||||
|
||||
@@ -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<T> = {
|
||||
@@ -43,33 +44,14 @@ export function VirtualCardGrid<T>({
|
||||
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,
|
||||
|
||||
@@ -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<HTMLDivElement>(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) {
|
||||
<>
|
||||
<div ref={scrollParentRef} className="virtual-song-list-scroll">
|
||||
<SongListHeader />
|
||||
<div style={{ height: totalSize, width: '100%', position: 'relative' }}>
|
||||
<div ref={virtualWrapRef} style={{ height: totalSize, width: '100%', position: 'relative' }}>
|
||||
{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)`,
|
||||
}}
|
||||
>
|
||||
<SongRow
|
||||
|
||||
@@ -9,6 +9,7 @@ import { ArtistCardAvatar } from './ArtistAvatars';
|
||||
|
||||
export type ArtistsGridVirtualization = {
|
||||
virtualizer: Virtualizer<HTMLElement, Element>;
|
||||
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 (
|
||||
<div
|
||||
@@ -141,7 +142,7 @@ export function ArtistsGridView({
|
||||
top: 0,
|
||||
left: 0,
|
||||
width: '100%',
|
||||
transform: `translateY(${vRow.start}px)`,
|
||||
transform: `translateY(${vRow.start - scrollMargin}px)`,
|
||||
display: 'grid',
|
||||
gridTemplateColumns: `repeat(${cols}, minmax(0, 1fr))`,
|
||||
gap: 'var(--space-4)',
|
||||
|
||||
@@ -72,6 +72,8 @@ interface Props {
|
||||
letters: string[];
|
||||
artistListFlatRows: ArtistListFlatRow[];
|
||||
artistListVirtualizer: Virtualizer<HTMLElement, Element>;
|
||||
artistListWrapRef: React.RefObject<HTMLDivElement | null>;
|
||||
artistListScrollMargin: number;
|
||||
selectionMode: boolean;
|
||||
selectedIds: Set<string>;
|
||||
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 (
|
||||
<div style={{ position: 'relative', width: '100%' }}>
|
||||
<div ref={artistListWrapRef} style={{ position: 'relative', width: '100%' }}>
|
||||
<div
|
||||
style={{
|
||||
height: artistListFlatRows.length === 0 ? 0 : artistListVirtualizer.getTotalSize(),
|
||||
@@ -153,7 +157,7 @@ export function ArtistsListView({
|
||||
top: 0,
|
||||
left: 0,
|
||||
width: '100%',
|
||||
transform: `translateY(${vi.start}px)`,
|
||||
transform: `translateY(${vi.start - artistListScrollMargin}px)`,
|
||||
}}
|
||||
>
|
||||
<h3 className="letter-heading">{row.letter}</h3>
|
||||
@@ -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,
|
||||
}}
|
||||
>
|
||||
|
||||
@@ -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<HTMLElement | null>,
|
||||
getScrollElement: () => HTMLElement | null,
|
||||
options: {
|
||||
active: boolean;
|
||||
/** Caller-supplied dependencies that affect the wrapper's position. */
|
||||
deps: ReadonlyArray<unknown>;
|
||||
},
|
||||
): 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;
|
||||
}
|
||||
+25
-1
@@ -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<HTMLDivElement>(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}
|
||||
|
||||
+15
-3
@@ -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<HTMLDivElement>(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() {
|
||||
))}
|
||||
</>
|
||||
) : (
|
||||
<div style={{ position: 'relative', width: '100%' }}>
|
||||
<div ref={composerListWrapRef} style={{ position: 'relative', width: '100%' }}>
|
||||
<div
|
||||
style={{
|
||||
height: composerListFlatRows.length === 0 ? 0 : composerListVirtualizer.getTotalSize(),
|
||||
@@ -357,7 +369,7 @@ export default function Composers() {
|
||||
top: 0,
|
||||
left: 0,
|
||||
width: '100%',
|
||||
transform: `translateY(${vi.start}px)`,
|
||||
transform: `translateY(${vi.start - composerListScrollMargin}px)`,
|
||||
}}
|
||||
>
|
||||
<h3 className="letter-heading">{row.letter}</h3>
|
||||
@@ -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,
|
||||
}}
|
||||
>
|
||||
|
||||
Reference in New Issue
Block a user