fix: keep virtualized async ListBox focus visible#10165
Open
000vincent999 wants to merge 3 commits into
Open
Conversation
| ); | ||
| let refreshVisibleRectRef = useContext(RefreshVisibleRectContext); | ||
| if (refreshVisibleRectRef) { | ||
| refreshVisibleRectRef.current = refreshVisibleRect; |
Member
There was a problem hiding this comment.
This breaks the rules of hooks, cannot read or write to a ref during render
snowystinger
left a comment
Member
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
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 #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