-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Remove half-second wait, rework auto submit #4282
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
Changes from all commits
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 |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| import React, { useRef, useState, useEffect, useMemo } from 'react'; | ||
| import React, { useRef, useState, useEffect, useMemo, useCallback } from 'react'; | ||
| import { FolderKey, ScrollText } from 'lucide-react'; | ||
| import { Tooltip, TooltipContent, TooltipTrigger } from './ui/Tooltip'; | ||
| import { Button } from './ui/button'; | ||
|
|
@@ -84,6 +84,7 @@ interface ChatInputProps { | |
| recipeConfig?: Recipe | null; | ||
| recipeAccepted?: boolean; | ||
| initialPrompt?: string; | ||
| autoSubmit: boolean; | ||
| } | ||
|
|
||
| export default function ChatInput({ | ||
|
|
@@ -106,6 +107,7 @@ export default function ChatInput({ | |
| recipeConfig, | ||
| recipeAccepted, | ||
| initialPrompt, | ||
| autoSubmit = false, | ||
| }: ChatInputProps) { | ||
| const [_value, setValue] = useState(initialValue); | ||
| const [displayValue, setDisplayValue] = useState(initialValue); // For immediate visual feedback | ||
|
|
@@ -357,6 +359,7 @@ export default function ChatInput({ | |
| const [hasUserTyped, setHasUserTyped] = useState(false); | ||
| const textAreaRef = useRef<HTMLTextAreaElement>(null); | ||
| const timeoutRefsRef = useRef<Set<ReturnType<typeof setTimeout>>>(new Set()); | ||
| const [didAutoSubmit, setDidAutoSubmit] = useState<boolean>(false); | ||
|
|
||
| // Use shared file drop hook for ChatInput | ||
| const { | ||
|
|
@@ -367,7 +370,10 @@ export default function ChatInput({ | |
| } = useFileDrop(); | ||
|
|
||
| // Merge local dropped files with parent dropped files | ||
| const allDroppedFiles = [...droppedFiles, ...localDroppedFiles]; | ||
| const allDroppedFiles = useMemo( | ||
| () => [...droppedFiles, ...localDroppedFiles], | ||
| [droppedFiles, localDroppedFiles] | ||
| ); | ||
|
|
||
| const handleRemoveDroppedFile = (idToRemove: string) => { | ||
| // Remove from local dropped files | ||
|
|
@@ -936,69 +942,89 @@ export default function ChatInput({ | |
| return true; // Return true if message was queued | ||
| }; | ||
|
|
||
| const performSubmit = () => { | ||
| const validPastedImageFilesPaths = pastedImages | ||
| .filter((img) => img.filePath && !img.error && !img.isLoading) | ||
| .map((img) => img.filePath as string); | ||
|
|
||
| // Get paths from all dropped files (both parent and local) | ||
| const droppedFilePaths = allDroppedFiles | ||
| .filter((file) => !file.error && !file.isLoading) | ||
| .map((file) => file.path); | ||
|
|
||
| let textToSend = displayValue.trim(); | ||
| const performSubmit = useCallback( | ||
| (text?: string) => { | ||
| const validPastedImageFilesPaths = pastedImages | ||
| .filter((img) => img.filePath && !img.error && !img.isLoading) | ||
| .map((img) => img.filePath as string); | ||
| // Get paths from all dropped files (both parent and local) | ||
| const droppedFilePaths = allDroppedFiles | ||
| .filter((file) => !file.error && !file.isLoading) | ||
| .map((file) => file.path); | ||
|
|
||
| let textToSend = text ?? displayValue.trim(); | ||
|
|
||
| // Combine pasted images and dropped files | ||
| const allFilePaths = [...validPastedImageFilesPaths, ...droppedFilePaths]; | ||
| if (allFilePaths.length > 0) { | ||
| const pathsString = allFilePaths.join(' '); | ||
| textToSend = textToSend ? `${textToSend} ${pathsString}` : pathsString; | ||
| } | ||
|
|
||
| // Combine pasted images and dropped files | ||
| const allFilePaths = [...validPastedImageFilesPaths, ...droppedFilePaths]; | ||
| if (allFilePaths.length > 0) { | ||
| const pathsString = allFilePaths.join(' '); | ||
| textToSend = textToSend ? `${textToSend} ${pathsString}` : pathsString; | ||
| } | ||
| if (textToSend) { | ||
| if (displayValue.trim()) { | ||
| LocalMessageStorage.addMessage(displayValue); | ||
| } else if (allFilePaths.length > 0) { | ||
| LocalMessageStorage.addMessage(allFilePaths.join(' ')); | ||
| } | ||
|
|
||
| if (textToSend) { | ||
| if (displayValue.trim()) { | ||
| LocalMessageStorage.addMessage(displayValue); | ||
| } else if (allFilePaths.length > 0) { | ||
| LocalMessageStorage.addMessage(allFilePaths.join(' ')); | ||
| } | ||
| handleSubmit( | ||
| new CustomEvent('submit', { detail: { value: textToSend } }) as unknown as React.FormEvent | ||
| ); | ||
|
|
||
| handleSubmit( | ||
| new CustomEvent('submit', { detail: { value: textToSend } }) as unknown as React.FormEvent | ||
| ); | ||
| // Auto-resume queue after sending a NON-interruption message (if it was paused due to interruption) | ||
| if ( | ||
| queuePausedRef.current && | ||
| lastInterruption && | ||
| textToSend && | ||
| !detectInterruption(textToSend) | ||
| ) { | ||
| queuePausedRef.current = false; | ||
| setLastInterruption(null); | ||
| } | ||
|
|
||
| // Auto-resume queue after sending a NON-interruption message (if it was paused due to interruption) | ||
| if ( | ||
| queuePausedRef.current && | ||
| lastInterruption && | ||
| textToSend && | ||
| !detectInterruption(textToSend) | ||
| ) { | ||
| queuePausedRef.current = false; | ||
| setLastInterruption(null); | ||
| } | ||
| setDisplayValue(''); | ||
| setValue(''); | ||
| setPastedImages([]); | ||
| setHistoryIndex(-1); | ||
| setSavedInput(''); | ||
| setIsInGlobalHistory(false); | ||
| setHasUserTyped(false); | ||
|
|
||
| setDisplayValue(''); | ||
| setValue(''); | ||
| setPastedImages([]); | ||
| setHistoryIndex(-1); | ||
| setSavedInput(''); | ||
| setIsInGlobalHistory(false); | ||
| setHasUserTyped(false); | ||
| // Clear draft when message is sent | ||
| if (chatContext && chatContext.clearDraft) { | ||
| chatContext.clearDraft(); | ||
| } | ||
|
|
||
| // Clear draft when message is sent | ||
| if (chatContext && chatContext.clearDraft) { | ||
| chatContext.clearDraft(); | ||
| // Clear both parent and local dropped files after processing | ||
| if (onFilesProcessed && droppedFiles.length > 0) { | ||
| onFilesProcessed(); | ||
| } | ||
| if (localDroppedFiles.length > 0) { | ||
| setLocalDroppedFiles([]); | ||
| } | ||
| } | ||
| }, | ||
| [ | ||
| allDroppedFiles, | ||
| chatContext, | ||
| displayValue, | ||
| droppedFiles.length, | ||
| handleSubmit, | ||
| lastInterruption, | ||
| localDroppedFiles.length, | ||
| onFilesProcessed, | ||
| pastedImages, | ||
| setLocalDroppedFiles, | ||
| ] | ||
| ); | ||
|
Collaborator
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. what happened here? did we not get warnings about this?
Collaborator
Author
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. The key change here is running performSubmit() in a useEffect, but in order to do that I wrapped that in a useCallback, and that meant enumerating its dependencies. We could have gotten away with keeping this a function re-created on every render, since the submit effect should only actually do something just once anyway, but we'd have to suppress some warnings and potentially invite some future bugs. Using hooks for all of them seemed the right way to go |
||
|
|
||
| // Clear both parent and local dropped files after processing | ||
| if (onFilesProcessed && droppedFiles.length > 0) { | ||
| onFilesProcessed(); | ||
| } | ||
| if (localDroppedFiles.length > 0) { | ||
| setLocalDroppedFiles([]); | ||
| } | ||
| useEffect(() => { | ||
| if (!!autoSubmit && !didAutoSubmit) { | ||
| setDidAutoSubmit(true); | ||
| performSubmit(initialValue); | ||
| } | ||
| }; | ||
| }, [autoSubmit, didAutoSubmit, initialValue, performSubmit]); | ||
|
|
||
| const handleKeyDown = (evt: React.KeyboardEvent<HTMLTextAreaElement>) => { | ||
| // If mention popover is open, handle arrow keys and enter | ||
|
|
||
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.
does this help?
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.
it's a dependency of a hook now, so we don't want to make a new list every time