Repository navigation
fix(Virtual, VirtualGrid): animate grid rows past row 3; scroll auto rows of displaySize items - #86
Merged
Conversation
…rows of displaySize items VirtualGrid: the row change read the rows' old position in applyShift, after the post-mutation flex pass had already laid out the new window. Once the window slid (row 3 on with the default buffer of 2) the rows jumped a row and the y transition did not run. The position is now read in onSelectedChanged, before the window changes. Virtual: with scroll="auto" (no wrap) and exactly displaySize items, the row never scrolled and the last item ended one slot past where it does for any other list length, out of view. That case now goes through the windowed path. Wrap and the other scroll modes keep the all-mounted path: switching them too broke "always" (last item unreachable) and wrap (items mounted twice, row off by a slot). Co-Authored-By: Claude Opus 5.5 <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.
Two bugs reported from the field.
VirtualGrid: the vertical transition stops working after the third row
When the focused row changes,
applyShiftrecords where the rows were so that it can animate them from there. It took that reading inside the microtask. The post-mutation flex pass had already run by then, so it read the new layout and the compensation was zero. As soon as the window starts sliding, every row change jumped a row instead of animating. With the defaultbufferof 2 that happens at row index 3, hence "after the third row".Fix: read
this.y + active.yinonSelectedChanged, beforesetSlicechanges the window.Virtual.tsxalready does this withshiftPrevChildPos.Virtual: with
items.length === displaySize, the last tile is not scrolled into viewWith
scroll="auto"(the default) and no wrap, the row scrolls one slot for any longer list, and the last item ends at slotdisplaySize - 2. A list of exactlydisplaySizeitems took the "everything fits" path instead. The row never scrolled, and the last item ended one slot further right, out of view.The suggested fix was to change
total <= displaySizetototal < displaySize. I measured the focused item's position at each press in every scroll mode, with and without wrap, for 3–6 items withdisplaySize4. That change fixesauto, but it breaks:scroll="always": the last item can no longer be reached (focus stops one short).wrapmode: the row is one slot off and the same item is mounted twice.Fix: only
autowithout wrap at exactlydisplaySizeitems takes the windowed path now. After this change, that case is the only one whose positions changed. Going back left, it follows the same path as longer lists.Tests
contract-virtual.test.tsx:autowith 4 items anddisplaySize4, Right ×4 then Left ×3, checking the focused item's screen x at each step.contract-virtual-grid.test.tsx: with animations on (frames driven the same way as the existing B14 test), Down to row 6 and Up back to row 0. Each press must start aytransition, no mounted cell may move before it starts, and the grid must end at the righty. Without the fix it fails withArrowDown to row 3: no y transition.Both tests fail without their fix.
npm test(757 passed),npm run tscandnpm run lintpass. I have not checked either fix in an app on the WebGL renderer; the tests run on the DOM renderer in jsdom.Not fixed (pre-existing, for a reviewer to know)
scroll="always"already skips an item when the count is betweendisplaySizeanddisplaySize + bufferSize. For example, with 5 items anddisplaySize4 the focus goes v2 → v4. Theselectedmath assumes the window start was never clamped.wrapwith exactlydisplaySizeitems still doesn't scroll. Changing that means deciding how wrap fills a window that has more slots than there are items.🤖 Generated with Claude Code