Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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}`;
}
82 changes: 82 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,82 @@
import { act, cleanup, fireEvent, render, renderHook, screen } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { TaskPR } from "@/lib/types/github";
import {
ReviewPRDiffBoundary,
shouldBlockReviewForPR,
useReviewDialogTransientState,
} from "./review-dialog-pr-state";

afterEach(cleanup);

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

describe("ReviewPRDiffBoundary", () => {
it("keeps expanded Review usable when local files exist during a PR failure", () => {
expect(
shouldBlockReviewForPR([
{
path: "src/local.ts",
diff: "@@ -1 +1 @@",
status: "modified",
additions: 1,
deletions: 1,
staged: false,
source: "uncommitted",
},
]),
).toBe(false);
Comment thread
zeval marked this conversation as resolved.
});

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();
});
});

describe("useReviewDialogTransientState", () => {
it("derives cleared state for a new source and keeps subsequent edits on that source", () => {
const { result, rerender } = renderHook(
({ sourceKey }) => useReviewDialogTransientState(sourceKey),
{ initialProps: { sourceKey: "session-a:pr-1" } },
);

act(() => {
result.current.setSelectedFile("src/old.ts");
result.current.setFilter("old");
});
rerender({ sourceKey: "session-b:pr-1" });

expect(result.current.selectedFile).toBeNull();
expect(result.current.filter).toBe("");

act(() => result.current.setFilter("new"));
expect(result.current.filter).toBe("new");
});
});
156 changes: 156 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,156 @@
"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";
import type { ReviewFile } from "./types";

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 shouldBlockReviewForPR(files: ReviewFile[]): boolean {
return !files.some((file) => file.source !== "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);
useEffect(() => {
setState((current) => resolveReviewTransientState(current, sourceKey));
}, [sourceKey]);

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;
}
Loading
Loading