-
-
Notifications
You must be signed in to change notification settings - Fork 923
Add arrow key navigation for node catalog #3236
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?
Add arrow key navigation for node catalog #3236
Conversation
!build |
|
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.
Thanks for the PR, Good UX improvement. Personally looking forward to using this.
{disabled} | ||
label={nodeType.name} | ||
tooltip={$nodeGraph.nodeDescriptions.get(nodeType.name)} | ||
emphasized={nodeType == flatNodeList[focusedNodeIndex]} |
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.
Replaced emphasized with focused in NodeCatalog
@timon-schelling I added 'focused' prop to TextButton, is that ok ? |
!build |
I will take a look once the build is done. |
|
!build |
}); | ||
} | ||
function keyboardNavigationHandler(e: KeyboardEvent) { |
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.
I'm relatively sure keavon will not like that this doesn't use the standard keyboard mapping. Should probably be handled in editor and only the visual aspect should be in the frontend. Would also allow showing the shortcuts in the bar at the bottom of the screen. You will need to look into how similar things are implemented.
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.
If it doesn't break editor interactions, that's an ok compromise for now to not route this through the editor backend. The intention is to fully replace the node catalog menu in the future, it has always been a placeholder.
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.
It doesn't break editor interactions. Then LGTM
|
Implements navigation in node catalog using up and down arrow keys and node selection using enter key.
Selected node is emphasized.
Closes #3235