fix(library): stabilize list-view virtual scroller (#675)#676
Open
s3ntin3l8 wants to merge 1 commit into
Open
Conversation
The audiobooks list view used a single sampled row height (via a ref shared with grid view) applied to every row, an unthrottled scroll handler that reassigned visibleRange on every tick, a visibleRange watcher that re-measured and re-ran updateVisibleRange, and a column header excluded from the scroll height. Together these produced an oversized scroll area, overscroll past both ends, an unreachable last row, and a freeze on fast scrolling. Changing the sort order (toolbar or clickable headers) reshuffles which row is sampled first, re-arming the same freeze on the next scroll. - Give list rows a fixed CSS height and return that constant directly from getRowHeight() (no measurement, no grid-height leak); add a taller constant for the show-details mode. - rAF-throttle the scroll handler so updateVisibleRange runs at most once per animation frame; cancel the pending frame on unmount. - Only reassign visibleRange when the computed range actually changes. - Measure grid rows once (not on every scroll-driven range change), which breaks the scroll -> measure -> resize -> scroll feedback loop. - Reserve the fixed list-header height in totalHeight so the last row stays fully scrollable. Adds Vitest coverage for the deterministic list row-height math, the header space reservation, sort-invariant scroll geometry, no-churn and applied range updates, scroll-event coalescing, and rAF cleanup on unmount. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Closes #675
Summary
The audiobooks list view uses a hand-rolled, uniform-height virtual scroller in
AudiobooksView.vue. On a library of a few hundred books it produced an oversized scroll area (large empty region below the last item), let you overscroll past both ends, left the last row partially unreachable, and froze the page ("infinite loading") on fast scrolling. Changing the sort order (toolbar or the clickable column headers) and then scrolling triggered the same freeze.This reproduces on a clean
canarycheckout (the list view + scroller already ship there; it is not specific to the grouped-list-view PR #585). The sort-then-scroll variant reproduces via the toolbar sort oncanaryand via the clickable sort headers downstream — it is the same bug, not a separate one.Root cause
syncMeasuredRowHeight()measures only the first rendered.audiobook-list-itemand applies that height to every row viagetRowHeight()→totalHeight = ceil(n) × getRowHeight().measuredRowHeightis a single ref shared with grid view (grid rows ≈ 220–240px). An inflated/stale sample sticks → oversized scroll area. Re-sorting reshuffles which row is sampled first, re-arming the problem.@scroll="updateVisibleRange"ran on every scroll event and always assigned a newvisibleRangeobject, so thevisibleRangewatcher fired every tick and forced a synchronousgetBoundingClientRectreflow per event — layout thrash on a fast fling.updateVisibleRange(); with variable row heights there is no stable fixpoint, so it oscillated (overscroll) and saturated the main thread (freeze)..list-headerlives inside thetranslateY-shifted list but wasn't counted intotalHeight, pushing the last row below the scrollable region.Changes
height+box-sizing+overflow:hidden);getRowHeight()returns that constant directly for list view and no longer consultsmeasuredRowHeight(measurement stays grid-only). A taller fixed constant is used for the "show details" mode. This makestotalHeightdeterministic and kills the oversize/oscillation by construction.updateVisibleRangeruns at most once per animation frame; the pending frame is cancelled on unmount.updateVisibleRangeonly reassignsvisibleRangewhen the computed range actually changes (no per-tick object churn).visibleRangewatcher measures grid rows once instead of on every scroll-driven range change, breaking the scroll → measure → resize → scroll loop.totalHeightso the last row stays fully scrollable.Deliberate tradeoffs (reviewer note)
overflow:hidden, so content that would exceed the row is clipped (titles already ellipsize; the show-details row uses a taller constant sized for its two detail lines).Tests
Adds Vitest coverage in
AudiobooksView.spec.tsfor: the deterministic list row-height (ignoring a stale/leakedmeasuredRowHeight), the taller show-details height, header-space reservation intotalHeight, sort-invariant scroll geometry, no-churn vs. appliedvisibleRangeupdates, scroll-event coalescing, and rAF cleanup on unmount. Full frontend suite,type-check, andlintpass.Verification
Reproduced on a clean
canarycheckout (fast-fling + sort-then-scroll), confirmed fixed after the change: scrolling clamps at both ends, the scroll area matches the item count, the last row is fully reachable, and no freeze — including after changing sort.