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:
Lukas May
2026-03-06 10:43:36 +01:00
parent 4656627a59
commit 1e2819eeff
3 changed files with 68 additions and 46 deletions

View File

@@ -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"

View File

@@ -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"

View File

@@ -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) => {