-
Notifications
You must be signed in to change notification settings - Fork 59.3k
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
feat: allow send image only #5599
feat: allow send image only #5599
Conversation
WalkthroughThe changes in this pull request involve modifications to the chat component and its associated store. In Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@Dogtiti is attempting to deploy a commit to the NextChat Team on Vercel. A member of the Team first needs to authorize it. |
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
app/store/chat.ts (1)
340-349
: LGTM! Improved handling of multimodal content.The changes effectively support text-only, image-only, and combined text-and-image inputs, aligning with the PR objective. The implementation is correct and maintains backwards compatibility.
Consider adding a brief comment explaining the structure of
mContent
for better code readability. For example:// mContent structure: // - For text-only: string // - For image-only or text+image: MultimodalContent[] let mContent: string | MultimodalContent[] = userContent;app/components/chat.tsx (3)
1021-1022
: Improved input validationThe addition of the
isEmpty(attachImages)
check is a good improvement. It prevents submission when both text input and image attachments are empty, enhancing the user experience.Consider extracting this condition into a separate function for better readability:
const isInputEmpty = (input: string, images: string[]) => input.trim() === "" && isEmpty(images); if (isInputEmpty(userInput, attachImages)) return;This would make the code more self-documenting and easier to maintain.
Line range hint
1482-1528
: Enhanced paste functionality for image supportThe changes to
handlePaste
function add support for pasting images directly into the chat, which is a great usability improvement. However, there are a few points to consider:
Error Handling: Consider adding error handling for the image upload process. Currently, if an error occurs during upload, it's caught but not handled or reported to the user.
File Type Checking: The function checks if the item type starts with "image/", but it might be more robust to have a whitelist of accepted image formats.
Image Limit: The code limits the number of images to 3, but this limit is not communicated to the user. Consider adding a user-facing message when this limit is reached.
Performance: For large images, the upload process might take a while. Consider adding a loading indicator or progress bar for better user feedback.
Accessibility: Ensure that this feature is accessible to keyboard users and screen readers.
Here's a suggestion for improved error handling and user feedback:
async function uploadImage(file: File): Promise<string> { try { setUploading(true); return await uploadImageRemote(file); } catch (error) { console.error("Image upload failed:", error); showToast("Image upload failed. Please try again."); throw error; } finally { setUploading(false); } } // In handlePaste function: if (file) { if (attachImages.length >= 3) { showToast("Maximum of 3 images allowed."); return; } try { const dataUrl = await uploadImage(file); setAttachImages(prev => [...prev, dataUrl].slice(0, 3)); } catch { // Error already handled in uploadImage } }This approach provides better error handling and user feedback.
Line range hint
1530-1570
: Multiple image upload functionality addedThe changes to the
uploadImage
function add support for multiple image uploads, which is a great feature. However, there are some similarities with thehandlePaste
function that could be addressed:
Code Duplication: The image processing and limiting logic is similar in both functions. Consider extracting this into a shared utility function.
Consistency: The error handling and user feedback could be made more consistent between the two functions.
Type Safety: Consider using TypeScript's type system more effectively, especially for the event handling.
Here's a suggestion for refactoring:
// In a separate utility file export async function processAndLimitImages( newImages: File[], existingImages: string[], maxImages: number = 3 ): Promise<string[]> { const processedImages: string[] = [...existingImages]; for (const file of newImages) { if (processedImages.length >= maxImages) { showToast(`Maximum of ${maxImages} images allowed.`); break; } try { const dataUrl = await uploadImage(file); processedImages.push(dataUrl); } catch (error) { console.error("Image upload failed:", error); showToast("Image upload failed. Please try again."); } } return processedImages; } // In your component async function uploadImage() { const fileInput = document.createElement("input"); fileInput.type = "file"; fileInput.accept = "image/png, image/jpeg, image/webp, image/heic, image/heif"; fileInput.multiple = true; fileInput.onchange = async (event: Event) => { const files = (event.target as HTMLInputElement).files; if (files) { setUploading(true); const newImages = await processAndLimitImages(Array.from(files), attachImages); setAttachImages(newImages); setUploading(false); } }; fileInput.click(); }This refactoring would make the code more DRY, consistent, and easier to maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- app/components/chat.tsx (2 hunks)
- app/store/chat.ts (1 hunks)
🧰 Additional context used
📓 Learnings (1)
app/components/chat.tsx (1)
Learnt from: Dakai PR: ChatGPTNextWeb/ChatGPT-Next-Web#5585 File: app/components/chat.tsx:1482-1528 Timestamp: 2024-10-06T07:59:31.943Z Learning: In `app/components/chat.tsx`, maintain similarity between `uploadDocument()` and `uploadImage()` functions to aid future maintenance.
🔇 Additional comments (1)
app/components/chat.tsx (1)
122-123
: New import addedA new import for the
isEmpty
function fromlodash-es
has been added. This is a good practice as it allows for a more robust empty check.
Your build has completed! |
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
📝 补充信息 | Additional Information
Summary by CodeRabbit
New Features
Bug Fixes