Adds support for image pasting from clipboard, and multiple file attachments #12
Conversation
Summary of ChangesHello @sdrinf, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the messaging interface by enabling direct image pasting from the clipboard. It also expands the attachment capabilities to support multiple images, providing users with a more flexible and intuitive method for sharing visual information. The changes include UI updates for attachment management and backend logic to process these new image sources. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds support for pasting images from the clipboard and attaching multiple files. The changes are well-implemented, introducing new props and state to handle attachments throughout the component hierarchy. My feedback focuses on two areas: improving the user experience for the new clear attachments feature by making the UI more intuitive, and refactoring the clipboard paste handler for better code clarity and maintainability. Overall, this is a great addition to the application's functionality.
| <IconButton | ||
| aria-label="Attach file" | ||
| aria-label="Clear attachments" | ||
| variant="ghost" | ||
| {...footerStyles.footer.attachButton} | ||
| onClick={onClearAttachments} | ||
| > | ||
| {attachmentsCount > 0 && (<> | ||
| {attachmentsCount} | ||
| </>)} | ||
| <BsPaperclip size="24" /> | ||
| </IconButton> |
There was a problem hiding this comment.
Using the paperclip icon (BsPaperclip) for an action that clears attachments is counter-intuitive, as this icon is universally understood to mean "attach". This can lead to a confusing user experience, even though the aria-label is correct. To improve clarity, consider changing the icon when there are attachments to visually indicate the "clear" action. For example, you could show a close icon (IoClose from react-icons/io5, which would need to be imported) next to the attachment count.
| <IconButton | |
| aria-label="Attach file" | |
| aria-label="Clear attachments" | |
| variant="ghost" | |
| {...footerStyles.footer.attachButton} | |
| onClick={onClearAttachments} | |
| > | |
| {attachmentsCount > 0 && (<> | |
| {attachmentsCount} | |
| </>)} | |
| <BsPaperclip size="24" /> | |
| </IconButton> | |
| <IconButton | |
| aria-label="Clear attachments" | |
| variant="ghost" | |
| {...footerStyles.footer.attachButton} | |
| onClick={onClearAttachments} | |
| disabled={attachmentsCount === 0} | |
| > | |
| {attachmentsCount > 0 ? ( | |
| <HStack spacing={1}> | |
| <Box as="span">{attachmentsCount}</Box> | |
| <IoClose size="20" /> | |
| </HStack> | |
| ) : ( | |
| <BsPaperclip size="24" /> | |
| )} | |
| </IconButton> |
| const handlePaste = (e: React.ClipboardEvent<HTMLInputElement | HTMLTextAreaElement>) => { | ||
| const { items } = e.clipboardData; | ||
| let foundImage = false; | ||
|
|
||
| for (let i = 0; i < items.length; i += 1) { | ||
| const item = items[i]; | ||
| if (item.type.startsWith('image/')) { | ||
| const file = item.getAsFile(); | ||
| if (file) { | ||
| foundImage = true; | ||
| const reader = new FileReader(); | ||
| reader.onload = () => { | ||
| setAttachedImages(prev => [ | ||
| ...prev, | ||
| { | ||
| source: 'clipboard', | ||
| data: reader.result as string, | ||
| mime_type: file.type, | ||
| }, | ||
| ]); | ||
| }; | ||
| reader.readAsDataURL(file); | ||
| } | ||
| } | ||
| } | ||
| // Prevent the raw image from being inserted as text | ||
| if (foundImage) e.preventDefault(); | ||
| }; |
There was a problem hiding this comment.
The handlePaste function can be refactored to be more declarative and easier to read. Instead of using a for loop and a boolean flag (foundImage), you can convert e.clipboardData.items to an array and use modern JavaScript array methods like filter and forEach. This makes the intent of the code clearer and improves maintainability.
const handlePaste = (e: React.ClipboardEvent<HTMLInputElement | HTMLTextAreaElement>) => {
const imageItems = Array.from(e.clipboardData.items).filter((item) =>
item.type.startsWith('image/'),
);
if (imageItems.length > 0) {
e.preventDefault();
imageItems.forEach((item) => {
const file = item.getAsFile();
if (file) {
const reader = new FileReader();
reader.onload = () => {
setAttachedImages((prev) => [
...prev,
{
source: 'clipboard',
data: reader.result as string,
mime_type: file.type,
},
]);
};
reader.readAsDataURL(file);
}
});
}
};
This PR enables ctrl+v in the inputbox to paste images to the outgoing message. Changelog: