Skip to content

feat(ui): tab cycles files/diff focus with sidebar+divider color shift#274

Open
aldevv wants to merge 3 commits intomodem-dev:mainfrom
aldevv:feat/section-focus
Open

feat(ui): tab cycles files/diff focus with sidebar+divider color shift#274
aldevv wants to merge 3 commits intomodem-dev:mainfrom
aldevv:feat/section-focus

Conversation

@aldevv
Copy link
Copy Markdown
Contributor

@aldevv aldevv commented May 10, 2026

Why

Changesets often have many files when you really want to read one in particular. Tab cycled the sidebar and the filter input instead of the diff, and nothing visually marked which pane held focus, so picking a file and settling into it was harder than it should be.

User-visible

Tab previously toggled between the sidebar file list and the file filter input, which made the filter the only path back to keyboard control of the diff. After this change Tab cycles between the sidebar and the diff stream itself, default focus on launch is the diff, and the filter is reached with /. While the sidebar holds focus its top border and the pane divider shift to theme.accent, and / step the file selection instead of scrolling the diff (the diff still follows because the new file header is aligned to the top).

image

What it does

  • New "diff" FocusArea, hoisted into src/ui/lib/focus.ts so App.tsx and useAppKeyboardShortcuts.ts share one source of truth.
  • Default focusArea is "diff" on launch.
  • SidebarPane accepts a focused prop; PaneDivider accepts highlighted. Both shift their border to theme.accent when the sidebar holds focus.
  • The filter input owns every keystroke (including Tab) so users can type literal characters; Esc still exits via the StatusBar.
  • New moveToFile(delta) on the review controller, wired through the keyboard hook so / step files when the sidebar is focused.
  • Step logic lives in a shared stepFileInList(files, currentId, delta, "clamp" | "wrap") helper. Both moveToFile (clamp) and findNextAnnotatedFile (wrap) delegate to it, which fixes a no-current-selection edge case: with no file selected, the first now lands on index 0 instead of skipping to index 1.

Tests

  • bun test src/ui/lib/files.test.ts — 8 new unit tests for stepFileInList (empty list, zero delta, clamp/wrap, boundary clamping, no-selection edge cases, single-item wrap).
  • bun test src/ui/hooks/useReviewController.test.tsxmoveToFile controller test stepping forward, backward, and clamping at the last file.
  • bun test src/ui/components/ui-components.test.tsxSidebarPane top-border colour shifts to theme.accent when focused.
  • bun test src/ui/AppHost.interactions.test.tsxTab cycles sidebar ↔ diff focus and only steps the file selection while the sidebar holds focus (also covers default-focus = "diff").
  • Existing PTY snapshot in test/pty/ui-integration.test.ts updated to match the new "Toggle files/diff focus" menu label.
  • bun run typecheck and bun run lint clean.

No docs or workflow changes needed.

Add a `"diff"` focus area so Tab cycles between the sidebar and the
diff stream rather than between the sidebar and the filter input.
Default focus on launch is `"diff"`. The sidebar's top border and the
pane divider shift to `theme.accent` while the sidebar holds focus and
revert to `theme.border` once focus returns to the diff. The filter
input is reached with `/`; Tab inside that input stays in the input so
users can type the literal character.

When the sidebar holds focus, `↑`/`↓` step the file selection (next or
previous visible file) instead of scrolling the diff. The diff still
follows because `selectFile` aligns the file header to the top.

Adds `moveToFile(delta)` to the review controller. The step logic lives
in a shared `stepFileInList` helper that both `moveToFile` and
`findNextAnnotatedFile` delegate to, so the no-current-selection edge
case (first ↓ landing on index 0, not 1) is fixed in one place. The
`FocusArea` type lives in `src/ui/lib/focus.ts` so the App and the
keyboard hook share one source of truth.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 10, 2026

Greptile Summary

This PR reworks the keyboard focus model to add a "diff" focus area as a third state alongside "files" and "filter", making Tab cycle between the sidebar and the diff stream (previously it cycled between the sidebar and the filter input), and defaulting focus to "diff" on launch. Arrow keys route to file navigation or diff scrolling based on which area is active, and the sidebar border/divider shift to theme.accent when the sidebar holds focus.

  • FocusArea type hoisted to src/ui/lib/focus.ts so App.tsx and useAppKeyboardShortcuts.ts share one definition; the old inline "files" | "filter" type is removed from both files.
  • stepFileInList helper added to files.ts, used by both the new moveToFile controller method (clamp) and the refactored findNextAnnotatedFile (wrap), fixing a prior edge case where a forward step with no selection skipped index 0.
  • Visual feedback: SidebarPane accepts focused and PaneDivider accepts highlighted; both shift their top border to theme.accent while the sidebar holds focus.

Confidence Score: 5/5

Safe to merge — the changes are confined to keyboard routing and visual feedback, with no data mutations or async side-effects.

All three focus-area transitions are covered by integration tests, stepFileInList has eight dedicated unit tests including every edge case called out in the PR description, and the findNextAnnotatedFile refactor is a clean delegation that also fixes the skip-index-0 bug. No logic changes touch persistence, networking, or auth.

No files require special attention.

Important Files Changed

Filename Overview
src/ui/lib/focus.ts New file that centralises the FocusArea type with clear JSDoc for all three states; clean single-responsibility module.
src/ui/lib/files.ts Adds stepFileInList helper with clean edge-case handling for clamp and wrap modes; correctly handles empty list, zero delta, out-of-bounds currentId, and boundary clamping.
src/ui/lib/reviewState.ts Refactors findNextAnnotatedFile to delegate to stepFileInList; changes no-selection behavior from skipping index 0 to landing on it — intentional fix, consistent with the new helper's design.
src/ui/hooks/useAppKeyboardShortcuts.ts Tab handling moved from handleFilterShortcut (where it toggled focus) to handleAppShortcut (where it still toggles between diff/files); filter handler now consumes all keys so the input can own Tab for literal typing.
src/ui/hooks/useReviewController.ts Adds moveToFile(delta) using stepFileInList with clamp mode and alignFileHeaderTop; dependency array correctly includes selectedFile?.id and visibleFiles.
src/ui/components/panes/SidebarPane.tsx Adds required focused: boolean prop; border color switches to theme.accent when focused — straightforward and correct.
src/ui/components/panes/PaneDivider.tsx Adds optional highlighted prop (default false); combines with isResizing into accent boolean for border color — logic is clean.
src/ui/App.tsx Default focusArea changed to 'diff', toggle function updated to cycle diff↔files, and moveToFile wired through keyboard options alongside the new component props.
src/ui/AppHost.interactions.test.tsx Adds integration test for Tab cycling and ↓ file stepping; updates existing filter tests to use '/' instead of Tab to open the filter — aligns with new UX.
src/ui/lib/files.test.ts Adds 8 unit tests for stepFileInList covering all edge cases (empty, zero delta, clamp/wrap boundaries, no-selection forward/backward, single-item wrap).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    Launch([App Launch]) --> FD[focusArea = 'diff']

    FD -->|Tab| FF[focusArea = 'files']
    FF -->|Tab| FD
    FD -->|'/'| FI[focusArea = 'filter']
    FF -->|'/'| FI
    FI -->|Esc via StatusBar| FD

    FF -->|up/down| MF[moveToFile stepFileInList clamp]
    FD -->|up/down| SD[scrollDiff step]
    FI -->|any key| OWN[Filter input owns keystroke]

    MF --> SF[selectFile alignFileHeaderTop]
    SF --> VF[diff scrolls to new file header]

    FF -->|border| ACC[SidebarPane + PaneDivider borderColor = theme.accent]
    FD -->|border| NRM[SidebarPane + PaneDivider borderColor = theme.border]

    style FD fill:#334,stroke:#66f
    style FF fill:#343,stroke:#6f6
    style FI fill:#433,stroke:#f66
Loading

Reviews (1): Last reviewed commit: "feat(ui): tab cycles files/diff focus wi..." | 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.

1 participant