fix: file attachment UX, thread status, and homepage hydration - #11
Conversation
Re-add file upload UI (paperclip button, drag-and-drop, file chips) to the homepage and parse files client-side before navigating to chat. Parsed text is appended to the prompt in sessionStorage config. Also fix SSR hydration mismatch by deferring localStorage reads for locale, responseLength, and rounds to useEffect.
aiedwardyi
changed the title
There was a problem hiding this comment.
Pull request overview
This PR improves the end-to-end file attachment flow (homepage → chat), addresses a thread-status edge case when loading threads, and eliminates a homepage SSR hydration mismatch by deferring persisted-setting reads to post-mount.
Changes:
- Add client-side unsupported file type validation with auto-dismissing inline feedback (EN/KO).
- Restore homepage file attachment by parsing files client-side and appending extracted text to the prompt before navigating to
/chat. - Prevent a “continue completed thread” effect from triggering incorrectly during thread load/reset; defer homepage localStorage hydration to
useEffect.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/components/MessageInput.tsx | Adds unsupported file validation + inline error banner and related state for chat input attachments. |
| src/app/page.tsx | Adds homepage attachment UI + parse-before-navigate behavior; moves persisted settings hydration to useEffect to avoid SSR mismatch. |
| src/app/chat/page.tsx | Resets prevShowSummary during thread load to avoid misinterpreting a reset as “continuing” a completed thread. |
Comments suppressed due to low confidence (1)
src/components/MessageInput.tsx:142
- IDs for attached files are generated with Math.random() + substr(), which is both collision-prone and uses a deprecated string API. Prefer crypto.randomUUID() (or at least .slice()) to produce stable unique IDs so remove/revoke logic can’t target the wrong file.
if (supported.length === 0) return
const newFiles = supported.map((file) => ({
id: Math.random().toString(36).substr(2, 9),
file,
preview: file.type.startsWith("image/") ? URL.createObjectURL(file) : undefined,
}))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| setFiles((prev) => [ | ||
| ...prev, | ||
| ...supported.map((file) => ({ | ||
| id: Math.random().toString(36).substr(2, 9), | ||
| file, | ||
| preview: file.type.startsWith("image/") ? URL.createObjectURL(file) : undefined, | ||
| })), | ||
| ]) |
There was a problem hiding this comment.
IDs for attached files are generated with Math.random() + substr(), which is collision-prone and uses a deprecated API. Prefer crypto.randomUUID() (or a shared helper) to avoid potential key collisions that could remove the wrong file chip.
| @@ -287,15 +334,55 @@ | |||
| setShowGate(true) | |||
| return | |||
There was a problem hiding this comment.
When the login gate triggers, pending debate config is saved using only prompt: prompt.trim() and ignores any attached files. This means users who attach files and then hit the gate will lose the file content after signing in. Consider parsing/appending file contents before calling savePendingDebate (store the resulting prompt string), or disable file attachment when gating is possible.
| onDragOver={(e) => { e.preventDefault(); setIsDragging(true) }} | ||
| onDragLeave={(e) => { e.preventDefault(); setIsDragging(false) }} | ||
| onDrop={(e) => { | ||
| e.preventDefault() | ||
| setIsDragging(false) |
There was a problem hiding this comment.
Drag-and-drop still calls addFiles even while isParsing is true. This can lead to confusing UI/state (files appear to attach during parsing but won’t be included in the in-flight submit). Consider short-circuiting drag/drop handlers (and remove actions) while parsing, consistent with the disabled attach button.
| onDragOver={(e) => { e.preventDefault(); setIsDragging(true) }} | |
| onDragLeave={(e) => { e.preventDefault(); setIsDragging(false) }} | |
| onDrop={(e) => { | |
| e.preventDefault() | |
| setIsDragging(false) | |
| onDragOver={(e) => { | |
| e.preventDefault() | |
| if (isParsing) return | |
| setIsDragging(true) | |
| }} | |
| onDragLeave={(e) => { | |
| e.preventDefault() | |
| setIsDragging(false) | |
| if (isParsing) return | |
| }} | |
| onDrop={(e) => { | |
| e.preventDefault() | |
| setIsDragging(false) | |
| if (isParsing) return |
| ] | ||
|
|
||
| const SUPPORTED_EXTENSIONS = new Set(["pdf", "docx", "xlsx", "xls", "txt", "md", "csv"]) | ||
|
|
||
| /* ─── Translations ─── */ |
There was a problem hiding this comment.
SUPPORTED_EXTENSIONS is duplicated here and in MessageInput, while parseFile also has its own extension allowlist. To avoid the UI allowlist drifting from the parser behavior over time, consider exporting a single source of truth (e.g., from file-parser) and reusing it in both places.
| const addFiles = (files: File[]) => { | ||
| const newFiles = files.map((file) => ({ | ||
| const supported: File[] = [] | ||
| let hasUnsupported = false | ||
| for (const file of files) { | ||
| const ext = file.name.split(".").pop()?.toLowerCase() ?? "" | ||
| if (SUPPORTED_EXTENSIONS.has(ext)) { | ||
| supported.push(file) | ||
| } else { | ||
| hasUnsupported = true | ||
| } | ||
| } | ||
| if (hasUnsupported) { | ||
| setFileError(t.unsupported) | ||
| } | ||
| if (supported.length === 0) return |
There was a problem hiding this comment.
addFiles() doesn’t guard against isParsing. Because handleSend clears attachedFiles after parsing/sending, any files added during an in-flight parse can be lost unexpectedly. Consider early-returning from addFiles when isParsing is true (or queuing additions until parsing completes).
…ough login gate - Export SUPPORTED_EXTENSIONS from file-parser.ts as single source of truth - Parse files before login gate check so file content is preserved in pending debate config - Guard drag-and-drop on homepage during file parsing
Summary
Test plan