fix(code-review): navigate with rendered file anchors

Generated-By: PostHog Code
Task-Id: 71e782ac-16eb-409f-a5d3-2aa4ad9117f8

Author: MattPua

packages/ui/src/features/code-review/components/ReviewShell.tsx

@@ -15,7 +15,11 @@ import { useReviewDraftsStore } from "../reviewDraftsStore";
 import { REVIEW_HOST, type ReviewHost } from "../reviewHost";
 import { useReviewNavigationStore } from "../reviewNavigationStore";
 import type { ReviewListItem, ReviewShellProps } from "../reviewShellParts";
-import { isFileViewed } from "../reviewShellParts";
+import {
+  findActiveScrollKey,
+  findRenderedScrollAnchor,
+  isFileViewed,
+} from "../reviewShellParts";
 import { ReviewViewedContext } from "../reviewViewedContext";
 import { useReviewViewedStore } from "../reviewViewedStore";
 import { PendingReviewBar } from "./PendingReviewBar";
@@ -127,8 +131,10 @@ export function ReviewShell({
   const reviewHost = useService<ReviewHost>(REVIEW_HOST);
   const taskId = task.id;
   const listRef = useRef<VListHandle | null>(null);
+  const listContainerRef = useRef<HTMLDivElement | null>(null);
   const lastActiveRef = useRef<string | null>(null);
-  const navigationLockedRef = useRef(false);
+  const pendingNavigationRef = useRef<string | null>(null);
+  const navigationFrameRef = useRef<number | null>(null);
 
   const workerFactory = useCallback(
     () => reviewHost.diffWorkerFactory(),
@@ -203,6 +209,9 @@ export function ReviewShell({
 
   useEffect(() => {
     return () => {
+      if (navigationFrameRef.current !== null) {
+        cancelAnimationFrame(navigationFrameRef.current);
+      }
       clearTask(taskId);
       useReviewDraftsStore.getState().clearDrafts(taskId);
     };
@@ -217,14 +226,37 @@ export function ReviewShell({
     const viewed =
       currentSignature !== undefined &&
       isFileViewed(viewedRecord[scrollRequest], currentSignature);
-    navigationLockedRef.current = true;
+    if (navigationFrameRef.current !== null) {
+      cancelAnimationFrame(navigationFrameRef.current);
+    }
+    pendingNavigationRef.current = scrollRequest;
     if (!viewed) onUncollapseFile?.(scrollRequest);
-    requestAnimationFrame(() => {
+
+    const scrollToAnchor = (remainingAttempts: number) => {
       listRef.current?.scrollToIndex(targetIndex, { align: "start" });
-      lastActiveRef.current = scrollRequest;
-      setActiveFilePath(taskId, scrollRequest);
-      clearScrollRequest(taskId);
-    });
+      navigationFrameRef.current = requestAnimationFrame(() => {
+        const root = listContainerRef.current;
+        const anchor = root
+          ? findRenderedScrollAnchor(root, scrollRequest)
+          : null;
+
+        if (!anchor && remainingAttempts > 0) {
+          scrollToAnchor(remainingAttempts - 1);
+          return;
+        }
+
+        anchor?.scrollIntoView({ block: "start", inline: "nearest" });
+        lastActiveRef.current = scrollRequest;
+        setActiveFilePath(taskId, scrollRequest);
+        clearScrollRequest(taskId);
+        navigationFrameRef.current = requestAnimationFrame(() => {
+          pendingNavigationRef.current = null;
+          navigationFrameRef.current = null;
+        });
+      });
+    };
+
+    scrollToAnchor(5);
   }, [
     clearScrollRequest,
     currentSignatures,
@@ -236,24 +268,17 @@ export function ReviewShell({
     viewedRecord,
   ]);
 
-  const handleScroll = useCallback(
-    (offset: number) => {
-      if (navigationLockedRef.current) return;
-      const handle = listRef.current;
-      if (!handle) return;
-      const index = handle.findItemIndex(offset);
-      const item = items[index];
-      const scrollKey = item?.scrollKey;
-      if (!scrollKey || scrollKey === lastActiveRef.current) return;
-      lastActiveRef.current = scrollKey;
-      setActiveFilePath(taskId, scrollKey);
-    },
-    [items, setActiveFilePath, taskId],
-  );
-
-  const handleUserScrollIntent = useCallback(() => {
-    navigationLockedRef.current = false;
-  }, []);
+  const handleScroll = useCallback(() => {
+    if (pendingNavigationRef.current !== null) return;
+    const scrollRoot = listContainerRef.current?.querySelector<HTMLElement>(
+      ".pierre-scroll-root",
+    );
+    if (!scrollRoot) return;
+    const scrollKey = findActiveScrollKey(scrollRoot);
+    if (!scrollKey || scrollKey === lastActiveRef.current) return;
+    lastActiveRef.current = scrollKey;
+    setActiveFilePath(taskId, scrollKey);
+  }, [setActiveFilePath, taskId]);
 
   const renderItem = useCallback(
     (item: ReviewListItem) => (
@@ -314,11 +339,9 @@ export function ReviewShell({
           />
           <Flex className="min-h-0 flex-1">
             <Flex
+              ref={listContainerRef}
               direction="column"
               className="min-w-0 flex-1"
-              onPointerDownCapture={handleUserScrollIntent}
-              onWheelCapture={handleUserScrollIntent}
-              onKeyDownCapture={handleUserScrollIntent}
             >
               {isLoading ? (
                 <Flex

packages/ui/src/features/code-review/reviewShellParts.test.tsx

@@ -14,7 +14,12 @@ vi.mock("../../primitives/FileIcon", () => ({
   FileIcon: () => <span data-testid="file-icon" />,
 }));
 
-import { DeferredDiffPlaceholder, DiffFileHeader } from "./reviewShellParts";
+import {
+  DeferredDiffPlaceholder,
+  DiffFileHeader,
+  findActiveScrollKey,
+  findRenderedScrollAnchor,
+} from "./reviewShellParts";
 
 type FileDiffMetadata = import("@pierre/diffs/react").FileDiffMetadata;
 
@@ -107,3 +112,35 @@ describe.each([
     expect(text.indexOf("2 comments")).toBeLessThan(text.indexOf(additions));
   });
 });
+
+function setRect(element: HTMLElement, top: number, bottom: number) {
+  element.getBoundingClientRect = vi.fn(() => ({ top, bottom }) as DOMRect);
+}
+
+describe("review scroll anchors", () => {
+  it("finds a rendered anchor by its exact file key", () => {
+    const root = document.createElement("div");
+    const anchor = document.createElement("div");
+    anchor.dataset.scrollKey = "src/[id]/file.ts";
+    root.append(anchor);
+
+    expect(findRenderedScrollAnchor(root, "src/[id]/file.ts")).toBe(anchor);
+  });
+
+  it("selects the first rendered file crossing the scroll root top", () => {
+    const root = document.createElement("div");
+    const above = document.createElement("div");
+    const active = document.createElement("div");
+    const below = document.createElement("div");
+    above.dataset.scrollKey = "above.ts";
+    active.dataset.scrollKey = "active.ts";
+    below.dataset.scrollKey = "below.ts";
+    root.append(above, active, below);
+    setRect(root, 100, 500);
+    setRect(above, 20, 90);
+    setRect(active, 80, 180);
+    setRect(below, 180, 280);
+
+    expect(findActiveScrollKey(root)).toBe("active.ts");
+  });
+});

packages/ui/src/features/code-review/reviewShellParts.tsx

@@ -34,6 +34,32 @@ export {
 } from "@posthog/core/code-review/reviewShellGeometry";
 
 const STICKY_HEADER_CSS = `[data-diffs-header] { position: sticky; top: 0; z-index: 1; background: var(--gray-2); }`;
+const SCROLL_ANCHOR_SELECTOR = "[data-scroll-key]";
+
+export function findRenderedScrollAnchor(
+  root: HTMLElement,
+  scrollKey: string,
+): HTMLElement | null {
+  for (const anchor of root.querySelectorAll<HTMLElement>(
+    SCROLL_ANCHOR_SELECTOR,
+  )) {
+    if (anchor.dataset.scrollKey === scrollKey) return anchor;
+  }
+  return null;
+}
+
+export function findActiveScrollKey(root: HTMLElement): string | null {
+  const rootTop = root.getBoundingClientRect().top;
+  for (const anchor of root.querySelectorAll<HTMLElement>(
+    SCROLL_ANCHOR_SELECTOR,
+  )) {
+    const scrollKey = anchor.dataset.scrollKey;
+    if (scrollKey && anchor.getBoundingClientRect().bottom > rootTop + 1) {
+      return scrollKey;
+    }
+  }
+  return null;
+}
 
 export function useDiffOptions() {
   const viewMode = useDiffViewerStore((s) => s.viewMode);