Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,44 @@ export const useImageOperations = (projectId: string, branchId: string, activeFo
}
};

// Handle file rename
const handleRename = async (oldPath: string, newName: string) => {
if (!codeEditor) throw new Error('Code editor not available');

const directory = path.dirname(oldPath);
const sanitizedName = sanitizeFilename(newName);
const newPath = path.join(directory, sanitizedName);

// Check if it's still an image file
if (!isImageFile(sanitizedName)) {
throw new Error('File must be an image');
}

// Read the existing file content
const content = await codeEditor.readFile(oldPath);
if (!content) {
throw new Error('Could not read file content');
}

// Write to new path and delete old file
await codeEditor.writeFile(newPath, content);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider checking if a file already exists at the target (newPath) before renaming to avoid accidental overwrites.

await codeEditor.deleteFile(oldPath);
};
Comment on lines 62 to 117
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add collision check and improve error handling.

The rename operation has two issues:

  1. File collision risk: No check if newPath already exists, which could silently overwrite an existing file.
  2. Partial failure state: If writeFile succeeds but deleteFile fails, you'll have both the old and new files, creating an inconsistent state.

Apply this diff to add collision check:

 const handleRename = async (oldPath: string, newName: string) => {
     if (!codeEditor) throw new Error('Code editor not available');

     const directory = path.dirname(oldPath);
     const sanitizedName = sanitizeFilename(newName);
     const newPath = path.join(directory, sanitizedName);

     // Check if it's still an image file
     if (!isImageFile(sanitizedName)) {
         throw new Error('File must be an image');
     }
+
+    // Check if the target path already exists
+    if (newPath !== oldPath) {
+        try {
+            const existingContent = await codeEditor.readFile(newPath);
+            if (existingContent) {
+                throw new Error('A file with this name already exists');
+            }
+        } catch (error) {
+            // File doesn't exist, which is what we want
+            if (!(error instanceof Error) || !error.message.includes('not found')) {
+                throw error;
+            }
+        }
+    }

     // Read the existing file content
     const content = await codeEditor.readFile(oldPath);
     if (!content) {
         throw new Error('Could not read file content');
     }

-    // Write to new path and delete old file
-    await codeEditor.writeFile(newPath, content);
-    await codeEditor.deleteFile(oldPath);
+    // Write to new path and delete old file
+    try {
+        await codeEditor.writeFile(newPath, content);
+        await codeEditor.deleteFile(oldPath);
+    } catch (error) {
+        // If write succeeded but delete failed, attempt cleanup
+        try {
+            await codeEditor.deleteFile(newPath);
+        } catch {
+            // Cleanup failed, log the inconsistent state
+            console.error('Failed to cleanup after partial rename failure');
+        }
+        throw error;
+    }
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleRename = async (oldPath: string, newName: string) => {
if (!codeEditor) throw new Error('Code editor not available');
const directory = path.dirname(oldPath);
const sanitizedName = sanitizeFilename(newName);
const newPath = path.join(directory, sanitizedName);
// Check if it's still an image file
if (!isImageFile(sanitizedName)) {
throw new Error('File must be an image');
}
// Read the existing file content
const content = await codeEditor.readFile(oldPath);
if (!content) {
throw new Error('Could not read file content');
}
// Write to new path and delete old file
await codeEditor.writeFile(newPath, content);
await codeEditor.deleteFile(oldPath);
};
const handleRename = async (oldPath: string, newName: string) => {
if (!codeEditor) throw new Error('Code editor not available');
const directory = path.dirname(oldPath);
const sanitizedName = sanitizeFilename(newName);
const newPath = path.join(directory, sanitizedName);
// Check if it's still an image file
if (!isImageFile(sanitizedName)) {
throw new Error('File must be an image');
}
// Check if the target path already exists
if (newPath !== oldPath) {
try {
const existingContent = await codeEditor.readFile(newPath);
if (existingContent) {
throw new Error('A file with this name already exists');
}
} catch (error) {
// File doesn't exist, which is what we want
if (!(error instanceof Error) || !error.message.includes('not found')) {
throw error;
}
}
}
// Read the existing file content
const content = await codeEditor.readFile(oldPath);
if (!content) {
throw new Error('Could not read file content');
}
// Write to new path and delete old file
try {
await codeEditor.writeFile(newPath, content);
await codeEditor.deleteFile(oldPath);
} catch (error) {
// If write succeeded but delete failed, attempt cleanup
try {
await codeEditor.deleteFile(newPath);
} catch {
// Cleanup failed, log the inconsistent state
console.error('Failed to cleanup after partial rename failure');
}
throw error;
}
};
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-operations.tsx
around lines 60 to 81, the rename flow lacks a collision check and can leave a
partial failure if writeFile succeeds but deleteFile fails; add an existence
check for newPath before writing and throw a clear error if it exists, then
perform writeFile inside a try/catch and if deleteFile fails attempt to rollback
by deleting the newly written file (and surface combined error information), and
ensure all thrown errors include contextual messages (oldPath/newPath) so
callers can handle them.


// Handle file delete
const handleDelete = async (filePath: string) => {
if (!codeEditor) throw new Error('Code editor not available');
await codeEditor.deleteFile(filePath);
};

return {
folders,
images,
loading,
error,
isUploading,
handleUpload,
handleRename,
handleDelete,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@ interface ImageGridProps {
branchId: string;
search: string;
onUpload: (files: FileList) => Promise<void>;
onRename: (oldPath: string, newName: string) => Promise<void>;
onDelete: (filePath: string) => Promise<void>;
onAddToChat: (image: any) => void;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Replace any with specific type.

The any type violates the coding guidelines. Based on the relevant code snippets, the correct type should be ImageContentData from @onlook/models.

As per coding guidelines.

Apply this diff:

-    onAddToChat: (image: any) => void;
+    onAddToChat: (image: ImageContentData) => void;

Import the type at the top of the file:

import type { ImageContentData } from '@onlook/models';
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsx
around line 17, the prop type uses `any` for onAddToChat; update the signature
to use the specific `ImageContentData` type and add an import at the top of the
file: import the type from '@onlook/models' (import type { ImageContentData }
from '@onlook/models';) and replace `onAddToChat: (image: any) => void;` with
`onAddToChat: (image: ImageContentData) => void;`.

}

export const ImageGrid = ({ images, projectId, branchId, search, onUpload }: ImageGridProps) => {
export const ImageGrid = ({ images, projectId, branchId, search, onUpload, onRename, onDelete, onAddToChat }: ImageGridProps) => {
const {
handleDragEnter, handleDragLeave, handleDragOver, handleDrop, isDragging,
onImageDragStart, onImageDragEnd, onImageMouseDown, onImageMouseUp
Expand All @@ -41,6 +44,9 @@ export const ImageGrid = ({ images, projectId, branchId, search, onUpload }: Ima
onImageDragEnd={onImageDragEnd}
onImageMouseDown={onImageMouseDown}
onImageMouseUp={onImageMouseUp}
onRename={onRename}
onDelete={onDelete}
onAddToChat={onAddToChat}
/>
))}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,25 @@

import { useFile } from '@onlook/file-system/hooks';
import type { ImageContentData } from '@onlook/models';
import {
AlertDialog,
AlertDialogAction,
AlertDialogCancel,
AlertDialogContent,
AlertDialogDescription,
AlertDialogFooter,
AlertDialogHeader,
AlertDialogTitle
} from '@onlook/ui/alert-dialog';
import { Button } from '@onlook/ui/button';
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuTrigger
} from '@onlook/ui/dropdown-menu';
import { Icons } from '@onlook/ui/icons';
import { Input } from '@onlook/ui/input';
import { useEffect, useState } from 'react';

interface ImageItemProps {
Expand All @@ -17,12 +35,18 @@ interface ImageItemProps {
onImageDragEnd: () => void;
onImageMouseDown: () => void;
onImageMouseUp: () => void;
onRename: (oldPath: string, newName: string) => Promise<void>;
onDelete: (filePath: string) => Promise<void>;
onAddToChat: (image: ImageContentData) => void;
}

export const ImageItem = ({ image, projectId, branchId, onImageDragStart, onImageDragEnd, onImageMouseDown, onImageMouseUp }: ImageItemProps) => {
export const ImageItem = ({ image, projectId, branchId, onImageDragStart, onImageDragEnd, onImageMouseDown, onImageMouseUp, onRename, onDelete, onAddToChat }: ImageItemProps) => {
const { content, loading } = useFile(projectId, branchId, image.path);
const [imageUrl, setImageUrl] = useState<string | null>(null);
const [isDisabled, setIsDisabled] = useState(false);
const [isRenaming, setIsRenaming] = useState(false);
const [newName, setNewName] = useState(image.name);
const [showDeleteDialog, setShowDeleteDialog] = useState(false);

// Convert content to data URL for display
useEffect(() => {
Expand Down Expand Up @@ -87,24 +111,158 @@ export const ImageItem = ({ image, projectId, branchId, onImageDragStart, onImag
onImageDragStart(e, imageContentData);
};

const handleRename = async () => {
if (newName.trim() && newName !== image.name) {
try {
await onRename(image.path, newName.trim());
} catch (error) {
console.error('Failed to rename file:', error);
setNewName(image.name); // Reset on error
}
}
setIsRenaming(false);
};

const handleDelete = async () => {
try {
await onDelete(image.path);
setShowDeleteDialog(false);
} catch (error) {
console.error('Failed to delete file:', error);
}
};

const handleAddToChat = () => {
const imageContentData: ImageContentData = {
fileName: image.name,
content: content as string,
mimeType: imageUrl || '',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mimeType property is being assigned the entire data URL value from imageUrl rather than extracting just the MIME type information. This could cause issues downstream where only the actual MIME type (e.g., image/png, image/jpeg) is expected. Consider either:

  1. Extracting the MIME type portion from the data URL using a regex or string manipulation
  2. Determining the MIME type based on the file extension
  3. Using a dedicated function to properly identify the content type

This would ensure the correct content type information is passed to the chat system.

Suggested change
mimeType: imageUrl || '',
mimeType: imageUrl ? imageUrl.split(';')[0].split(':')[1] : 'image/png',

Spotted by Diamond

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

originPath: image.path,
};
onAddToChat(imageContentData);
};
Comment on lines 153 to 155
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix incorrect mimeType assignment.

Line 139 has the same issue as line 108: mimeType is set to imageUrl (a data URL) instead of the actual MIME type.

Apply this diff:

     const handleAddToChat = () => {
         const imageContentData: ImageContentData = {
             fileName: image.name,
             content: content as string,
-            mimeType: imageUrl || '',
+            mimeType: image.mimeType || 'image/*',
             originPath: image.path,
         };
         onAddToChat(imageContentData);
     };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleAddToChat = () => {
const imageContentData: ImageContentData = {
fileName: image.name,
content: content as string,
mimeType: imageUrl || '',
originPath: image.path,
};
onAddToChat(imageContentData);
};
const handleAddToChat = () => {
const imageContentData: ImageContentData = {
fileName: image.name,
content: content as string,
mimeType: image.mimeType || 'image/*',
originPath: image.path,
};
onAddToChat(imageContentData);
};
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-item.tsx
around lines 135 to 143, the mimeType for ImageContentData is incorrectly set to
imageUrl (a data URL); change the assignment to use the actual MIME type from
the image object (e.g. image.mimeType || ''), ensuring the field contains a MIME
type string rather than a data URL and preserving types when calling
onAddToChat.


const handleKeyDown = (e: React.KeyboardEvent) => {
if (e.key === 'Enter') {
void handleRename();
} else if (e.key === 'Escape') {
setNewName(image.name);
setIsRenaming(false);
}
};

return (
<div className="aspect-square bg-background-secondary rounded-md border border-border-primary overflow-hidden cursor-pointer hover:border-border-onlook transition-colors"
onDragStart={handleDragStart}
onDragEnd={onImageDragEnd}
onMouseDown={onImageMouseDown}
onMouseUp={onImageMouseUp}
>
<img
src={imageUrl}
alt={image.name}
className="w-full h-full object-cover"
loading="lazy"
/>
<div className="p-1 bg-background-primary/80 backdrop-blur-sm">
<div className="text-xs text-foreground-primary truncate" title={image.name}>
{image.name}
</div>
<div className="group">
<div
className="aspect-square bg-background-secondary rounded-md border border-border-primary overflow-hidden cursor-pointer hover:border-border-onlook transition-colors relative"
onDragStart={handleDragStart}
onDragEnd={onImageDragEnd}
onMouseDown={onImageMouseDown}
onMouseUp={onImageMouseUp}
>
<img
src={imageUrl}
alt={image.name}
className="w-full h-full object-cover"
loading="lazy"
/>

{/* Action menu */}
{!isRenaming && (
<div className="absolute top-2 right-2 opacity-0 group-hover:opacity-100 transition-opacity">
<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
size="icon"
variant="secondary"
className="h-6 w-6 bg-background-secondary/90 hover:bg-background-onlook"
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
}}
>
<Icons.DotsHorizontal className="h-3 w-3" />
</Button>
</DropdownMenuTrigger>
<DropdownMenuContent align="end" className="w-40">
<DropdownMenuItem
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
handleAddToChat();
}}
className="flex items-center gap-2"
>
<Icons.Plus className="h-3 w-3" />
Add to Chat
</DropdownMenuItem>
<DropdownMenuItem
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
setIsRenaming(true);
}}
className="flex items-center gap-2"
>
<Icons.Edit className="h-3 w-3" />
Rename
</DropdownMenuItem>
<DropdownMenuItem
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
setShowDeleteDialog(true);
}}
className="flex items-center gap-2 text-red-500 hover:text-red-600 focus:text-red-600"
>
<Icons.Trash className="h-3 w-3" />
Delete
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
</div>
)}
</div>

{/* Name section with rename functionality */}
<div className="mt-1 px-1">
{isRenaming ? (
<Input
value={newName}
onChange={(e) => setNewName(e.target.value)}
onKeyDown={handleKeyDown}
onBlur={() => void handleRename()}
className="h-6 text-xs p-1 border-0 bg-transparent focus-visible:ring-1 focus-visible:ring-ring"
autoFocus
onClick={(e) => e.stopPropagation()}
/>
) : (
<div className="text-xs text-foreground-primary truncate" title={image.name}>
{image.name}
</div>
)}
</div>

{/* Delete confirmation dialog */}
<AlertDialog open={showDeleteDialog} onOpenChange={setShowDeleteDialog}>
<AlertDialogContent>
<AlertDialogHeader>
<AlertDialogTitle>Delete Image</AlertDialogTitle>
<AlertDialogDescription>
Are you sure you want to delete {image.name}? This action cannot be undone.
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel>Cancel</AlertDialogCancel>
<AlertDialogAction
onClick={() => void handleDelete()}
className="bg-destructive text-white hover:bg-destructive/90"
>
Delete
</AlertDialogAction>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>
</div>
);
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
'use client';

import { useEditorEngine } from '@/components/store/editor';
import { MessageContextType, type FileMessageContext } from '@onlook/models/chat';
import { Icons } from '@onlook/ui/icons';
import { toast } from '@onlook/ui/sonner';
import { observer } from 'mobx-react-lite';
import { BreadcrumbNavigation } from './breadcrumb-navigation';
import { FolderList } from './folder-list';
Expand Down Expand Up @@ -37,11 +39,55 @@ export const ImagesTab = observer(() => {
error,
isUploading,
handleUpload,
handleRename,
handleDelete,
} = useImageOperations(projectId, branchId, activeFolder, branchData?.codeEditor);

// Filter images based on search
const images = filterImages(allImages);

// Handler functions with error handling and feedback
const handleRenameWithFeedback = async (oldPath: string, newName: string) => {
try {
await handleRename(oldPath, newName);
toast.success('Image renamed successfully');
} catch (error) {
console.error('Failed to rename image:', error);
toast.error(`Failed to rename image: ${error instanceof Error ? error.message : 'Unknown error'}`);
throw error;
}
};

const handleDeleteWithFeedback = async (filePath: string) => {
try {
await handleDelete(filePath);
toast.success('Image deleted successfully');
} catch (error) {
console.error('Failed to delete image:', error);
toast.error(`Failed to delete image: ${error instanceof Error ? error.message : 'Unknown error'}`);
throw error;
}
};

const handleAddToChat = async (imageContentData: any) => {
try {
// Convert the image content data to file context for chat
const fileContext: FileMessageContext = {
type: MessageContextType.FILE,
content: '', // File content will be loaded by the chat system
displayName: imageContentData.fileName,
path: imageContentData.originPath,
branchId: branchId,
};

editorEngine.chat.context.addContexts([fileContext]);
toast.success('Image added to chat');
} catch (error) {
console.error('Failed to add image to chat:', error);
toast.error('Failed to add image to chat');
}
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix type annotation and clarify empty content field.

Two issues:

  1. Type violation: Parameter uses any type, which violates coding guidelines.
  2. Empty content: Line 77 sets content: '' with a comment saying it will be loaded by the chat system, but imageContentData already contains the content. This seems inconsistent and may cause issues if the chat system expects pre-loaded content.

As per coding guidelines.

Apply this diff:

-const handleAddToChat = async (imageContentData: any) => {
+const handleAddToChat = async (imageContentData: ImageContentData) => {
     try {
         // Convert the image content data to file context for chat
         const fileContext: FileMessageContext = {
             type: MessageContextType.FILE,
-            content: '', // File content will be loaded by the chat system
+            content: imageContentData.content,
             displayName: imageContentData.fileName,
             path: imageContentData.originPath,
             branchId: branchId,
         };

Also add the import:

import type { ImageContentData } from '@onlook/models';
🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx
around lines 72-89, the handler uses an any parameter and sets content to an
empty string; change the parameter type to ImageContentData (add import: import
type { ImageContentData } from '@onlook/models') and populate the
FileMessageContext.content with the actual imageContentData content field (e.g.,
imageContentData.content or correct property name), keep the other fields
(displayName, path, branchId) as-is, and remove the any usage so the function
signature and context object are fully typed.


if (loading) {
return (
<div className="w-full h-full flex items-center justify-center gap-2">
Expand Down Expand Up @@ -84,6 +130,9 @@ export const ImagesTab = observer(() => {
branchId={branchId}
search={search}
onUpload={handleUpload}
onRename={handleRenameWithFeedback}
onDelete={handleDeleteWithFeedback}
onAddToChat={handleAddToChat}
/>
</div>
);
Expand Down