Skip to content

feat(ui): issue review banner with approve / request changes #4435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ramonmatias19 wants to merge 1 commit into
base: master
Choose a base branch
from
+64 −2
Open

feat(ui): issue review banner with approve / request changes #4435

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 64 additions & 2 deletions ui/src/components/IssueChatThread.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ import { cn, formatDateTime, formatShortDate } from "../lib/utils";
import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip";
import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover";
import { Textarea } from "@/components/ui/textarea";
import { AlertTriangle, ArrowRight, Brain, Check, ChevronDown, Copy, Hammer, Loader2, MoreHorizontal, Paperclip, PauseCircle, Search, Square, ThumbsDown, ThumbsUp } from "lucide-react";
import { AlertTriangle, ArrowRight, Brain, Check, CheckCircle2, ChevronDown, Copy, Hammer, Loader2, MoreHorizontal, Paperclip, PauseCircle, Search, Square, ThumbsDown, ThumbsUp, XCircle } from "lucide-react";
import { IssueLinkQuicklook } from "./IssueLinkQuicklook";

interface IssueChatMessageContext {
Expand Down Expand Up @@ -295,6 +295,8 @@ interface IssueChatThreadProps {
answers: AskUserQuestionsAnswer[],
) => Promise<void> | void;
composerRef?: Ref<IssueChatComposerHandle>;
onReviewApprove?: () => Promise<void>;
onReviewRequestChanges?: () => Promise<void>;
}

