fix: Request changes uses in-app confirmation, requires review comments
- Replace window.prompt with in-app dropdown confirmation (matches merge dialog pattern) - Disable button when no unresolved comments exist (comments-only, no summary) - Remove initiative-level request changes button (no comment system there)
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
import { useCallback, useMemo, useRef, useState } from "react";
|
import { useCallback, useMemo, useRef, useState } from "react";
|
||||||
import { toast } from "sonner";
|
import { toast } from "sonner";
|
||||||
import { Loader2, GitBranch, ArrowRight, FileCode, Plus, Minus, Upload, GitMerge, RotateCcw } from "lucide-react";
|
import { Loader2, GitBranch, ArrowRight, FileCode, Plus, Minus, Upload, GitMerge } from "lucide-react";
|
||||||
import { Button } from "@/components/ui/button";
|
import { Button } from "@/components/ui/button";
|
||||||
import { trpc } from "@/lib/trpc";
|
import { trpc } from "@/lib/trpc";
|
||||||
import { parseUnifiedDiff } from "./parse-diff";
|
import { parseUnifiedDiff } from "./parse-diff";
|
||||||
@@ -82,14 +82,6 @@ export function InitiativeReview({ initiativeId, onCompleted }: InitiativeReview
|
|||||||
onError: (err) => toast.error(`Failed to stop: ${err.message}`),
|
onError: (err) => toast.error(`Failed to stop: ${err.message}`),
|
||||||
});
|
});
|
||||||
|
|
||||||
const requestChangesMutation = trpc.requestInitiativeChanges.useMutation({
|
|
||||||
onSuccess: () => {
|
|
||||||
toast.success("Changes requested — review task created");
|
|
||||||
onCompleted();
|
|
||||||
},
|
|
||||||
onError: (err) => toast.error(err.message),
|
|
||||||
});
|
|
||||||
|
|
||||||
const approveMutation = trpc.approveInitiativeReview.useMutation({
|
const approveMutation = trpc.approveInitiativeReview.useMutation({
|
||||||
onSuccess: (_data, variables) => {
|
onSuccess: (_data, variables) => {
|
||||||
const msg = variables.strategy === "merge_and_push"
|
const msg = variables.strategy === "merge_and_push"
|
||||||
@@ -197,24 +189,6 @@ export function InitiativeReview({ initiativeId, onCompleted }: InitiativeReview
|
|||||||
{/* Right: preview + action buttons */}
|
{/* Right: preview + action buttons */}
|
||||||
<div className="flex items-center gap-2 shrink-0">
|
<div className="flex items-center gap-2 shrink-0">
|
||||||
{previewState && <PreviewControls preview={previewState} />}
|
{previewState && <PreviewControls preview={previewState} />}
|
||||||
<Button
|
|
||||||
variant="outline"
|
|
||||||
size="sm"
|
|
||||||
onClick={() => {
|
|
||||||
const summary = window.prompt("Describe the changes needed:");
|
|
||||||
if (!summary?.trim()) return;
|
|
||||||
requestChangesMutation.mutate({ initiativeId, summary: summary.trim() });
|
|
||||||
}}
|
|
||||||
disabled={requestChangesMutation.isPending}
|
|
||||||
className="h-8 text-xs px-3 text-status-error-fg border-status-error-border hover:bg-status-error-bg"
|
|
||||||
>
|
|
||||||
{requestChangesMutation.isPending ? (
|
|
||||||
<Loader2 className="h-3 w-3 animate-spin" />
|
|
||||||
) : (
|
|
||||||
<RotateCcw className="h-3 w-3" />
|
|
||||||
)}
|
|
||||||
Request Changes
|
|
||||||
</Button>
|
|
||||||
<Button
|
<Button
|
||||||
variant="outline"
|
variant="outline"
|
||||||
size="sm"
|
size="sm"
|
||||||
|
|||||||
@@ -64,22 +64,32 @@ export function ReviewHeader({
|
|||||||
const totalAdditions = files.reduce((s, f) => s + f.additions, 0);
|
const totalAdditions = files.reduce((s, f) => s + f.additions, 0);
|
||||||
const totalDeletions = files.reduce((s, f) => s + f.deletions, 0);
|
const totalDeletions = files.reduce((s, f) => s + f.deletions, 0);
|
||||||
const [showConfirmation, setShowConfirmation] = useState(false);
|
const [showConfirmation, setShowConfirmation] = useState(false);
|
||||||
|
const [showRequestConfirm, setShowRequestConfirm] = useState(false);
|
||||||
const confirmRef = useRef<HTMLDivElement>(null);
|
const confirmRef = useRef<HTMLDivElement>(null);
|
||||||
|
const requestConfirmRef = useRef<HTMLDivElement>(null);
|
||||||
|
|
||||||
// Click-outside handler to dismiss confirmation
|
// Click-outside handler to dismiss confirmation dropdowns
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!showConfirmation) return;
|
if (!showConfirmation && !showRequestConfirm) return;
|
||||||
function handleClickOutside(e: MouseEvent) {
|
function handleClickOutside(e: MouseEvent) {
|
||||||
if (
|
if (
|
||||||
|
showConfirmation &&
|
||||||
confirmRef.current &&
|
confirmRef.current &&
|
||||||
!confirmRef.current.contains(e.target as Node)
|
!confirmRef.current.contains(e.target as Node)
|
||||||
) {
|
) {
|
||||||
setShowConfirmation(false);
|
setShowConfirmation(false);
|
||||||
}
|
}
|
||||||
|
if (
|
||||||
|
showRequestConfirm &&
|
||||||
|
requestConfirmRef.current &&
|
||||||
|
!requestConfirmRef.current.contains(e.target as Node)
|
||||||
|
) {
|
||||||
|
setShowRequestConfirm(false);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
document.addEventListener("mousedown", handleClickOutside);
|
document.addEventListener("mousedown", handleClickOutside);
|
||||||
return () => document.removeEventListener("mousedown", handleClickOutside);
|
return () => document.removeEventListener("mousedown", handleClickOutside);
|
||||||
}, [showConfirmation]);
|
}, [showConfirmation, showRequestConfirm]);
|
||||||
|
|
||||||
const viewed = viewedCount ?? 0;
|
const viewed = viewedCount ?? 0;
|
||||||
const total = totalCount ?? 0;
|
const total = totalCount ?? 0;
|
||||||
@@ -187,20 +197,60 @@ export function ReviewHeader({
|
|||||||
<>
|
<>
|
||||||
{status === "pending" && (
|
{status === "pending" && (
|
||||||
<>
|
<>
|
||||||
<Button
|
<div className="relative" ref={requestConfirmRef}>
|
||||||
variant="outline"
|
<Button
|
||||||
size="sm"
|
variant="outline"
|
||||||
onClick={onRequestChanges}
|
size="sm"
|
||||||
disabled={isRequestingChanges}
|
onClick={() => setShowRequestConfirm(true)}
|
||||||
className="h-8 text-xs px-3 border-status-error-border/50 text-status-error-fg hover:bg-status-error-bg/50 hover:border-status-error-border"
|
disabled={isRequestingChanges || unresolvedCount === 0}
|
||||||
>
|
className="h-8 text-xs px-3 border-status-error-border/50 text-status-error-fg hover:bg-status-error-bg/50 hover:border-status-error-border"
|
||||||
{isRequestingChanges ? (
|
>
|
||||||
<Loader2 className="h-3 w-3 animate-spin" />
|
{isRequestingChanges ? (
|
||||||
) : (
|
<Loader2 className="h-3 w-3 animate-spin" />
|
||||||
<X className="h-3 w-3" />
|
) : (
|
||||||
|
<X className="h-3 w-3" />
|
||||||
|
)}
|
||||||
|
Request Changes
|
||||||
|
</Button>
|
||||||
|
|
||||||
|
{showRequestConfirm && (
|
||||||
|
<div className="absolute right-0 top-full mt-1 z-30 w-64 rounded-lg border border-border bg-card shadow-lg p-4">
|
||||||
|
<p className="text-sm font-semibold mb-3">
|
||||||
|
Request changes?
|
||||||
|
</p>
|
||||||
|
<div className="space-y-1.5 mb-4">
|
||||||
|
<div className="flex items-center gap-2 text-xs">
|
||||||
|
<AlertCircle className="h-3.5 w-3.5 text-status-error-fg" />
|
||||||
|
<span className="text-muted-foreground">
|
||||||
|
{unresolvedCount} unresolved {unresolvedCount === 1 ? "comment" : "comments"} will be sent
|
||||||
|
</span>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
<div className="flex items-center justify-end gap-2">
|
||||||
|
<Button
|
||||||
|
variant="ghost"
|
||||||
|
size="sm"
|
||||||
|
onClick={() => setShowRequestConfirm(false)}
|
||||||
|
className="h-8 text-xs"
|
||||||
|
>
|
||||||
|
Cancel
|
||||||
|
</Button>
|
||||||
|
<Button
|
||||||
|
variant="outline"
|
||||||
|
size="sm"
|
||||||
|
onClick={() => {
|
||||||
|
setShowRequestConfirm(false);
|
||||||
|
onRequestChanges();
|
||||||
|
}}
|
||||||
|
className="h-8 px-4 text-xs font-semibold shadow-sm border-status-error-border text-status-error-fg hover:bg-status-error-bg"
|
||||||
|
>
|
||||||
|
<X className="h-3.5 w-3.5" />
|
||||||
|
Request Changes
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
)}
|
)}
|
||||||
Request Changes
|
</div>
|
||||||
</Button>
|
|
||||||
<div className="relative" ref={confirmRef}>
|
<div className="relative" ref={confirmRef}>
|
||||||
<Button
|
<Button
|
||||||
size="sm"
|
size="sm"
|
||||||
|
|||||||
@@ -249,9 +249,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) {
|
|||||||
|
|
||||||
const handleRequestChanges = useCallback(() => {
|
const handleRequestChanges = useCallback(() => {
|
||||||
if (!activePhaseId) return;
|
if (!activePhaseId) return;
|
||||||
const summary = window.prompt("Optional: describe what needs to change (leave blank for comments only)");
|
requestChangesMutation.mutate({ phaseId: activePhaseId });
|
||||||
if (summary === null) return; // cancelled
|
|
||||||
requestChangesMutation.mutate({ phaseId: activePhaseId, summary: summary || undefined });
|
|
||||||
}, [activePhaseId, requestChangesMutation]);
|
}, [activePhaseId, requestChangesMutation]);
|
||||||
|
|
||||||
const handleFileClick = useCallback((filePath: string) => {
|
const handleFileClick = useCallback((filePath: string) => {
|
||||||
|
|||||||
Reference in New Issue
Block a user