Skip to content

Commit cbe03f7

Browse files
ammar-agentammar-agent
committed
🤖 fix: improve pending reviews UX based on feedback
- Use draft construct for error recovery (no duplication) - Hide attached reviews from banner to prevent double-send - Move 'Send to chat' action left, away from delete - Make 'Clear' button smaller (just text link) - Grey chat icon when no pending reviews - Reduce padding/margins in ReviewBlock for compact display - Notify parent when attached reviews change _Generated with mux_
1 parent 5e57730 commit cbe03f7

File tree

5 files changed

+100
-75
lines changed

5 files changed

+100
-75
lines changed

‎src/browser/components/AIView.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ const AIViewInner: React.FC<AIViewProps> = ({
111111

112112
// Pending reviews state
113113
const pendingReviews = usePendingReviews(workspaceId);
114+
const [attachedReviewIds, setAttachedReviewIds] = useState<string[]>([]);
114115
const { options } = useProviderOptions();
115116
const use1M = options.anthropic?.use1MContext ?? false;
116117
// Get pending model for auto-compaction settings (threshold is per-model)
@@ -616,7 +617,11 @@ const AIViewInner: React.FC<AIViewProps> = ({
616617
onCompactClick={handleCompactClick}
617618
/>
618619
)}
619-
<PendingReviewsBanner workspaceId={workspaceId} chatInputAPI={chatInputAPI} />
620+
<PendingReviewsBanner
621+
workspaceId={workspaceId}
622+
chatInputAPI={chatInputAPI}
623+
attachedReviewIds={attachedReviewIds}
624+
/>
620625
<ChatInput
621626
variant="workspace"
622627
workspaceId={workspaceId}
@@ -634,6 +639,7 @@ const AIViewInner: React.FC<AIViewProps> = ({
634639
autoCompactionCheck={autoCompactionResult}
635640
getReview={pendingReviews.getReview}
636641
onReviewsSent={(ids) => ids.forEach((id) => pendingReviews.checkReview(id))}
642+
onAttachedReviewsChange={setAttachedReviewIds}
637643
/>
638644
</div>
639645

‎src/browser/components/ChatInput/index.tsx‎

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -154,24 +154,26 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
154154
const inputRef = useRef<HTMLTextAreaElement>(null);
155155
const modelSelectorRef = useRef<ModelSelectorRef>(null);
156156

157-
// Draft state combines text input and image attachments
158-
// Use these helpers to avoid accidentally losing images when modifying text
157+
// Draft state combines text input, image attachments, and attached reviews
158+
// Use these helpers to avoid accidentally losing content when modifying text
159159
interface DraftState {
160160
text: string;
161161
images: ImageAttachment[];
162+
reviewIds: string[];
162163
}
163164
const getDraft = useCallback(
164-
(): DraftState => ({ text: input, images: imageAttachments }),
165-
[input, imageAttachments]
165+
(): DraftState => ({ text: input, images: imageAttachments, reviewIds: attachedReviewIds }),
166+
[input, imageAttachments, attachedReviewIds]
166167
);
167168
const setDraft = useCallback(
168169
(draft: DraftState) => {
169170
setInput(draft.text);
170171
setImageAttachments(draft.images);
172+
setAttachedReviewIds(draft.reviewIds);
171173
},
172174
[setInput]
173175
);
174-
const preEditDraftRef = useRef<DraftState>({ text: "", images: [] });
176+
const preEditDraftRef = useRef<DraftState>({ text: "", images: [], reviewIds: [] });
175177
const { open } = useSettings();
176178
const { selectedWorkspace } = useWorkspaceContext();
177179
const [mode, setMode] = useMode();
@@ -356,6 +358,13 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
356358
// Get currently attached reviews
357359
const getAttachedReviews = useCallback(() => attachedReviewIds, [attachedReviewIds]);
358360

361+
// Notify parent when attached reviews change (workspace variant only)
362+
useEffect(() => {
363+
if (variant === "workspace" && props.onAttachedReviewsChange) {
364+
props.onAttachedReviewsChange(attachedReviewIds);
365+
}
366+
}, [variant, attachedReviewIds, props]);
367+
359368
// Provide API to parent via callback
360369
useEffect(() => {
361370
if (props.onReady) {
@@ -411,7 +420,7 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
411420
useEffect(() => {
412421
if (editingMessage) {
413422
preEditDraftRef.current = getDraft();
414-
setDraft({ text: editingMessage.content, images: [] });
423+
setDraft({ text: editingMessage.content, images: [], reviewIds: [] });
415424
// Auto-resize textarea and focus
416425
setTimeout(() => {
417426
if (inputRef.current) {
@@ -968,8 +977,8 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
968977
}
969978
setIsSending(true);
970979

971-
// Save current state for restoration on error
972-
const previousImageAttachments = [...imageAttachments];
980+
// Save current draft state for restoration on error
981+
const preSendDraft = getDraft();
973982

974983
// Auto-compaction check (workspace variant only)
975984
// Check if we should auto-compact before sending this message
@@ -1002,8 +1011,7 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
10021011

10031012
if (!result.success) {
10041013
// Restore on error
1005-
setInput(messageText);
1006-
setImageAttachments(previousImageAttachments);
1014+
setDraft(preSendDraft);
10071015
setToast({
10081016
id: Date.now().toString(),
10091017
type: "error",
@@ -1020,8 +1028,7 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
10201028
}
10211029
} catch (error) {
10221030
// Restore on unexpected error
1023-
setInput(messageText);
1024-
setImageAttachments(previousImageAttachments);
1031+
setDraft(preSendDraft);
10251032
setToast({
10261033
id: Date.now().toString(),
10271034
type: "error",
@@ -1036,9 +1043,6 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
10361043
return; // Skip normal send
10371044
}
10381045

1039-
// Store attached review IDs before try block so catch block can access them
1040-
const sentReviewIds = [...attachedReviewIds];
1041-
10421046
try {
10431047
// Prepare image parts if any
10441048
const imageParts = imageAttachments.map((img, index) => {
@@ -1135,10 +1139,8 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
11351139
console.error("Failed to send message:", result.error);
11361140
// Show error using enhanced toast
11371141
setToast(createErrorToast(result.error));
1138-
// Restore input, images, and attached reviews on error so user can try again
1139-
setInput(messageText);
1140-
setImageAttachments(previousImageAttachments);
1141-
setAttachedReviewIds(sentReviewIds);
1142+
// Restore draft on error so user can try again
1143+
setDraft(preSendDraft);
11421144
} else {
11431145
// Track telemetry for successful message send
11441146
telemetry.messageSent(
@@ -1151,8 +1153,8 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
11511153
);
11521154

11531155
// Mark attached reviews as completed
1154-
if (sentReviewIds.length > 0) {
1155-
props.onReviewsSent?.(sentReviewIds);
1156+
if (preSendDraft.reviewIds.length > 0) {
1157+
props.onReviewsSent?.(preSendDraft.reviewIds);
11561158
}
11571159

11581160
// Exit editing mode if we were editing
@@ -1170,9 +1172,8 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
11701172
raw: error instanceof Error ? error.message : "Failed to send message",
11711173
})
11721174
);
1173-
setInput(messageText);
1174-
setImageAttachments(previousImageAttachments);
1175-
setAttachedReviewIds(sentReviewIds);
1175+
// Restore draft on error
1176+
setDraft(preSendDraft);
11761177
} finally {
11771178
setIsSending(false);
11781179
}
@@ -1354,7 +1355,7 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
13541355

13551356
{/* Attached reviews preview - show styled blocks with remove buttons */}
13561357
{variant === "workspace" && attachedReviewIds.length > 0 && props.getReview && (
1357-
<div className="border-border max-h-40 space-y-1 overflow-y-auto border-b px-2 py-1.5">
1358+
<div className="border-border max-h-36 space-y-0.5 overflow-y-auto border-b px-1.5 py-1">
13581359
{attachedReviewIds.map((reviewId) => {
13591360
const review = props.getReview!(reviewId);
13601361
if (!review) return null;
@@ -1364,10 +1365,10 @@ export const ChatInput: React.FC<ChatInputProps> = (props) => {
13641365
<button
13651366
type="button"
13661367
onClick={() => detachReview(reviewId)}
1367-
className="bg-dark/80 text-muted hover:text-error absolute top-3 right-3 rounded-full p-0.5 opacity-0 transition-opacity group-hover:opacity-100"
1368+
className="bg-dark/80 text-muted hover:text-error absolute top-1.5 right-1.5 rounded-full p-0.5 opacity-0 transition-opacity group-hover:opacity-100"
13681369
title="Remove from message"
13691370
>
1370-
<X className="size-3.5" />
1371+
<X className="size-3" />
13711372
</button>
13721373
</div>
13731374
);

‎src/browser/components/ChatInput/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ export interface ChatInputWorkspaceVariant {
3838
autoCompactionCheck?: AutoCompactionCheckResult; // Computed in parent (AIView) to avoid duplicate calculation
3939
/** Called after reviews are sent in a message - allows parent to mark them as checked */
4040
onReviewsSent?: (reviewIds: string[]) => void;
41+
/** Called when attached reviews change (for syncing with banner) */
42+
onAttachedReviewsChange?: (reviewIds: string[]) => void;
4143
/** Get a pending review by ID (for resolving attached review IDs to data) */
4244
getReview?: (id: string) => PendingReview | undefined;
4345
}

‎src/browser/components/PendingReviewsBanner.tsx‎

Lines changed: 57 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,19 @@ const ReviewItem: React.FC<ReviewItemProps> = ({
188188
<Tooltip align="center">{isChecked ? "Mark as pending" : "Mark as done"}</Tooltip>
189189
</TooltipWrapper>
190190

191+
{/* Send to chat - near safe actions, away from delete */}
192+
<TooltipWrapper inline>
193+
<Button
194+
variant="ghost"
195+
size="icon"
196+
className="size-5 shrink-0 opacity-0 transition-opacity group-hover:opacity-100 [&_svg]:size-3"
197+
onClick={onSendToChat}
198+
>
199+
<Send />
200+
</Button>
201+
<Tooltip align="center">Send to chat</Tooltip>
202+
</TooltipWrapper>
203+
191204
{/* File path and age */}
192205
<button
193206
type="button"
@@ -200,32 +213,18 @@ const ReviewItem: React.FC<ReviewItemProps> = ({
200213
<span className="text-muted shrink-0 text-[10px]">{age}</span>
201214
</button>
202215

203-
{/* Actions */}
204-
<div className="flex shrink-0 items-center gap-0.5 opacity-0 transition-opacity group-hover:opacity-100">
205-
<TooltipWrapper inline>
206-
<Button
207-
variant="ghost"
208-
size="icon"
209-
className="size-5 [&_svg]:size-3"
210-
onClick={onSendToChat}
211-
>
212-
<Send />
213-
</Button>
214-
<Tooltip align="center">Send to chat</Tooltip>
215-
</TooltipWrapper>
216-
217-
<TooltipWrapper inline>
218-
<Button
219-
variant="ghost"
220-
size="icon"
221-
className="text-error size-5 [&_svg]:size-3"
222-
onClick={onRemove}
223-
>
224-
<Trash2 />
225-
</Button>
226-
<Tooltip align="center">Remove</Tooltip>
227-
</TooltipWrapper>
228-
</div>
216+
{/* Delete action - separate from safe actions */}
217+
<TooltipWrapper inline>
218+
<Button
219+
variant="ghost"
220+
size="icon"
221+
className="text-error size-5 shrink-0 opacity-0 transition-opacity group-hover:opacity-100 [&_svg]:size-3"
222+
onClick={onRemove}
223+
>
224+
<Trash2 />
225+
</Button>
226+
<Tooltip align="center">Remove</Tooltip>
227+
</TooltipWrapper>
229228
</div>
230229

231230
{/* Expanded content */}
@@ -300,24 +299,33 @@ const ReviewItem: React.FC<ReviewItemProps> = ({
300299
interface PendingReviewsBannerInnerProps {
301300
workspaceId: string;
302301
chatInputAPI: React.RefObject<ChatInputAPI | null>;
302+
attachedReviewIds: string[];
303303
}
304304

305305
const PendingReviewsBannerInner: React.FC<PendingReviewsBannerInnerProps> = ({
306306
workspaceId,
307307
chatInputAPI,
308+
attachedReviewIds,
308309
}) => {
309310
const pendingReviews = usePendingReviews(workspaceId);
310311
const [isExpanded, setIsExpanded] = useState(false);
311312
const [showAllCompleted, setShowAllCompleted] = useState(false);
312313

313314
const INITIAL_COMPLETED_COUNT = 3;
314315

315-
// Separate pending and completed reviews
316+
// Set of attached review IDs for quick lookup
317+
const attachedSet = useMemo(() => new Set(attachedReviewIds), [attachedReviewIds]);
318+
319+
// Separate pending and completed reviews, excluding those already attached to chat
316320
const { pendingList, completedList } = useMemo(() => {
317-
const pending = pendingReviews.reviews.filter((r) => r.status === "pending");
318-
const completed = pendingReviews.reviews.filter((r) => r.status === "checked");
321+
const pending = pendingReviews.reviews.filter(
322+
(r) => r.status === "pending" && !attachedSet.has(r.id)
323+
);
324+
const completed = pendingReviews.reviews.filter(
325+
(r) => r.status === "checked" && !attachedSet.has(r.id)
326+
);
319327
return { pendingList: pending, completedList: completed };
320-
}, [pendingReviews.reviews]);
328+
}, [pendingReviews.reviews, attachedSet]);
321329

322330
// Completed reviews to display (limited unless expanded)
323331
const displayedCompleted = useMemo(() => {
@@ -358,7 +366,12 @@ const PendingReviewsBannerInner: React.FC<PendingReviewsBannerInnerProps> = ({
358366
onClick={handleToggle}
359367
className="hover:bg-hover flex w-full items-center gap-2 px-3 py-1.5 text-xs transition-colors"
360368
>
361-
<MessageSquare className="size-3.5 text-[var(--color-review-accent)]" />
369+
<MessageSquare
370+
className={cn(
371+
"size-3.5",
372+
pendingReviews.pendingCount > 0 ? "text-[var(--color-review-accent)]" : "text-muted"
373+
)}
374+
/>
362375
<span className="text-secondary">
363376
{pendingReviews.pendingCount > 0 ? (
364377
<>
@@ -415,15 +428,13 @@ const PendingReviewsBannerInner: React.FC<PendingReviewsBannerInnerProps> = ({
415428
Completed ({completedList.length})
416429
</div>
417430
{completedList.length > 0 && (
418-
<Button
419-
variant="ghost"
420-
size="sm"
421-
className="text-error h-5 px-2 text-xs"
431+
<button
432+
type="button"
422433
onClick={pendingReviews.clearChecked}
434+
className="text-muted hover:text-error text-[10px] transition-colors"
423435
>
424-
<Trash2 className="mr-1 size-3" />
425-
Clear all
426-
</Button>
436+
Clear
437+
</button>
427438
)}
428439
</div>
429440
{displayedCompleted.map((review) => (
@@ -467,6 +478,8 @@ const PendingReviewsBannerInner: React.FC<PendingReviewsBannerInnerProps> = ({
467478
interface PendingReviewsBannerProps {
468479
workspaceId: string;
469480
chatInputAPI: React.RefObject<ChatInputAPI | null>;
481+
/** Review IDs currently attached to chat input (hidden from banner to prevent double-send) */
482+
attachedReviewIds: string[];
470483
}
471484

472485
/**
@@ -476,12 +489,17 @@ interface PendingReviewsBannerProps {
476489
export const PendingReviewsBanner: React.FC<PendingReviewsBannerProps> = ({
477490
workspaceId,
478491
chatInputAPI,
492+
attachedReviewIds,
479493
}) => {
480494
const pendingReviews = usePendingReviews(workspaceId);
481495

482496
return (
483497
<BannerErrorBoundary onClear={pendingReviews.clearAll}>
484-
<PendingReviewsBannerInner workspaceId={workspaceId} chatInputAPI={chatInputAPI} />
498+
<PendingReviewsBannerInner
499+
workspaceId={workspaceId}
500+
chatInputAPI={chatInputAPI}
501+
attachedReviewIds={attachedReviewIds}
502+
/>
485503
</BannerErrorBoundary>
486504
);
487505
};

‎src/browser/components/shared/ReviewBlock.tsx‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,20 +56,18 @@ export const ReviewBlock: React.FC<ReviewBlockProps> = ({ content }) => {
5656
}, [parsed.code]);
5757

5858
return (
59-
<div className="my-2 overflow-hidden rounded border border-[var(--color-review-accent)]/30 bg-[var(--color-review-accent)]/5">
59+
<div className="overflow-hidden rounded border border-[var(--color-review-accent)]/30 bg-[var(--color-review-accent)]/5">
6060
{/* Header */}
61-
<div className="flex items-center gap-2 border-b border-[var(--color-review-accent)]/20 bg-[var(--color-review-accent)]/10 px-3 py-1.5 text-xs">
62-
<MessageSquare className="size-3.5 text-[var(--color-review-accent)]" />
63-
<span className="font-medium text-[var(--color-review-accent)]">Review Note</span>
64-
<span className="text-muted">·</span>
61+
<div className="flex items-center gap-1.5 border-b border-[var(--color-review-accent)]/20 bg-[var(--color-review-accent)]/10 px-2 py-1 text-xs">
62+
<MessageSquare className="size-3 text-[var(--color-review-accent)]" />
6563
<span className="text-secondary font-mono">
6664
{parsed.filePath}:{parsed.lineRange}
6765
</span>
6866
</div>
6967

7068
{/* Code snippet */}
7169
{parsed.code && (
72-
<div className="max-h-32 overflow-auto border-b border-[var(--color-review-accent)]/20 text-[11px]">
70+
<div className="max-h-28 overflow-auto border-b border-[var(--color-review-accent)]/20 text-[11px]">
7371
<DiffRenderer
7472
content={diffContent}
7573
showLineNumbers={false}
@@ -81,8 +79,8 @@ export const ReviewBlock: React.FC<ReviewBlockProps> = ({ content }) => {
8179

8280
{/* Comment */}
8381
{parsed.comment && (
84-
<div className="px-3 py-2">
85-
<blockquote className="text-primary border-l-2 border-[var(--color-review-accent)] pl-2 text-sm italic">
82+
<div className="px-2 py-1">
83+
<blockquote className="text-primary border-l-2 border-[var(--color-review-accent)] pl-1.5 text-xs italic">
8684
{parsed.comment}
8785
</blockquote>
8886
</div>

0 commit comments

Comments
 (0)