-
Notifications
You must be signed in to change notification settings - Fork 535
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(TreeView): add support for PageUp, PageDown #2546
Conversation
🦋 Changeset detectedLatest commit: 870904a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
I think most of this code probably belongs in the useRovingTabIndex.ts file
I checked out the branch locally, and this is the behavior I'm seeing: CleanShot.2022-11-08.at.10.18.33.mp4Looks like maybe a race condition between the focus change and the default PageUp/Down behavior? |
@colebemis seeing that too on the stress test one, let me dig into it and also check on other browsers too where |
size-limit report 📦
|
@colebemis just updated with a different approach, let me know what you think! Quick cross-browser check: Screen.Recording.2022-11-08.at.3.52.07.PM.mov |
@colebemis thanks for the feedback! Updated to try another heuristic with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works so well! ✨ Excellent job on the implementation. This ended up being a lot trickier than I expected.
Add in support for
PageUp
andPageDown
onTreeView
. The way this works is that when a user pressesPageDown
while focus is within aTreeView
we look for the firsttreeitem
following the currently active item that is not currently in the viewport. If one exists, we focus it.PageUp
works the same way but in reverse, look fortreeitem
's before the current item and if one is currently not in the viewport focus it.Approaches
useRovingTabIndex
but unfortunatelyuseFocusZone
will alwayspreventDefault()
on the PageUp, PageDown keys