fix: store scroll pos when virtualized#8767
Merged
Merged
Conversation
5 tasks
Member
|
Thanks for the PR. There still seems to be some bad scroll behaviours, I'm not sure if it's related? I've built your PR over at #8773
|
Contributor
|
@snowystinger Thanks for the build, we can take care of that issue also 👍 |
Contributor
|
@snowystinger The issue you discovered only exists in storybook because the keyboard modality is never recognized when the relatedTarget is outside the owner document. When you test this with adjacent inputs the issue is gone 👍 |
Member
|
I added buttons to the story in the PR where I built this to ease testing on our end. Thanks for chasing that down. |
snowystinger
approved these changes
Sep 5, 2025
Member
snowystinger
left a comment
There was a problem hiding this comment.
This appears to make sense, will get others on the team to review it now. Thank you!
reidbarber
approved these changes
Jan 7, 2026
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 #8234
This PR makes three changes:
1. Use isFocused instead of focusedKey for tabIndex
Previously, the tabIndex was determined by whether focusedKey was null. However, focusedKey persists even after tabbing out of the collection. This caused the collection root to remain non-tabbable, and as a result the browser would default to focusing the first or last focusable element inside the collection (e.g., a button) when tabbing back in. Once the browser focused that button, the focus logic ran and scrolled the focusedKey item into view — which led to the “jump” that was observed. By switching the condition to use isFocused instead, the collection root becomes tabbable whenever the collection itself is not focused. This prevents the browser from landing on inner focusable elements and eliminates the initial jump when tabbing back into the collection.
2. Always track scroll position
The useEvent(scrollRef, 'scroll', ...) handler is now always active, rather than being disabled for virtualized collections. This ensures the current scroll position is always stored, even when virtualization is used.
3. Always restore scroll position on focus
Similarly, the scroll position is now restored whenever the collection regains focus, regardless of whether it is virtualized. This prevents jumps when shift+tabbing back into the collection. I guess the issue #2233 is connected to the problem.
I am still curious why scroll position saving/restoring was originally disabled for virtualized collections. My assumption is that it may have been excluded to avoid interfering with virtualization behavior or for performance reasons.
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: