feat(web): resizable review dialog sidebar with persistence (#1245)

apps/web/app/layout.tsx

@@ -52,6 +52,15 @@ export default async function RootLayout({
     <html lang="en" suppressHydrationWarning>
       <head>
         <meta name="apple-mobile-web-app-title" content="Kandev" />
+        {/* Preload the Seti icon webfont so file-tree glyphs (review dialog,
+            file browser) don't flash blank on first render. */}
+        <link
+          rel="preload"
+          href="/fonts/seti/seti.woff"
+          as="font"
+          type="font/woff"
+          crossOrigin="anonymous"
+        />
       </head>
       <body className="antialiased font-sans">
         {apiPort || debugMode ? (

apps/web/components/review/review-dialog.tsx

@@ -8,6 +8,7 @@ import type { PRDiffFile } from "@/lib/types/github";
 import { useCommentsStore, isDiffComment } from "@/lib/state/slices/comments";
 import { useSessionFileReviews } from "@/hooks/use-session-file-reviews";
 import { useGitOperations } from "@/hooks/use-git-operations";
+import { useReviewSidebarResize } from "@/hooks/use-review-sidebar-resize";
 import { useAppStore } from "@/components/state-provider";
 import { useToast } from "@/components/toast-provider";
 import { ReviewTopBar } from "./review-top-bar";
@@ -418,13 +419,18 @@ function useReviewDialogState(props: ReviewDialogProps) {
 export const ReviewDialog = memo(function ReviewDialog(props: ReviewDialogProps) {
   const { open, onOpenChange, sessionId, baseBranch, onOpenFile } = props;
   const s = useReviewDialogState(props);
+  const splitRowRef = useRef<HTMLDivElement>(null);
+  const sidebar = useReviewSidebarResize(splitRowRef, open);
 
   return (
     <Dialog open={open} onOpenChange={onOpenChange}>
       <DialogContent
         className="!max-w-[100vw] !w-[100vw] sm:!max-w-[80vw] sm:!w-[80vw] max-h-[85vh] h-[85vh] p-0 gap-0 flex flex-col shadow-2xl"
         showCloseButton={false}
-        overlayClassName="bg-transparent"
+        // Use a fixed black tint (not a theme token) so the backdrop reads
+        // as "a little dark" in both light and dark mode — `foreground/N`
+        // would invert to a light overlay in dark mode.
+        overlayClassName="bg-black/40"
       >
         <DialogTitle className="sr-only">Review Changes</DialogTitle>
         <ReviewTopBar
@@ -442,8 +448,12 @@ export const ReviewDialog = memo(function ReviewDialog(props: ReviewDialogProps)
           getPendingComments={s.getPendingComments}
           markCommentsSent={s.markCommentsSent}
         />
-        <div className="flex flex-1 min-h-0">
-          <div className="w-[280px] sm:w-[220px] min-w-[100px] border-r border-border flex-shrink-0 overflow-hidden hidden sm:flex flex-col">
+        <div ref={splitRowRef} className="flex flex-1 min-h-0">
+          <div
+            data-testid="review-dialog-sidebar"
+            className="border-r border-border flex-shrink-0 overflow-hidden hidden sm:flex flex-col"
+            style={{ width: `${sidebar.width}px` }}
+          >
             <ReviewFileTree
               files={s.filteredFiles}
               reviewedFiles={s.reviewedFiles}
@@ -456,6 +466,16 @@ export const ReviewDialog = memo(function ReviewDialog(props: ReviewDialogProps)
               onToggleReviewed={s.handleToggleReviewed}
             />
           </div>
+          <button
+            data-testid="review-dialog-sidebar-resize"
+            type="button"
+            tabIndex={-1}
+            aria-label="Resize file list"
+            className="hidden sm:block w-1 bg-border hover:bg-primary cursor-col-resize flex-shrink-0 relative group transition-colors p-0"
+            {...sidebar.resizeHandleProps}
+          >
+            <span className="absolute inset-y-0 -left-1 -right-1" />
+          </button>
           <div className="flex-1 min-w-0 overflow-hidden">
             {s.filteredFiles.length > 0 ? (
               <ReviewDiffList

apps/web/e2e/tests/review/review-sidebar-resize.spec.ts

@@ -0,0 +1,154 @@
+import { test, expect } from "../../fixtures/test-base";
+import { SessionPage } from "../../pages/session-page";
+import { REVIEW_SIDEBAR_LIMITS } from "../../../hooks/use-review-sidebar-resize";
+import type { ApiClient } from "../../helpers/api-client";
+import type { SeedData } from "../../fixtures/test-base";
+import type { Page, Locator } from "@playwright/test";
+
+const STORAGE_KEY = REVIEW_SIDEBAR_LIMITS.storageKey;
+
+async function seedReviewTask(testPage: Page, apiClient: ApiClient, seedData: SeedData) {
+  const task = await apiClient.createTaskWithAgent(
+    seedData.workspaceId,
+    "Review Sidebar Resize E2E",
+    seedData.agentProfileId,
+    {
+      description: "/e2e:review-cumulative-setup",
+      workflow_id: seedData.workflowId,
+      workflow_step_id: seedData.startStepId,
+      repository_ids: [seedData.repositoryId],
+    },
+  );
+  await testPage.goto(`/t/${task.id}`);
+  const session = new SessionPage(testPage);
+  await session.waitForLoad();
+  await expect(
+    session.chat.getByText("review-cumulative-setup complete", { exact: false }),
+  ).toBeVisible({ timeout: 45_000 });
+  return task;
+}
+
+async function openDialogWithChanges(testPage: Page) {
+  const changesTab = testPage.locator(".dv-default-tab", { hasText: "Changes" });
+  await expect(changesTab).toBeVisible({ timeout: 10_000 });
+  await changesTab.click();
+  await expect(testPage.getByTestId("file-row-review_cumulative_test.txt")).toBeVisible({
+    timeout: 15_000,
+  });
+  await testPage.evaluate(() => window.dispatchEvent(new CustomEvent("open-review-dialog")));
+  const dialog = testPage.getByRole("dialog", { name: "Review Changes" });
+  await expect(dialog).toBeVisible({ timeout: 10_000 });
+  return dialog;
+}
+
+async function getSidebarWidth(sidebar: Locator): Promise<number> {
+  const box = await sidebar.boundingBox();
+  if (!box) throw new Error("sidebar has no bounding box");
+  return Math.round(box.width);
+}
+
+// boundingBox returns floats and sub-pixel rounding can shift the measured
+// width by a pixel across renderers, so compare with a small tolerance.
+function expectWidthNear(actual: number, expected: number, tolerance = 1) {
+  expect(actual).toBeGreaterThanOrEqual(expected - tolerance);
+  expect(actual).toBeLessThanOrEqual(expected + tolerance);
+}
+
+async function dragHandle(testPage: Page, handle: Locator, deltaX: number) {
+  const box = await handle.boundingBox();
+  if (!box) throw new Error("resize handle has no bounding box");
+  const cx = box.x + box.width / 2;
+  const cy = box.y + box.height / 2;
+  await testPage.mouse.move(cx, cy);
+  await testPage.mouse.down();
+  await testPage.mouse.move(cx + deltaX, cy, { steps: 10 });
+  await testPage.mouse.up();
+}
+
+test.describe("Review dialog sidebar resize", () => {
+  test.describe.configure({ retries: 2, timeout: 120_000 });
+
+  test("dragging the handle resizes the sidebar and persists to sessionStorage", async ({
+    testPage,
+    apiClient,
+    seedData,
+  }) => {
+    await seedReviewTask(testPage, apiClient, seedData);
+    const dialog = await openDialogWithChanges(testPage);
+
+    const sidebar = dialog.getByTestId("review-dialog-sidebar");
+    const handle = dialog.getByTestId("review-dialog-sidebar-resize");
+    await expect(sidebar).toBeVisible();
+    await expect(handle).toBeVisible();
+
+    const before = await getSidebarWidth(sidebar);
+    expectWidthNear(before, 220); // default width
+
+    await dragHandle(testPage, handle, 120); // drag right by 120px
+
+    await expect
+      .poll(async () => getSidebarWidth(sidebar), { timeout: 5_000 })
+      .toBeGreaterThan(before + 80);
+    const after = await getSidebarWidth(sidebar);
+
+    // sessionStorage persistence (within 1px of the rendered width)
+    const stored = await testPage.evaluate(
+      (key) => window.sessionStorage.getItem(key),
+      STORAGE_KEY,
+    );
+    expect(stored).not.toBeNull();
+    expectWidthNear(Number(stored), after);
+  });
+
+  test("clamps to max width when dragged far past the limit", async ({
+    testPage,
+    apiClient,
+    seedData,
+  }) => {
+    await seedReviewTask(testPage, apiClient, seedData);
+    const dialog = await openDialogWithChanges(testPage);
+    const sidebar = dialog.getByTestId("review-dialog-sidebar");
+    const handle = dialog.getByTestId("review-dialog-sidebar-resize");
+
+    await dragHandle(testPage, handle, 2000); // way past max=600
+
+    // At default Playwright viewport (~1280px) the dialog is ~1024px, so the
+    // (containerWidth - minDiffPaneWidth) clamp (~704) is wider than the
+    // hard max (600); we should land on the hard max.
+    await expect
+      .poll(async () => getSidebarWidth(sidebar), { timeout: 5_000 })
+      .toBeGreaterThanOrEqual(599);
+    const finalWidth = await getSidebarWidth(sidebar);
+    expectWidthNear(finalWidth, 600);
+  });
+
+  test("persisted width is reapplied when the dialog is reopened in the same tab", async ({
+    testPage,
+    apiClient,
+    seedData,
+  }) => {
+    // Pre-seed sessionStorage via addInitScript so the value is written
+    // before any page script runs. The ReviewDialog is mounted as soon as
+    // the session layout renders (see dockview-desktop-layout.tsx) — well
+    // before this test could `page.evaluate` after navigation — so the
+    // hook's useState initializer reads the seeded value on first mount.
+    await testPage.addInitScript(
+      ({ key, val }) => {
+        try {
+          sessionStorage.setItem(key, val);
+        } catch {
+          // sessionStorage may be unavailable in some contexts; ignore.
+        }
+      },
+      { key: STORAGE_KEY, val: "340" },
+    );
+    await seedReviewTask(testPage, apiClient, seedData);
+
+    const dialog = await openDialogWithChanges(testPage);
+    const sidebar = dialog.getByTestId("review-dialog-sidebar");
+    await expect
+      .poll(async () => getSidebarWidth(sidebar), { timeout: 5_000 })
+      .toBeGreaterThanOrEqual(339);
+    expectWidthNear(await getSidebarWidth(sidebar), 340);
+  });
+});

apps/web/hooks/use-review-sidebar-resize.test.ts

@@ -0,0 +1,184 @@
+import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import { renderHook, act } from "@testing-library/react";
+import { useRef } from "react";
+import {
+  useReviewSidebarResize,
+  clampSidebarWidth,
+  REVIEW_SIDEBAR_LIMITS,
+} from "./use-review-sidebar-resize";
+
+function mouseMove(clientX: number) {
+  document.dispatchEvent(new MouseEvent("mousemove", { clientX, bubbles: true }));
+}
+
+function mouseUp() {
+  document.dispatchEvent(new MouseEvent("mouseup", { bubbles: true }));
+}
+
+function reactMouse(clientX: number) {
+  return { clientX, preventDefault: () => {} } as unknown as React.MouseEvent;
+}
+
+describe("clampSidebarWidth", () => {
+  it("returns the value when within [min, max]", () => {
+    expect(clampSidebarWidth(250)).toBe(250);
+  });
+
+  it("clamps below min", () => {
+    expect(clampSidebarWidth(50)).toBe(REVIEW_SIDEBAR_LIMITS.minWidth);
+  });
+
+  it("clamps above max", () => {
+    expect(clampSidebarWidth(9999)).toBe(REVIEW_SIDEBAR_LIMITS.maxWidth);
+  });
+
+  it("clamps against (containerWidth - minDiffPaneWidth) when it bites first", () => {
+    // container 700 - minDiffPane 320 → effective max 380
+    expect(clampSidebarWidth(500, 700)).toBe(700 - REVIEW_SIDEBAR_LIMITS.minDiffPaneWidth);
+  });
+
+  it("does not let containerWidth force effective max below min", () => {
+    // container 200 - minDiff 320 = -120 → floor at MIN_SIDEBAR_WIDTH
+    expect(clampSidebarWidth(500, 200)).toBe(REVIEW_SIDEBAR_LIMITS.minWidth);
+  });
+
+  it("does not let containerWidth raise effective max above hard max", () => {
+    // container 5000 - 320 = 4680 → capped by hard max
+    expect(clampSidebarWidth(9999, 5000)).toBe(REVIEW_SIDEBAR_LIMITS.maxWidth);
+  });
+});
+
+describe("useReviewSidebarResize", () => {
+  beforeEach(() => {
+    window.sessionStorage.clear();
+    document.body.style.cursor = "";
+    document.body.style.userSelect = "";
+  });
+
+  afterEach(() => {
+    document.body.style.cursor = "";
+    document.body.style.userSelect = "";
+  });
+
+  it("defaults to DEFAULT_SIDEBAR_WIDTH when sessionStorage is empty", () => {
+    const { result } = renderHook(() => useReviewSidebarResize());
+    expect(result.current.width).toBe(REVIEW_SIDEBAR_LIMITS.defaultWidth);
+    expect(typeof result.current.resizeHandleProps.onMouseDown).toBe("function");
+  });
+
+  it("reads a previously persisted width from sessionStorage on mount", () => {
+    window.sessionStorage.setItem(REVIEW_SIDEBAR_LIMITS.storageKey, "350");
+    const { result } = renderHook(() => useReviewSidebarResize());
+    expect(result.current.width).toBe(350);
+  });
+
+  it("clamps a persisted width that's outside [min, max] on mount", () => {
+    window.sessionStorage.setItem(REVIEW_SIDEBAR_LIMITS.storageKey, "9999");
+    const { result } = renderHook(() => useReviewSidebarResize());
+    expect(result.current.width).toBe(REVIEW_SIDEBAR_LIMITS.maxWidth);
+  });
+
+  it("falls back to default when sessionStorage contains a non-numeric value", () => {
+    window.sessionStorage.setItem(REVIEW_SIDEBAR_LIMITS.storageKey, "not-a-number");
+    const { result } = renderHook(() => useReviewSidebarResize());
+    expect(result.current.width).toBe(REVIEW_SIDEBAR_LIMITS.defaultWidth);
+  });
+
+  it("updates width live during mousemove and clamps the result", () => {
+    const { result } = renderHook(() => useReviewSidebarResize());
+    act(() => result.current.resizeHandleProps.onMouseDown(reactMouse(300)));
+    // move right by 80px → 220 + 80 = 300
+    act(() => mouseMove(380));
+    expect(result.current.width).toBe(300);
+
+    // overshoot past max → clamps to maxWidth
+    act(() => mouseMove(2000));
+    expect(result.current.width).toBe(REVIEW_SIDEBAR_LIMITS.maxWidth);
+  });
+
+  it("persists final width to sessionStorage on mouseup", () => {
+    const { result } = renderHook(() => useReviewSidebarResize());
+    act(() => result.current.resizeHandleProps.onMouseDown(reactMouse(300)));
+    act(() => mouseMove(380)); // → 300
+    act(() => mouseUp());
+
+    expect(window.sessionStorage.getItem(REVIEW_SIDEBAR_LIMITS.storageKey)).toBe("300");
+    expect(result.current.width).toBe(300);
+  });
+
+  it("stops responding to mousemove after mouseup", () => {
+    const { result } = renderHook(() => useReviewSidebarResize());
+    act(() => result.current.resizeHandleProps.onMouseDown(reactMouse(300)));
+    act(() => mouseMove(380));
+    expect(result.current.width).toBe(300);
+
+    act(() => mouseUp());
+    act(() => mouseMove(500)); // would push to 420 if still listening
+
+    expect(result.current.width).toBe(300);
+  });
+
+  it("sets body cursor + userSelect during drag and restores them on mouseup", () => {
+    const { result } = renderHook(() => useReviewSidebarResize());
+    act(() => result.current.resizeHandleProps.onMouseDown(reactMouse(300)));
+    expect(document.body.style.cursor).toBe("col-resize");
+    expect(document.body.style.userSelect).toBe("none");
+
+    act(() => mouseUp());
+    expect(document.body.style.cursor).toBe("");
+    expect(document.body.style.userSelect).toBe("");
+  });
+
+  it("restores body styles when the component unmounts mid-drag", () => {
+    const { result, unmount } = renderHook(() => useReviewSidebarResize());
+    act(() => result.current.resizeHandleProps.onMouseDown(reactMouse(300)));
+    expect(document.body.style.cursor).toBe("col-resize");
+
+    act(() => unmount());
+
+    expect(document.body.style.cursor).toBe("");
+    expect(document.body.style.userSelect).toBe("");
+    // After unmount, the now-detached window listeners must not be able to
+    // mutate any state — a stray mousemove should be a no-op.
+    act(() => mouseMove(800));
+    // Nothing to assert on result.current after unmount, but the absence of
+    // a thrown error from the listener confirms cleanup ran.
+  });
+
+  it("clamps width against the container width when one is provided", () => {
+    // jsdom doesn't ship ResizeObserver; the initial reclamp still runs.
+    window.sessionStorage.setItem(REVIEW_SIDEBAR_LIMITS.storageKey, "500");
+    const { result } = renderHook(() => {
+      const ref = useRef<HTMLDivElement>(null);
+      // Simulate a container element that reports a 600px width.
+      const fakeEl = { getBoundingClientRect: () => ({ width: 600 }) as DOMRect };
+      (ref as React.MutableRefObject<HTMLDivElement | null>).current =
+        fakeEl as unknown as HTMLDivElement;
+      return useReviewSidebarResize(ref);
+    });
+    // 600 - 320 = 280 → stored 500 should be clamped down to 280
+    expect(result.current.width).toBe(600 - REVIEW_SIDEBAR_LIMITS.minDiffPaneWidth);
+  });
+
+  it("reclamps when the dialog transitions from closed to open", () => {
+    // Mirrors the real lifecycle: Radix doesn't mount the DialogContent
+    // portal while open=false, so containerRef.current is null on first
+    // effect run. When open flips to true, the effect must re-fire and
+    // pick up the now-populated element.
+    window.sessionStorage.setItem(REVIEW_SIDEBAR_LIMITS.storageKey, "500");
+    const ref = { current: null } as React.MutableRefObject<HTMLDivElement | null>;
+    const { result, rerender } = renderHook(({ open }) => useReviewSidebarResize(ref, open), {
+      initialProps: { open: false },
+    });
+    // Closed → stored width returned unclamped by container (since no el).
+    expect(result.current.width).toBe(500);
+
+    // Dialog opens: portal mounts, ref populates with a 600px container.
+    const fakeEl = { getBoundingClientRect: () => ({ width: 600 }) as DOMRect };
+    ref.current = fakeEl as unknown as HTMLDivElement;
+    rerender({ open: true });
+
+    // Effect must re-run and reclamp to 600 - 320 = 280.
+    expect(result.current.width).toBe(600 - REVIEW_SIDEBAR_LIMITS.minDiffPaneWidth);
+  });
+});

apps/web/hooks/use-review-sidebar-resize.ts

@@ -0,0 +1,128 @@
+"use client";
+
+import { useCallback, useEffect, useRef, useState } from "react";
+
+const SIDEBAR_WIDTH_KEY = "review-dialog-sidebar-width";
+const DEFAULT_SIDEBAR_WIDTH = 220;
+const MIN_SIDEBAR_WIDTH = 160;
+const MAX_SIDEBAR_WIDTH = 600;
+// Reserve at least this much horizontal space for the diff pane so a
+// previously-saved 600px sidebar can't push the diff list to ~0 on a small
+// desktop viewport (e.g. the dialog is 80vw → 512px at the sm breakpoint).
+const MIN_DIFF_PANE_WIDTH = 320;
+
+export const REVIEW_SIDEBAR_LIMITS = {
+  storageKey: SIDEBAR_WIDTH_KEY,
+  defaultWidth: DEFAULT_SIDEBAR_WIDTH,
+  minWidth: MIN_SIDEBAR_WIDTH,
+  maxWidth: MAX_SIDEBAR_WIDTH,
+  minDiffPaneWidth: MIN_DIFF_PANE_WIDTH,
+} as const;
+
+export function clampSidebarWidth(w: number, containerWidth?: number): number {
+  const effectiveMax = containerWidth
+    ? Math.max(MIN_SIDEBAR_WIDTH, Math.min(MAX_SIDEBAR_WIDTH, containerWidth - MIN_DIFF_PANE_WIDTH))
+    : MAX_SIDEBAR_WIDTH;
+  return Math.max(MIN_SIDEBAR_WIDTH, Math.min(effectiveMax, w));
+}
+
+function readStoredWidth(): number {
+  if (typeof window === "undefined") return DEFAULT_SIDEBAR_WIDTH;
+  try {
+    const stored = window.sessionStorage.getItem(SIDEBAR_WIDTH_KEY);
+    const parsed = stored ? Number(stored) : NaN;
+    return Number.isFinite(parsed) ? clampSidebarWidth(parsed) : DEFAULT_SIDEBAR_WIDTH;
+  } catch {
+    return DEFAULT_SIDEBAR_WIDTH;
+  }
+}
+
+function persistWidth(width: number) {
+  try {
+    window.sessionStorage.setItem(SIDEBAR_WIDTH_KEY, String(width));
+  } catch {
+    // sessionStorage may be unavailable (private mode, quota); ignore.
+  }
+}
+
+function restoreBodyStyles() {
+  document.body.style.cursor = "";
+  document.body.style.userSelect = "";
+}
+
+export function useReviewSidebarResize(
+  containerRef?: React.RefObject<HTMLElement | null>,
+  open = true,
+) {
+  const [width, setWidth] = useState<number>(readStoredWidth);
+  const isDragging = useRef(false);
+  const startX = useRef(0);
+  const startWidth = useRef(0);
+
+  // Re-clamp when the container becomes available or resizes so a stored
+  // width carried over from a wider viewport gets pulled down on a narrower
+  // one (rather than overflowing the dialog). `open` is in the dep array
+  // because the container element only exists once Radix mounts the
+  // DialogContent portal — on the closed → open transition, `containerRef`
+  // (a stable ref *object*) doesn't change identity, so the effect needs
+  // `open` to re-fire and pick up the now-populated `current`.
+  useEffect(() => {
+    if (!open) return;
+    const el = containerRef?.current;
+    if (!el) return;
+    const reclamp = () => {
+      const cw = el.getBoundingClientRect().width;
+      setWidth((w) => clampSidebarWidth(w, cw));
+    };
+    reclamp();
+    if (typeof ResizeObserver === "undefined") return;
+    const ro = new ResizeObserver(reclamp);
+    ro.observe(el);
+    return () => ro.disconnect();
+  }, [containerRef, open]);
+
+  const handleMouseDown = useCallback(
+    (e: React.MouseEvent) => {
+      e.preventDefault();
+      isDragging.current = true;
+      startX.current = e.clientX;
+      startWidth.current = width;
+      document.body.style.cursor = "col-resize";
+      document.body.style.userSelect = "none";
+    },
+    [width],
+  );
+
+  useEffect(() => {
+    const handleMouseMove = (e: MouseEvent) => {
+      if (!isDragging.current) return;
+      const cw = containerRef?.current?.getBoundingClientRect().width;
+      const delta = e.clientX - startX.current;
+      setWidth(clampSidebarWidth(startWidth.current + delta, cw));
+    };
+    const handleMouseUp = () => {
+      if (!isDragging.current) return;
+      isDragging.current = false;
+      restoreBodyStyles();
+      setWidth((current) => {
+        persistWidth(current);
+        return current;
+      });
+    };
+    document.addEventListener("mousemove", handleMouseMove);
+    document.addEventListener("mouseup", handleMouseUp);
+    return () => {
+      document.removeEventListener("mousemove", handleMouseMove);
+      document.removeEventListener("mouseup", handleMouseUp);
+      if (isDragging.current) {
+        isDragging.current = false;
+        restoreBodyStyles();
+      }
+    };
+  }, [containerRef]);
+
+  return {
+    width,
+    resizeHandleProps: { onMouseDown: handleMouseDown },
+  };
+}