-
-
Notifications
You must be signed in to change notification settings - Fork 7
Auto-convert large pasted text to files #474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
9a54a27
d5d090d
5b0855e
88731f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |||||||
| import { useEffect, useState, useRef, ChangeEvent, forwardRef, useImperativeHandle, useCallback } from 'react' | ||||||||
| import type { AI, UIState } from '@/app/actions' | ||||||||
| import { useUIState, useActions, readStreamableValue } from 'ai/rsc' | ||||||||
| import { toast } from 'sonner' | ||||||||
| // Removed import of useGeospatialToolMcp as it's no longer used/available | ||||||||
| import { cn } from '@/lib/utils' | ||||||||
| import { UserMessage } from './user-message' | ||||||||
|
|
@@ -69,13 +70,35 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i | |||||||
| const file = e.target.files?.[0] | ||||||||
| if (file) { | ||||||||
| if (file.size > 10 * 1024 * 1024) { | ||||||||
| alert('File size must be less than 10MB') | ||||||||
| toast.error('File size must be less than 10MB') | ||||||||
| return | ||||||||
| } | ||||||||
| setSelectedFile(file) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| const handlePaste = (e: React.ClipboardEvent) => { | ||||||||
| const pastedText = e.clipboardData.getData('text') | ||||||||
| if (pastedText.length > 500) { | ||||||||
| e.preventDefault() | ||||||||
| if (pastedText.length > 10 * 1024 * 1024) { | ||||||||
| toast.error('Pasted text exceeds 10MB limit') | ||||||||
| return | ||||||||
| } | ||||||||
| if (selectedFile) { | ||||||||
| toast.error( | ||||||||
| 'Please remove the current attachment to convert large paste to file' | ||||||||
| ) | ||||||||
| return | ||||||||
| } | ||||||||
| const file = new File([pastedText], 'pasted-text.txt', { | ||||||||
| type: 'text/plain' | ||||||||
| }) | ||||||||
| setSelectedFile(file) | ||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||
| setInput('') | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If a user types some context before pasting large text, this call erases their typed content. Per PR discussion, Consider preserving existing input or showing a success toast so users understand why their input changed. ♻️ Option A: Remove setInput to preserve typed text setSelectedFile(file)
- setInput('')
+ toast.success('Large text converted to file attachment')♻️ Option B: Add ref guard to prevent race condition (per PR discussion)If you need to clear input to prevent race conditions with + const isConvertingPaste = useRef(false)
+
const handlePaste = (e: React.ClipboardEvent) => {
const pastedText = e.clipboardData.getData('text')
if (pastedText.length > 1000) {
e.preventDefault()
// ... size and attachment checks ...
+ isConvertingPaste.current = true
const file = new File([pastedText], 'pasted-text.txt', {
type: 'text/plain'
})
setSelectedFile(file)
setInput('')
+ toast.success('Large text converted to file attachment')
+ // Reset flag after state updates
+ setTimeout(() => { isConvertingPaste.current = false }, 0)
}
}Then guard the onChange: onChange={e => {
+ if (isConvertingPaste.current) return
setInput(e.target.value)
debouncedGetSuggestions(e.target.value)
}}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| const handleAttachmentClick = () => { | ||||||||
| fileInputRef.current?.click() | ||||||||
| } | ||||||||
|
|
@@ -249,6 +272,7 @@ export const ChatPanel = forwardRef<ChatPanelRef, ChatPanelProps>(({ messages, i | |||||||
| setInput(e.target.value) | ||||||||
| debouncedGetSuggestions(e.target.value) | ||||||||
| }} | ||||||||
| onPaste={handlePaste} | ||||||||
| onKeyDown={e => { | ||||||||
| if ( | ||||||||
| e.key === 'Enter' && | ||||||||
|
|
||||||||
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,82 @@ | ||||||||||||||||||||||||||||||||
| import { test, expect } from '@playwright/test'; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| test.describe('Paste to File Conversion', () => { | ||||||||||||||||||||||||||||||||
| test.beforeEach(async ({ page }) => { | ||||||||||||||||||||||||||||||||
| await page.goto('/'); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| test('converts large pasted text to file', async ({ page }) => { | ||||||||||||||||||||||||||||||||
| const chatInput = page.getByTestId('chat-input'); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Create a large text string (> 500 chars) | ||||||||||||||||||||||||||||||||
| const largeText = 'A'.repeat(501); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Simulate paste | ||||||||||||||||||||||||||||||||
| await chatInput.focus(); | ||||||||||||||||||||||||||||||||
| await page.evaluate((text) => { | ||||||||||||||||||||||||||||||||
| const dt = new DataTransfer(); | ||||||||||||||||||||||||||||||||
| dt.setData('text/plain', text); | ||||||||||||||||||||||||||||||||
| const event = new ClipboardEvent('paste', { | ||||||||||||||||||||||||||||||||
| clipboardData: dt, | ||||||||||||||||||||||||||||||||
| bubbles: true, | ||||||||||||||||||||||||||||||||
| cancelable: true | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
| document.activeElement?.dispatchEvent(event); | ||||||||||||||||||||||||||||||||
| }, largeText); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Check if attachment exists | ||||||||||||||||||||||||||||||||
| const attachment = page.getByText('pasted-text.txt'); | ||||||||||||||||||||||||||||||||
| await expect(attachment).toBeVisible(); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Check if input is empty | ||||||||||||||||||||||||||||||||
| await expect(chatInput).toHaveValue(''); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| test('does not convert small pasted text', async ({ page }) => { | ||||||||||||||||||||||||||||||||
| const chatInput = page.getByTestId('chat-input'); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const smallText = 'Small snippet'; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Simulate paste | ||||||||||||||||||||||||||||||||
| await chatInput.focus(); | ||||||||||||||||||||||||||||||||
| await page.evaluate((text) => { | ||||||||||||||||||||||||||||||||
| const dt = new DataTransfer(); | ||||||||||||||||||||||||||||||||
| dt.setData('text/plain', text); | ||||||||||||||||||||||||||||||||
| const event = new ClipboardEvent('paste', { | ||||||||||||||||||||||||||||||||
| clipboardData: dt, | ||||||||||||||||||||||||||||||||
| bubbles: true, | ||||||||||||||||||||||||||||||||
| cancelable: true | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
| document.activeElement?.dispatchEvent(event); | ||||||||||||||||||||||||||||||||
| }, smallText); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Check that attachment does NOT exist | ||||||||||||||||||||||||||||||||
| const attachment = page.getByText('pasted-text.txt'); | ||||||||||||||||||||||||||||||||
| await expect(attachment).not.toBeVisible(); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| test('shows error when pasting while file already attached', async ({ page }) => { | ||||||||||||||||||||||||||||||||
| const chatInput = page.getByTestId('chat-input'); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const largeText1 = 'A'.repeat(501); | ||||||||||||||||||||||||||||||||
| await chatInput.focus(); | ||||||||||||||||||||||||||||||||
| await page.evaluate((text) => { | ||||||||||||||||||||||||||||||||
| const dt = new DataTransfer(); | ||||||||||||||||||||||||||||||||
| dt.setData('text/plain', text); | ||||||||||||||||||||||||||||||||
| const event = new ClipboardEvent('paste', { clipboardData: dt, bubbles: true, cancelable: true }); | ||||||||||||||||||||||||||||||||
| document.activeElement?.dispatchEvent(event); | ||||||||||||||||||||||||||||||||
| }, largeText1); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| await expect(page.getByText('pasted-text.txt')).toBeVisible(); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const largeText2 = 'B'.repeat(501); | ||||||||||||||||||||||||||||||||
| await page.evaluate((text) => { | ||||||||||||||||||||||||||||||||
| const dt = new DataTransfer(); | ||||||||||||||||||||||||||||||||
| dt.setData('text/plain', text); | ||||||||||||||||||||||||||||||||
| const event = new ClipboardEvent('paste', { clipboardData: dt, bubbles: true, cancelable: true }); | ||||||||||||||||||||||||||||||||
| document.activeElement?.dispatchEvent(event); | ||||||||||||||||||||||||||||||||
| }, largeText2); | ||||||||||||||||||||||||||||||||
|
Comment on lines
+72
to
+78
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing focus before second paste may cause test flakiness. After the first paste creates an attachment, the focus might be lost. The second paste (lines 73-78) dispatches the event to 🛡️ Proposed fix await expect(page.getByText('pasted-text.txt')).toBeVisible();
const largeText2 = 'B'.repeat(501);
+ await chatInput.focus();
await page.evaluate((text) => {
const dt = new DataTransfer();
dt.setData('text/plain', text);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| await expect(page.getByText('Please remove the current attachment')).toBeVisible(); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Size check compares character count against byte threshold.
pastedText.lengthreturns the number of characters (UTF-16 code units), not bytes. For multi-byte Unicode text, the actual byte size can be significantly larger than the character count. This check should validate byte size to be consistent withhandleFileChange.🐛 Proposed fix
const handlePaste = (e: React.ClipboardEvent) => { const pastedText = e.clipboardData.getData('text') if (pastedText.length > 1000) { e.preventDefault() - if (pastedText.length > 10 * 1024 * 1024) { + const byteSize = new TextEncoder().encode(pastedText).length + if (byteSize > 10 * 1024 * 1024) { toast.error('Pasted text exceeds 10MB limit') return }📝 Committable suggestion
🤖 Prompt for AI Agents