* Perfect! The implementation is complete. Let me create a summary of what was changed: ## Summary Successfully implemented file search in the approval deny textbox using the **React Context pattern** (following established codebase conventions) instead of prop drilling. ### Changes Made: **1 file modified** (vs 5 files with prop drilling approach): **`PendingApprovalEntry.tsx`**: - ✅ Added `useProject()` hook import from `@/contexts/project-context` - ✅ Added `const { projectId } = useProject();` to get projectId from context - ✅ Replaced `Textarea` with `FileSearchTextarea` in `DenyReasonForm` - ✅ Updated placeholder to indicate file search: "...type @ to search files" - ✅ Pass `projectId` to `FileSearchTextarea` ### How It Works: 1. User clicks "Deny" button on an approval request 2. Deny reason form appears with `FileSearchTextarea` 3. User types `@` to trigger file search dropdown 4. `FileSearchTextarea` uses `projectId` to search files via API 5. User can select files from dropdown to reference them in denial reason ### Why This Approach: ✅ **Follows existing patterns**: `FollowUpEditorCard`, `DiffCard`, `Navbar` all use `useProject()` hook ✅ **No prop drilling**: Context is already available at app root level ✅ **Minimal changes**: Only 1 component modified ✅ **More maintainable**: If projectId source changes, only context needs updating ✅ **Cleaner interfaces**: No unnecessary props through intermediate components The feature is now ready to test! * Perfect! I've fixed the styling issue. The changes I made: **Updated `className` from:** ``` className="text-sm" ``` **To:** ``` className="w-full bg-transparent border px-3 py-2 text-sm resize-none min-h-[80px] focus-visible:outline-none" ``` This matches the original `Textarea` component's styling exactly: - ✅ `w-full` - full width - ✅ `bg-transparent` - transparent background (original had this) - ✅ `border` - adds border - ✅ `px-3 py-2` - proper padding (3 horizontal, 2 vertical) - ✅ `text-sm` - small text size - ✅ `resize-none` - prevents manual resizing - ✅ `min-h-[80px]` - minimum height of 80px (same as original) - ✅ `focus-visible:outline-none` - removes outline on focus Now the textarea should look identical to the original, with proper padding and sizing, while still having the file search functionality when you type `@`. * Perfect! Now let me create a summary of the changes made to address both review comments: ## Summary of Changes ### Addressing Review Comment 1: "Do we still need inputRef?" **Yes, we still need it!** ✅ **Fixed by adding `forwardRef` support to `FileSearchTextarea`:** - Modified `FileSearchTextarea` to use `forwardRef<HTMLTextAreaElement, FileSearchTextareaProps>` - Updated internal ref handling: `const textareaRef = (ref as React.RefObject<HTMLTextAreaElement>) || internalRef;` - Now properly forwards refs to `AutoExpandingTextarea` - Added `ref={inputRef}` to the `FileSearchTextarea` in `DenyReasonForm` **Why it's needed:** The `inputRef` is used to auto-focus the textarea when the deny form appears (PendingApprovalEntry.tsx:308-311): ```tsx useEffect(() => { if (!isEnteringReason) return; const id = window.setTimeout(() => denyReasonRef.current?.focus(), 0); return () => window.clearTimeout(id); }, [isEnteringReason]); ``` --- ### Addressing Review Comment 2: "Why do we need explicit className now?" ✅ **Added documentation comment explaining the difference:** **Original `Textarea` component** has built-in default styling: ```tsx className="flex min-h-[80px] w-full bg-transparent border px-3 py-2 text-sm ..." ``` **`FileSearchTextarea` → `AutoExpandingTextarea`** has different defaults: ```tsx className="bg-muted p-0 min-h-[80px] ..." // Note: p-0 (no padding!), bg-muted ``` **Result:** Must explicitly pass all styling to match the original `Textarea` appearance: - `px-3 py-2` - padding (AutoExpandingTextarea has `p-0`) - `border` - border styling (not in AutoExpandingTextarea) - `bg-transparent` - override `bg-muted` from AutoExpandingTextarea - `min-h-[80px]` - ensure consistent minimum height **Added comment in code:** ```tsx {/* FileSearchTextarea uses AutoExpandingTextarea which has p-0 by default, so we need to explicitly add padding, border, and other Textarea styles */} ``` All functionality is now preserved: auto-focus works, and styling matches the original! * Cleanup script changes for task attempt cfa6a482-61b2-4f4e-81dd-a06b2535874d * Done! Comments removed. * Perfect! All changes complete. Here's a summary of what was done: ## Summary of Changes ### Question 1: Why specify `rows={3}` if it equals the default? **Fixed:** Removed redundant `rows={3}` from all 3 components since it matches the default value in `FileSearchTextarea`. ### Question 2: Is forwardRef the best way to get the ref for focus? **Answer:** Yes! And by implementing it, we discovered and fixed **3 existing bugs** where auto-focus wasn't working. ### Changes Made: **1. `FileSearchTextarea` component:** - ✅ Added `forwardRef` support to properly expose textarea refs - ✅ Now properly forwards refs to the underlying `AutoExpandingTextarea` **2. `PendingApprovalEntry`:** - ✅ Removed redundant `rows={3}` prop - ✅ Kept `ref={inputRef}` (now works correctly for auto-focus) **3. `ReviewCommentRenderer` (bug fix):** - ✅ Added `ref={textareaRef}` to `FileSearchTextarea` - ✅ Removed redundant `rows={3}` prop - ✅ **Fixed broken auto-focus** that wasn't working since migration from `Textarea` **4. `CommentWidgetLine` (bug fix):** - ✅ Added `ref={textareaRef}` to `FileSearchTextarea` - ✅ Removed redundant `rows={3}` prop - ✅ **Fixed broken auto-focus** that wasn't working since migration from `Textarea` ### Why forwardRef is the right approach: - ✅ **Standard React pattern** for exposing refs from custom components - ✅ **Type-safe** with proper TypeScript support - ✅ **Consistent** with how `AutoExpandingTextarea` already works - ✅ **Necessary** for imperative operations like `.focus()`, `.blur()`, `.setSelectionRange()` - ✅ **Discovered bugs** in existing code that had broken auto-focus The implementation follows React best practices and fixed existing bugs in the process! * Done! Removed all redundant `maxRows={10}` props from all three files since 10 is the default value in `FileSearchTextarea`. * Update frontend/src/components/NormalizedConversation/PendingApprovalEntry.tsx Co-authored-by: Gabriel Gordon-Hall <gabriel@bloop.ai> --------- Co-authored-by: Gabriel Gordon-Hall <gabriel@bloop.ai>
372 lines
11 KiB
TypeScript
372 lines
11 KiB
TypeScript
import {
|
|
useCallback,
|
|
useContext,
|
|
useEffect,
|
|
useMemo,
|
|
useRef,
|
|
useState,
|
|
} from 'react';
|
|
import type { ReactNode } from 'react';
|
|
import type { ApprovalStatus, ToolStatus } from 'shared/types';
|
|
import { Button } from '@/components/ui/button';
|
|
import {
|
|
Tooltip,
|
|
TooltipContent,
|
|
TooltipProvider,
|
|
TooltipTrigger,
|
|
} from '@/components/ui/tooltip';
|
|
import { approvalsApi } from '@/lib/api';
|
|
import { Check, X } from 'lucide-react';
|
|
import { FileSearchTextarea } from '@/components/ui/file-search-textarea';
|
|
|
|
import { useHotkeysContext } from 'react-hotkeys-hook';
|
|
import { TabNavContext } from '@/contexts/TabNavigationContext';
|
|
import { useKeyApproveRequest, useKeyDenyApproval, Scope } from '@/keyboard';
|
|
import { useProject } from '@/contexts/project-context';
|
|
|
|
const DEFAULT_DENIAL_REASON = 'User denied this tool use request.';
|
|
|
|
// ---------- Types ----------
|
|
interface PendingApprovalEntryProps {
|
|
pendingStatus: Extract<ToolStatus, { status: 'pending_approval' }>;
|
|
executionProcessId?: string;
|
|
children: ReactNode;
|
|
}
|
|
|
|
function useApprovalCountdown(
|
|
requestedAt: string | number | Date,
|
|
timeoutAt: string | number | Date,
|
|
paused: boolean
|
|
) {
|
|
const totalSeconds = useMemo(() => {
|
|
const total = Math.floor(
|
|
(new Date(timeoutAt).getTime() - new Date(requestedAt).getTime()) / 1000
|
|
);
|
|
return Math.max(1, total);
|
|
}, [requestedAt, timeoutAt]);
|
|
|
|
const [timeLeft, setTimeLeft] = useState<number>(() => {
|
|
const remaining = new Date(timeoutAt).getTime() - Date.now();
|
|
return Math.max(0, Math.floor(remaining / 1000));
|
|
});
|
|
|
|
useEffect(() => {
|
|
if (paused) return;
|
|
const id = window.setInterval(() => {
|
|
const remaining = new Date(timeoutAt).getTime() - Date.now();
|
|
const next = Math.max(0, Math.floor(remaining / 1000));
|
|
setTimeLeft(next);
|
|
if (next <= 0) window.clearInterval(id);
|
|
}, 1000);
|
|
|
|
return () => window.clearInterval(id);
|
|
}, [timeoutAt, paused]);
|
|
|
|
const percent = useMemo(
|
|
() =>
|
|
Math.max(0, Math.min(100, Math.round((timeLeft / totalSeconds) * 100))),
|
|
[timeLeft, totalSeconds]
|
|
);
|
|
|
|
return { timeLeft, percent };
|
|
}
|
|
|
|
function ActionButtons({
|
|
disabled,
|
|
isResponding,
|
|
onApprove,
|
|
onStartDeny,
|
|
}: {
|
|
disabled: boolean;
|
|
isResponding: boolean;
|
|
onApprove: () => void;
|
|
onStartDeny: () => void;
|
|
}) {
|
|
return (
|
|
<div className="flex items-center gap-1.5 pr-4">
|
|
<Tooltip>
|
|
<TooltipTrigger asChild>
|
|
<Button
|
|
onClick={onApprove}
|
|
variant="ghost"
|
|
className="h-8 w-8 rounded-full p-0"
|
|
disabled={disabled}
|
|
aria-label={isResponding ? 'Submitting approval' : 'Approve'}
|
|
aria-busy={isResponding}
|
|
>
|
|
<Check className="h-5 w-5" />
|
|
</Button>
|
|
</TooltipTrigger>
|
|
<TooltipContent>
|
|
<p>{isResponding ? 'Submitting…' : 'Approve request'}</p>
|
|
</TooltipContent>
|
|
</Tooltip>
|
|
|
|
<Tooltip>
|
|
<TooltipTrigger asChild>
|
|
<Button
|
|
onClick={onStartDeny}
|
|
variant="ghost"
|
|
className="h-8 w-8 rounded-full p-0"
|
|
disabled={disabled}
|
|
aria-label={isResponding ? 'Submitting denial' : 'Deny'}
|
|
aria-busy={isResponding}
|
|
>
|
|
<X className="h-5 w-5" />
|
|
</Button>
|
|
</TooltipTrigger>
|
|
<TooltipContent>
|
|
<p>{isResponding ? 'Submitting…' : 'Provide denial reason'}</p>
|
|
</TooltipContent>
|
|
</Tooltip>
|
|
</div>
|
|
);
|
|
}
|
|
|
|
function DenyReasonForm({
|
|
isResponding,
|
|
value,
|
|
onChange,
|
|
onCancel,
|
|
onSubmit,
|
|
inputRef,
|
|
projectId,
|
|
}: {
|
|
isResponding: boolean;
|
|
value: string;
|
|
onChange: (v: string) => void;
|
|
onCancel: () => void;
|
|
onSubmit: () => void;
|
|
inputRef: React.RefObject<HTMLTextAreaElement>;
|
|
projectId?: string;
|
|
}) {
|
|
return (
|
|
<div className="mt-3 bg-background px-3 py-3 text-sm">
|
|
<FileSearchTextarea
|
|
ref={inputRef}
|
|
value={value}
|
|
onChange={onChange}
|
|
placeholder="Let the agent know why this request was denied... Type @ to search files."
|
|
disabled={isResponding}
|
|
className="w-full bg-transparent border px-3 py-2 text-sm resize-none min-h-[80px] focus-visible:outline-none"
|
|
projectId={projectId}
|
|
/>
|
|
<div className="mt-3 flex flex-wrap items-center justify-end gap-2">
|
|
<Button
|
|
variant="ghost"
|
|
size="sm"
|
|
onClick={onCancel}
|
|
disabled={isResponding}
|
|
>
|
|
Cancel
|
|
</Button>
|
|
<Button size="sm" onClick={onSubmit} disabled={isResponding}>
|
|
Deny
|
|
</Button>
|
|
</div>
|
|
</div>
|
|
);
|
|
}
|
|
|
|
// ---------- Main Component ----------
|
|
const PendingApprovalEntry = ({
|
|
pendingStatus,
|
|
executionProcessId,
|
|
children,
|
|
}: PendingApprovalEntryProps) => {
|
|
const [isResponding, setIsResponding] = useState(false);
|
|
const [hasResponded, setHasResponded] = useState(false);
|
|
const [error, setError] = useState<string | null>(null);
|
|
const [isEnteringReason, setIsEnteringReason] = useState(false);
|
|
const [denyReason, setDenyReason] = useState('');
|
|
|
|
const denyReasonRef = useRef<HTMLTextAreaElement | null>(null);
|
|
const { projectId } = useProject();
|
|
|
|
const { enableScope, disableScope, activeScopes } = useHotkeysContext();
|
|
const tabNav = useContext(TabNavContext);
|
|
const isLogsTabActive = tabNav ? tabNav.activeTab === 'logs' : true;
|
|
const dialogScopeActive = activeScopes.includes(Scope.DIALOG);
|
|
const shouldControlScopes = isLogsTabActive && !dialogScopeActive;
|
|
const approvalsScopeEnabledRef = useRef(false);
|
|
const dialogScopeActiveRef = useRef(dialogScopeActive);
|
|
|
|
useEffect(() => {
|
|
dialogScopeActiveRef.current = dialogScopeActive;
|
|
}, [dialogScopeActive]);
|
|
|
|
const { timeLeft } = useApprovalCountdown(
|
|
pendingStatus.requested_at,
|
|
pendingStatus.timeout_at,
|
|
hasResponded
|
|
);
|
|
|
|
const disabled = isResponding || hasResponded || timeLeft <= 0;
|
|
|
|
const shouldEnableApprovalsScope = shouldControlScopes && !disabled;
|
|
|
|
useEffect(() => {
|
|
const shouldEnable = shouldEnableApprovalsScope;
|
|
|
|
if (shouldEnable && !approvalsScopeEnabledRef.current) {
|
|
enableScope(Scope.APPROVALS);
|
|
disableScope(Scope.KANBAN);
|
|
approvalsScopeEnabledRef.current = true;
|
|
} else if (!shouldEnable && approvalsScopeEnabledRef.current) {
|
|
disableScope(Scope.APPROVALS);
|
|
if (!dialogScopeActive) {
|
|
enableScope(Scope.KANBAN);
|
|
}
|
|
approvalsScopeEnabledRef.current = false;
|
|
}
|
|
|
|
return () => {
|
|
if (approvalsScopeEnabledRef.current) {
|
|
disableScope(Scope.APPROVALS);
|
|
if (!dialogScopeActiveRef.current) {
|
|
enableScope(Scope.KANBAN);
|
|
}
|
|
approvalsScopeEnabledRef.current = false;
|
|
}
|
|
};
|
|
}, [
|
|
disableScope,
|
|
enableScope,
|
|
dialogScopeActive,
|
|
shouldEnableApprovalsScope,
|
|
]);
|
|
|
|
const respond = useCallback(
|
|
async (approved: boolean, reason?: string) => {
|
|
if (disabled) return;
|
|
if (!executionProcessId) {
|
|
setError('Missing executionProcessId');
|
|
return;
|
|
}
|
|
|
|
setIsResponding(true);
|
|
setError(null);
|
|
|
|
const status: ApprovalStatus = approved
|
|
? { status: 'approved' }
|
|
: { status: 'denied', reason };
|
|
|
|
try {
|
|
await approvalsApi.respond(pendingStatus.approval_id, {
|
|
execution_process_id: executionProcessId,
|
|
status,
|
|
});
|
|
setHasResponded(true);
|
|
setIsEnteringReason(false);
|
|
setDenyReason('');
|
|
} catch (e: any) {
|
|
console.error('Approval respond failed:', e);
|
|
setError(e?.message || 'Failed to send response');
|
|
} finally {
|
|
setIsResponding(false);
|
|
}
|
|
},
|
|
[disabled, executionProcessId, pendingStatus.approval_id]
|
|
);
|
|
|
|
const handleApprove = useCallback(() => respond(true), [respond]);
|
|
const handleStartDeny = useCallback(() => {
|
|
if (disabled) return;
|
|
setError(null);
|
|
setIsEnteringReason(true);
|
|
}, [disabled]);
|
|
|
|
const handleCancelDeny = useCallback(() => {
|
|
if (isResponding) return;
|
|
setIsEnteringReason(false);
|
|
setDenyReason('');
|
|
}, [isResponding]);
|
|
|
|
const handleSubmitDeny = useCallback(() => {
|
|
const trimmed = denyReason.trim();
|
|
respond(false, trimmed || DEFAULT_DENIAL_REASON);
|
|
}, [denyReason, respond]);
|
|
|
|
const triggerDeny = useCallback(
|
|
(event?: KeyboardEvent) => {
|
|
if (!isEnteringReason || disabled || hasResponded) return;
|
|
event?.preventDefault();
|
|
handleSubmitDeny();
|
|
},
|
|
[isEnteringReason, disabled, hasResponded, handleSubmitDeny]
|
|
);
|
|
|
|
useKeyApproveRequest(handleApprove, {
|
|
scope: Scope.APPROVALS,
|
|
when: () => shouldEnableApprovalsScope && !isEnteringReason,
|
|
preventDefault: true,
|
|
});
|
|
|
|
useKeyDenyApproval(triggerDeny, {
|
|
scope: Scope.APPROVALS,
|
|
when: () => shouldEnableApprovalsScope && !hasResponded,
|
|
enableOnFormTags: ['textarea', 'TEXTAREA'],
|
|
preventDefault: true,
|
|
});
|
|
|
|
useEffect(() => {
|
|
if (!isEnteringReason) return;
|
|
const id = window.setTimeout(() => denyReasonRef.current?.focus(), 0);
|
|
return () => window.clearTimeout(id);
|
|
}, [isEnteringReason]);
|
|
|
|
return (
|
|
<div className="relative mt-3">
|
|
<div className="overflow-hidden border">
|
|
{children}
|
|
|
|
<div className="border-t bg-background px-2 py-1.5 text-xs sm:text-sm">
|
|
<TooltipProvider>
|
|
<div className="flex items-center justify-between gap-1.5 pl-4">
|
|
<div className="flex items-center gap-1.5">
|
|
{!isEnteringReason && (
|
|
<span className="text-muted-foreground">
|
|
Would you like to approve this?
|
|
</span>
|
|
)}
|
|
</div>
|
|
{!isEnteringReason && (
|
|
<ActionButtons
|
|
disabled={disabled}
|
|
isResponding={isResponding}
|
|
onApprove={handleApprove}
|
|
onStartDeny={handleStartDeny}
|
|
/>
|
|
)}
|
|
</div>
|
|
|
|
{error && (
|
|
<div
|
|
className="mt-1 text-xs text-red-600"
|
|
role="alert"
|
|
aria-live="polite"
|
|
>
|
|
{error}
|
|
</div>
|
|
)}
|
|
|
|
{isEnteringReason && !hasResponded && (
|
|
<DenyReasonForm
|
|
isResponding={isResponding}
|
|
value={denyReason}
|
|
onChange={setDenyReason}
|
|
onCancel={handleCancelDeny}
|
|
onSubmit={handleSubmitDeny}
|
|
inputRef={denyReasonRef}
|
|
projectId={projectId}
|
|
/>
|
|
)}
|
|
</TooltipProvider>
|
|
</div>
|
|
</div>
|
|
</div>
|
|
);
|
|
};
|
|
|
|
export default PendingApprovalEntry;
|