feat(virtual): derive visibleCount and overscan from container size#13
Open
chiefcll wants to merge 5 commits into
Open
feat(virtual): derive visibleCount and overscan from container size#13chiefcll wants to merge 5 commits into
chiefcll wants to merge 5 commits into
Conversation
Removes displaySize, bufferSize, uniformSize, and factorScale props from VirtualRow / VirtualColumn. Visible count and buffer are now computed from container width/height and the first child's measured size on a single layout pass; the result is cached for the lifetime of the component (item size assumed uniform). A single probe item is rendered until measurement completes, then the full slice expands in. Container must have a measurable width (Row) or height (Column) on first layout. VirtualGrid is untouched. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
494bf14 to
dbd9595
Compare
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.
Summary
Replaces
VirtualRow/VirtualColumnwith a version that derives window size from the rendered layout instead of asking the caller for it. The outer API is otherwise unchanged.Removed props (all auto-derived now):
displaySizefloor(container size / (first child size + gap))bufferSizemax(2, ceil(visibleCount * 0.3))uniformSizefactorScaleAll other props (
each,wrap,scrollIndex,onEndReached,onEndReachedThreshold,debugInfo,children, plus inheritedscroll/selected/onSelectedChanged) are unchanged.VirtualGridis untouched.How it works
updateLayout()the container'swidth/heightand the first child's size are read;visibleCount,bufferSize, anditemSizeare cached in a single signal.computeSlicestate machine takes over — all the scroll / wrap / edge logic from before is preserved verbatim.Behavior changes (beyond the removed props)
width(Row) orheight(Column) on first layout — either explicit, or sized by a flex parent. If the container has no size on the first measurement attempt, the effect re-runs wheneachupdates.onEndReachedis now latched: it fires once when the cursor crosses the threshold, re-arms when the cursor leaves the zone oreachchanges. Previously it fired on every keypress inside the zone, so callers doing infinite scroll got duplicate fetches.flexBoundary='fixed'is now set on the container, and the default styles no longer includeflexWrap: wrapor theytransition.Context: what users are hitting with Virtual today
Two open community PRs (both against the wrap logic this PR carries over verbatim) are worth reading alongside this one — teams are patching these locally in their own libs while they wait:
wrapandConfig.animationsEnabled === false, every keypress overshoots the scroll position. Root cause: the non-animated path inonSelectedChangedappliesshiftByon top of a stale container position instead of first normalizing viaprevChildPos - active[axis]like the animated path does. This PR preserves that bug because it keepsonSelectedChangedunchanged; it should either rebase on fix: position calculation in createVirtual function with animations disabled #24 or absorb the one-line fix.wrapapplies its left-buffer offset immediately on mount, which looks wrong for a "first time on the page" state and makes an initial left press jump to the last item. The proposedskipInitialWrapdefers wrap until the user first navigates. This PR keeps the samedoOncemount-offset effect, so it neither fixes nor conflicts with the desire — but it will merge-conflict with feat(Virtual): add skipInitialWrap to defer wrap behavior until first navigation #25's implementation.The common thread: the incremental, mutation-based scroll bookkeeping (
shiftBy,originalPosition,targetPosition,doOnce,atStart) is where all the user-reported wrap bugs live. DerivingvisibleCountautomatically is nice, but it doesn't touch the part of Virtual that is actually costing users time.Known issues in this PR (to fix before merge)
src/primitives/index.tsaddsexport * from './Rail.jsx', butRail.jsxdoes not exist on this branch — leftover from another branch. Remove it (keep the withScrolling/handleNavigation reorder, which is legitimate).eachchanges. If the container is sized by a flex parent that hasn't laid out yet and the data never changes again, the component renders one item forever. Needs a retry on layout (e.g.onLayout) rather than only on data.scrollToIndex,updateSelected, andonSelectedChangedall silently no-op whilederivedDims()is undefined. Apps that restore scroll position right after mount will lose the call — queue it and replay after measurement instead.docs/primitives/virtual.mdsays the buffer isceil(visibleCount * 0.25); the code uses0.3.floor(containerSize / itemSize)undercounts when the design intentionally shows a partial item at the edge (standard TV rail pattern). The buffer usually hides this, butvisibleCountalso drives thetotal <= vc"render everything" branch and scroll anchoring — worth aceil(or+1) and a test.// viewRef.updateLayout();inmeasureAndInit.Ideas to simplify further
shiftBymutations with one absolute formula:position = base - windowOffset * itemSize. The animated path animates to that target; the non-animated path assigns it. One code path, and the entire class of stale-position bugs from fix: position calculation in createVirtual function with animations disabled #24 becomes impossible — no moreprevChildPos - active[axis]normalization,originalPosition, ortargetPositionbookkeeping.computeSlicea pure function of(cursor, total, vc, buf, scrollIndex, mode, wrap). Today it's a state machine overprevwithdelta,atStart, andshiftBythreading through a 3-mode × wrap switch. Start position is derivable directly: anchor the cursor atscrollIndex(or the edge), clamp ormod, andselected = cursor - start. That deletes most of the switch and makes the slice trivially testable.displaySizeas an optional override. When provided: no probe phase, no measurement, works with unmeasurable containers, and existing callers migrate with zero changes. When omitted: derive as this PR does. This turns a breaking change into an additive one and doubles as the escape hatch for the probe-mode edge cases above.min(each.length, smallDefault)items immediately and correct the window after measuring. Kills the one-frame single-item flash on slow TV hardware.doOnceeffect entirely instead of adding a prop to gate it.Test plan
npm run tsc— currently fails on the missingRail.jsxexport; re-verify after removing itnpm run lint— 0 errors (warnings are pre-existing)npm test— newtests/virtual.test.tsxcovers probe phase, slice windowing, navigation, latchedonEndReached,scrollToIndexclamping, wrap traversalVirtualRowandVirtualColumnto confirm one-frame measurement is imperceptible and scroll behavior matches v1wrap,scrollIndex, andscroll: 'edge' | 'always' | 'auto'modes still behave as before — specifically withConfig.animationsEnabled = false(the fix: position calculation in createVirtual function with animations disabled #24 scenario)width/height) measures correctly on first layout🤖 Generated with Claude Code