[Refactor] UI - Playground: Extract FilePreviewCard from ChatUI#23973
[Refactor] UI - Playground: Extract FilePreviewCard from ChatUI#23973yuneng-jiang merged 1 commit intolitellm_yj_march_17_2026from
Conversation
Extract duplicate file preview JSX blocks (responses and chat image previews) into a reusable FilePreviewCard component, reducing ~50 lines of duplicated markup in ChatUI.tsx. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR is a clean UI refactor that extracts two nearly-identical file-preview JSX blocks from Key changes:
Two minor style improvements are flagged on the new component: the delete button is missing Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/components/playground/chat_ui/FilePreviewCard.tsx | New reusable component faithfully extracted from the two duplicate JSX blocks in ChatUI.tsx. Minor accessibility gap (missing aria-label on delete button) and a pre-existing empty-src issue when previewUrl is null for non-PDF files. |
| ui/litellm-dashboard/src/components/playground/chat_ui/FilePreviewCard.test.tsx | 8 well-scoped unit tests covering rendering, PDF/image label branching, case-insensitive .pdf detection, image preview presence/absence, and the onRemove callback. Tests 1 and 2 are slightly redundant (both assert file-name rendering) but this is harmless. |
| ui/litellm-dashboard/src/components/playground/chat_ui/ChatUI.tsx | Two ~30-line duplicate JSX blocks replaced with clean FilePreviewCard usages. Import added, no logic changes. All existing ChatUI tests continue to pass. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[ChatUI renders file upload area] --> B{endpointType?}
B -->|RESPONSES| C{responsesUploadedImage?}
B -->|CHAT| D{chatUploadedImage?}
C -->|yes| E["<FilePreviewCard\n file={responsesUploadedImage}\n previewUrl={responsesImagePreviewUrl}\n onRemove={handleRemoveResponsesImage}\n/>"]
C -->|no| F[Nothing rendered]
D -->|yes| G["<FilePreviewCard\n file={chatUploadedImage}\n previewUrl={chatImagePreviewUrl}\n onRemove={handleRemoveChatImage}\n/>"]
D -->|no| H[Nothing rendered]
E --> I[FilePreviewCard]
G --> I
I --> J{file.name ends with .pdf?}
J -->|yes| K[PDF icon + 'PDF' label]
J -->|no| L["img src={previewUrl} + 'Image' label"]
K --> M[Remove button → onRemove]
L --> M
Last reviewed commit: "[Refactor] UI - Play..."
| <button | ||
| className="flex items-center justify-center w-6 h-6 text-gray-400 hover:text-gray-600 hover:bg-gray-200 rounded-full transition-colors" | ||
| onClick={onRemove} | ||
| > | ||
| <DeleteOutlined style={{ fontSize: "12px" }} /> | ||
| </button> |
There was a problem hiding this comment.
Missing
aria-label and type="button" on remove button
The icon-only delete button has no accessible text for screen readers, so assistive technologies will announce it as a nameless "button". It also lacks type="button", which is a best-practice safeguard against accidental form submission if this card is ever nested inside a <form>. These issues existed in the original inline JSX, but the refactor is a good opportunity to fix both.
| <button | |
| className="flex items-center justify-center w-6 h-6 text-gray-400 hover:text-gray-600 hover:bg-gray-200 rounded-full transition-colors" | |
| onClick={onRemove} | |
| > | |
| <DeleteOutlined style={{ fontSize: "12px" }} /> | |
| </button> | |
| <button | |
| type="button" | |
| aria-label="Remove file" | |
| className="flex items-center justify-center w-6 h-6 text-gray-400 hover:text-gray-600 hover:bg-gray-200 rounded-full transition-colors" | |
| onClick={onRemove} | |
| > | |
| <DeleteOutlined style={{ fontSize: "12px" }} /> | |
| </button> |
| <img | ||
| src={previewUrl || ""} | ||
| alt="Upload preview" | ||
| className="w-10 h-10 rounded-md border border-gray-200 object-cover" | ||
| /> |
There was a problem hiding this comment.
Empty
src for image when previewUrl is null
When the file is not a PDF but previewUrl is null, the <img> renders with src="", which triggers a broken-image indicator in most browsers and fires an unnecessary network request to the current page's URL. Consider guarding the render — consistent with how the PDF branch already hides the <img> entirely:
| <img | |
| src={previewUrl || ""} | |
| alt="Upload preview" | |
| className="w-10 h-10 rounded-md border border-gray-200 object-cover" | |
| /> | |
| ) : previewUrl ? ( | |
| <img | |
| src={previewUrl} | |
| alt="Upload preview" | |
| className="w-10 h-10 rounded-md border border-gray-200 object-cover" | |
| /> | |
| ) : ( | |
| <div className="w-10 h-10 rounded-md bg-gray-200 flex items-center justify-center" /> |
This also keeps behaviour consistent: callers that haven't yet generated a preview URL (e.g. while the object URL is being created) won't show a broken image.
bbabdaa
into
litellm_yj_march_17_2026
Summary
Problem
ChatUI.tsx contains two nearly identical JSX blocks (~30 lines each) for rendering file preview cards — one for the Responses endpoint image upload and one for the Chat endpoint image upload.
Fix
Extracted the duplicate markup into a reusable
FilePreviewCardcomponent with a clean 3-prop interface (file,previewUrl,onRemove), reducing ChatUI.tsx by ~50 lines.Testing
FilePreviewCardcovering:.pdfdetectionType
🧹 Refactoring
✅ Test