type IssueChatErrorBoundaryProps = {
Expand Down Expand Up @@ -1675,6 +1677,12 @@ function IssueChatFeedbackButtons({
<Textarea
value={downvoteReason}
onChange={(event) => setDownvoteReason(event.target.value)}
onKeyDown={(event) => {
if (event.key === "Enter" && !event.shiftKey) {
event.preventDefault();
handleSubmitReason();
}
}}
placeholder="Add a short note"
className="min-h-20 resize-y bg-background text-sm"
disabled={isSaving}
Expand Down Expand Up @@ -2522,8 +2530,12 @@ export function IssueChatThread({
onRejectInteraction,
onSubmitInteractionAnswers,
composerRef,
onReviewApprove,
onReviewRequestChanges,
}: IssueChatThreadProps) {
const location = useLocation();
const [reviewSubmitting, setReviewSubmitting] = useState(false);
const showReviewBanner = issueStatus === "in_review" && (onReviewApprove || onReviewRequestChanges);
Comment on lines +2537 to +2538
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Shared loading state conflates both buttons

reviewSubmitting is shared, so clicking either button sets both to disabled and shows "Enviando..." on each. A user who clicks "Solicitar alterações" will also see "Enviando..." on the "Aprovar" button, which is confusing. Consider tracking which action is in-flight (e.g., reviewSubmitting: "approve" | "request-changes" | false) and showing the spinner only on the clicked button.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: ui/src/components/IssueChatThread.tsx
Line: 2397-2398

Comment:
**Shared loading state conflates both buttons**

`reviewSubmitting` is shared, so clicking either button sets both to disabled and shows "Enviando..." on each. A user who clicks "Solicitar alterações" will also see "Enviando..." on the "Aprovar" button, which is confusing. Consider tracking which action is in-flight (e.g., `reviewSubmitting: "approve" | "request-changes" | false`) and showing the spinner only on the clicked button.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 showReviewBanner evaluates to a function reference, not a boolean

const showReviewBanner = issueStatus === "in_review" && (onReviewApprove || onReviewRequestChanges) — when issueStatus === "in_review" and onReviewApprove is defined, this evaluates to the function itself rather than true. React treats a function reference as a truthy value in {showReviewBanner && (...)} so the component renders correctly, but TypeScript's --strictNullChecks may emit a warning and it silently departs from conventional boolean-flag patterns. Prefer an explicit cast:

Suggested change
const showReviewBanner = issueStatus === "in_review" && (onReviewApprove || onReviewRequestChanges);
const showReviewBanner = issueStatus === "in_review" && !!(onReviewApprove || onReviewRequestChanges);
Prompt To Fix With AI 
This is a comment left during a code review.
Path: ui/src/components/IssueChatThread.tsx
Line: 2398

Comment:
**`showReviewBanner` evaluates to a function reference, not a boolean**

`const showReviewBanner = issueStatus === "in_review" && (onReviewApprove || onReviewRequestChanges)` — when `issueStatus === "in_review"` and `onReviewApprove` is defined, this evaluates to the function itself rather than `true`. React treats a function reference as a truthy value in `{showReviewBanner && (...)}` so the component renders correctly, but TypeScript's `--strictNullChecks` may emit a warning and it silently departs from conventional boolean-flag patterns. Prefer an explicit cast:

```suggestion
  const showReviewBanner = issueStatus === "in_review" && !!(onReviewApprove || onReviewRequestChanges);
```

How can I resolve this? If you propose a fix, please make it concise.

const hasScrolledRef = useRef(false);
const bottomAnchorRef = useRef<HTMLDivElement | null>(null);
const composerViewportAnchorRef = useRef<HTMLDivElement | null>(null);
Expand Down Expand Up @@ -2644,12 +2656,13 @@ export function IssueChatThread({
return map;
}, [feedbackVotes]);

const issueIsClosed = issueStatus === "done" || issueStatus === "cancelled";
const runtime = usePaperclipIssueRuntime({
messages,
isRunning,
onSend: ({ body, reopen, reassignment }) => {
pendingSubmitScrollRef.current = true;
return onAdd(body, reopen, reassignment);
return onAdd(body, reopen ?? (issueIsClosed ? true : undefined), reassignment);
},
onCancel: onCancelRun,
});
Expand Down Expand Up @@ -2854,6 +2867,55 @@ export function IssueChatThread({
</div>
</IssueChatErrorBoundary>

{showReviewBanner && (
<div className="rounded-lg border border-amber-300/70 bg-amber-50/60 dark:border-amber-500/40 dark:bg-amber-500/10 px-4 py-3 flex items-center justify-between gap-3">
<div className="flex items-center gap-2 text-sm font-medium text-amber-900 dark:text-amber-100">
<span className="relative flex h-2 w-2">
<span className="animate-pulse absolute inline-flex h-full w-full rounded-full bg-amber-400 opacity-75" />
<span className="relative inline-flex rounded-full h-2 w-2 bg-amber-500" />
</span>
Aguardando revisão
</div>
<div className="flex items-center gap-2">
{onReviewRequestChanges && (
<Button
variant="outline"
size="sm"
disabled={reviewSubmitting}
onClick={async () => {
setReviewSubmitting(true);
try {
await onReviewRequestChanges();
} finally {
setReviewSubmitting(false);
}
}}
>
<XCircle className="h-3.5 w-3.5 mr-1.5" />
{reviewSubmitting ? "Enviando..." : "Solicitar alterações"}
</Button>
)}
{onReviewApprove && (
<Button
size="sm"
disabled={reviewSubmitting}
onClick={async () => {
setReviewSubmitting(true);
try {
await onReviewApprove();
} finally {
setReviewSubmitting(false);
}
}}
>
<CheckCircle2 className="h-3.5 w-3.5 mr-1.5" />
Comment on lines +2899 to +2911
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Missing rejection reason UI

The PR description states that "Request changes" sets status=in_progress with a rejection reason, but the onReviewRequestChanges callback signature is () => Promise<void> — no reason string is passed, and the button immediately fires the callback without prompting the user for input. Either the callback should accept a reason: string parameter (and the banner should include an inline text input or open a dialog to collect it), or the PR description overstates what this action does.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: ui/src/components/IssueChatThread.tsx
Line: 2759-2771

Comment:
**Missing rejection reason UI**

The PR description states that "Request changes" sets `status=in_progress` **with a rejection reason**, but the `onReviewRequestChanges` callback signature is `() => Promise<void>` — no reason string is passed, and the button immediately fires the callback without prompting the user for input. Either the callback should accept a `reason: string` parameter (and the banner should include an inline text input or open a dialog to collect it), or the PR description overstates what this action does.

How can I resolve this? If you propose a fix, please make it concise.

{reviewSubmitting ? "Enviando..." : "Aprovar"}
</Button>
)}
</div>
</div>
)}

{showComposer ? (
<div
Comment on lines +2881 to 2920
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent failure on callback error

Both onClick handlers swallow errors via finally { setReviewSubmitting(false) }. If onReviewApprove() or onReviewRequestChanges() rejects, the button simply returns to its idle state with no feedback to the user. Consider surfacing the error (e.g., via a toast or an inline error message) so the reviewer knows the action did not complete.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: ui/src/components/IssueChatThread.tsx
Line: 2741-2780

Comment:
**Silent failure on callback error**

Both `onClick` handlers swallow errors via `finally { setReviewSubmitting(false) }`. If `onReviewApprove()` or `onReviewRequestChanges()` rejects, the button simply returns to its idle state with no feedback to the user. Consider surfacing the error (e.g., via a toast or an inline error message) so the reviewer knows the action did not complete.

How can I resolve this? If you propose a fix, please make it concise.

ref={composerViewportAnchorRef}
Expand Down
Loading