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

Author: benvinegar

src/ui/App.tsx

@@ -14,6 +14,11 @@ import { StatusBar } from "./components/chrome/StatusBar";
 import { DiffPane } from "./components/panes/DiffPane";
 import { SidebarPane } from "./components/panes/SidebarPane";
 import { PaneDivider } from "./components/panes/PaneDivider";
+import {
+  findMaxLineNumber,
+  maxFileCodeLineWidth,
+  resolveCodeViewportWidth,
+} from "./diff/codeColumns";
 import { useAppKeyboardShortcuts } from "./hooks/useAppKeyboardShortcuts";
 import { useHunkSessionBridge } from "./hooks/useHunkSessionBridge";
 import { useMenuController } from "./hooks/useMenuController";
@@ -26,6 +31,8 @@ import { resolveTheme, THEMES } from "./themes";
 
 type FocusArea = "files" | "filter";
 
+const FAST_CODE_HORIZONTAL_SCROLL_COLUMNS = 8;
+
 const LazyHelpDialog = lazy(async () => ({
   default: (await import("./components/chrome/HelpDialog")).HelpDialog,
 }));
@@ -99,6 +106,7 @@ export function App({
   const [showAgentNotes, setShowAgentNotes] = useState(bootstrap.initialShowAgentNotes ?? false);
   const [showLineNumbers, setShowLineNumbers] = useState(bootstrap.initialShowLineNumbers ?? true);
   const [wrapLines, setWrapLines] = useState(bootstrap.initialWrapLines ?? false);
+  const [codeHorizontalOffset, setCodeHorizontalOffset] = useState(0);
   const [showHunkHeaders, setShowHunkHeaders] = useState(bootstrap.initialShowHunkHeaders ?? true);
   const [sidebarVisible, setSidebarVisible] = useState(true);
   const [forceSidebarOpen, setForceSidebarOpen] = useState(false);
@@ -168,6 +176,26 @@ export function App({
   const diffPaneWidth = renderSidebar
     ? Math.max(DIFF_MIN_WIDTH, availableCenterWidth - clampedSidebarWidth)
     : Math.max(0, availableCenterWidth);
+  const diffContentWidth = Math.max(12, diffPaneWidth - 2);
+  const maxVisibleLineNumber = useMemo(
+    () =>
+      filteredFiles.reduce(
+        (maxLineNumber, file) => Math.max(maxLineNumber, findMaxLineNumber(file)),
+        1,
+      ),
+    [filteredFiles],
+  );
+  const maxLineNumberDigits = String(maxVisibleLineNumber).length;
+  const codeViewportWidth = useMemo(
+    () =>
+      resolveCodeViewportWidth(
+        resolvedLayout,
+        diffContentWidth,
+        maxLineNumberDigits,
+        showLineNumbers,
+      ),
+    [diffContentWidth, maxLineNumberDigits, resolvedLayout, showLineNumbers],
+  );
   const isResizingSidebar = resizeDragOriginX !== null && resizeStartWidth !== null;
   const dividerHitLeft = Math.max(
     1,
@@ -219,6 +247,34 @@ export function App({
     diffScrollRef.current?.scrollBy(delta, unit);
   };
 
+  const maxCodeHorizontalOffset = useMemo(
+    () =>
+      Math.max(
+        0,
+        filteredFiles.reduce(
+          (maxWidth, file) => Math.max(maxWidth, maxFileCodeLineWidth(file)),
+          0,
+        ) - codeViewportWidth,
+      ),
+    [codeViewportWidth, filteredFiles],
+  );
+
+  useEffect(() => {
+    setCodeHorizontalOffset((current) => clamp(current, 0, maxCodeHorizontalOffset));
+  }, [maxCodeHorizontalOffset]);
+
+  /** Shift the visible code columns horizontally without moving gutters or headers. */
+  const scrollCodeHorizontally = useCallback(
+    (delta: number) => {
+      if (wrapLines || delta === 0 || maxCodeHorizontalOffset <= 0) {
+        return;
+      }
+
+      setCodeHorizontalOffset((current) => clamp(current + delta, 0, maxCodeHorizontalOffset));
+    },
+    [maxCodeHorizontalOffset, wrapLines],
+  );
+
   /** Toggle the global agent note layer on or off. */
   const toggleAgentNotes = () => {
     setShowAgentNotes((current) => !current);
@@ -234,6 +290,7 @@ export function App({
     // Capture the pre-toggle viewport position synchronously so DiffPane can restore the same
     // top-most source row after wrapped row heights change.
     wrapToggleScrollTopRef.current = diffScrollRef.current?.scrollTop ?? 0;
+    setCodeHorizontalOffset(0);
     setWrapLines((current) => !current);
   };
 
@@ -491,6 +548,7 @@ export function App({
     openMenu,
     pagerMode,
     requestQuit,
+    scrollCodeHorizontally,
     scrollDiff,
     selectLayoutMode: setLayoutMode,
     showHelp,
@@ -559,7 +617,6 @@ export function App({
   );
   const topTitle = `${bootstrap.changeset.title}  +${totalAdditions}  -${totalDeletions}`;
   const sidebarTextWidth = Math.max(8, clampedSidebarWidth - 2);
-  const diffContentWidth = Math.max(12, diffPaneWidth - 2);
   const diffHeaderStatsWidth = Math.min(24, Math.max(16, Math.floor(diffContentWidth / 3)));
   const diffHeaderLabelWidth = Math.max(8, diffContentWidth - diffHeaderStatsWidth - 1);
   const diffSeparatorWidth = Math.max(4, diffContentWidth - 2);
@@ -636,6 +693,7 @@ export function App({
         ) : null}
 
         <DiffPane
+          codeHorizontalOffset={codeHorizontalOffset}
           diffContentWidth={diffContentWidth}
           files={filteredFiles}
           pagerMode={pagerMode}
@@ -656,6 +714,9 @@ export function App({
           theme={activeTheme}
           width={diffPaneWidth}
           onOpenAgentNotesAtHunk={openAgentNotesAtHunk}
+          onScrollCodeHorizontally={(delta) => {
+            scrollCodeHorizontally(delta * FAST_CODE_HORIZONTAL_SCROLL_COLUMNS);
+          }}
           onSelectFile={jumpToFile}
         />
       </box>

src/ui/AppHost.interactions.test.tsx

@@ -297,6 +297,10 @@ function firstVisibleAddedLine(frame: string) {
   return frame.match(/line\d{2} = 1\d{2}/)?.[0] ?? null;
 }
 
+function firstVisibleAddedLineNumber(frame: string) {
+  return frame.match(/▌\s*(\d+)\s+\+/)?.[1] ?? null;
+}
+
 describe("App interactions", () => {
   test("keyboard shortcuts toggle notes, line numbers, and hunk metadata", async () => {
     const setup = await testRender(<AppHost bootstrap={createSingleFileBootstrap()} />, {
@@ -450,6 +454,235 @@ describe("App interactions", () => {
     }
   });
 
+  test("left and right arrows can reveal offscreen code columns in nowrap mode", async () => {
+    const setup = await testRender(<AppHost bootstrap={createWrapBootstrap()} />, {
+      width: 92,
+      height: 20,
+    });
+
+    try {
+      await flush(setup);
+
+      let frame = setup.captureCharFrame();
+      expect(frame).toContain("this is a very");
+      expect(frame).not.toContain("interaction coverage");
+
+      for (let index = 0; index < 64; index += 1) {
+        await act(async () => {
+          await setup.mockInput.pressArrow("right");
+        });
+        await flush(setup);
+        frame = setup.captureCharFrame();
+        if (frame.includes("interaction coverage")) {
+          break;
+        }
+      }
+
+      expect(frame).toContain("interaction coverage");
+      expect(frame).not.toContain("this is a very");
+
+      for (let index = 0; index < 64; index += 1) {
+        await act(async () => {
+          await setup.mockInput.pressArrow("left");
+        });
+        await flush(setup);
+        frame = setup.captureCharFrame();
+        if (frame.includes("this is a very")) {
+          break;
+        }
+      }
+
+      expect(frame).toContain("this is a very");
+    } finally {
+      await act(async () => {
+        setup.renderer.destroy();
+      });
+    }
+  });
+
+  test("shift plus left and right arrows scroll code horizontally faster", async () => {
+    const setup = await testRender(<AppHost bootstrap={createWrapBootstrap()} />, {
+      width: 92,
+      height: 20,
+    });
+
+    try {
+      await flush(setup);
+
+      let frame = setup.captureCharFrame();
+      expect(frame).toContain("this is a very");
+      expect(frame).not.toContain("interaction coverage");
+
+      for (let index = 0; index < 8; index += 1) {
+        await act(async () => {
+          await setup.mockInput.pressArrow("right", { shift: true });
+        });
+        await flush(setup);
+        frame = setup.captureCharFrame();
+        if (frame.includes("interaction coverage")) {
+          break;
+        }
+      }
+
+      expect(frame).toContain("interaction coverage");
+      expect(frame).not.toContain("this is a very");
+
+      for (let index = 0; index < 8; index += 1) {
+        await act(async () => {
+          await setup.mockInput.pressArrow("left", { shift: true });
+        });
+        await flush(setup);
+        frame = setup.captureCharFrame();
+        if (frame.includes("this is a very")) {
+          break;
+        }
+      }
+
+      expect(frame).toContain("this is a very");
+      expect(frame).not.toContain("interaction coverage");
+    } finally {
+      await act(async () => {
+        setup.renderer.destroy();
+      });
+    }
+  });
+
+  test("shift plus mouse wheel scrolls code horizontally", async () => {
+    const setup = await testRender(<AppHost bootstrap={createWrapBootstrap()} />, {
+      width: 92,
+      height: 20,
+    });
+
+    try {
+      await flush(setup);
+
+      let frame = setup.captureCharFrame();
+      expect(frame).toContain("this is a very");
+      expect(frame).not.toContain("interaction coverage");
+
+      for (let index = 0; index < 8; index += 1) {
+        await act(async () => {
+          await setup.mockMouse.scroll(60, 10, "down", { modifiers: { shift: true } });
+        });
+        await flush(setup);
+        frame = setup.captureCharFrame();
+        if (frame.includes("interaction coverage")) {
+          break;
+        }
+      }
+
+      expect(frame).toContain("interaction coverage");
+      expect(frame).not.toContain("this is a very");
+
+      for (let index = 0; index < 8; index += 1) {
+        await act(async () => {
+          await setup.mockMouse.scroll(60, 10, "up", { modifiers: { shift: true } });
+        });
+        await flush(setup);
+        frame = setup.captureCharFrame();
+        if (frame.includes("this is a very")) {
+          break;
+        }
+      }
+
+      expect(frame).toContain("this is a very");
+      expect(frame).not.toContain("interaction coverage");
+    } finally {
+      await act(async () => {
+        setup.renderer.destroy();
+      });
+    }
+  });
+
+  test("shift plus mouse wheel does not move the vertical review position", async () => {
+    const setup = await testRender(<AppHost bootstrap={createWrapScrollBootstrap()} />, {
+      width: 92,
+      height: 20,
+    });
+
+    try {
+      await flush(setup);
+
+      let frame = setup.captureCharFrame();
+      const initialTopLine = firstVisibleAddedLineNumber(frame);
+      expect(initialTopLine).toBeTruthy();
+      expect(frame).not.toContain("viewport anchoring");
+
+      for (let index = 0; index < 8; index += 1) {
+        await act(async () => {
+          await setup.mockMouse.scroll(60, 10, "down", { modifiers: { shift: true } });
+        });
+        await flush(setup);
+        frame = setup.captureCharFrame();
+        if (frame.includes("viewport anchoring")) {
+          break;
+        }
+      }
+
+      expect(frame).toContain("viewport anchoring");
+      expect(firstVisibleAddedLineNumber(frame)).toBe(initialTopLine);
+    } finally {
+      await act(async () => {
+        setup.renderer.destroy();
+      });
+    }
+  });
+
+  test("wrap toggles reset the horizontal code offset", async () => {
+    const setup = await testRender(<AppHost bootstrap={createWrapBootstrap()} />, {
+      width: 92,
+      height: 20,
+    });
+
+    try {
+      await flush(setup);
+
+      let frame = setup.captureCharFrame();
+      expect(frame).toContain("this is a very");
+
+      for (let index = 0; index < 8; index += 1) {
+        await act(async () => {
+          await setup.mockInput.pressArrow("right", { shift: true });
+        });
+        await flush(setup);
+        frame = setup.captureCharFrame();
+        if (frame.includes("interaction coverage")) {
+          break;
+        }
+      }
+
+      expect(frame).toContain("interaction coverage");
+      expect(frame).not.toContain("this is a very");
+
+      await act(async () => {
+        await setup.mockInput.typeText("w");
+      });
+      await settleWrapToggle(setup);
+
+      frame = await waitForFrame(setup, (nextFrame) => nextFrame.includes("overage';"));
+      expect(frame).toContain("this is a very");
+      expect(frame).toContain("long wrapped line");
+      expect(frame).toContain("overage';");
+
+      await act(async () => {
+        await setup.mockInput.typeText("w");
+      });
+      await settleWrapToggle(setup);
+
+      frame = await waitForFrame(
+        setup,
+        (nextFrame) =>
+          nextFrame.includes("this is a very") && !nextFrame.includes("interaction coverage"),
+      );
+      expect(frame).toContain("this is a very");
+      expect(frame).not.toContain("interaction coverage");
+    } finally {
+      await act(async () => {
+        setup.renderer.destroy();
+      });
+    }
+  });
+
   test("bootstrap preferences initialize the visible view state", async () => {
     const setup = await testRender(
       <AppHost

src/ui/components/chrome/HelpDialog.tsx

@@ -27,6 +27,7 @@ export function HelpDialog({
         ["d / u", "half page down / up"],
         ["[ / ]", "previous / next hunk"],
         ["{ / }", "previous / next comment"],
+        ["← / →", "scroll code (Shift = faster)"],
         ["Home / End", "jump to top / bottom"],
       ],
     },