feat(web): show image thumbnails in composer attachment chips#511
Open
GeT-LeFt wants to merge 4 commits intotiann:mainfrom
Open
feat(web): show image thumbnails in composer attachment chips#511GeT-LeFt wants to merge 4 commits intotiann:mainfrom
GeT-LeFt wants to merge 4 commits intotiann:mainfrom
Conversation
Tool results (Read, Bash, Markdown, Generic views) currently render only text content — any base64 image blocks returned by tools are dropped. This means screenshots read by the Read tool, image output from bash commands, etc. are invisible in the chat. Changes: - Add ImageLightbox component with Portal overlay, ESC/backdrop close, and open-in-new-tab action. - Extract and render base64 image blocks in tool result content. - Integrate ResultImages into Read/Bash/Markdown/Generic result views. - Add click-to-preview for user-uploaded image attachments.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review mode: initial
Findings
- [Major] The new composer thumbnail upload state is currently unreachable.
web/src/components/AssistantChat/AttachmentItem.tsx:42treatspreviewUrlas the image/non-image switch, butpreviewUrlis only assigned after upload completion inweb/src/lib/attachmentAdapter.ts:93-107, so therunningoverlay path never renders. - [Major]
web/src/components/ImageLightbox.tsx:43opens user-controlledpreviewUrlvalues withwindow.open(src, '_blank')and keepswindow.openerattached for non-data:URLs.previewUrlis accepted as an arbitrary string inshared/src/schemas.ts:147and passed through inweb/src/chat/normalizeUser.ts:23, so this adds a reverse-tabnabbing path.
Summary
- Review mode: initial
- 2 issues found in the latest head diff. Residual gap: no automated coverage was added for the new composer-thumbnail path or the lightbox open-in-new-tab path.
Testing
- Not run (automation):
bunis not available in this runner (/bin/bash: bun: command not found).
HAPI Bot
| const isError = status.type === 'incomplete' | ||
| const [lightboxOpen, setLightboxOpen] = useState(false) | ||
|
|
||
| const previewUrl = (attachment as Record<string, unknown>).previewUrl as string | undefined |
There was a problem hiding this comment.
[MAJOR] isImage now depends on previewUrl, but previewUrl is only added after the adapter switches the attachment to requires-action (web/src/lib/attachmentAdapter.ts:93-107). During both running yields there is still no preview URL, so this new thumbnail + spinner state never appears while the upload is in progress.
Suggested fix:
const previewUrl =
isImageMimeType(contentType) && file.size <= MAX_PREVIEW_BYTES
? await fileToDataUrl(file)
: undefined
yield {
id,
type: 'file',
name: file.name,
contentType,
file,
status: { type: 'running', reason: 'uploading', progress: 0 },
previewUrl
}| title="在新标签页打开" | ||
| onClick={(e) => { | ||
| e.stopPropagation() | ||
| window.open(src, '_blank') |
There was a problem hiding this comment.
[MAJOR] src is ultimately user-controlled attachment metadata (shared/src/schemas.ts:147, web/src/chat/normalizeUser.ts:23). Opening it with window.open(src, '_blank') leaves window.opener available for non-data: URLs, which lets the opened page navigate the HAPI tab.
Suggested fix:
const newWindow = window.open(src, '_blank', 'noopener,noreferrer')
if (newWindow) newWindow.opener = null
added 3 commits
April 22, 2026 14:59
The Read tool view used a ternary that swapped file content for images when a result contained image blocks, so mixed text+image payloads (e.g. Read on a notebook with embedded images) lost the textual output. Render the CodeBlock unconditionally and append ResultImages when images are present, mirroring how Bash / Markdown / Generic views already handle the same extraction.
Image attachments in the composer currently display as plain filename text (same chip style as non-image files), even though previewUrl is available. This hides useful context from the user before they send the message. Render images as thumbnails instead: - 64px thumbnail with object-cover, max-width 120px, click to open the shared ImageLightbox - Uploading state shown as a dark overlay with a spinner on top of the thumbnail (previously the chip was hidden entirely until upload finished) - Filename gradient label along the bottom edge - Remove button repositioned as a floating circular button in the top-right, visible on hover Non-image attachments and error state are unchanged.
- attachmentAdapter.ts: generate previewUrl for images before the initial 'running' yield so the composer thumbnail and upload overlay are visible during upload, not only after completion. - ImageLightbox.tsx: pass 'noopener,noreferrer' to window.open for the 'open in new tab' action to prevent reverse tabnabbing via the user-controlled previewUrl.
GeT-LeFt
force-pushed
the
feat/composer-image-thumbnails
branch
from
3cf7d6b to
03abe29
Compare
There was a problem hiding this comment.
Review mode: follow-up after new commits
Findings
- [Major] Hover-only remove action regresses the mobile composer flow —
group-hover:opacity-100on the new image-chip remove button leaves it invisible on touch-first surfaces, so selected images are easy to add but hard to undo before send. Evidence:web/src/components/AssistantChat/AttachmentItem.tsx:61.
Suggested fix:<AttachmentPrimitive.Remove className="absolute right-1 top-1 z-10 flex h-5 w-5 items-center justify-center rounded-full bg-black/60 text-white/80 opacity-100 transition-opacity group-focus-within:opacity-100 md:opacity-0 md:group-hover:opacity-100 hover:bg-black/80 hover:text-white" aria-label="Remove attachment" title="Remove attachment" >
- [Minor] New lightbox controls bypass the existing locale system — the added
titlestrings are hardcoded in Chinese, so English sessions now get untranslated tooltips. Evidence:web/src/components/ImageLightbox.tsx:40,web/src/components/ImageLightbox.tsx:54.
Suggested fix:import { useTranslation } from '@/lib/use-translation' const { t } = useTranslation() <button title={t('imageLightbox.openInNewTab')} aria-label={t('imageLightbox.openInNewTab')} /> <button title={t('button.close')} aria-label={t('button.close')} />
Summary
- Review mode: follow-up after new commits
- 2 issues found in the latest head diff.
- Residual risk: the new touch removal path and localized lightbox controls have no automated coverage in this diff.
Testing
- Not run (automation):
bunis not available in this runner.
HAPI Bot
| </div> | ||
| )} | ||
| <AttachmentPrimitive.Remove | ||
| className="absolute -right-1 -top-1 z-10 flex h-5 w-5 items-center justify-center rounded-full bg-black/60 text-white/80 opacity-0 transition-opacity group-hover:opacity-100 hover:bg-black/80 hover:text-white" |
There was a problem hiding this comment.
[MAJOR] group-hover:opacity-100 makes the new remove affordance hover-only, so on touch devices the image chip has no visible undo action before send. That is a regression for the PWA/phone flow this composer is used in.
Suggested fix:
<AttachmentPrimitive.Remove
className="absolute right-1 top-1 z-10 flex h-5 w-5 items-center justify-center rounded-full bg-black/60 text-white/80 opacity-100 transition-opacity group-focus-within:opacity-100 md:opacity-0 md:group-hover:opacity-100 hover:bg-black/80 hover:text-white"
aria-label="Remove attachment"
title="Remove attachment"
>| <div className="fixed right-4 top-4 z-[101] flex items-center gap-2"> | ||
| <button | ||
| className="rounded-lg bg-white/10 p-2 text-white/80 backdrop-blur-sm transition-colors hover:bg-white/20 hover:text-white" | ||
| title="在新标签页打开" |
There was a problem hiding this comment.
[MINOR] These new tooltip strings bypass the existing locale tables, so English sessions now show Chinese-only UI here.
Suggested fix:
import { useTranslation } from '@/lib/use-translation'
const { t } = useTranslation()
<button title={t('imageLightbox.openInNewTab')} aria-label={t('imageLightbox.openInNewTab')} />
<button title={t('button.close')} aria-label={t('button.close')} />
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
1 participant
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
Image attachments in the composer render as a 64px thumbnail (max-width 120px) instead of a plain filename chip. The schema already has `previewUrl` and sent-message attachments already display thumbnails — this aligns the composer stage with that behavior.
Non-image attachments and error state are unchanged.
Dependencies
This PR depends on #509, which introduces the shared `ImageLightbox` component. The diff shown here will include #509's changes until that one lands — rebasing onto `main` after #509 is merged will leave this PR as a single-file, ~40-line change.
Closes #510
Test plan