-
-
Notifications
You must be signed in to change notification settings - Fork 565
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: keyboard status navigation with j
/k
#2739
Conversation
Run & review this pull request in StackBlitz Codeflow. |
✅ Deploy Preview for elk-docs canceled.
|
✅ Deploy Preview for elk-zone ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
j
/k
b50ce2b
to
08f847f
Compare
3746e16
to
9d327f1
Compare
This implementation is working well if we disable the experimental virtual scrolling. When enabled and a post has a large height, the next post is not loaded until it is on-screen. As a result, this keyboard navigation code fails to find the next post to focus on and stack to the last post or is forced to go to the top post. We are using I'll disable this keyboard shortcut when the virtual scroller is enabled. But that means most users cannot use it as it is enabled by default. |
aa171a8
to
2d53047
Compare
I'm not sure about adding j/k only when virtual scrolling is disabled. It may push users to disable it and we could get performance reports later on. Maybe we could wait to see if a way to have it always enabled comes up? |
Right, that's not an ideal situation. OK, let's revert the message to encourage disabling virtual scrolling and seek a solution. |
…in shortcut help" This reverts commit aa171a8.
Thanks! |
resolves
j
/k
navigation part of #1858