From cd06bf01b0ea78aa6bce5865a2065d3f537c614b Mon Sep 17 00:00:00 2001 From: Alex Netsch Date: Wed, 8 Oct 2025 11:23:57 +0100 Subject: [PATCH] File search in approval text box (vibe-kanban) (#966) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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` - Updated internal ref handling: `const textareaRef = (ref as React.RefObject) || 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 --------- Co-authored-by: Gabriel Gordon-Hall --- .../PendingApprovalEntry.tsx | 16 ++++--- .../src/components/diff/CommentWidgetLine.tsx | 3 +- .../components/diff/ReviewCommentRenderer.tsx | 3 +- .../components/ui/file-search-textarea.tsx | 42 +++++++++++-------- 4 files changed, 38 insertions(+), 26 deletions(-) diff --git a/frontend/src/components/NormalizedConversation/PendingApprovalEntry.tsx b/frontend/src/components/NormalizedConversation/PendingApprovalEntry.tsx index 78816d96..47c8a0b2 100644 --- a/frontend/src/components/NormalizedConversation/PendingApprovalEntry.tsx +++ b/frontend/src/components/NormalizedConversation/PendingApprovalEntry.tsx @@ -17,11 +17,12 @@ import { } from '@/components/ui/tooltip'; import { approvalsApi } from '@/lib/api'; import { Check, X } from 'lucide-react'; -import { Textarea } from '@/components/ui/textarea'; +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.'; @@ -129,6 +130,7 @@ function DenyReasonForm({ onCancel, onSubmit, inputRef, + projectId, }: { isResponding: boolean; value: string; @@ -136,16 +138,18 @@ function DenyReasonForm({ onCancel: () => void; onSubmit: () => void; inputRef: React.RefObject; + projectId?: string; }) { return (
-