Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions apps/web/components/github/pr-detail-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { ScrollArea } from "@kandev/ui/scroll-area";
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
import { useAppStore } from "@/components/state-provider";
import { useActiveTaskPR, useTaskPR } from "@/hooks/domains/github/use-task-pr";
import { prPanelLabel } from "@/components/github/pr-utils";
import { prPanelLabel, prTaskKey } from "@/components/github/pr-utils";
import { usePRFeedback } from "@/hooks/domains/github/use-pr-feedback";
import { useGitHubStatus } from "@/hooks/domains/github/use-github-status";
import { useCommentsStore, isPRFeedbackComment } from "@/lib/state/slices/comments";
Expand Down Expand Up @@ -78,11 +78,6 @@ export function PRDetailPanelComponent({ panelId, params }: PRDetailPanelProps)
);
}

/** Stable per-PR key used by addPRPanel and the multi-PR topbar buttons. */
export function prTaskKey(pr: TaskPR): string {
return `${pr.owner}/${pr.repo}/${pr.pr_number}`;
}

// --- Add PR feedback as chat context ---

function useAddPRFeedbackAsContext(sessionId: string, prNumber: number) {
Expand Down
3 changes: 1 addition & 2 deletions apps/web/components/github/pr-topbar-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ import {
isPRReadyToMerge,
isPRWaitingOnBranchProtection,
} from "@/components/github/pr-task-icon";
import { prTaskKey } from "@/components/github/pr-detail-panel";
import { prIdentitySlug } from "@/components/github/pr-utils";
import { prIdentitySlug, prTaskKey } from "@/components/github/pr-utils";
import { PR_CI_DESKTOP_POPOVER_SCROLL_CLASS, PRCIPopover } from "@/components/github/pr-ci-popover";
import { MultiPRCIPopover } from "@/components/github/multi-pr-ci-popover";
import { useAppStore } from "@/components/state-provider";
Expand Down
5 changes: 5 additions & 0 deletions apps/web/components/github/pr-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,8 @@ export function prPanelLabel(prNumber: number): string {
export function prIdentitySlug(pr: TaskPR): string {
return `${pr.owner}-${pr.repo}-${pr.pr_number}`;
}

/** Stable per-PR key used by task-scoped state and dockview panels. */
export function prTaskKey(pr: TaskPR): string {
return `${pr.owner}/${pr.repo}/${pr.pr_number}`;
}
41 changes: 41 additions & 0 deletions apps/web/components/review/review-dialog-pr-state.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { TaskPR } from "@/lib/types/github";
import { ReviewPRDiffBoundary } from "./review-dialog-pr-state";

afterEach(cleanup);

const selectedPR = {
repo: "widgets",
pr_number: 42,
} as TaskPR;

describe("ReviewPRDiffBoundary", () => {
it("retries a failed selected PR without rendering stale children", () => {
const onRetry = vi.fn();
render(
<ReviewPRDiffBoundary
selectedPR={selectedPR}
loading={false}
error="Could not load PR changes"
onRetry={onRetry}
>
<span>stale diff</span>
</ReviewPRDiffBoundary>,
);

expect(screen.queryByText("stale diff")).toBeNull();
fireEvent.click(screen.getByRole("button", { name: "Retry" }));
expect(onRetry).toHaveBeenCalledOnce();
});

it("ignores PR fetch state for a local-only review source", () => {
render(
<ReviewPRDiffBoundary selectedPR={null} loading error="Could not load PR changes">
<span>local diff</span>
</ReviewPRDiffBoundary>,
);

expect(screen.getByText("local diff")).toBeTruthy();
});
});
149 changes: 149 additions & 0 deletions apps/web/components/review/review-dialog-pr-state.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
"use client";

import { useCallback, useEffect, useRef, useState, type ReactNode } from "react";
import { IconLoader2, IconRefresh } from "@tabler/icons-react";
import { Button } from "@kandev/ui/button";
import type { TaskPR } from "@/lib/types/github";

type AutoCloseReviewDialogInput = {
open: boolean;
previousFileCount: number | null;
fileCount: number;
prDiffLoading: boolean;
};

export function shouldAutoCloseReviewDialog({
open,
previousFileCount,
fileCount,
prDiffLoading,
}: AutoCloseReviewDialogInput): boolean {
return (
open && !prDiffLoading && previousFileCount !== null && previousFileCount > 0 && fileCount === 0
);
}

export function useReviewDialogAutoClose(opts: {
open: boolean;
fileCount: number;
prDiffLoading: boolean;
onOpenChange: (open: boolean) => void;
}) {
const previousFileCountRef = useRef<number | null>(null);
useEffect(() => {
if (
shouldAutoCloseReviewDialog({
open: opts.open,
previousFileCount: previousFileCountRef.current,
fileCount: opts.fileCount,
prDiffLoading: opts.prDiffLoading,
})
) {
opts.onOpenChange(false);
}
previousFileCountRef.current = opts.fileCount;
}, [opts.open, opts.fileCount, opts.prDiffLoading, opts.onOpenChange]);
}

export type ReviewTransientState = {
sourceKey: string;
selectedFile: string | null;
filter: string;
};

export function reviewDialogSourceKey(sessionId: string, selectedPRKey: string | null): string {
return `${sessionId}\u0000${selectedPRKey ?? "review-without-pr"}`;
}

export function resolveReviewTransientState(
state: ReviewTransientState,
sourceKey: string,
): ReviewTransientState {
if (state.sourceKey === sourceKey) return state;
return { sourceKey, selectedFile: null, filter: "" };
}

export function useReviewDialogTransientState(sourceKey: string) {
const [state, setState] = useState<ReviewTransientState>(() => ({
sourceKey,
selectedFile: null,
filter: "",
}));
const resolvedState = resolveReviewTransientState(state, sourceKey);
if (resolvedState !== state) setState(resolvedState);
Comment thread
zeval marked this conversation as resolved.
Outdated

Comment thread
zeval marked this conversation as resolved.
const setSelectedFile = useCallback(
(value: string | null) =>
setState((current) => ({
...resolveReviewTransientState(current, sourceKey),
selectedFile: value,
})),
[sourceKey],
);
const setFilter = useCallback(
(value: string) =>
setState((current) => ({
...resolveReviewTransientState(current, sourceKey),
filter: value,
})),
[sourceKey],
);

return {
selectedFile: resolvedState.selectedFile,
filter: resolvedState.filter,
setSelectedFile,
setFilter,
};
}

export function usePRKeyedReviewFileSelection(
selectFile: (path: string, setSelectedFile: (value: string | null) => void) => void,
setSelectedFile: (value: string | null) => void,
) {
return useCallback(
(path: string) => selectFile(path, setSelectedFile),
[selectFile, setSelectedFile],
);
}

type ReviewPRDiffBoundaryProps = {
selectedPR: TaskPR | null;
loading: boolean;
error: string | null;
onRetry?: () => void;
children: ReactNode;
};

export function ReviewPRDiffBoundary({
selectedPR,
loading,
error,
onRetry,
children,
}: ReviewPRDiffBoundaryProps) {
if (selectedPR && loading) {
return (
<div className="flex h-full flex-col items-center justify-center gap-2 px-6 text-center text-sm text-muted-foreground">
<IconLoader2 className="h-5 w-5 animate-spin" />
<span>
Loading {selectedPR.repo} #{selectedPR.pr_number} changes…
</span>
</div>
);
}
if (selectedPR && error) {
return (
<div className="flex h-full flex-col items-center justify-center gap-3 px-6 text-center text-sm text-muted-foreground">
<span>{error}</span>
{onRetry && (
<Button className="min-h-11" variant="outline" size="sm" onClick={onRetry}>
<IconRefresh className="h-4 w-4" />
Retry
</Button>
)}
</div>
);
}
return children;
}
146 changes: 146 additions & 0 deletions apps/web/components/review/review-dialog-surface.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
"use client";

import { useRef } from "react";
import { Dialog, DialogContent, DialogTitle } from "@kandev/ui/dialog";
import { useReviewSidebarResize } from "@/hooks/use-review-sidebar-resize";
import type { TaskPR } from "@/lib/types/github";
import type { ReviewDialogViewState } from "./review-dialog";
import { ReviewDiffList } from "./review-diff-list";
import { ReviewFileTree } from "./review-file-tree";
import { ReviewPRDiffBoundary } from "./review-dialog-pr-state";
import { ReviewTopBar } from "./review-top-bar";

type ReviewDialogSurfaceProps = {
open: boolean;
onOpenChange: (open: boolean) => void;
sessionId: string;
baseBranch?: string;
onOpenFile?: (filePath: string) => void;
prs: TaskPR[];
selectedPR: TaskPR | null;
onSelectPR?: (pr: TaskPR) => void;
prDiffLoading: boolean;
prDiffError: string | null;
onRetryPRDiff?: () => void;
onRequestWalkthrough: () => void;
state: ReviewDialogViewState;
};

function ReviewDialogDiffContent({
sessionId,
onOpenFile,
selectedPR,
prDiffLoading,
prDiffError,
onRetryPRDiff,
state,
}: Pick<
ReviewDialogSurfaceProps,
| "sessionId"
| "onOpenFile"
| "selectedPR"
| "prDiffLoading"
| "prDiffError"
| "onRetryPRDiff"
| "state"
>) {
return (
<ReviewPRDiffBoundary
selectedPR={selectedPR}
loading={prDiffLoading}
error={prDiffError}
onRetry={onRetryPRDiff}
>
Comment thread
zeval marked this conversation as resolved.
{state.filteredFiles.length > 0 ? (
<ReviewDiffList
files={state.filteredFiles}
selectedFile={state.selectedFile}
reviewedFiles={state.reviewedFiles}
staleFiles={state.staleFiles}
sessionId={sessionId}
autoMarkOnScroll={state.autoMarkOnScroll}
wordWrap={state.wordWrap}
onToggleReviewed={state.handleToggleReviewed}
onDiscard={state.handleDiscard}
onOpenFile={onOpenFile}
fileRefs={state.fileRefs}
/>
) : (
<div className="flex h-full items-center justify-center text-sm text-muted-foreground">
{state.filter.trim() ? "No files match the filter" : "No changes to review"}
</div>
)}
</ReviewPRDiffBoundary>
);
}

export function ReviewDialogSurface(props: ReviewDialogSurfaceProps) {
const { open, onOpenChange, sessionId, state } = props;
const splitRowRef = useRef<HTMLDivElement>(null);
const sidebar = useReviewSidebarResize(splitRowRef, open, state.reviewSourceKey);

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-black/40"
>
<DialogTitle className="sr-only">Review Changes</DialogTitle>
<ReviewTopBar
sessionId={sessionId}
reviewedCount={state.reviewedFiles.size}
totalCount={state.allFiles.length}
commentCount={state.totalCommentCount}
baseBranch={props.baseBranch}
splitView={state.splitView}
onToggleSplitView={state.handleToggleSplitView}
wordWrap={state.wordWrap}
onToggleWordWrap={state.setWordWrap}
onSendComments={state.handleSendComments}
onClose={() => onOpenChange(false)}
onRequestWalkthrough={props.onRequestWalkthrough}
requestWalkthroughDisabled={state.allFiles.length === 0}
getPendingComments={state.getPendingComments}
markCommentsSent={state.markCommentsSent}
prs={props.prs}
selectedPR={props.selectedPR}
onSelectPR={props.onSelectPR}
prDiffLoading={props.prDiffLoading}
/>
<div key={state.reviewSourceKey} ref={splitRowRef} className="flex min-h-0 flex-1">
<div
data-testid="review-dialog-sidebar"
className="hidden flex-shrink-0 flex-col overflow-hidden border-r border-border sm:flex"
style={{ width: `${sidebar.width}px` }}
>
<ReviewFileTree
files={state.filteredFiles}
reviewedFiles={state.reviewedFiles}
staleFiles={state.staleFiles}
commentCountByFile={state.commentCountByFile}
selectedFile={state.selectedFile}
filter={state.filter}
onFilterChange={state.setFilter}
onSelectFile={state.handleSelectFile}
onToggleReviewed={state.handleToggleReviewed}
/>
</div>
<button
data-testid="review-dialog-sidebar-resize"
type="button"
tabIndex={-1}
aria-label="Resize file list"
className="group relative hidden w-1 flex-shrink-0 cursor-col-resize bg-border p-0 transition-colors hover:bg-primary sm:block"
{...sidebar.resizeHandleProps}
>
<span className="absolute inset-y-0 -left-1 -right-1" />
</button>
<div className="min-w-0 flex-1 overflow-hidden">
<ReviewDialogDiffContent {...props} />
</div>
</div>
</DialogContent>
</Dialog>
);
}
Loading
Loading