feat: add image preview on /files page#1604
Merged
sweetmantech merged 10 commits intotestfrom Mar 28, 2026
Merged
Conversation
When clicking an image file on the /files tab, show the image using a signed URL instead of "Preview not available for this file type". Co-Authored-By: Paperclip <noreply@paperclip.ing>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR extends file preview functionality to support image files via base64 encoding. Changes include a new loading skeleton component, updated file content fetching to detect and encode images as base64 data URLs, and threading the Changes
Sequence Diagram(s)sequenceDiagram
participant Component as SandboxFileTree
participant Hook as useSandboxFileContent
participant API as getFileContents
participant Render as FilePreview
Component->>Hook: Fetch selectedPath
activate Hook
Hook->>API: getFileContents(token, path)
activate API
API->>API: Check response.encoding
alt base64 encoded
API->>API: getMimeFromPath(path)<br/>Generate data URL
API-->>Hook: {content: null, imageUrl: "data:..."}
else text content
API-->>Hook: {content: string, imageUrl: null}
end
deactivate API
Hook-->>Component: {content, imageUrl}
deactivate Hook
Component->>Render: FilePreview({imageUrl, content, loading})
activate Render
alt imageUrl provided
Render->>Render: Render <img src={imageUrl}>
else loading
Render->>Render: Render FilePreviewSkeleton
else text content
Render->>Render: Render markdown/text
else error
Render->>Render: Render error card
end
deactivate Render
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
components/Files/FilePreview.tsx (1)
25-37: Consider adding loading and error states for image preview.The image rendering logic is clean and the URL encoding is properly handled. However, unlike the text file branch which has explicit loading/error states, the image preview lacks feedback while the image loads or if the signed URL fails.
For a better UX, consider showing a loading spinner while the image loads and handling the
onErrorcase gracefully.♻️ Suggested improvement with loading/error handling
+import { useState } from "react"; import ReactMarkdown from "react-markdown"; import remarkBreaks from "remark-breaks"; import remarkGfm from "remark-gfm"; import isImagePath from "@/utils/isImagePath";Then update the image rendering block:
if (isImage && storageKey) { const signedUrl = `/api/files/signed-url?key=${encodeURIComponent(storageKey)}`; + const [imgLoading, setImgLoading] = useState(true); + const [imgError, setImgError] = useState(false); + + if (imgError) { + return ( + <div className="flex items-center justify-center h-full min-h-[300px] border border-border rounded-lg bg-background"> + <p className="text-sm text-destructive">Failed to load image</p> + </div> + ); + } + return ( <div className="flex-1 border border-border rounded-lg bg-background overflow-hidden flex items-center justify-center min-h-[300px]"> + {imgLoading && ( + <p className="text-sm text-muted-foreground absolute">Loading...</p> + )} {/* eslint-disable-next-line `@next/next/no-img-element` */} <img src={signedUrl} alt={fileName || "Image preview"} - className="max-w-full max-h-[70vh] object-contain" + className={`max-w-full max-h-[70vh] object-contain ${imgLoading ? 'opacity-0' : 'opacity-100'}`} + onLoad={() => setImgLoading(false)} + onError={() => setImgError(true)} /> </div> ); }Note: If you adopt this, you'll need to extract the image preview into a separate component since hooks cannot be called conditionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Files/FilePreview.tsx` around lines 25 - 37, The image preview branch in FilePreview.tsx (where isImage && storageKey constructs signedUrl and renders an <img>) needs loading and error states: extract the image preview into its own component (e.g., ImagePreview) so you can use hooks, track isLoading and hasError via onLoad/onError handlers, show a loading spinner while isLoading is true, show a friendly error/fallback UI when hasError is true, and continue to use the encoded signedUrl for src; update FilePreview to render <ImagePreview signedUrl={signedUrl} alt={fileName} /> instead of the raw <img>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@components/Files/FilePreview.tsx`:
- Around line 25-37: The image preview branch in FilePreview.tsx (where isImage
&& storageKey constructs signedUrl and renders an <img>) needs loading and error
states: extract the image preview into its own component (e.g., ImagePreview) so
you can use hooks, track isLoading and hasError via onLoad/onError handlers,
show a loading spinner while isLoading is true, show a friendly error/fallback
UI when hasError is true, and continue to use the encoded signedUrl for src;
update FilePreview to render <ImagePreview signedUrl={signedUrl} alt={fileName}
/> instead of the raw <img>.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 45f4f8f5-d73d-4e67-a584-420b4d103ec8
📒 Files selected for processing (2)
components/Files/FileInfoDialogContent.tsxcomponents/Files/FilePreview.tsx
Add image preview support for sandbox (GitHub repo) files on the /files page. The previous commit only handled Supabase-stored files via FileInfoDialog. This adds support for the SandboxFilePreview path by: - Adding getImageContent() to fetch base64 image data via API format=base64 - Extending useSandboxFileContent hook to detect images and return data URLs - Adding imageUrl prop to FilePreview as alternative to storageKey - Passing imageUrl through SandboxFilePreview and SandboxFileTree Depends on API PR #364 (base64 format support for sandbox file endpoint). Co-Authored-By: Paperclip <noreply@paperclip.ing> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/useSandboxFileContent.ts`:
- Around line 28-31: In useSandboxFileContent, normalize fileName by stripping
query/hash fragments (e.g., split on ? and # and take the first part) once into
a normalizedFileName and use that normalizedFileName for isImagePath checks and
when calling getImageContent (instead of the raw path.split("/").pop()), keeping
the empty-string fallback; ensure the returned metadata and any MIME-resolution
logic use normalizedFileName so downstream image MIME detection won't be
misclassified.
In `@lib/sandboxes/getImageContent.ts`:
- Around line 31-36: The code currently assumes data.content exists when
response.ok and data.status === "success"; update the getImageContent flow to
validate that data.content is a non-empty string before constructing the data
URL: check the fetched data (the variable data) for a present, non-empty
data.content and throw a clear Error (including context such as fileName and
data.status) if missing or empty; only call getMimeType(fileName) and return the
`data:${mimeType};base64,${data.content}` string after this validation so
broken/empty previews are caught and handled upstream.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 58b592e0-e91b-4fe4-9c07-83b7049b062a
📒 Files selected for processing (5)
components/Files/FilePreview.tsxcomponents/Sandboxes/SandboxFilePreview.tsxcomponents/Sandboxes/SandboxFileTree.tsxhooks/useSandboxFileContent.tslib/sandboxes/getImageContent.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- components/Files/FilePreview.tsx
hooks/useSandboxFileContent.ts
Outdated
Comment on lines
+28
to
+31
| const fileName = path.split("/").pop() ?? ""; | ||
| if (isImagePath(fileName)) { | ||
| const dataUrl = await getImageContent(accessToken, path, fileName); | ||
| return { content: null, imageUrl: dataUrl }; |
Contributor
There was a problem hiding this comment.
Normalize fileName before using it for image fetch metadata.
At Line 28, fileName may retain query/hash fragments. isImagePath still detects the file as image, but MIME resolution downstream can misclassify it. Normalize once before both checks and fetch.
Proposed fix
- const fileName = path.split("/").pop() ?? "";
+ const fileName = (path.split("/").pop() ?? "").split("?")[0].split("#")[0];
if (isImagePath(fileName)) {
const dataUrl = await getImageContent(accessToken, path, fileName);
return { content: null, imageUrl: dataUrl };
}📝 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 fileName = path.split("/").pop() ?? ""; | |
| if (isImagePath(fileName)) { | |
| const dataUrl = await getImageContent(accessToken, path, fileName); | |
| return { content: null, imageUrl: dataUrl }; | |
| const fileName = (path.split("/").pop() ?? "").split("?")[0].split("#")[0]; | |
| if (isImagePath(fileName)) { | |
| const dataUrl = await getImageContent(accessToken, path, fileName); | |
| return { content: null, imageUrl: dataUrl }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useSandboxFileContent.ts` around lines 28 - 31, In
useSandboxFileContent, normalize fileName by stripping query/hash fragments
(e.g., split on ? and # and take the first part) once into a normalizedFileName
and use that normalizedFileName for isImagePath checks and when calling
getImageContent (instead of the raw path.split("/").pop()), keeping the
empty-string fallback; ensure the returned metadata and any MIME-resolution
logic use normalizedFileName so downstream image MIME detection won't be
misclassified.
lib/sandboxes/getImageContent.ts
Outdated
Comment on lines
+31
to
+36
| if (!response.ok || data.status === "error") { | ||
| throw new Error(data.error || "Failed to fetch image"); | ||
| } | ||
|
|
||
| const mimeType = getMimeType(fileName); | ||
| return `data:${mimeType};base64,${data.content}`; |
Contributor
There was a problem hiding this comment.
Validate content before constructing the data URL.
At Line 35, successful responses are assumed to always include data.content. If the API returns "success" with missing/empty content, Line 36 builds a broken URL and the preview silently fails.
Proposed fix
- if (!response.ok || data.status === "error") {
- throw new Error(data.error || "Failed to fetch image");
+ if (!response.ok || data.status === "error" || !data.content) {
+ throw new Error(data.error || "Failed to fetch image");
}
const mimeType = getMimeType(fileName);
return `data:${mimeType};base64,${data.content}`;📝 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
| if (!response.ok || data.status === "error") { | |
| throw new Error(data.error || "Failed to fetch image"); | |
| } | |
| const mimeType = getMimeType(fileName); | |
| return `data:${mimeType};base64,${data.content}`; | |
| if (!response.ok || data.status === "error" || !data.content) { | |
| throw new Error(data.error || "Failed to fetch image"); | |
| } | |
| const mimeType = getMimeType(fileName); | |
| return `data:${mimeType};base64,${data.content}`; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/sandboxes/getImageContent.ts` around lines 31 - 36, The code currently
assumes data.content exists when response.ok and data.status === "success";
update the getImageContent flow to validate that data.content is a non-empty
string before constructing the data URL: check the fetched data (the variable
data) for a present, non-empty data.content and throw a clear Error (including
context such as fileName and data.status) if missing or empty; only call
getMimeType(fileName) and return the `data:${mimeType};base64,${data.content}`
string after this validation so broken/empty previews are caught and handled
upstream.
…iles The API now auto-detects binary files by extension and returns base64 encoding automatically — no need to pass format=base64 in the request. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
There was a problem hiding this comment.
YAGNI - do we need storage key? can these changes be reverted?
components/Files/FilePreview.tsx
Outdated
| error: string | null; | ||
| isTextFile: boolean; | ||
| fileName?: string; | ||
| storageKey?: string; |
Collaborator
There was a problem hiding this comment.
YAGNI - do we need storage key? can these changes be reverted?
lib/sandboxes/getImageContent.ts
Outdated
Collaborator
There was a problem hiding this comment.
Why do we need a separate file for getting image content?
…ng UX Address PR review feedback: - Eliminate getImageContent.ts — the API auto-detects binary files, so getFileContents now handles base64 responses and builds data URLs - Validate data.content before constructing data URLs - Fix loading state showing false "Preview not available" for images by moving loading/error checks before image/isTextFile checks - Add a spinner to the loading state Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
storageKey/signed-URL image previews for Supabase-stored files are not part of this PR's scope. Remove the prop to keep the change focused on sandbox GitHub image previews only. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Not part of this PR's scope — keeps the diff focused. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
lib/sandboxes/getFileContents.ts
Outdated
| imageUrl: string | null; | ||
| } | ||
|
|
||
| function getMimeType(fileName: string): string { |
Collaborator
There was a problem hiding this comment.
SRP - new lib file for getMimeType.
…SRP) Add image MIME types to the existing getMimeFromPath utility and reuse it in getFileContents instead of an inline function. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
components/Files/FilePreview.tsx
Outdated
Comment on lines
24
to
29
| <div className="flex items-center justify-center h-full min-h-[300px] border border-border rounded-lg bg-background"> | ||
| <div className="flex flex-col items-center gap-2"> | ||
| <div className="h-6 w-6 animate-spin rounded-full border-2 border-muted-foreground border-t-transparent" /> | ||
| <p className="text-sm text-muted-foreground">Loading...</p> | ||
| </div> | ||
| </div> |
Collaborator
There was a problem hiding this comment.
Please replace loading text + spinner with a skeleton. If possible, reuse an existing image skeleton component from this codebase.
- Replace loading spinner with shadcn Skeleton component in FilePreview - Create lib/files/getImageMimeType.ts instead of modifying utils/ (SRP) - Revert utils/getMimeFromPath.ts to base branch state Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…IME types Move getMimeFromPath to lib/files/ per project conventions (no utils/ path). Add image MIME types (png, jpg, jpeg, gif, webp, svg) to the existing map so it can be reused for both text and image MIME resolution (DRY). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
components/Files/FilePreview.tsx
Outdated
Comment on lines
25
to
33
| <div className="flex-1 border border-border rounded-lg bg-background overflow-hidden p-6 sm:p-8"> | ||
| <div className="space-y-3"> | ||
| <Skeleton className="h-4 w-3/4" /> | ||
| <Skeleton className="h-4 w-full" /> | ||
| <Skeleton className="h-4 w-5/6" /> | ||
| <Skeleton className="h-4 w-2/3" /> | ||
| <Skeleton className="h-32 w-full" /> | ||
| </div> | ||
| </div> |
Collaborator
There was a problem hiding this comment.
SRP - new file for the loading file component.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/filespage for both Supabase-stored files and sandbox (GitHub repo) filesstorageKeythroughFileInfoDialogContenttoFilePreviewto render via signed URLformat=base64), convert to data URL, and render inFilePreviewvia newimageUrlpropgetImageContent()utility for fetching base64 image data from sandbox APIuseSandboxFileContenthook to detect image files and return data URLsDependencies
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements