Add image attachment support to follow-up chat input#518
Add image attachment support to follow-up chat input#518ngoiyaeric wants to merge 1 commit intomainfrom
Conversation
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
|
WalkthroughEnvironment variables are added for authentication and API access, with placeholder keys configured. The follow-up panel component is extended to accept file attachments, adding file selection UI, validation, local state management, and FormData submission logic alongside text input. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @.env:
- Around line 1-5: Remove the tracked .env file from git and replace it with a
template: rename the committed file to .env.example (preserving keys like
DATABASE_URL, OPENAI_API_KEY, NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN,
MAPBOX_ACCESS_TOKEN, and ENABLE_AUTH) and commit that; add .env to .gitignore
and run git rm --cached .env to stop tracking the real file; update project
docs/readme to instruct developers to copy .env.example -> .env and populate
secrets locally; and remove or avoid shipping ENABLE_AUTH=false in any
production config (ensure production uses asecure setting or omits this key).
In `@components/followup-panel.tsx`:
- Around line 127-130: The submit disabled state and the Enter-key empty-input
check are inconsistent: change the submit button's disabled prop to use the same
trimmed-empty check as the key handler (use input.trim().length === 0) so
whitespace-only input is treated as empty; ensure any other empty checks in this
component (e.g., the onKeyDown/Enter handler that references selectedFile and
input) use input.trim() as well so both visual disabled state and actual submit
behavior match.
- Around line 85-163: This duplicates the file-attachment logic present in
ChatPanel; extract the shared state and handlers (selectedFile, fileInputRef,
handleAttachmentClick, handleFileChange, clearAttachment, and any
validation/FormData construction) into a reusable hook (e.g., useFileAttachment)
and move the UI for the selected-file preview into a shared AttachmentPreview
component; update FollowupPanel and ChatPanel to consume useFileAttachment
(provide callbacks for submit) and render AttachmentPreview instead of keeping
their own local refs/state/JSX so both panels share the same attachment behavior
and validation.
- Around line 50-59: The UI currently accepts text/plain files but the content
builder only handles images and input text, so selected .txt files are silently
dropped; either remove 'text/plain' from the file input accept attribute (so
only images can be chosen) or add handling in the content construction: when
selectedFile exists and selectedFile.type === 'text/plain' (or
!type.startsWith('image/')), read the file contents (e.g., FileReader or
file.text()) and push a { type: 'text', text: <file contents> } entry into the
content array alongside the existing image handling; update references to
selectedFile, content, and the file input accept attribute accordingly.
- Around line 21-30: The file input handler handleFileChange fails to clear the
input when validation rejects a file, so the same filename won't retrigger
onChange; update handleFileChange to, on validation failure (file.size > 10 *
1024 * 1024), check fileInputRef.current exists and set
fileInputRef.current.value = '' (and optionally setSelectedFile(null)) before
returning, ensuring the DOM input is reset so selecting the same file later will
fire onChange.
- Line 64: The rendering can pass an empty string to UserMessage because content
can be [] when a non-image file is attached and input is "", so update the guard
around the UserMessage prop (component: <UserMessage ...>) to ensure you never
pass an empty value: check content.length first, then check input for non-empty,
and if both are empty but selectedFile exists and selectedFile.type
startsWith('text/') supply a text fallback (or read the file's text into
content) so UserMessage always receives meaningful text; adjust the logic that
populates content/input accordingly in the FollowupPanel component to prevent
rendering an empty string.
- Around line 54-59: selectedFile's image preview is created with
URL.createObjectURL inline and never revoked, causing a memory leak; instead,
generate the blob URL when the file is selected (in handleFileChange), store it
in a state variable (e.g., previewUrl), and use previewUrl where
URL.createObjectURL(selectedFile) is currently used (inside the content push).
Ensure you revoke any existing previewUrl before creating a new one, call
URL.revokeObjectURL(previewUrl) in clearAttachment, and also revoke in a
useEffect cleanup on unmount to avoid leaks.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.envcomponents/followup-panel.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
components/followup-panel.tsx (4)
components/map/map-data-context.tsx (1)
useMapData(51-57)components/user-message.tsx (1)
UserMessage(15-53)lib/utils/index.ts (1)
cn(10-12)components/chat-panel.tsx (6)
ref(30-306)e(88-126)handleAttachmentClick(47-54)ChatPanelRef(25-28)e(66-75)handleAttachmentClick(48-50)
.env (3)
mapbox_mcp/composio-mapbox.ts (1)
validateEnvironmentVariables(8-48)components/map/google-map.tsx (1)
apiKey(20-29)lib/agents/tools/geospatial.tsx (1)
getGoogleStaticMapUrl(158-162)
🪛 dotenv-linter (4.0.0)
.env
[warning] 4-4: [UnorderedKey] The NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 5-5: [UnorderedKey] The MAPBOX_ACCESS_TOKEN key should go before the NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN key
(UnorderedKey)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| DATABASE_URL="postgresql://user:password@host:port/db" | ||
| ENABLE_AUTH=false | ||
| OPENAI_API_KEY=sk-dummy | ||
| NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN=pk.dummy | ||
| MAPBOX_ACCESS_TOKEN=pk.dummy |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if .env is gitignored
echo "=== .gitignore contents for .env ==="
grep -n '\.env' .gitignore || echo ".env is NOT in .gitignore"
# Check if .env.example or .env.local already exists
echo ""
echo "=== Existing env files ==="
fd -H -d 1 '\.env'
# Check if .env is actually tracked in git
echo ""
echo "=== Is .env tracked in git? ==="
git ls-files | grep -E '^\.env$' && echo ".env IS tracked in git" || echo ".env is NOT tracked in git"Repository: QueueLab/QCX
Length of output: 283
Remove .env from version control—it is currently tracked in git.
.env is committed to the repository and not protected by .gitignore (which only excludes .env*.local files). This means secrets are permanently stored in git history. Even if sk-dummy and other values are currently placeholders, this pattern enables accidental commits of real credentials.
Rename this file to .env.example, add .env to .gitignore, and update documentation to instruct developers to copy .env.example to .env locally. Additionally, the ENABLE_AUTH=false setting should not ship to any environment that reaches production.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 1-1: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [UnorderedKey] The NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN key should go before the OPENAI_API_KEY key
(UnorderedKey)
[warning] 5-5: [UnorderedKey] The MAPBOX_ACCESS_TOKEN key should go before the NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN key
(UnorderedKey)
🤖 Prompt for AI Agents
In @.env around lines 1 - 5, Remove the tracked .env file from git and replace
it with a template: rename the committed file to .env.example (preserving keys
like DATABASE_URL, OPENAI_API_KEY, NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN,
MAPBOX_ACCESS_TOKEN, and ENABLE_AUTH) and commit that; add .env to .gitignore
and run git rm --cached .env to stop tracking the real file; update project
docs/readme to instruct developers to copy .env.example -> .env and populate
secrets locally; and remove or avoid shipping ENABLE_AUTH=false in any
production config (ensure production uses asecure setting or omits this key).
| const handleFileChange = (e: ChangeEvent<HTMLInputElement>) => { | ||
| const file = e.target.files?.[0] | ||
| if (file) { | ||
| if (file.size > 10 * 1024 * 1024) { | ||
| alert('File size must be less than 10MB') | ||
| return | ||
| } | ||
| setSelectedFile(file) | ||
| } | ||
| } |
There was a problem hiding this comment.
File input is not reset when validation fails, preventing re-selection of the same file.
When a file exceeds 10MB and the early return fires (line 26), fileInputRef.current.value is not cleared. If the user then tries to select the same file again (after perhaps compressing it—unlikely, but also for a different file with the same name), the browser's onChange won't fire because the input's value hasn't changed.
Proposed fix
const handleFileChange = (e: ChangeEvent<HTMLInputElement>) => {
const file = e.target.files?.[0]
if (file) {
if (file.size > 10 * 1024 * 1024) {
alert('File size must be less than 10MB')
+ if (fileInputRef.current) {
+ fileInputRef.current.value = ''
+ }
return
}
setSelectedFile(file)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleFileChange = (e: ChangeEvent<HTMLInputElement>) => { | |
| const file = e.target.files?.[0] | |
| if (file) { | |
| if (file.size > 10 * 1024 * 1024) { | |
| alert('File size must be less than 10MB') | |
| return | |
| } | |
| setSelectedFile(file) | |
| } | |
| } | |
| const handleFileChange = (e: ChangeEvent<HTMLInputElement>) => { | |
| const file = e.target.files?.[0] | |
| if (file) { | |
| if (file.size > 10 * 1024 * 1024) { | |
| alert('File size must be less than 10MB') | |
| if (fileInputRef.current) { | |
| fileInputRef.current.value = '' | |
| } | |
| return | |
| } | |
| setSelectedFile(file) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@components/followup-panel.tsx` around lines 21 - 30, The file input handler
handleFileChange fails to clear the input when validation rejects a file, so the
same filename won't retrigger onChange; update handleFileChange to, on
validation failure (file.size > 10 * 1024 * 1024), check fileInputRef.current
exists and set fileInputRef.current.value = '' (and optionally
setSelectedFile(null)) before returning, ensuring the DOM input is reset so
selecting the same file later will fire onChange.
| const content: ({ type: 'text'; text: string } | { type: 'image'; image: string })[] = [] | ||
| if (input) { | ||
| content.push({ type: 'text', text: input }) | ||
| } | ||
| if (selectedFile && selectedFile.type.startsWith('image/')) { | ||
| content.push({ | ||
| type: 'image', | ||
| image: URL.createObjectURL(selectedFile) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Text files are accepted but silently dropped from the user message content.
The accept attribute on line 96 allows text/plain, but the content construction on line 54 only adds an entry when selectedFile.type.startsWith('image/'). A user who attaches a .txt file will see the file preview and the file is sent in FormData, but the UserMessage will show no indication of the attachment—creating a confusing UX disconnect.
Either remove text/plain from the accept attribute (if only images are intended), or add handling for text file content.
Also applies to: 91-96
🤖 Prompt for AI Agents
In `@components/followup-panel.tsx` around lines 50 - 59, The UI currently accepts
text/plain files but the content builder only handles images and input text, so
selected .txt files are silently dropped; either remove 'text/plain' from the
file input accept attribute (so only images can be chosen) or add handling in
the content construction: when selectedFile exists and selectedFile.type ===
'text/plain' (or !type.startsWith('image/')), read the file contents (e.g.,
FileReader or file.text()) and push a { type: 'text', text: <file contents> }
entry into the content array alongside the existing image handling; update
references to selectedFile, content, and the file input accept attribute
accordingly.
| if (selectedFile && selectedFile.type.startsWith('image/')) { | ||
| content.push({ | ||
| type: 'image', | ||
| image: URL.createObjectURL(selectedFile) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Memory leak: URL.createObjectURL is never revoked.
Each call to URL.createObjectURL allocates a blob URL that persists until the document is unloaded or explicitly revoked. Since clearAttachment (called on submit) doesn't call URL.revokeObjectURL, repeated attach/submit cycles will leak memory.
Also, this blob URL is ephemeral and only valid in the current browsing context—it won't survive a page refresh or be meaningful to the server. Is this intentional for a local preview only?
Proposed fix
Track the object URL and revoke it on cleanup:
+ const [previewUrl, setPreviewUrl] = useState<string | null>(null)
const clearAttachment = () => {
+ if (previewUrl) {
+ URL.revokeObjectURL(previewUrl)
+ setPreviewUrl(null)
+ }
setSelectedFile(null)
if (fileInputRef.current) {
fileInputRef.current.value = ''
}
}Then generate the URL when the file is selected (in handleFileChange) and use previewUrl where URL.createObjectURL(selectedFile) is currently called.
🤖 Prompt for AI Agents
In `@components/followup-panel.tsx` around lines 54 - 59, selectedFile's image
preview is created with URL.createObjectURL inline and never revoked, causing a
memory leak; instead, generate the blob URL when the file is selected (in
handleFileChange), store it in a state variable (e.g., previewUrl), and use
previewUrl where URL.createObjectURL(selectedFile) is currently used (inside the
content push). Ensure you revoke any existing previewUrl before creating a new
one, call URL.revokeObjectURL(previewUrl) in clearAttachment, and also revoke in
a useEffect cleanup on unmount to avoid leaks.
| id: nanoid(), | ||
| isGenerating: false, | ||
| component: <UserMessage content={input} /> | ||
| component: <UserMessage content={content.length > 0 ? content : input} /> |
There was a problem hiding this comment.
content can be an empty array when a non-image file is attached with no text.
If input is empty and selectedFile is a text file (not image/*), then content stays [], and the fallback content.length > 0 ? content : input evaluates to input which is "". The UserMessage would render with an empty string as content.
Proposed fix
Guard submission against this state, or always include a text entry even if empty (if the server expects it):
- component: <UserMessage content={content.length > 0 ? content : input} />
+ component: <UserMessage content={content.length > 0 ? content : input || '[Attachment]'} />Though the better fix is to address the text/plain acceptance issue noted above.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| component: <UserMessage content={content.length > 0 ? content : input} /> | |
| component: <UserMessage content={content.length > 0 ? content : input || '[Attachment]'} /> |
🤖 Prompt for AI Agents
In `@components/followup-panel.tsx` at line 64, The rendering can pass an empty
string to UserMessage because content can be [] when a non-image file is
attached and input is "", so update the guard around the UserMessage prop
(component: <UserMessage ...>) to ensure you never pass an empty value: check
content.length first, then check input for non-empty, and if both are empty but
selectedFile exists and selectedFile.type startsWith('text/') supply a text
fallback (or read the file's text into content) so UserMessage always receives
meaningful text; adjust the logic that populates content/input accordingly in
the FollowupPanel component to prevent rendering an empty string.
| return ( | ||
| <form | ||
| onSubmit={handleSubmit} | ||
| className="relative flex items-center space-x-1" | ||
| > | ||
| <Input | ||
| type="text" | ||
| name="input" | ||
| placeholder="Explore" | ||
| value={input} | ||
| className="pr-14 h-12" | ||
| onChange={e => setInput(e.target.value)} | ||
| /> | ||
| <Button | ||
| type="submit" | ||
| size={'icon'} | ||
| disabled={input.length === 0} | ||
| variant={'ghost'} | ||
| className="absolute right-1" | ||
| <div className="flex flex-col w-full"> | ||
| <form | ||
| onSubmit={handleSubmit} | ||
| className="relative flex items-start w-full" | ||
| > | ||
| <ArrowRight size={20} /> | ||
| </Button> | ||
| </form> | ||
| <input | ||
| type="file" | ||
| ref={fileInputRef} | ||
| onChange={handleFileChange} | ||
| className="hidden" | ||
| accept="text/plain,image/png,image/jpeg,image/webp" | ||
| /> | ||
| <Button | ||
| type="button" | ||
| variant={'ghost'} | ||
| size={'icon'} | ||
| className="absolute left-3 top-1/2 transform -translate-y-1/2" | ||
| onClick={handleAttachmentClick} | ||
| data-testid="followup-attachment-button" | ||
| > | ||
| <Paperclip size={20} /> | ||
| </Button> | ||
| <Textarea | ||
| name="input" | ||
| rows={1} | ||
| maxRows={5} | ||
| tabIndex={0} | ||
| placeholder="Explore" | ||
| spellCheck={false} | ||
| value={input} | ||
| data-testid="followup-input" | ||
| className={cn( | ||
| 'resize-none w-full min-h-12 rounded-fill border border-input pl-14 pr-14 pt-3 pb-1 text-sm ring-offset-background file:border-0 file:bg-transparent file:text-sm file:font-medium placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 bg-muted' | ||
| )} | ||
| onChange={e => setInput(e.target.value)} | ||
| onKeyDown={e => { | ||
| if ( | ||
| e.key === 'Enter' && | ||
| !e.shiftKey && | ||
| !e.nativeEvent.isComposing | ||
| ) { | ||
| if (input.trim().length === 0 && !selectedFile) { | ||
| e.preventDefault() | ||
| return | ||
| } | ||
| e.preventDefault() | ||
| const form = e.currentTarget.form | ||
| if (form) { | ||
| form.requestSubmit() | ||
| } | ||
| } | ||
| }} | ||
| /> | ||
| <Button | ||
| type="submit" | ||
| size={'icon'} | ||
| disabled={input.length === 0 && !selectedFile} | ||
| variant={'ghost'} | ||
| data-testid="followup-submit" | ||
| className="absolute right-3 top-1/2 transform -translate-y-1/2" | ||
| > | ||
| <ArrowRight size={20} /> | ||
| </Button> | ||
| </form> | ||
| {selectedFile && ( | ||
| <div className="w-full mt-2"> | ||
| <div className="flex items-center justify-between p-2 bg-muted rounded-lg"> | ||
| <span className="text-sm text-muted-foreground truncate max-w-xs"> | ||
| {selectedFile.name} | ||
| </span> | ||
| <Button variant="ghost" size="icon" onClick={clearAttachment} data-testid="followup-clear-attachment-button"> | ||
| <X size={16} /> | ||
| </Button> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Significant code duplication with ChatPanel.
Per the PR objectives and the relevant snippets, ChatPanel already implements the same file attachment pattern (hidden file input, handleAttachmentClick, handleFileChange, clearAttachment, FormData construction). Consider extracting the shared attachment logic (state, refs, handlers, validation, preview UI) into a custom hook (e.g., useFileAttachment) and a shared AttachmentPreview component to keep both panels in sync.
🤖 Prompt for AI Agents
In `@components/followup-panel.tsx` around lines 85 - 163, This duplicates the
file-attachment logic present in ChatPanel; extract the shared state and
handlers (selectedFile, fileInputRef, handleAttachmentClick, handleFileChange,
clearAttachment, and any validation/FormData construction) into a reusable hook
(e.g., useFileAttachment) and move the UI for the selected-file preview into a
shared AttachmentPreview component; update FollowupPanel and ChatPanel to
consume useFileAttachment (provide callbacks for submit) and render
AttachmentPreview instead of keeping their own local refs/state/JSX so both
panels share the same attachment behavior and validation.
| if (input.trim().length === 0 && !selectedFile) { | ||
| e.preventDefault() | ||
| return | ||
| } |
There was a problem hiding this comment.
Inconsistent empty-input checks: input.trim().length vs input.length.
Line 127 uses input.trim().length === 0 (correct—whitespace-only input is empty), but line 142 uses input.length === 0 for the submit button's disabled prop. This means typing only spaces enables the submit button visually, but pressing Enter won't submit. Use the same trimmed check for both.
Proposed fix
- disabled={input.length === 0 && !selectedFile}
+ disabled={input.trim().length === 0 && !selectedFile}Also applies to: 142-142
🤖 Prompt for AI Agents
In `@components/followup-panel.tsx` around lines 127 - 130, The submit disabled
state and the Enter-key empty-input check are inconsistent: change the submit
button's disabled prop to use the same trimmed-empty check as the key handler
(use input.trim().length === 0) so whitespace-only input is treated as empty;
ensure any other empty checks in this component (e.g., the onKeyDown/Enter
handler that references selectedFile and input) use input.trim() as well so both
visual disabled state and actual submit behavior match.
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The main issues are behavioral inconsistencies around attachments: accept allows text/plain but the optimistic message only supports images, and the text portion isn’t aligned with the trim()-based submit guard. Additionally, URL.createObjectURL() is never revoked, which can cause a memory leak over repeated usage. Finally, alert() is a UX/testing smell for validation feedback.
Summary of changes
FollowupPanel: add attachment support & richer input UI
- Replaced single-line
<Input />with an autosizing<Textarea />(viareact-textarea-autosize) and addedEnter-to-submit behavior (withShift+Enterfor newline). - Added an attachment flow:
- Hidden
<input type="file" />triggered by aPaperclipbutton. - Local state for a single selected file with 10MB size validation.
- A preview row showing the filename and an
Xbutton to clear the selection.
- Hidden
- Updated submit behavior:
- Prevents submission when both text and attachment are empty.
- Builds multi-part
contentfor the optimisticUserMessage(text + image preview viaURL.createObjectURL). - Appends
input, optionalfile, anddrawnFeaturestoFormData.
- Synchronized layout with
ChatPanel-style positioning (left attachment button, right submit button).
| const handleFileChange = (e: ChangeEvent<HTMLInputElement>) => { | ||
| const file = e.target.files?.[0] | ||
| if (file) { | ||
| if (file.size > 10 * 1024 * 1024) { | ||
| alert('File size must be less than 10MB') | ||
| return | ||
| } | ||
| setSelectedFile(file) | ||
| } | ||
| } | ||
|
|
||
| const handleAttachmentClick = () => { | ||
| fileInputRef.current?.click() | ||
| } | ||
|
|
||
| const clearAttachment = () => { | ||
| setSelectedFile(null) | ||
| if (fileInputRef.current) { | ||
| fileInputRef.current.value = '' | ||
| } | ||
| } | ||
|
|
||
| const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => { | ||
| event.preventDefault() | ||
| const formData = new FormData() | ||
| formData.append("input", input) | ||
|
|
||
| if (!input.trim() && !selectedFile) { | ||
| return | ||
| } | ||
|
|
||
| const content: ({ type: 'text'; text: string } | { type: 'image'; image: string })[] = [] | ||
| if (input) { | ||
| content.push({ type: 'text', text: input }) | ||
| } | ||
| if (selectedFile && selectedFile.type.startsWith('image/')) { | ||
| content.push({ | ||
| type: 'image', | ||
| image: URL.createObjectURL(selectedFile) | ||
| }) | ||
| } | ||
|
|
||
| const userMessage = { | ||
| id: nanoid(), | ||
| isGenerating: false, | ||
| component: <UserMessage content={input} /> | ||
| component: <UserMessage content={content.length > 0 ? content : input} /> | ||
| } | ||
|
|
||
| // Include drawn features in the form data | ||
| const formData = new FormData() | ||
| formData.append('input', input) | ||
| if (selectedFile) { | ||
| formData.append('file', selectedFile) | ||
| } |
There was a problem hiding this comment.
handleFileChange allows selecting text/plain (per accept) but the submit path only treats image/* as content; non-image files will be uploaded in FormData but won’t be represented in the optimistic UserMessage content. That creates a mismatch between what the user sees locally and what the backend receives.
Related: accept includes text/plain yet there’s no UI affordance indicating that text files are supported (and no rendering path for them).
Suggestion
Make attachment handling consistent: either (a) restrict to images only in both accept and FormData append, or (b) support non-image attachments end-to-end (optimistic UI + server contract).
Option A (images only):
- Change accept to images:
accept="image/png,image/jpeg,image/webp" - In
handleFileChange, reject non-images with a user-facing message. - In submit, append
fileonly if it’s an image.
Option B (support text/plain):
- Add a
{ type: 'file'; fileName: string }(or similar) entry tocontentso theUserMessagecan render it. - Keep the
FormDataappend as-is.
Reply with "@CharlieHelps yes please" if you’d like me to add a commit implementing the chosen option.
| const content: ({ type: 'text'; text: string } | { type: 'image'; image: string })[] = [] | ||
| if (input) { | ||
| content.push({ type: 'text', text: input }) | ||
| } | ||
| if (selectedFile && selectedFile.type.startsWith('image/')) { | ||
| content.push({ | ||
| type: 'image', | ||
| image: URL.createObjectURL(selectedFile) | ||
| }) | ||
| } |
There was a problem hiding this comment.
URL.createObjectURL(selectedFile) is created during submission and never revoked. Over time (especially with repeated attachments) this can leak memory. Even though the preview is “optimistic”, the object URL should be cleaned up once it’s no longer needed.
Also, note that sending an image field containing an object URL is only meaningful for local rendering; it should not be relied upon for any server-side processing.
Suggestion
Track and revoke the object URL.
Example:
- Store a
previewUrlin state when selecting the file:const [previewUrl, setPreviewUrl] = useState<string | null>(null)- On file select:
const url = URL.createObjectURL(file); setPreviewUrl(url)
- Use
previewUrlincontent. - Revoke when clearing/changing/unmounting:
- In
clearAttachment:if (previewUrl) URL.revokeObjectURL(previewUrl) - Or via
useEffect(() => () => { if (previewUrl) URL.revokeObjectURL(previewUrl) }, [previewUrl])
- In
Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this change.
| if (!input.trim() && !selectedFile) { | ||
| return | ||
| } | ||
|
|
||
| const content: ({ type: 'text'; text: string } | { type: 'image'; image: string })[] = [] | ||
| if (input) { | ||
| content.push({ type: 'text', text: input }) | ||
| } | ||
| if (selectedFile && selectedFile.type.startsWith('image/')) { | ||
| content.push({ | ||
| type: 'image', | ||
| image: URL.createObjectURL(selectedFile) | ||
| }) | ||
| } | ||
|
|
||
| const userMessage = { | ||
| id: nanoid(), | ||
| isGenerating: false, | ||
| component: <UserMessage content={input} /> | ||
| component: <UserMessage content={content.length > 0 ? content : input} /> | ||
| } | ||
|
|
There was a problem hiding this comment.
The optimistic UserMessage uses content.length > 0 ? content : input, but content is only populated when input is truthy (not trimmed) and when the attachment is an image. This can lead to odd cases:
- If
inputis just whitespace, it’s included as a text item (if (input)) but the form blocks empty submissions using!input.trim(). That means the optimistic message could contain whitespace text even though the intent is “empty text”. - If a non-image file is attached,
contentmay end up containing only text (or nothing) even though a file is being sent.
This is a correctness/UI consistency issue in the optimistic rendering logic.
Suggestion
Align the optimistic content construction with the submission guard:
- Use trimmed text:
const trimmed = input.trim()if (trimmed) content.push({ type: 'text', text: trimmed })
- Decide how to represent non-image attachments (see earlier comment) or prevent them.
- Then set
UserMessagecontent unconditionally tocontent(no fallback to rawinput), sincecontentwould always represent what’s being submitted.
Reply with "@CharlieHelps yes please" if you’d like me to add a commit applying this refactor.
| const handleFileChange = (e: ChangeEvent<HTMLInputElement>) => { | ||
| const file = e.target.files?.[0] | ||
| if (file) { | ||
| if (file.size > 10 * 1024 * 1024) { | ||
| alert('File size must be less than 10MB') | ||
| return | ||
| } | ||
| setSelectedFile(file) | ||
| } |
There was a problem hiding this comment.
Using alert() for validation is a poor UX and hard to test/standardize. It also blocks the main thread and doesn’t match typical app notification patterns.
Given this component already uses data-testid hooks, a non-blocking, in-app error message (or shared toast system) would be more consistent and testable.
Suggestion
Replace alert() with an inline error state or the app’s toast/notification mechanism.
Example:
- Add
const [attachmentError, setAttachmentError] = useState<string | null>(null) - Set it when validation fails, render it below the input, and clear it on successful selection.
Reply with "@CharlieHelps yes please" if you’d like me to add a commit implementing an inline error message.
2 participants
This change adds image attachment functionality to the
FollowupPanelcomponent, making it feature-equivalent to the mainChatPanel.Key changes:
Inputwith aTextareafromreact-textarea-autosizefor a better typing experience.Paperclipbutton to trigger a hidden file input.handleSubmitfunction to construct multi-part content (text and images) and correctly append files to theFormData.ChatPaneldesign.PR created automatically by Jules for task 17041392562262869585 started by @ngoiyaeric
Summary by CodeRabbit