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

Easy keyboard-only traversal between files #76

Open
lptr opened this issue Nov 5, 2024 · 2 comments
Open

Easy keyboard-only traversal between files #76

lptr opened this issue Nov 5, 2024 · 2 comments
Labels
enhancement New feature or request github This issue is about our GitHub App

Comments

@lptr
Copy link

lptr commented Nov 5, 2024

Is your feature request related to a problem? Please describe.

Currently it requires clicking on individual files to move between them while doing a review. This is tedious and breaks the flow of the review. GitHub shows the diff in a continuous stream where scrolling back-and-forth is easy.

Describe the solution you'd like

Adopting GitHub's continuous mode would be one way, perhaps as an option that can be toggled off and on.

Another option would be to add some keyboard shortcuts (j and k comes to mind) to move between files.

The two are not mutually exclusive, and indeed it would be nice to have both.

@lptr lptr added enhancement New feature or request github This issue is about our GitHub App labels Nov 5, 2024
@mmueller2012
Copy link
Contributor

Adding another keyboard shortcut is simple if we can find a suitable key combination.

Most CTRL/⌘ based shortcuts are already registered by either the browser or the operating system. We can override most of these shortcuts but this may not provide the best user experience.

Simply using keys without modifiers has its own drawbacks. The user may be in the middle of writing a comment, click on the code and lose focus on the text area without realizing it. If they continue typing, they could easily trigger a navigation event.

It is also not easy to find two keys that are next to each other in all/most keyboard layouts.

My current proposal would be CTRL/⌘ + arrow left/right:

  • We already register CTRL/⌘ + arrow up/down to jump between changes within a file.
  • The keys are always next to each other.
  • I don't think this combination is registered by any browser, but I'll have to double check Safari on macOS.
  • The combination can be used in input fields to jump between words, but I don't think it is a problem in this case. You need to look at the input field to see the cursor position and thus notice if it has lost focus.

@slackner: What do you think about this key combination and should the behavior depend on the current sidebar tab (i.e. jump between threads if the threads tab is selected)?

@slackner
Copy link
Contributor

slackner commented Nov 8, 2024

@mmueller2012 No objections from my side, but I'd wait for some more feedback, especially from @lptr.

Usage of single letter shortcuts like J/K would be easier, but I also see a risk that users will type it by accident while writing a comment. If we go in such a direction, we'd have to make sure that there are confirmation dialogs if there are still any unsubmitted comments etc.

Since the sidebar is collapsed on mobile and it's not easy to see which thread is selected, I don't think the meaning of these keyboard shortcuts should change. It may be better to add separate shortcuts for jumping between threads, e.g., by using a different keyboard modifier. Most obvious would be alt+left/right, maybe? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request github This issue is about our GitHub App
Projects
None yet
Development

No branches or pull requests

3 participants