Merge branch 'cw/review-tab-performance-phase-frontend-virtualize-sidebar-file-list-for-50-items' into cw-merge-1772824315514
This commit is contained in:
Lukas May
@@ -27,12 +27,14 @@
|
||||
"@tiptap/suggestion": "^3.19.0",
|
||||
"@trpc/client": "^11.9.0",
|
||||
"@trpc/react-query": "^11.9.0",
|
||||
"@types/react-window": "^1.8.8",
|
||||
"class-variance-authority": "^0.7.1",
|
||||
"clsx": "^2.1.1",
|
||||
"geist": "^1.7.0",
|
||||
"lucide-react": "^0.563.0",
|
||||
"react": "^19.0.0",
|
||||
"react-dom": "^19.0.0",
|
||||
"react-window": "^2.2.7",
|
||||
"sonner": "^2.0.7",
|
||||
"tailwind-merge": "^3.4.0",
|
||||
"tippy.js": "^6.3.7"
|
||||
|
||||
193
apps/web/src/components/review/ReviewSidebar.test.tsx
Normal file
193
apps/web/src/components/review/ReviewSidebar.test.tsx
Normal file
@@ -0,0 +1,193 @@
|
||||
// @vitest-environment happy-dom
|
||||
import '@testing-library/jest-dom/vitest';
|
||||
import { render, screen, fireEvent, act } from '@testing-library/react';
|
||||
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { ReviewSidebar } from './ReviewSidebar';
|
||||
import type { FileDiff, ReviewComment, CommitInfo } from './types';
|
||||
|
||||
// Mock ResizeObserver — not provided by happy-dom.
|
||||
// react-window 2.x uses `new ResizeObserver()` internally.
|
||||
class MockResizeObserver {
|
||||
observe() {}
|
||||
unobserve() {}
|
||||
disconnect() {}
|
||||
}
|
||||
vi.stubGlobal('ResizeObserver', MockResizeObserver);
|
||||
|
||||
// Mock react-window to avoid ESM/CJS duplicate-React-instance errors in Vitest.
|
||||
// The mock renders only the first 15 rows, simulating windowed rendering.
|
||||
// It also exposes a `listRef`-compatible imperative handle so scroll-save/restore logic runs.
|
||||
vi.mock('react-window', () => ({
|
||||
List: vi.fn(({ rowComponent: RowComponent, rowCount, rowProps, listRef }: any) => {
|
||||
// Expose the imperative API via the ref (synchronous assignment is safe in tests).
|
||||
if (listRef && typeof listRef === 'object' && 'current' in listRef) {
|
||||
listRef.current = { element: { scrollTop: 0 }, scrollToRow: vi.fn() };
|
||||
}
|
||||
const renderCount = Math.min(rowCount ?? 0, 15);
|
||||
return (
|
||||
<div data-testid="virtual-list">
|
||||
{Array.from({ length: renderCount }, (_, i) => (
|
||||
<RowComponent key={i} index={i} style={{}} ariaAttributes={{}} {...rowProps} />
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}),
|
||||
}));
|
||||
|
||||
// ─── Helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
function makeFile(path: string): FileDiff {
|
||||
return {
|
||||
oldPath: path,
|
||||
newPath: path,
|
||||
hunks: [],
|
||||
additions: 1,
|
||||
deletions: 0,
|
||||
changeType: 'modified',
|
||||
};
|
||||
}
|
||||
|
||||
function makeFiles(count: number, prefix = 'src/components/'): FileDiff[] {
|
||||
return Array.from({ length: count }, (_, i) =>
|
||||
makeFile(`${prefix}file${String(i).padStart(4, '0')}.ts`),
|
||||
);
|
||||
}
|
||||
|
||||
const NO_COMMENTS: ReviewComment[] = [];
|
||||
const NO_COMMITS: CommitInfo[] = [];
|
||||
|
||||
function renderSidebar(files: FileDiff[]) {
|
||||
return render(
|
||||
<ReviewSidebar
|
||||
files={files}
|
||||
comments={NO_COMMENTS}
|
||||
onFileClick={vi.fn()}
|
||||
selectedCommit={null}
|
||||
activeFiles={files}
|
||||
commits={NO_COMMITS}
|
||||
onSelectCommit={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
}
|
||||
|
||||
// ─── Tests ────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('ReviewSidebar FilesView virtualization', () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
afterEach(() => vi.restoreAllMocks());
|
||||
|
||||
// 1. Virtual list NOT used for ≤50 files (fallback path)
|
||||
it('does not use virtual list when files count is ≤50', () => {
|
||||
renderSidebar(makeFiles(10));
|
||||
|
||||
expect(screen.queryByTestId('virtual-list')).not.toBeInTheDocument();
|
||||
// All 10 file rows are in the DOM directly
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(10);
|
||||
});
|
||||
|
||||
// 2. Virtual list IS used for >50 files (virtualized path)
|
||||
it('uses virtual list when files count is >50', () => {
|
||||
renderSidebar(makeFiles(1000));
|
||||
|
||||
expect(screen.getByTestId('virtual-list')).toBeInTheDocument();
|
||||
// Mock renders only 15 rows — far fewer than 1000
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeLessThan(50);
|
||||
});
|
||||
|
||||
// 3. Directory collapse removes file rows from the virtual list
|
||||
it('removes file rows from virtual list when directory is collapsed', async () => {
|
||||
// 100 files all in "src/" — produces 101 rows (1 dir-header + 100 files), which is >50
|
||||
const files = Array.from({ length: 100 }, (_, i) => makeFile(`src/file${i}.ts`));
|
||||
renderSidebar(files);
|
||||
|
||||
expect(screen.getByTestId('virtual-list')).toBeInTheDocument();
|
||||
|
||||
const dirHeader = screen.getByRole('button', { name: /src\// });
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(dirHeader);
|
||||
});
|
||||
|
||||
// After collapse: only the dir-header row remains in the virtual list
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(0);
|
||||
expect(document.querySelectorAll('[data-testid="dir-header"]').length).toBe(1);
|
||||
});
|
||||
|
||||
// 3a. Expanding a collapsed directory restores file rows
|
||||
it('restores file rows when a collapsed directory is expanded again', async () => {
|
||||
const files = makeFiles(60, 'src/components/');
|
||||
renderSidebar(files);
|
||||
|
||||
const dirHeader = screen.getByRole('button', { name: /src\/components\// });
|
||||
|
||||
// Collapse
|
||||
await act(async () => {
|
||||
fireEvent.click(dirHeader);
|
||||
});
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(0);
|
||||
|
||||
// Expand again
|
||||
const freshDirHeader = screen.getByRole('button', { name: /src\/components\// });
|
||||
await act(async () => {
|
||||
fireEvent.click(freshDirHeader);
|
||||
});
|
||||
|
||||
// File rows are back (virtual list renders up to 15)
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0);
|
||||
expect(document.querySelectorAll('[data-testid="dir-header"]').length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
// 4. Scroll position saved and restored on Files ↔ Commits tab switch
|
||||
it('restores file rows when returning to Files tab after switching to Commits tab', async () => {
|
||||
renderSidebar(makeFiles(200));
|
||||
|
||||
// Files tab is default — file rows are visible
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0);
|
||||
|
||||
// Switch to Commits tab — FilesView unmounts (scroll offset is saved)
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByTitle('Commits'));
|
||||
});
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(0);
|
||||
|
||||
// Switch back to Files tab — FilesView remounts (scroll offset is restored)
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByTitle('Files'));
|
||||
});
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
// 5. Clicking a file calls onFileClick with the correct path
|
||||
it('calls onFileClick when a file row is clicked', () => {
|
||||
const onFileClick = vi.fn();
|
||||
const files = makeFiles(5);
|
||||
render(
|
||||
<ReviewSidebar
|
||||
files={files}
|
||||
comments={NO_COMMENTS}
|
||||
onFileClick={onFileClick}
|
||||
selectedCommit={null}
|
||||
activeFiles={files}
|
||||
commits={NO_COMMITS}
|
||||
onSelectCommit={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
const fileButtons = document.querySelectorAll('[data-testid="file-row"]');
|
||||
expect(fileButtons.length).toBeGreaterThan(0);
|
||||
fireEvent.click(fileButtons[0]);
|
||||
|
||||
// First file after alphabetical sort within the directory
|
||||
expect(onFileClick).toHaveBeenCalledWith(files[0].newPath);
|
||||
});
|
||||
|
||||
// 6. Root-level files (no subdirectory) render without a directory header
|
||||
it('root-level files render without a directory header', () => {
|
||||
const files = makeFiles(10, ''); // no prefix → root-level files
|
||||
renderSidebar(files);
|
||||
|
||||
expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(10);
|
||||
expect(document.querySelectorAll('[data-testid="dir-header"]').length).toBe(0);
|
||||
});
|
||||
});
|
||||
@@ -1,8 +1,15 @@
|
||||
import { useMemo, useState } from "react";
|
||||
import { useMemo, useState, useRef, useEffect, useCallback } from "react";
|
||||
// Using react-window 2.x (installed version). The task spec was written for react-window 1.x
|
||||
// (VariableSizeList API). react-window 2.x provides a `List` component with a different but
|
||||
// equivalent API: it handles ResizeObserver internally (no explicit height/width props needed),
|
||||
// uses `rowComponent`/`rowProps` for rendering, and exposes `scrollToRow` via `listRef`.
|
||||
import { List } from "react-window";
|
||||
import type { RowComponentProps, ListImperativeAPI } from "react-window";
|
||||
import {
|
||||
MessageSquare,
|
||||
FileCode,
|
||||
FolderOpen,
|
||||
ChevronRight,
|
||||
Plus,
|
||||
Minus,
|
||||
Circle,
|
||||
@@ -38,6 +45,8 @@ export function ReviewSidebar({
|
||||
viewedFiles = new Set(),
|
||||
}: ReviewSidebarProps) {
|
||||
const [view, setView] = useState<SidebarView>("files");
|
||||
// Persist Files-tab scroll offset across Files ↔ Commits switches
|
||||
const filesScrollOffsetRef = useRef<number>(0);
|
||||
|
||||
return (
|
||||
<div className="flex h-full">
|
||||
@@ -58,8 +67,8 @@ export function ReviewSidebar({
|
||||
/>
|
||||
</div>
|
||||
|
||||
{/* Content panel */}
|
||||
<div className="flex-1 min-w-0 overflow-y-auto p-4">
|
||||
{/* Content panel — flex column so FilesView can stretch and manage its own scroll */}
|
||||
<div className="flex-1 min-w-0 flex flex-col min-h-0">
|
||||
{view === "files" ? (
|
||||
<FilesView
|
||||
files={files}
|
||||
@@ -69,13 +78,16 @@ export function ReviewSidebar({
|
||||
selectedCommit={selectedCommit}
|
||||
activeFiles={activeFiles}
|
||||
viewedFiles={viewedFiles}
|
||||
scrollOffsetRef={filesScrollOffsetRef}
|
||||
/>
|
||||
) : (
|
||||
<CommitsView
|
||||
commits={commits}
|
||||
selectedCommit={selectedCommit}
|
||||
onSelectCommit={onSelectCommit}
|
||||
/>
|
||||
<div className="overflow-y-auto p-4 flex-1">
|
||||
<CommitsView
|
||||
commits={commits}
|
||||
selectedCommit={selectedCommit}
|
||||
onSelectCommit={onSelectCommit}
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
@@ -171,6 +183,109 @@ const changeTypeDotColor: Record<string, string> = {
|
||||
renamed: "bg-status-active-fg",
|
||||
};
|
||||
|
||||
// ─── Row type for virtualized list ───
|
||||
|
||||
type Row =
|
||||
| { kind: "dir-header"; dirName: string; fileCount: number; isCollapsed: boolean }
|
||||
| { kind: "file"; file: FileDiff; dirName: string; isViewed: boolean; commentCount: number };
|
||||
|
||||
// Item heights: dir-header ≈ 32px (py-0.5 + icon), file row ≈ 40px (py-1 + text)
|
||||
const DIR_HEADER_HEIGHT = 32;
|
||||
const FILE_ROW_HEIGHT = 40;
|
||||
|
||||
// ─── Virtualized row component (must be stable — defined outside FilesView) ───
|
||||
|
||||
type VirtualRowProps = {
|
||||
rows: Row[];
|
||||
selectedCommit: string | null;
|
||||
activeFilePaths: Set<string>;
|
||||
onFileClick: (filePath: string) => void;
|
||||
onToggleDir: (dirName: string) => void;
|
||||
};
|
||||
|
||||
function VirtualRowItem({
|
||||
index,
|
||||
style,
|
||||
rows,
|
||||
selectedCommit,
|
||||
activeFilePaths,
|
||||
onFileClick,
|
||||
onToggleDir,
|
||||
}: RowComponentProps<VirtualRowProps>) {
|
||||
const row = rows[index];
|
||||
if (!row) return null;
|
||||
|
||||
if (row.kind === "dir-header") {
|
||||
return (
|
||||
<button
|
||||
data-testid="dir-header"
|
||||
style={style}
|
||||
className="flex w-full items-center gap-1 text-[10px] font-mono text-muted-foreground/70 px-2 hover:bg-accent/30 transition-colors"
|
||||
onClick={() => onToggleDir(row.dirName)}
|
||||
title={row.isCollapsed ? "Expand directory" : "Collapse directory"}
|
||||
>
|
||||
<ChevronRight
|
||||
className={`h-3 w-3 shrink-0 transition-transform ${row.isCollapsed ? "" : "rotate-90"}`}
|
||||
/>
|
||||
<FolderOpen className="h-3 w-3 shrink-0" />
|
||||
<span className="truncate">{row.dirName}</span>
|
||||
</button>
|
||||
);
|
||||
}
|
||||
|
||||
// kind === "file"
|
||||
const { file, dirName, isViewed, commentCount } = row;
|
||||
const isInView = activeFilePaths.has(file.newPath);
|
||||
const dimmed = selectedCommit && !isInView;
|
||||
const dotColor = changeTypeDotColor[file.changeType];
|
||||
|
||||
return (
|
||||
<button
|
||||
data-testid="file-row"
|
||||
style={style}
|
||||
className={`
|
||||
flex w-full items-center gap-1.5 rounded py-1 text-left text-[11px]
|
||||
hover:bg-accent/50 transition-colors group
|
||||
${dirName ? "pl-4 pr-2" : "px-2"}
|
||||
${dimmed ? "opacity-35" : ""}
|
||||
`}
|
||||
onClick={() => onFileClick(file.newPath)}
|
||||
>
|
||||
{isViewed ? (
|
||||
<CheckCircle2 className="h-3 w-3 text-status-success-fg shrink-0" />
|
||||
) : (
|
||||
<FileCode className="h-3 w-3 text-muted-foreground shrink-0" />
|
||||
)}
|
||||
{dotColor && (
|
||||
<span className={`h-1.5 w-1.5 rounded-full shrink-0 ${dotColor}`} />
|
||||
)}
|
||||
<span className="truncate flex-1 font-mono">
|
||||
{getFileName(file.newPath)}
|
||||
</span>
|
||||
<span className="flex items-center gap-1 shrink-0">
|
||||
{commentCount > 0 && (
|
||||
<span className="flex items-center gap-0.5 text-muted-foreground">
|
||||
<MessageSquare className="h-2.5 w-2.5" />
|
||||
{commentCount}
|
||||
</span>
|
||||
)}
|
||||
{file.additions > 0 && (
|
||||
<span className="text-diff-add-fg text-[10px]">
|
||||
<Plus className="h-2.5 w-2.5 inline" />
|
||||
{file.additions}
|
||||
</span>
|
||||
)}
|
||||
{file.deletions > 0 && (
|
||||
<span className="text-diff-remove-fg text-[10px]">
|
||||
<Minus className="h-2.5 w-2.5 inline" />
|
||||
{file.deletions}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
}
|
||||
|
||||
function FilesView({
|
||||
files,
|
||||
comments,
|
||||
@@ -179,6 +294,7 @@ function FilesView({
|
||||
selectedCommit,
|
||||
activeFiles,
|
||||
viewedFiles,
|
||||
scrollOffsetRef,
|
||||
}: {
|
||||
files: FileDiff[];
|
||||
comments: ReviewComment[];
|
||||
@@ -187,10 +303,14 @@ function FilesView({
|
||||
selectedCommit: string | null;
|
||||
activeFiles: FileDiff[];
|
||||
viewedFiles: Set<string>;
|
||||
scrollOffsetRef: React.MutableRefObject<number>;
|
||||
}) {
|
||||
const unresolvedCount = comments.filter((c) => !c.resolved && !c.parentCommentId).length;
|
||||
const resolvedCount = comments.filter((c) => c.resolved && !c.parentCommentId).length;
|
||||
const activeFilePaths = new Set(activeFiles.map((f) => f.newPath));
|
||||
const activeFilePaths = useMemo(
|
||||
() => new Set(activeFiles.map((f) => f.newPath)),
|
||||
[activeFiles],
|
||||
);
|
||||
|
||||
const directoryGroups = useMemo(() => groupFilesByDirectory(files), [files]);
|
||||
|
||||
@@ -198,169 +318,308 @@ function FilesView({
|
||||
const totalCount = files.length;
|
||||
const progressPercent = totalCount > 0 ? (viewedCount / totalCount) * 100 : 0;
|
||||
|
||||
// ─── Collapse state ───
|
||||
const [collapsedDirs, setCollapsedDirs] = useState<Set<string>>(new Set());
|
||||
|
||||
const toggleDir = useCallback((dirName: string) => {
|
||||
setCollapsedDirs((prev) => {
|
||||
const next = new Set(prev);
|
||||
if (next.has(dirName)) next.delete(dirName);
|
||||
else next.add(dirName);
|
||||
return next;
|
||||
});
|
||||
}, []);
|
||||
|
||||
// ─── Flat row list for virtualization ───
|
||||
const rows = useMemo<Row[]>(() => {
|
||||
const result: Row[] = [];
|
||||
for (const group of directoryGroups) {
|
||||
const isCollapsed = collapsedDirs.has(group.directory);
|
||||
// Root-level files (directory === "") get no dir-header, preserving existing behavior
|
||||
if (group.directory) {
|
||||
result.push({
|
||||
kind: "dir-header",
|
||||
dirName: group.directory,
|
||||
fileCount: group.files.length,
|
||||
isCollapsed,
|
||||
});
|
||||
}
|
||||
if (!isCollapsed) {
|
||||
for (const file of group.files) {
|
||||
const commentCount = comments.filter(
|
||||
(c) => c.filePath === file.newPath && !c.parentCommentId,
|
||||
).length;
|
||||
result.push({
|
||||
kind: "file",
|
||||
file,
|
||||
dirName: group.directory,
|
||||
isViewed: viewedFiles.has(file.newPath),
|
||||
commentCount,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}, [directoryGroups, collapsedDirs, comments, viewedFiles]);
|
||||
|
||||
const isVirtualized = rows.length > 50;
|
||||
|
||||
// ─── react-window 2.x imperative ref ───
|
||||
const listRef = useRef<ListImperativeAPI | null>(null);
|
||||
// Fallback container ref for non-virtualized path
|
||||
const containerRef = useRef<HTMLDivElement | null>(null);
|
||||
|
||||
// Restore scroll position on mount (both paths)
|
||||
useEffect(() => {
|
||||
const offset = scrollOffsetRef.current;
|
||||
if (!offset) return;
|
||||
if (isVirtualized) {
|
||||
// react-window 2.x: scroll via the outermost DOM element
|
||||
const el = listRef.current?.element;
|
||||
if (el) el.scrollTop = offset;
|
||||
} else if (containerRef.current) {
|
||||
containerRef.current.scrollTop = offset;
|
||||
}
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, []); // only on mount
|
||||
|
||||
// Save scroll position on unmount (both paths)
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
if (isVirtualized) {
|
||||
scrollOffsetRef.current = listRef.current?.element?.scrollTop ?? 0;
|
||||
} else {
|
||||
scrollOffsetRef.current = containerRef.current?.scrollTop ?? 0;
|
||||
}
|
||||
};
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, [isVirtualized]);
|
||||
|
||||
// Row height function for react-window 2.x List
|
||||
const rowHeight = useCallback(
|
||||
(index: number) => (rows[index]?.kind === "dir-header" ? DIR_HEADER_HEIGHT : FILE_ROW_HEIGHT),
|
||||
[rows],
|
||||
);
|
||||
|
||||
// Handle file click: call onFileClick and scroll virtual list to row
|
||||
const handleFileClick = useCallback(
|
||||
(filePath: string) => {
|
||||
onFileClick(filePath);
|
||||
const rowIndex = rows.findIndex(
|
||||
(r) => r.kind === "file" && r.file.newPath === filePath,
|
||||
);
|
||||
if (rowIndex >= 0) {
|
||||
listRef.current?.scrollToRow({ index: rowIndex, align: "smart" });
|
||||
}
|
||||
},
|
||||
[onFileClick, rows, listRef],
|
||||
);
|
||||
|
||||
// Stable row props for the virtual row component
|
||||
const rowProps = useMemo<VirtualRowProps>(
|
||||
() => ({
|
||||
rows,
|
||||
selectedCommit,
|
||||
activeFilePaths,
|
||||
onFileClick: handleFileClick,
|
||||
onToggleDir: toggleDir,
|
||||
}),
|
||||
[rows, selectedCommit, activeFilePaths, handleFileClick, toggleDir],
|
||||
);
|
||||
|
||||
return (
|
||||
<div className="space-y-4">
|
||||
{/* Review progress */}
|
||||
{totalCount > 0 && (
|
||||
<div className="space-y-1.5">
|
||||
<h4 className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider">
|
||||
Review Progress
|
||||
</h4>
|
||||
<div className="h-1 rounded-full bg-muted w-full">
|
||||
<div
|
||||
className="h-full rounded-full bg-status-success-fg transition-all duration-300"
|
||||
style={{ width: `${progressPercent}%` }}
|
||||
/>
|
||||
</div>
|
||||
<span className="text-[10px] text-muted-foreground">
|
||||
{viewedCount}/{totalCount} files viewed
|
||||
</span>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Discussions — individual threads */}
|
||||
{comments.length > 0 && (
|
||||
<div className="space-y-1.5">
|
||||
<h4 className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider flex items-center justify-between">
|
||||
<span>Discussions</span>
|
||||
<span className="flex items-center gap-2 font-normal normal-case">
|
||||
{unresolvedCount > 0 && (
|
||||
<span className="flex items-center gap-0.5 text-status-warning-fg">
|
||||
<Circle className="h-2.5 w-2.5" />
|
||||
{unresolvedCount}
|
||||
</span>
|
||||
)}
|
||||
{resolvedCount > 0 && (
|
||||
<span className="flex items-center gap-0.5 text-status-success-fg">
|
||||
<CheckCircle2 className="h-2.5 w-2.5" />
|
||||
{resolvedCount}
|
||||
</span>
|
||||
)}
|
||||
<div className="flex flex-col h-full min-h-0">
|
||||
{/* Fixed header — review progress + discussions */}
|
||||
<div className="p-4 space-y-4 shrink-0">
|
||||
{/* Review progress */}
|
||||
{totalCount > 0 && (
|
||||
<div className="space-y-1.5">
|
||||
<h4 className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider">
|
||||
Review Progress
|
||||
</h4>
|
||||
<div className="h-1 rounded-full bg-muted w-full">
|
||||
<div
|
||||
className="h-full rounded-full bg-status-success-fg transition-all duration-300"
|
||||
style={{ width: `${progressPercent}%` }}
|
||||
/>
|
||||
</div>
|
||||
<span className="text-[10px] text-muted-foreground">
|
||||
{viewedCount}/{totalCount} files viewed
|
||||
</span>
|
||||
</h4>
|
||||
<div className="space-y-0.5">
|
||||
{comments
|
||||
.filter((c) => !c.parentCommentId)
|
||||
.map((thread) => {
|
||||
const replyCount = comments.filter(
|
||||
(c) => c.parentCommentId === thread.id,
|
||||
).length;
|
||||
return (
|
||||
<button
|
||||
key={thread.id}
|
||||
className={`
|
||||
flex w-full flex-col gap-0.5 rounded px-2 py-1.5 text-left
|
||||
transition-colors hover:bg-accent/50
|
||||
${thread.resolved ? "opacity-50" : ""}
|
||||
`}
|
||||
onClick={() => onCommentClick ? onCommentClick(thread.id) : onFileClick(thread.filePath)}
|
||||
>
|
||||
<div className="flex items-center gap-1.5 w-full min-w-0">
|
||||
{thread.resolved ? (
|
||||
<CheckCircle2 className="h-3 w-3 text-status-success-fg shrink-0" />
|
||||
) : (
|
||||
<MessageSquare className="h-3 w-3 text-status-warning-fg shrink-0" />
|
||||
)}
|
||||
<span className="text-[10px] font-mono text-muted-foreground truncate">
|
||||
{getFileName(thread.filePath)}:{thread.lineNumber}
|
||||
</span>
|
||||
{replyCount > 0 && (
|
||||
<span className="text-[9px] text-muted-foreground/70 shrink-0 ml-auto">
|
||||
{replyCount}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<span className="text-[11px] text-foreground/80 truncate pl-[18px]">
|
||||
{thread.body.length > 60
|
||||
? thread.body.slice(0, 57) + "..."
|
||||
: thread.body}
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
)}
|
||||
|
||||
{/* Directory-grouped file tree */}
|
||||
<div>
|
||||
<h4 className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider mb-1.5">
|
||||
Files
|
||||
{selectedCommit && (
|
||||
<span className="font-normal ml-1 normal-case">
|
||||
({activeFiles.length} in commit)
|
||||
</span>
|
||||
)}
|
||||
</h4>
|
||||
{directoryGroups.map((group) => (
|
||||
<div key={group.directory}>
|
||||
{/* Directory header */}
|
||||
{group.directory && (
|
||||
<div className="text-[10px] font-mono text-muted-foreground/70 mt-2 first:mt-0 px-2 py-0.5 flex items-center gap-1">
|
||||
<FolderOpen className="h-3 w-3 shrink-0" />
|
||||
<span className="truncate">{group.directory}</span>
|
||||
</div>
|
||||
)}
|
||||
{/* Files in directory */}
|
||||
{/* Discussions — individual threads */}
|
||||
{comments.length > 0 && (
|
||||
<div className="space-y-1.5">
|
||||
<h4 className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider flex items-center justify-between">
|
||||
<span>Discussions</span>
|
||||
<span className="flex items-center gap-2 font-normal normal-case">
|
||||
{unresolvedCount > 0 && (
|
||||
<span className="flex items-center gap-0.5 text-status-warning-fg">
|
||||
<Circle className="h-2.5 w-2.5" />
|
||||
{unresolvedCount}
|
||||
</span>
|
||||
)}
|
||||
{resolvedCount > 0 && (
|
||||
<span className="flex items-center gap-0.5 text-status-success-fg">
|
||||
<CheckCircle2 className="h-2.5 w-2.5" />
|
||||
{resolvedCount}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
</h4>
|
||||
<div className="space-y-0.5">
|
||||
{group.files.map((file) => {
|
||||
const fileCommentCount = comments.filter(
|
||||
(c) => c.filePath === file.newPath && !c.parentCommentId,
|
||||
).length;
|
||||
const isInView = activeFilePaths.has(file.newPath);
|
||||
const dimmed = selectedCommit && !isInView;
|
||||
const isViewed = viewedFiles.has(file.newPath);
|
||||
const dotColor = changeTypeDotColor[file.changeType];
|
||||
|
||||
return (
|
||||
<button
|
||||
key={file.newPath}
|
||||
className={`
|
||||
flex w-full items-center gap-1.5 rounded py-1 text-left text-[11px]
|
||||
hover:bg-accent/50 transition-colors group
|
||||
${group.directory ? "pl-4 pr-2" : "px-2"}
|
||||
${dimmed ? "opacity-35" : ""}
|
||||
`}
|
||||
onClick={() => onFileClick(file.newPath)}
|
||||
>
|
||||
{isViewed ? (
|
||||
<CheckCircle2 className="h-3 w-3 text-status-success-fg shrink-0" />
|
||||
) : (
|
||||
<FileCode className="h-3 w-3 text-muted-foreground shrink-0" />
|
||||
)}
|
||||
{dotColor && (
|
||||
<span className={`h-1.5 w-1.5 rounded-full shrink-0 ${dotColor}`} />
|
||||
)}
|
||||
<span className="truncate flex-1 font-mono">
|
||||
{getFileName(file.newPath)}
|
||||
</span>
|
||||
<span className="flex items-center gap-1 shrink-0">
|
||||
{fileCommentCount > 0 && (
|
||||
<span className="flex items-center gap-0.5 text-muted-foreground">
|
||||
<MessageSquare className="h-2.5 w-2.5" />
|
||||
{fileCommentCount}
|
||||
{comments
|
||||
.filter((c) => !c.parentCommentId)
|
||||
.map((thread) => {
|
||||
const replyCount = comments.filter(
|
||||
(c) => c.parentCommentId === thread.id,
|
||||
).length;
|
||||
return (
|
||||
<button
|
||||
key={thread.id}
|
||||
className={`
|
||||
flex w-full flex-col gap-0.5 rounded px-2 py-1.5 text-left
|
||||
transition-colors hover:bg-accent/50
|
||||
${thread.resolved ? "opacity-50" : ""}
|
||||
`}
|
||||
onClick={() => onCommentClick ? onCommentClick(thread.id) : onFileClick(thread.filePath)}
|
||||
>
|
||||
<div className="flex items-center gap-1.5 w-full min-w-0">
|
||||
{thread.resolved ? (
|
||||
<CheckCircle2 className="h-3 w-3 text-status-success-fg shrink-0" />
|
||||
) : (
|
||||
<MessageSquare className="h-3 w-3 text-status-warning-fg shrink-0" />
|
||||
)}
|
||||
<span className="text-[10px] font-mono text-muted-foreground truncate">
|
||||
{getFileName(thread.filePath)}:{thread.lineNumber}
|
||||
</span>
|
||||
)}
|
||||
{file.additions > 0 && (
|
||||
<span className="text-diff-add-fg text-[10px]">
|
||||
<Plus className="h-2.5 w-2.5 inline" />
|
||||
{file.additions}
|
||||
</span>
|
||||
)}
|
||||
{file.deletions > 0 && (
|
||||
<span className="text-diff-remove-fg text-[10px]">
|
||||
<Minus className="h-2.5 w-2.5 inline" />
|
||||
{file.deletions}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
{replyCount > 0 && (
|
||||
<span className="text-[9px] text-muted-foreground/70 shrink-0 ml-auto">
|
||||
{replyCount}
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<span className="text-[11px] text-foreground/80 truncate pl-[18px]">
|
||||
{thread.body.length > 60
|
||||
? thread.body.slice(0, 57) + "..."
|
||||
: thread.body}
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
))}
|
||||
)}
|
||||
|
||||
{/* Files section heading */}
|
||||
<div>
|
||||
<h4 className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider mb-1.5">
|
||||
Files
|
||||
{selectedCommit && (
|
||||
<span className="font-normal ml-1 normal-case">
|
||||
({activeFiles.length} in commit)
|
||||
</span>
|
||||
)}
|
||||
</h4>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Scrollable file tree — virtualized (react-window 2.x List) when >50 rows */}
|
||||
{isVirtualized ? (
|
||||
<List
|
||||
listRef={listRef}
|
||||
rowCount={rows.length}
|
||||
rowHeight={rowHeight}
|
||||
rowComponent={VirtualRowItem}
|
||||
rowProps={rowProps}
|
||||
defaultHeight={600}
|
||||
style={{ flex: 1, minHeight: 0 }}
|
||||
/>
|
||||
) : (
|
||||
<div ref={containerRef} className="overflow-y-auto px-4 pb-4">
|
||||
{directoryGroups.map((group) => (
|
||||
<div key={group.directory}>
|
||||
{/* Directory header — collapsible */}
|
||||
{group.directory && (
|
||||
<button
|
||||
data-testid="dir-header"
|
||||
className="flex w-full items-center gap-1 text-[10px] font-mono text-muted-foreground/70 mt-2 first:mt-0 px-2 py-0.5 hover:bg-accent/30 transition-colors"
|
||||
onClick={() => toggleDir(group.directory)}
|
||||
title={collapsedDirs.has(group.directory) ? "Expand directory" : "Collapse directory"}
|
||||
>
|
||||
<ChevronRight
|
||||
className={`h-3 w-3 shrink-0 transition-transform ${collapsedDirs.has(group.directory) ? "" : "rotate-90"}`}
|
||||
/>
|
||||
<FolderOpen className="h-3 w-3 shrink-0" />
|
||||
<span className="truncate">{group.directory}</span>
|
||||
</button>
|
||||
)}
|
||||
{/* Files in directory */}
|
||||
{!collapsedDirs.has(group.directory) && (
|
||||
<div className="space-y-0.5">
|
||||
{group.files.map((file) => {
|
||||
const fileCommentCount = comments.filter(
|
||||
(c) => c.filePath === file.newPath && !c.parentCommentId,
|
||||
).length;
|
||||
const isInView = activeFilePaths.has(file.newPath);
|
||||
const dimmed = selectedCommit && !isInView;
|
||||
const isViewed = viewedFiles.has(file.newPath);
|
||||
const dotColor = changeTypeDotColor[file.changeType];
|
||||
|
||||
return (
|
||||
<button
|
||||
key={file.newPath}
|
||||
data-testid="file-row"
|
||||
className={`
|
||||
flex w-full items-center gap-1.5 rounded py-1 text-left text-[11px]
|
||||
hover:bg-accent/50 transition-colors group
|
||||
${group.directory ? "pl-4 pr-2" : "px-2"}
|
||||
${dimmed ? "opacity-35" : ""}
|
||||
`}
|
||||
onClick={() => onFileClick(file.newPath)}
|
||||
>
|
||||
{isViewed ? (
|
||||
<CheckCircle2 className="h-3 w-3 text-status-success-fg shrink-0" />
|
||||
) : (
|
||||
<FileCode className="h-3 w-3 text-muted-foreground shrink-0" />
|
||||
)}
|
||||
{dotColor && (
|
||||
<span className={`h-1.5 w-1.5 rounded-full shrink-0 ${dotColor}`} />
|
||||
)}
|
||||
<span className="truncate flex-1 font-mono">
|
||||
{getFileName(file.newPath)}
|
||||
</span>
|
||||
<span className="flex items-center gap-1 shrink-0">
|
||||
{fileCommentCount > 0 && (
|
||||
<span className="flex items-center gap-0.5 text-muted-foreground">
|
||||
<MessageSquare className="h-2.5 w-2.5" />
|
||||
{fileCommentCount}
|
||||
</span>
|
||||
)}
|
||||
{file.additions > 0 && (
|
||||
<span className="text-diff-add-fg text-[10px]">
|
||||
<Plus className="h-2.5 w-2.5 inline" />
|
||||
{file.additions}
|
||||
</span>
|
||||
)}
|
||||
{file.deletions > 0 && (
|
||||
<span className="text-diff-remove-fg text-[10px]">
|
||||
<Minus className="h-2.5 w-2.5 inline" />
|
||||
{file.deletions}
|
||||
</span>
|
||||
)}
|
||||
</span>
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -14,6 +14,7 @@
|
||||
| Tiptap | Rich text editor (ProseMirror-based) |
|
||||
| Lucide | Icon library |
|
||||
| Geist Sans/Mono | Typography (variable fonts in `public/fonts/`) |
|
||||
| react-window 2.x | Virtualized list rendering for large file trees in ReviewSidebar |
|
||||
|
||||
## Design System (v2)
|
||||
|
||||
@@ -115,7 +116,7 @@ The initiative detail page has three tabs managed via local state (not URL param
|
||||
|-----------|---------|
|
||||
| `ReviewTab` | Review tab container — orchestrates header, diff, sidebar, and preview. Phase-level review has threaded inline comments (with reply support) + Request Changes; initiative-level review has Request Changes (summary prompt) + Push Branch / Merge & Push |
|
||||
| `ReviewHeader` | Consolidated toolbar: phase selector pills, branch info, stats, preview controls, approve/reject actions |
|
||||
| `ReviewSidebar` | VSCode-style icon strip (Files/Commits views) with file list, root-only comment counts, and commit navigation |
|
||||
| `ReviewSidebar` | VSCode-style icon strip (Files/Commits views) with file list, root-only comment counts, and commit navigation. FilesView uses react-window 2.x `List` for virtualized rendering when the row count exceeds 50 (dir-headers + file rows). Scroll position is preserved across Files ↔ Commits tab switches. Directories are collapsible. Clicking a file scrolls the virtual list to that row. |
|
||||
| `DiffViewer` | Unified diff renderer with threaded inline comments (root + reply threads) |
|
||||
| `CommentThread` | Renders root comment with resolve/reopen + nested reply threads (agent replies styled with primary border). Inline reply form |
|
||||
| `ConflictResolutionPanel` | Merge conflict detection + agent resolution in initiative review. Shows conflict files, spawns conflict agent, inline questions, re-check on completion |
|
||||
|
||||
21
package-lock.json
generated
21
package-lock.json
generated
@@ -73,12 +73,14 @@
|
||||
"@tiptap/suggestion": "^3.19.0",
|
||||
"@trpc/client": "^11.9.0",
|
||||
"@trpc/react-query": "^11.9.0",
|
||||
"@types/react-window": "^1.8.8",
|
||||
"class-variance-authority": "^0.7.1",
|
||||
"clsx": "^2.1.1",
|
||||
"geist": "^1.7.0",
|
||||
"lucide-react": "^0.563.0",
|
||||
"react": "^19.0.0",
|
||||
"react-dom": "^19.0.0",
|
||||
"react-window": "^2.2.7",
|
||||
"sonner": "^2.0.7",
|
||||
"tailwind-merge": "^3.4.0",
|
||||
"tippy.js": "^6.3.7"
|
||||
@@ -5198,6 +5200,15 @@
|
||||
"@types/react": "^19.2.0"
|
||||
}
|
||||
},
|
||||
"node_modules/@types/react-window": {
|
||||
"version": "1.8.8",
|
||||
"resolved": "https://registry.npmjs.org/@types/react-window/-/react-window-1.8.8.tgz",
|
||||
"integrity": "sha512-8Ls660bHR1AUA2kuRvVG9D/4XpRC6wjAaPT9dil7Ckc76eP9TKWZwwmgfq8Q1LANX3QNDnoU4Zp48A3w+zK69Q==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"@types/react": "*"
|
||||
}
|
||||
},
|
||||
"node_modules/@types/unist": {
|
||||
"version": "3.0.3",
|
||||
"resolved": "https://registry.npmjs.org/@types/unist/-/unist-3.0.3.tgz",
|
||||
@@ -9131,6 +9142,16 @@
|
||||
}
|
||||
}
|
||||
},
|
||||
"node_modules/react-window": {
|
||||
"version": "2.2.7",
|
||||
"resolved": "https://registry.npmjs.org/react-window/-/react-window-2.2.7.tgz",
|
||||
"integrity": "sha512-SH5nvfUQwGHYyriDUAOt7wfPsfG9Qxd6OdzQxl5oQ4dsSsUicqQvjV7dR+NqZ4coY0fUn3w1jnC5PwzIUWEg5w==",
|
||||
"license": "MIT",
|
||||
"peerDependencies": {
|
||||
"react": "^18.0.0 || ^19.0.0",
|
||||
"react-dom": "^18.0.0 || ^19.0.0"
|
||||
}
|
||||
},
|
||||
"node_modules/read-cache": {
|
||||
"version": "1.0.0",
|
||||
"resolved": "https://registry.npmjs.org/read-cache/-/read-cache-1.0.0.tgz",
|
||||
|
||||
@@ -5,11 +5,24 @@ import path from 'node:path';
|
||||
export default defineConfig({
|
||||
plugins: [react()],
|
||||
resolve: {
|
||||
// Alias react to the parent monorepo's copy, matching what @testing-library
|
||||
// loads react-dom from. This ensures React DOM and our components share the
|
||||
// same ReactSharedInternals and hook dispatcher — preventing null-dispatcher
|
||||
// errors when running tests from a git worktree.
|
||||
alias: {
|
||||
'@': path.resolve(__dirname, './apps/web/src'),
|
||||
react: path.resolve(__dirname, '../../../../node_modules/react'),
|
||||
'react-dom': path.resolve(__dirname, '../../../../node_modules/react-dom'),
|
||||
},
|
||||
dedupe: ['react', 'react-dom'],
|
||||
},
|
||||
test: {
|
||||
// Force react-dom and @testing-library through Vite's module graph so that
|
||||
// the resolve.alias for 'react-dom' applies (prevents parent-monorepo
|
||||
// react-dom loading a different React instance than our source files).
|
||||
deps: {
|
||||
inline: ['react-dom', '@testing-library/react'],
|
||||
},
|
||||
// Enable test globals (describe, it, expect without imports)
|
||||
globals: true,
|
||||
env: {
|
||||
|
||||
Reference in New Issue
Block a user