Skip to content
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

Add keyboard shortcuts to the GUI #210

Merged

Conversation

rbs-jacob
Copy link
Member

@rbs-jacob rbs-jacob commented Feb 6, 2023

One sentence summary of this PR (This should go in the CHANGELOG!)

Add keyboard shortcuts to the GUI.

Please describe the changes in your request.

This PR adds support for keyboard shortcuts to the GUI. There is an (importable) global object for storing the shortcut keybindings and their callbacks. Any time a key is pressed outside of an input or contentEditable element, that object is searched and the respective callback is called.

All of the toolbar buttons with associated shortcuts show their respective shortcut in a tooltip that appears when the button is hovered.

Any component can import and update the shortcuts object. Thus callbacks can close over reactive variables local to the component.

This design has a few nice properties:

  • It's pretty simple
  • Keyboard shortcuts can be created within the components that implement functionality, removing code duplication and indirection, and reducing maintenance burden
  • The importable list of shortcuts is viewable and modifiable by any component, so it is possible to list all of the bound shortcuts, as well as rebind them later
  • If we eventually support plugins in the form of components, they will be able to set their own keyboard shortcuts by modifying this object

For this first pass, I set a couple of shortcuts that can always be overridden later:

  • i – identify the current resource
  • u – unpack the current resource
  • a – analyze the current resource
  • p – pack the current resource
  • n – new resource
  • Shift+u – recursively unpack the current resource
  • Shift+p – recursively pack the current resource
  • g – focus on the "go to offset" input
  • h OR – collapse the current resource tree node
  • l OR – expand the current resource tree node
  • k OR – focus one resource up in the tree
  • j OR – focus one resource down in the tree

Anyone you think should look at this, specifically?

@whyitfor @EdwardLarson

@rbs-jacob rbs-jacob requested a review from whyitfor February 6, 2023 21:54
@SamL98
Copy link
Collaborator

SamL98 commented Feb 7, 2023

The shortcuts work great but I will say that not having j and k (or up and down) sort of defeats the purpose of having them imo.

@EdwardLarson
Copy link
Collaborator

The shortcuts work great but I will say that not having j and k (or up and down) sort of defeats the purpose of having them imo.

@SamL98 do you mean hotkeys to navigate around, selecting different resources? I sort of agree, because with that the GUI is nearly fully usable with hotkeys so the feature is "complete". But @rbs-jacob has said this is more just adding the infrastructure for hotkeys themselves, I think adding navigation will require a bit more code besides just adding a button. I think we could do without navigation, at least initially.

@rbs-jacob
Copy link
Member Author

I'll add j and k, and then this will be ready to merge.

@rbs-jacob rbs-jacob requested review from EdwardLarson and removed request for whyitfor February 8, 2023 21:47
@whyitfor
Copy link
Contributor

whyitfor commented Feb 8, 2023

@rbs-jacob this looks cool! Would be nice to have a key bindings page in the docs, no?

@rbs-jacob rbs-jacob merged commit 785ada4 into redballoonsecurity:master Feb 9, 2023
marczalik pushed a commit to marczalik/ofrak that referenced this pull request Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants