From d8cb73b4ab2b3c766185fd654100175cffd7bfb3 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 22:00:31 -0500 Subject: [PATCH 1/2] fix(desktop): open a composer image attachment in the lightbox, not the rail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking an attached image routed it through normalizeOrLocalPreviewTarget and into the right rail, where it rendered as a small contained inside a pane built for reading and editing files. The bytes were already in hand as a data URL, so the rail's whole read/edit/reload apparatus was doing nothing for a picture the user just wanted to look at. Route images with a resolved thumbnail to ImageLightbox — the same overlay the thread uses — so the attachment previews at full size with the download action, and drop the dataUrl/previewKind graft that only existed to make the rail render bytes it shouldn't have been handed. Non-image attachments, and images whose thumbnail never resolved, still fall through to the rail unchanged. --- .../src/app/chat/composer/attachments.tsx | 163 ++++++++++-------- 1 file changed, 92 insertions(+), 71 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/attachments.tsx b/apps/desktop/src/app/chat/composer/attachments.tsx index f50f74be668..cb42dc9f1c9 100644 --- a/apps/desktop/src/app/chat/composer/attachments.tsx +++ b/apps/desktop/src/app/chat/composer/attachments.tsx @@ -1,8 +1,11 @@ import { useStore } from '@nanostores/react' +import { useState } from 'react' import { useSessionView } from '@/app/chat/session-view' +import { ImageLightbox } from '@/components/chat/zoomable-image' import { Codicon } from '@/components/ui/codicon' import { Tip } from '@/components/ui/tooltip' +import { useImageDownload } from '@/hooks/use-image-download' import { useI18n } from '@/i18n' import { AlertCircle, FileText, FolderOpen, ImageIcon, Link, Loader2, Terminal } from '@/lib/icons' import { normalizeOrLocalPreviewTarget } from '@/lib/local-preview' @@ -38,12 +41,25 @@ function AttachmentPill({ attachment, onRemove }: { attachment: ComposerAttachme const hasUploadError = attachment.uploadState === 'error' const canPreview = attachment.kind !== 'folder' && attachment.kind !== 'terminal' && !isUploading const detail = attachment.detail && attachment.detail !== attachment.label ? attachment.detail : undefined + // An attached image already holds its full bytes as a data URL, so it belongs + // in the same lightbox the thread uses. The rail is for files you read or + // edit — not a picture you just want to look at. Images that never resolved a + // thumbnail still fall through to the rail rather than dead-clicking. + const lightboxSrc = attachment.kind === 'image' && !isUploading ? attachment.previewUrl : undefined + const [lightboxOpen, setLightboxOpen] = useState(false) + const { download, saving } = useImageDownload(lightboxSrc) - async function openAttachmentPreview() { + async function openAttachment() { if (!canPreview) { return } + if (lightboxSrc) { + setLightboxOpen(true) + + return + } + const rawTarget = attachment.path || attachment.detail || @@ -64,85 +80,90 @@ function AttachmentPill({ attachment, onRemove }: { attachment: ComposerAttachme throw new Error(c.couldNotPreview(attachment.label)) } - // We already hold the image bytes (the card thumbnail) — render those - // directly so a screenshot/clipboard image previews even when its only - // on-disk copy is a transient path the renderer can't re-read. - const withBytes = - attachment.kind === 'image' && attachment.previewUrl - ? { ...preview, dataUrl: attachment.previewUrl, previewKind: 'image' as const } - : preview - - openPreview(withBytes, 'manual') + openPreview(preview, 'manual') } catch (error) { notifyError(error, c.previewUnavailable) } } return ( - -
- - {onRemove && ( + <> + +
- )} -
- + {onRemove && ( + + )} +
+ + {lightboxSrc && ( + + )} + ) } From 71e66f3a11b4182f7dc98e330f0ed8904ea41e1f Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 22:00:31 -0500 Subject: [PATCH 2/2] test(desktop): cover the composer attachment click routing An image attachment opens the lightbox and leaves the preview rail empty; a file attachment still opens a rail tab. The lightbox case fails on main. --- .../app/chat/composer/attachments.test.tsx | 44 ++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/apps/desktop/src/app/chat/composer/attachments.test.tsx b/apps/desktop/src/app/chat/composer/attachments.test.tsx index 52e8f6bf98e..b405b011d62 100644 --- a/apps/desktop/src/app/chat/composer/attachments.test.tsx +++ b/apps/desktop/src/app/chat/composer/attachments.test.tsx @@ -1,11 +1,14 @@ -import { act, cleanup, render, screen } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' import { afterEach, describe, expect, it } from 'vitest' import { I18nProvider } from '@/i18n/context' import type { ComposerAttachment } from '@/store/composer' +import { $previewTabs } from '@/store/preview' import { AttachmentList } from './attachments' +const DATA_URL = 'data:image/png;base64,iVBORw0KGgoAAAANS' + function makeAttachment(id: string, label = 'test.pdf'): ComposerAttachment { return { id, kind: 'file', label } } @@ -66,4 +69,43 @@ describe('AttachmentList', () => { expect(screen.getByText('valid.txt')).toBeDefined() }) + + it('opens an attached image in the lightbox, not the preview rail', async () => { + $previewTabs.set([]) + + const image: ComposerAttachment = { + id: 'img', + kind: 'image', + label: 'shot.png', + path: '/tmp/shot.png', + previewUrl: DATA_URL + } + + await renderWithI18n() + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /shot\.png/ })) + }) + + // The lightbox renders the full-size image in a dialog; the rail stays empty. + const lightbox = await screen.findByRole('dialog') + + expect(lightbox.querySelector('img')?.src).toBe(DATA_URL) + expect($previewTabs.get()).toHaveLength(0) + }) + + it('still routes a non-image attachment to the preview rail', async () => { + $previewTabs.set([]) + + const file: ComposerAttachment = { id: 'doc', kind: 'file', label: 'notes.md', path: '/tmp/notes.md' } + + await renderWithI18n() + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /notes\.md/ })) + }) + + expect(screen.queryByRole('dialog')).toBeNull() + expect($previewTabs.get().map(tab => tab.target.path)).toEqual(['/tmp/notes.md']) + }) })