-
-
Notifications
You must be signed in to change notification settings - Fork 124
Allow stepping through search results using arrow up and down keys #483
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
base: master
Are you sure you want to change the base?
Conversation
e0b9354 to
2ea94f6
Compare
|
@manisandro Hi, did you manage to take a look at this pull request yet? If you have any suggestions on how to improve this feature, please let me know. Cheers |
|
@BenediktSeidlSWM Sorry, didn't yet get around properly reviewing this. As a very general remark, it would be great to introduce a generic and reusuable keyboard navigatable list component to hide the complexity in consumer components. |
| this.resultIndex = 0; | ||
| // Reset the reference to the selected result | ||
| this.selectedResultEl = null; | ||
| // Build the different sections of the search results |
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.
Can we be sure that the menu will never be re-rendered while key-navigating?
|
I will take a look into how a reusable list component could be split out of this plugin. One thing I noticed which increases the complexity of this component is the support of nested search results. It becomes relevant at this line. @manisandro Do you know how relevant it is that these results are nested in the DOM? Maybe the indentation of each result could be passed in some other way (recursion depth as a variable). If the new list component supported only a flat list without any nested structures, that would make a refactoring of the component less complex. |
|
I think a flat list with indentation should also work well |
No description provided.