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 key navigation between index and workdir files #921

Merged

Conversation

fo2rist
Copy link
Contributor

@fo2rist fo2rist commented Mar 11, 2023

Navigate to index files controller when Down button pressed while on the last file of the workdir list
Navigate to workdir files controller when Up button pressed while on the first files of the index list

Context: I'm using keyboard navigation for most of the actions in GitUp but there were no way to quickly navigate b/w staged and un-staged files, and while it's possible with Tab/Shift+Tab it breaks natural up/down navigation that's available in simple mode. This change adds that ability to standard mode as well.

Testing and considerations:

  • I checked that simple mode still works the same as before
  • I added checks to make sure there is not confusing behavior when Arrow key is pressed with a modifier or when multiple files selected to avoid losing selection
  • I added checks to prevent navigation to an empty list
  • No changes to the documentation page is needed since it simply mentions Arrow up & Arrow down to select a different file which is still true

Navigate to index files controller when Down button pressed while on the
last file of the workdir list
Navigate to workdir files controller when Up button pressed while on the
first files of the index list
@fo2rist
Copy link
Contributor Author

fo2rist commented Mar 11, 2023

Would like to discuss a possibility to add arrow navigation between individual lines in the diff view.
That would allow me to do complete change/review/commit cycle with keyboard.

@lucasderraugh
Copy link
Collaborator

Thanks for writing this up. I'll take some time tonight or tomorrow to review what's being changed here.

Copy link
Collaborator

@lucasderraugh lucasderraugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay in reviewing this. Looks good to me and welcome functionality.
Small nits just to change the type to match the modifierFlags property.
I was also thinking if there was an easier way to just say there were no modifiers, but I guess it includes caps lock and things like that, so this looks appropriate.

Other than that, it's ready to merge.

@@ -29,3 +30,5 @@ typedef NS_ENUM(unsigned short, GIKeyCode) {
kGIKeyCode_Return = 0x24,
kGIKeyCode_Delete = 0x33
};

static const long kGIKeyModifiersAll = NSEventModifierFlagShift | NSEventModifierFlagControl | NSEventModifierFlagCommand | NSEventModifierFlagOption;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static const long kGIKeyModifiersAll = NSEventModifierFlagShift | NSEventModifierFlagControl | NSEventModifierFlagCommand | NSEventModifierFlagOption;
static const NSEventModifierFlags kGIKeyModifiersAll = NSEventModifierFlagShift | NSEventModifierFlagControl | NSEventModifierFlagCommand | NSEventModifierFlagOption;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

GitUpKit/Views/GIAdvancedCommitViewController.m Outdated Show resolved Hide resolved
Use more precise type for long flags
@lucasderraugh lucasderraugh merged commit cf6f49d into git-up:master May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants