fix: keep virtualized async ListBox focus visible#10165
Conversation
| ); | ||
| let refreshVisibleRectRef = useContext(RefreshVisibleRectContext); | ||
| if (refreshVisibleRectRef) { | ||
| refreshVisibleRectRef.current = refreshVisibleRect; |
There was a problem hiding this comment.
This breaks the rules of hooks, cannot read or write to a ref during render
There was a problem hiding this comment.
Fixed the hook rule violation by moving the refreshVisibleRectRef.current update into a layout effect and clearing it on cleanup.
I also tried narrowing this to only the ListLayout persisted-key fix by removing the virtualizer refresh path, but the browser regression still failed: focus moved to the newly loaded item, while the item remained outside the listbox viewport. So the refresh path is still needed to keep the virtualizer visible rect in sync after programmatic scrolling.
Verified with:
yarn jest packages/react-stately/test/virtualizer/ListLayout.test.ts --runInBand
nvm use 22.22.2
yarn vitest --config=vitest.browser.config.ts packages/react-aria-components/test/ListBox.browser.test.tsx --run
snowystinger
left a comment
There was a problem hiding this comment.
Thank you for the interest. This is a lot of very involved changes, maybe it'd be good to write up a more formal RFC for this to get comments on the direction. https://github.com/adobe/react-spectrum/blob/main/rfcs/template.md
|
Opened an RFC PR for the broader virtualizer visible rect refresh direction: #10309. This implementation PR is still useful as a concrete reference for the proposed approach and regression coverage, but I’m happy to pause/adjust it based on the RFC discussion. The RFC separates the fix into two parts:
For context, I also tested narrowing this implementation to only the |
Closes #10129
Summary
This fixes keyboard navigation for virtualized async ListBox collections when pressing End or paging after more items have loaded.
Previously,
focusedKeycould move to the newest last item, butListLayoutdid not always compute layout info for that focused/persisted key. The old fallback only forced a full layout whenrequestedRect.area < contentSize.area. Because the virtualizer overscans on both axes, a vertical list can get extra cross-axis width inrequestedRect, making the area comparison look large enough even though the newly loaded focused item has not been laid out. As a result, the item was not mounted and could not be scrolled into view.The fix makes
ListLayoutkey-driven for this fallback: if a persisted key is missing fromlayoutNodesbut exists in the current collection, it computes the full layout so the focused item can be rendered. This avoids relying on cross-axis-sensitive area comparisons. The change also refreshes the virtualizer visible rect after programmatic focus scrolling and re-scrolls virtualized collections when the collection object changes while the focused key remains the same.✅ Pull Request Checklist:
📝 Test Instructions:
yarn jest packages/react-stately/test/virtualizer/ListLayout.test.ts --runInBandyarn vitest --config=vitest.browser.config.ts packages/react-aria-components/test/ListBox.browser.test.tsxLocal verification:
yarn jest packages/react-stately/test/virtualizer/ListLayout.test.ts --runInBandyarn playwright installbefore the browser test command.🧢 Your Project:
Personal contribution