Skip to content

feat(ui): add comma and period shortcuts to move between files#280

Open
mvanhorn wants to merge 1 commit intomodem-dev:mainfrom
mvanhorn:feat/255-file-list-keyboard-navigation
Open

feat(ui): add comma and period shortcuts to move between files#280
mvanhorn wants to merge 1 commit intomodem-dev:mainfrom
mvanhorn:feat/255-file-list-keyboard-navigation

Conversation

@mvanhorn
Copy link
Copy Markdown

Summary

Adds keyboard shortcuts to move between files in the current diff -- , for previous file, . for next file. Mirrors how [/] walks hunks, but at the file level.

Why this matters

Per #212 your suggestion: "If the purpose is to just scroll up/down on the file list, what if I just added a keyboard shortcut for file up/down?" This PR ships exactly that -- a global shortcut that works from the diff pane without needing a sidebar-focus state machine.

moveToAnnotatedFile (useReviewController.ts:238) was the architecturally analogous existing function -- it walks annotatedFiles and calls selectFile. This PR adds moveToFile next to it. Two deliberate divergences vs. moveToAnnotatedFile:

  • Walks visibleFiles (all files, not just annotated). The whole point of the shortcut is to reach every file.
  • Clamps at the boundaries rather than wrapping. Wrap is fine for a small annotated-file set; for arbitrary file lists, clamp matches scroll semantics and avoids surprise when the cursor leaves the visible range and reappears at the other end. Happy to switch to wrap if you'd prefer consistency with moveToAnnotatedFile.
  • Calls selectFile(nextFile.id, 0, { alignFileHeaderTop: true }) so the diff pane aligns the file header to the top, matching the mouse-click behavior the original issue described as jumpToFile(fileId, 0, { alignFileHeaderTop: true }).

Key binding choice

Picked , and . because both f and j/k are already taken (page-down and step-down/up respectively per keyboard.ts:13-35). The comma/period pair is unbound and reads naturally as prev/next in many TUIs.

If you'd prefer a different pair, let me know -- changing the binding is a one-line tweak.

Changes

  • src/ui/hooks/useReviewController.ts: new moveToFile(delta) method. Clamps to [0, visibleFiles.length - 1]; no-op at boundaries (no re-fire of selectFile).
  • src/ui/hooks/useAppKeyboardShortcuts.ts: binds ,moveToFile(-1) and .moveToFile(1) inside handleAppShortcut, so the filter-area short-circuit at lines 228-240 automatically prevents firing while typing in the filter.
  • src/ui/App.tsx: threads moveToFile from the review controller into the keyboard shortcuts hook (mirrors moveToHunk).
  • src/ui/components/chrome/HelpDialog.tsx: new row [", / ."]"previous / next file", placed next to the existing [ / ] hunk row.
  • src/ui/hooks/useReviewController.test.tsx: unit tests covering mid-list step, clamp at first file (no-op), clamp at last file (no-op), and that selectFile is called with alignFileHeaderTop: true.
  • src/ui/AppHost.interactions.test.tsx: integration test that exercises the key bindings end-to-end through the renderer.

Testing

  • bun run typecheck -- clean
  • bun run format:check -- clean
  • bun run lint -- clean (0 warnings)
  • bun test ./src/ui/hooks/useReviewController.test.tsx ./src/ui/AppHost.interactions.test.tsx -- 48 pass, 0 fail
  • Verified no collision: , and . are not bound anywhere in keyboard.ts or useAppKeyboardShortcuts.ts. Existing shortcuts [/], j/k, f, Space, etc. behave unchanged.

Existing CONTRIBUTING.md rule honored: "Selecting a file should jump within the main stream, not collapse the review to one file." This PR jumps within the main stream (no view-mode change).

Fixes #255

AI was used for assistance.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 10, 2026

Greptile Summary

Adds , and . keyboard shortcuts to move backwards and forwards through all visible files in the diff, analogous to how [/] walk hunks. The feature clamps at list boundaries instead of wrapping.

  • moveToFile(delta) is added to useReviewController.ts, walking visibleFiles with clamp, and calling selectFile with alignFileHeaderTop: true — resetting the hunk index to 0 and scrolling the file header to the top on every jump.
  • The shortcuts are wired through useAppKeyboardShortcuts.ts inside handleAppShortcut, so the existing handleFilterShortcut early-return automatically blocks them while the filter input has focus.
  • Unit tests in useReviewController.test.tsx and an end-to-end integration test in AppHost.interactions.test.tsx cover mid-list navigation, boundary clamping, and filter-focus suppression.

Confidence Score: 5/5

Safe to merge — the change adds a purely additive keyboard shortcut with no risk to existing navigation paths.

The implementation is self-contained: moveToFile cannot affect state when no file is selected or when already at a boundary, filter-focus suppression is inherited from the existing handler chain and verified by an integration test, and all six changed files have clear, narrowly scoped diffs with matching test coverage.

No files require special attention.

Important Files Changed

Filename Overview
src/ui/hooks/useReviewController.ts Adds moveToFile(delta) using a locally-defined clamp helper; correctly handles empty file list (via currentIndex < 0 guard), single-file list, and boundary clamping; resets hunk index to 0 on every jump.
src/ui/hooks/useAppKeyboardShortcuts.ts Adds , / . bindings inside handleAppShortcut; filter-focus protection is inherited from the existing handleFilterShortcut short-circuit. Slight style inconsistency: [/] check only key.name, while ,/. check both key.name and key.sequence, but this is harmless defensive coding.
src/ui/hooks/useReviewController.test.tsx Unit tests cover mid-list forward/backward navigation, forward clamp at last file (no-op), backward clamp at first file (no-op), and selectedFileTopAlignRequestId increment tracking; all assertions align with the implementation.
src/ui/AppHost.interactions.test.tsx Integration test verifies . advances to the second file, , returns to the first file, and that pressing Tab (to focus the filter) before . leaves the selection unchanged; covers the expected real-user path end-to-end.
src/ui/components/chrome/HelpDialog.tsx New [", / ."] row added between the hunk and comment shortcut entries; placement and description are consistent with existing entries.
src/ui/App.tsx Threads moveToFile from the review controller into useAppKeyboardShortcuts, mirroring the existing moveToHunk / moveToAnnotatedHunk pattern; no functional concerns.

Sequence Diagram

sequenceDiagram
    participant User
    participant useKeyboard
    participant handleFilterShortcut
    participant handleAppShortcut
    participant moveToFile
    participant selectFile

    User->>useKeyboard: press "," or "."
    useKeyboard->>handleFilterShortcut: key event
    alt filter is focused
        handleFilterShortcut-->>useKeyboard: return true (consumed)
    else filter not focused
        handleFilterShortcut-->>useKeyboard: return false
        useKeyboard->>handleAppShortcut: key event
        handleAppShortcut->>moveToFile: moveToFile(-1) or moveToFile(+1)
        moveToFile->>moveToFile: findIndex in visibleFiles
        alt no selected file or boundary reached
            moveToFile-->>handleAppShortcut: no-op (return early)
        else valid next file
            moveToFile->>selectFile: selectFile(nextFile.id, 0, alignFileHeaderTop: true)
            selectFile-->>moveToFile: selection updated
        end
    end
Loading

Reviews (1): Last reviewed commit: "feat(ui): add , and . shortcuts to move ..." | Re-trigger Greptile

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.

Keyboard navigation for sidebar file list (Tab to focus, arrows to move, Enter/Space to jump)

1 participant