Skip to content

[Refactor] UI - Playground: Extract FilePreviewCard from ChatUI#23973

Merged
yuneng-jiang merged 1 commit intolitellm_yj_march_17_2026from
litellm_/fervent-hypatia
Mar 18, 2026
Merged

[Refactor] UI - Playground: Extract FilePreviewCard from ChatUI#23973
yuneng-jiang merged 1 commit intolitellm_yj_march_17_2026from
litellm_/fervent-hypatia

Conversation

@yuneng-jiang
Copy link
Copy Markdown
Contributor

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 FilePreviewCard component with a clean 3-prop interface (file, previewUrl, onRemove), reducing ChatUI.tsx by ~50 lines.

Testing

  • All 9 existing ChatUI tests pass unchanged
  • Added 8 new tests for FilePreviewCard covering:
    • Rendering and file name display
    • PDF vs image type label branching
    • Image preview rendering (present for images, absent for PDFs)
    • Case-insensitive .pdf detection
    • Remove button callback

Type

🧹 Refactoring
✅ Test

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]>
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Mar 18, 2026 7:42am

Request Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 18, 2026

Greptile Summary

This PR is a clean UI refactor that extracts two nearly-identical file-preview JSX blocks from ChatUI.tsx into a reusable FilePreviewCard component, reducing duplication by ~50 lines and improving maintainability. The extracted component has a clear 3-prop interface (file, previewUrl, onRemove) and is accompanied by 8 new unit tests covering all major branches.

Key changes:

  • FilePreviewCard.tsx — new component faithfully reproducing the original rendering logic (PDF icon vs image thumbnail, file name, type label, remove button) with proper TypeScript typing
  • ChatUI.tsx — the two duplicate inline blocks replaced with <FilePreviewCard /> calls; no logic or behavioral changes
  • FilePreviewCard.test.tsx — 8 new tests covering rendering, PDF/image label branching, case-insensitive .pdf detection, image preview presence, and the onRemove callback; all 9 existing ChatUI tests are reported to pass unchanged

Two minor style improvements are flagged on the new component: the delete button is missing aria-label and type="button", and the <img> element renders with an empty src when previewUrl is null for a non-PDF file (both inherited from the original code, but the refactor is a natural opportunity to address them).

Confidence Score: 5/5

  • This PR is safe to merge — it is a pure refactor with no logic changes and good test coverage.
  • The change is a mechanical extraction of duplicate JSX into a well-typed component. Behavior is identical to the original, all existing tests continue to pass, and 8 new unit tests cover the new component's branches. The only open items are two minor accessibility/style suggestions inherited from the original code.
  • No files require special attention; both flagged items on FilePreviewCard.tsx are minor style improvements, not blockers.

Important Files Changed

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
Loading

Last reviewed commit: "[Refactor] UI - Play..."

Comment on lines +34 to +39
<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>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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.

Suggested change
<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>

Comment on lines +21 to +25
<img
src={previewUrl || ""}
alt="Upload preview"
className="w-10 h-10 rounded-md border border-gray-200 object-cover"
/>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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:

Suggested change
<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.

@yuneng-jiang yuneng-jiang merged commit bbabdaa into litellm_yj_march_17_2026 Mar 18, 2026
19 of 54 checks passed
@ishaan-berri ishaan-berri deleted the litellm_/fervent-hypatia branch March 26, 2026 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant