Skip to content

feat(review): add horizontal code-column scrolling#171

Merged
benvinegar merged 3 commits into
mainfrom
feat/review-horizontal-code-scroll
Apr 6, 2026
Merged

feat(review): add horizontal code-column scrolling#171
benvinegar merged 3 commits into
mainfrom
feat/review-horizontal-code-scroll

Conversation

@benvinegar

@benvinegar benvinegar commented Apr 6, 2026

Copy link
Copy Markdown
Member

Summary

  • add horizontal code-column scrolling for long diff lines in nowrap mode while keeping gutters, headers, separators, and note chrome fixed
  • support keyboard and mouse controls with arrow keys, faster shift-arrow movement, and shift-wheel scrolling while resetting the horizontal offset when wrap mode toggles
  • clamp scrolling to the visible code viewport and add renderer, app interaction, PTY integration, and TTY smoke coverage for the reveal behavior

Testing

  • bun run typecheck
  • bun test src/ui/AppHost.interactions.test.tsx src/ui/components/ui-components.test.tsx
  • bun run test:integration
  • bun run test:tty-smoke

Keep gutters and review chrome fixed while letting long diff lines be inspected without switching to wrapped mode. Add arrow-key controls, help text, and targeted coverage for the new reveal path.
@greptile-apps

greptile-apps Bot commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR prototypes shared horizontal code-column scrolling for the diff review stream. Left/right arrow keys update a single codeHorizontalOffset state in App, which is threaded as a prop through DiffPane → DiffSection → PierreDiffView → DiffRowView and applied in renderInlineSpans via a new sliceSpansWindow helper. Gutters, hunk headers, and note chrome are unaffected by the offset. The feature is correctly guarded against wrap mode and is covered by both interaction and renderer unit tests.

Key changes:

  • New sliceSpansWindow helper in renderRows.tsx slices styled span sequences to a visible horizontal window while preserving color runs
  • maxCodeHorizontalOffset computed from the widest line across all filtered files' addition, deletion, and context lines
  • codeHorizontalOffset clamped to [0, maxCodeHorizontalOffset] and no-ops when wrap mode is active
  • Help dialog updated with ← / → scroll code entry in the Navigation section
  • DiffRowView memo comparator extended with codeHorizontalOffset to avoid stale renders

Confidence Score: 5/5

Safe to merge — both remaining findings are P2 style suggestions with no correctness or crash risk

Prop threading is complete through all render layers, the memo comparator is correctly extended, left/right key handling respects menu-open precedence, and the feature is gated off in wrap mode. Integration and renderer unit tests provide solid coverage of the new behavior. Both flagged items are minor UX polish concerns.

src/ui/App.tsx lines 42–44 (tab-width assumption) and lines 240–250 (over-scroll bound)

Important Files Changed

Filename Overview
src/ui/App.tsx Adds codeHorizontalOffset state, maxCodeHorizontalOffset memo, and scrollCodeHorizontally callback; threads offset to DiffPane — max-offset bound is slightly over-permissive
src/ui/diff/renderRows.tsx Adds sliceSpansWindow helper and horizontalOffset parameter to renderInlineSpans; threads codeHorizontalOffset through renderRow and DiffRowView memo — tab-width assumption worth confirming
src/ui/hooks/useAppKeyboardShortcuts.ts Binds left/right arrow keys to scrollCodeHorizontally in both normal and pager mode with correct menu-precedence ordering
src/ui/diff/PierreDiffView.tsx Adds codeHorizontalOffset prop (default 0) and passes it to DiffRowView
src/ui/components/panes/DiffPane.tsx Passes codeHorizontalOffset prop down to DiffSection; no logic changes
src/ui/components/panes/DiffSection.tsx Passes codeHorizontalOffset to PierreDiffView; prop added to memo comparator correctly
src/ui/components/chrome/HelpDialog.tsx Adds left/right arrow scroll code entry to Navigation section
src/ui/AppHost.interactions.test.tsx Adds integration test verifying left/right arrows reveal offscreen columns and restore original position
src/ui/components/ui-components.test.tsx Adds PierreDiffView unit test verifying codeHorizontalOffset shifts visible content correctly

Sequence Diagram

sequenceDiagram
    actor User
    participant KB as useAppKeyboardShortcuts
    participant App
    participant DP as DiffPane
    participant DS as DiffSection
    participant PDV as PierreDiffView
    participant DRV as DiffRowView
    participant RIS as renderInlineSpans

    User->>KB: left/right arrow key
    KB->>App: scrollCodeHorizontally(+/-1)
    App->>App: guard: wrapLines=false AND maxCodeHorizontalOffset > 0
    App->>App: setCodeHorizontalOffset(clamp(current+/-1, 0, max))
    App->>DP: codeHorizontalOffset prop
    DP->>DS: codeHorizontalOffset prop
    DS->>PDV: codeHorizontalOffset prop
    PDV->>DRV: codeHorizontalOffset prop
    DRV->>RIS: horizontalOffset arg
    RIS->>RIS: sliceSpansWindow(spans, offset, width)
Loading

Reviews (1): Last reviewed commit: "feat(review): prototype horizontal code-..." | Re-trigger Greptile

Comment thread src/ui/App.tsx
Comment thread src/ui/App.tsx Outdated
Make long diff lines easier to inspect by supporting faster shift-arrow movement and shift-wheel code scrolling. Reset the horizontal offset when wrapping toggles so the reveal state stays predictable.
Clamp horizontal reveal to the visible code viewport and share the renderer's tab-expansion math so long lines stop overscrolling into blank space. Preserve vertical position for shift-wheel scrolling and add PTY coverage for real-terminal horizontal reveal and wrap reset behavior.
@benvinegar benvinegar changed the title feat(review): prototype horizontal code-column scrolling feat(review): add horizontal code-column scrolling Apr 6, 2026
@benvinegar
benvinegar merged commit 05416a3 into main Apr 6, 2026
3 checks passed
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