Skip to content

support image#9

Draft
ttdatt wants to merge 1 commit intomainfrom
support-image
Draft

support image#9
ttdatt wants to merge 1 commit intomainfrom
support-image

Conversation

@ttdatt
Copy link
Owner

@ttdatt ttdatt commented May 13, 2025

PR Type

Enhancement


Description

  • Add file upload support to chat threads

    • Introduce file attachment and removal atoms
    • Display attached files in chat input area
    • Enable drag-and-drop file upload via Dropzone
  • Update types to support files in threads

  • Add new FileItem component for file display

  • Update dependencies for file upload functionality


Changes walkthrough 📝

Relevant files
Enhancement
6 files
derivedAtoms.ts
Add atoms for file attachment and removal in threads         
+28/-1   
Message.ts
Extend types to support file attachments in threads           
+13/-0   
ChatInput.tsx
Show attached files in chat input area                                     
+12/-2   
FileItem.tsx
New component to display attached files with remove option
+25/-0   
MessageArea.tsx
Add drag-and-drop file upload with Dropzone                           
+10/-1   
main.tsx
Import Dropzone styles for file upload UI                               
+1/-0     
Dependencies
1 files
package.json
Add @mantine/dropzone and update jotai version                     
+4/-2     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @qodo-free-for-open-source-projects

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Duplicate Dropzone

    There appears to be both a Dropzone.FullScreen component and a regular div with the same CSS classes and purpose. This creates duplicate scrollable areas which may cause UI/UX issues.

    <Dropzone.FullScreen
    	activateOnDrag
    	className='flex-1 overflow-y-auto overscroll-none'
    	onDrop={(files) => {
    		addFiles(files);
    	}}
    />
    <div className='flex-1 overflow-y-auto overscroll-none'>
    
    Missing Return

    The addFilesToThreadAtom and removeFileAtom functions don't explicitly return a value when the early return conditions are met, which could lead to unexpected behavior.

    const addFilesToThreadAtom = atom(null, (get, set, files: FileWithPath[]) => {
    	set(threadsAtom, (threads) => {
    		const currentThreadId = get(currentThreadIdAtom);
    		const currentThread = threads[currentThreadId ?? ''];
    		if (!currentThread || !currentThreadId) return;
    
    		if (!currentThread.files) {
    			currentThread.files = [];
    		}
    
    		currentThread.files = currentThread.files.concat(files.map((f) => ({ file: f, id: uuidv4() })));
    	});
    });
    
    const removeFileAtom = atom(null, (get, set, file: CustomFileWithPath) => {
    	set(threadsAtom, (threads) => {
    		const currentThreadId = get(currentThreadIdAtom);
    		const currentThread = threads[currentThreadId ?? ''];
    		if (!currentThread || !currentThreadId) return;
    
    		currentThread.files = currentThread.files?.filter((f) => f.id !== file.id);
    	});
    });
    Missing File Size

    The FileItem component displays the file name but doesn't show the file size, which is important information for users when uploading files.

    <div className='flex flex-col'>
    	<p>{file.file.name}</p>
    	<p>File</p>
    </div>
    

    @qodo-free-for-open-source-projects

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Return updated state object

    The function doesn't return the updated threads object when modifying the state.
    In Jotai's immer-style updates, you need to return the modified state object to
    properly update the atom.

    src/atom/derivedAtoms.ts [254-266]

     const addFilesToThreadAtom = atom(null, (get, set, files: FileWithPath[]) => {
     	set(threadsAtom, (threads) => {
     		const currentThreadId = get(currentThreadIdAtom);
     		const currentThread = threads[currentThreadId ?? ''];
    -		if (!currentThread || !currentThreadId) return;
    +		if (!currentThread || !currentThreadId) return threads;
     
     		if (!currentThread.files) {
     			currentThread.files = [];
     		}
     
     		currentThread.files = currentThread.files.concat(files.map((f) => ({ file: f, id: uuidv4() })));
    +		return threads;
     	});
     });
    • Apply / Chat
    Suggestion importance[1-10]: 8

    __

    Why: This suggestion correctly identifies that the updater function for the atom should return the updated state object to ensure Jotai's state update mechanism works as intended. Not returning the state can cause updates to be ignored, so this is a critical fix for correct functionality.

    Medium
    Fix layout structure

    The Dropzone.FullScreen component and the message display div both have the same
    className with 'flex-1', causing layout conflicts. The Dropzone should wrap the
    message content instead of being a separate element.

    src/components/MessageArea.tsx [44-53]

     <Dropzone.FullScreen
     	activateOnDrag
    -	className='flex-1 overflow-y-auto overscroll-none'
     	onDrop={(files) => {
     		addFiles(files);
     	}}
    -/>
    -<div className='flex-1 overflow-y-auto overscroll-none'>
    -	{!!currentThread &&
    -		orderBy(Object.values(currentThread.messages), 'timestamp', 'asc').map((x) => {
    +>
    +	<div className='flex-1 overflow-y-auto overscroll-none'>
    +		{!!currentThread &&
    +			orderBy(Object.values(currentThread.messages), 'timestamp', 'asc').map((x) => {

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 7

    __

    Why: The suggestion improves layout by ensuring the Dropzone wraps the message area, preventing flexbox conflicts and potential UI bugs. While not a critical bug, it meaningfully enhances maintainability and user experience.

    Medium
    • More

    @cloudflare-workers-and-pages
    Copy link

    Deploying llmchat with  Cloudflare Pages  Cloudflare Pages

    Latest commit: 9e72a7e
    Status:🚫  Build failed.

    View logs

    @ttdatt ttdatt marked this pull request as draft May 13, 2025 02:14
    @ttdatt ttdatt force-pushed the main branch 3 times, most recently from b05ce9c to 20977c1 Compare June 26, 2025 01:34
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    1 participant