diff --git a/web/src/components/AssistantChat/AttachmentItem.test.tsx b/web/src/components/AssistantChat/AttachmentItem.test.tsx new file mode 100644 index 00000000..4bce8df1 --- /dev/null +++ b/web/src/components/AssistantChat/AttachmentItem.test.tsx @@ -0,0 +1,107 @@ +import type { ComponentProps, ReactNode } from 'react' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { I18nProvider } from '@/lib/i18n-context' + +const mocks = vi.hoisted(() => ({ + attachment: { + name: 'photo.png', + status: { type: 'requires-action', reason: 'composer-send' }, + previewUrl: 'data:image/png;base64,cGhvdG8=' + } as Record +})) + +vi.mock('@assistant-ui/react', () => ({ + useThreadComposerAttachment: () => mocks.attachment, + AttachmentPrimitive: { + Root: ({ children, ...props }: ComponentProps<'div'>) =>
{children}
, + Remove: ({ children, ...props }: ComponentProps<'button'> & { children?: ReactNode }) => ( + + ) + } +})) + +import { AttachmentItem } from './AttachmentItem' + +afterEach(() => cleanup()) + +function renderAttachment() { + return render( + + + + ) +} + +describe('AttachmentItem', () => { + it('renders an image preview with its filename and an always-visible remove button', () => { + mocks.attachment = { + name: 'photo.png', + status: { type: 'requires-action', reason: 'composer-send' }, + previewUrl: 'data:image/png;base64,cGhvdG8=' + } + + renderAttachment() + + expect(screen.getByRole('img', { name: 'photo.png' })).toHaveAttribute( + 'src', + 'data:image/png;base64,cGhvdG8=' + ) + expect(screen.getAllByText('photo.png')).toHaveLength(2) + expect(screen.getByRole('button', { name: 'Remove attachment' })).not.toHaveClass('opacity-0') + }) + + it('keeps the upload indicator on top of an image preview while uploading', () => { + mocks.attachment = { + name: 'uploading.png', + status: { type: 'running', reason: 'uploading', progress: 0 }, + previewUrl: 'data:image/png;base64,dXBsb2FkaW5n' + } + + const { container } = renderAttachment() + + expect(screen.getByRole('img', { name: 'uploading.png' })).toBeInTheDocument() + expect(container.querySelector('[class*="bg-black/40"]')).not.toBeNull() + }) + + it('opens the same zoomable image viewer used by sent attachments', () => { + mocks.attachment = { + name: 'zoom-me.png', + status: { type: 'requires-action', reason: 'composer-send' }, + previewUrl: 'data:image/png;base64,em9vbQ==' + } + + renderAttachment() + fireEvent.click(screen.getByTitle('Click to zoom')) + + const dialog = screen.getByRole('dialog', { name: 'zoom-me.png' }) + expect(dialog).toBeInTheDocument() + expect(screen.getAllByRole('img', { name: 'zoom-me.png' })).toHaveLength(2) + }) + + it('keeps non-image attachments in the filename chip layout', () => { + mocks.attachment = { + name: 'notes.txt', + status: { type: 'requires-action', reason: 'composer-send' } + } + + renderAttachment() + + expect(screen.queryByRole('img')).not.toBeInTheDocument() + expect(screen.getByText('notes.txt')).toBeInTheDocument() + }) + + it('keeps upload errors in the existing error layout', () => { + mocks.attachment = { + name: 'broken.png', + status: { type: 'incomplete', reason: 'error' }, + previewUrl: 'data:image/png;base64,YnJva2Vu' + } + + renderAttachment() + + expect(screen.queryByRole('img')).not.toBeInTheDocument() + expect(screen.getByText('Upload failed')).toBeInTheDocument() + expect(screen.getByText('broken.png')).toHaveClass('line-through') + }) +}) diff --git a/web/src/components/AssistantChat/AttachmentItem.tsx b/web/src/components/AssistantChat/AttachmentItem.tsx index 36dc5187..8890f3b7 100644 --- a/web/src/components/AssistantChat/AttachmentItem.tsx +++ b/web/src/components/AssistantChat/AttachmentItem.tsx @@ -1,7 +1,13 @@ import { AttachmentPrimitive, useThreadComposerAttachment } from '@assistant-ui/react' +import type { PendingAttachment } from '@assistant-ui/react' +import { ImagePreview } from '@/components/ImagePreview' import { Spinner } from '@/components/Spinner' import { useComposerParking } from '@/components/AssistantChat/composerParkingContext' +type ComposerAttachmentWithPreview = PendingAttachment & { + previewUrl?: string +} + function ErrorIcon() { return ( @@ -32,11 +38,45 @@ function RemoveIcon() { } export function AttachmentItem() { - const { name, status } = useThreadComposerAttachment() + const { name, status, previewUrl } = useThreadComposerAttachment() as ComposerAttachmentWithPreview const isParking = useComposerParking() const isUploading = status.type === 'running' const isError = status.type === 'incomplete' + if (previewUrl && !isError) { + return ( + + + {name} + + )} + /> + {isUploading ? ( +
+ +
+ ) : null} + {!isParking ? ( + + + + ) : null} + + ) + } + return ( {isUploading ? : null} diff --git a/web/src/components/ImagePreview.test.tsx b/web/src/components/ImagePreview.test.tsx index 14a98123..e4b8ebc8 100644 --- a/web/src/components/ImagePreview.test.tsx +++ b/web/src/components/ImagePreview.test.tsx @@ -40,4 +40,22 @@ describe('ImagePreview gallery navigation', () => { fireEvent.keyDown(window, { key: 'ArrowLeft' }) expect(screen.getByRole('dialog', { name: 'First image' })).toBeInTheDocument() }) + + it('keeps named galleries separate from ungrouped previews', () => { + render( + <> + + + + + ) + + fireEvent.click(screen.getByRole('button', { name: /first draft/i })) + + const dialog = screen.getByRole('dialog', { name: 'First draft' }) + expect(within(dialog).getByText('1 / 2')).toBeInTheDocument() + fireEvent.click(within(dialog).getByRole('button', { name: 'Next image' })) + expect(screen.getByRole('dialog', { name: 'Second draft' })).toBeInTheDocument() + expect(within(screen.getByRole('dialog')).queryByRole('img', { name: 'Sent image' })).not.toBeInTheDocument() + }) }) diff --git a/web/src/components/ImagePreview.tsx b/web/src/components/ImagePreview.tsx index ccf10ccd..b70f5564 100644 --- a/web/src/components/ImagePreview.tsx +++ b/web/src/components/ImagePreview.tsx @@ -36,6 +36,7 @@ export function ImagePreview(props: { buttonClassName?: string imageClassName?: string caption?: ReactNode + galleryId?: string }) { const [viewerOpen, setViewerOpen] = useState(false) const [previewImages, setPreviewImages] = useState([]) @@ -56,7 +57,9 @@ export function ImagePreview(props: { const openViewer = useCallback((event: MouseEvent) => { event.preventDefault() event.stopPropagation() + const galleryId = props.galleryId ?? '' const triggers = Array.from(document.querySelectorAll('[data-image-preview-trigger]')) + .filter((trigger) => (trigger.dataset.imagePreviewGallery ?? '') === galleryId) const images = triggers.flatMap((trigger): PreviewImage[] => { const image = trigger.querySelector('img') if (!image) return [] @@ -70,7 +73,7 @@ export function ImagePreview(props: { setPreviewImages(images) setPreviewIndex(index >= 0 ? index : 0) setViewerOpen(true) - }, []) + }, [props.galleryId]) const updateScale = useCallback((next: number | ((current: number) => number)) => { setScale((current) => { @@ -262,6 +265,7 @@ export function ImagePreview(props: { data-image-preview-trigger="" data-image-preview-file-name={props.fileName} data-image-preview-label={props.label} + data-image-preview-gallery={props.galleryId ?? ''} className={props.buttonClassName ?? 'group flex min-h-[18rem] w-full items-center justify-center overflow-auto rounded-md border border-[var(--app-border)] bg-[var(--app-code-bg)] p-3 text-left'} title="Click to zoom" > diff --git a/web/src/lib/attachmentAdapter.test.ts b/web/src/lib/attachmentAdapter.test.ts index 3d76b4e6..5ca09f9e 100644 --- a/web/src/lib/attachmentAdapter.test.ts +++ b/web/src/lib/attachmentAdapter.test.ts @@ -1,65 +1,19 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -function stubUploadThenPreviewReadFailure(): void { - let readCount = 0 +async function collectAdditions( + file: File, + uploadFile = vi.fn(async () => ({ success: true, path: '/uploads/file' })) +) { + const { createAttachmentAdapter } = await import('./attachmentAdapter') + const adapter = createAttachmentAdapter({ uploadFile } as never, 'session-1') + const additions = adapter.add({ file }) as AsyncIterable> + const emitted: Record[] = [] - class FileReaderMock { - result: string | ArrayBuffer | null = null - onload: FileReader['onload'] = null - onerror: FileReader['onerror'] = null - - readAsDataURL(): void { - readCount += 1 - if (readCount === 1) { - this.result = 'data:image/png;base64,dXBsb2Fk' - this.onload?.call(this as unknown as FileReader, {} as ProgressEvent) - return - } - this.onerror?.call(this as unknown as FileReader, {} as ProgressEvent) - } + for await (const attachment of additions) { + emitted.push(attachment) } - vi.stubGlobal('FileReader', FileReaderMock) -} - -function stubUploadThenDeferredPreviewReadFailure(): { - previewStarted: Promise - failPreview: () => void -} { - let readCount = 0 - let resolvePreviewStarted!: () => void - let failPreviewRead: (() => void) | undefined - const previewStarted = new Promise((resolve) => { - resolvePreviewStarted = resolve - }) - - class FileReaderMock { - result: string | ArrayBuffer | null = null - onload: FileReader['onload'] = null - onerror: FileReader['onerror'] = null - - readAsDataURL(): void { - readCount += 1 - if (readCount === 1) { - this.result = 'data:image/png;base64,dXBsb2Fk' - this.onload?.call(this as unknown as FileReader, {} as ProgressEvent) - return - } - failPreviewRead = () => { - this.onerror?.call(this as unknown as FileReader, {} as ProgressEvent) - } - resolvePreviewStarted() - } - } - - vi.stubGlobal('FileReader', FileReaderMock) - return { - previewStarted, - failPreview: () => { - if (!failPreviewRead) throw new Error('Preview read did not start') - failPreviewRead() - } - } + return { emitted, uploadFile } } describe('attachmentAdapter', () => { @@ -109,77 +63,65 @@ describe('attachmentAdapter', () => { })]) }) - it('keeps a successful upload ready when image preview generation fails', async () => { - stubUploadThenPreviewReadFailure() - const { createAttachmentAdapter } = await import('./attachmentAdapter') - const uploadFile = vi.fn().mockResolvedValue({ success: true, path: '/uploads/proof.png' }) - const deleteUploadFile = vi.fn().mockResolvedValue({ success: true }) - const adapter = createAttachmentAdapter({ uploadFile, deleteUploadFile } as never, 'session-1') + it('uploads an image when the initial preview read fails', async () => { + let readCount = 0 + class FileReaderMock { + result: string | ArrayBuffer | null = null + onload: FileReader['onload'] = null + onerror: FileReader['onerror'] = null + + readAsDataURL(): void { + readCount += 1 + if (readCount === 1) { + this.onerror?.call(this as unknown as FileReader, {} as ProgressEvent) + return + } + this.result = 'data:image/png;base64,dXBsb2Fk' + this.onload?.call(this as unknown as FileReader, {} as ProgressEvent) + } + } + vi.stubGlobal('FileReader', FileReaderMock) + const file = new File(['proof'], 'proof.png', { type: 'image/png' }) - const states: import('@assistant-ui/react').PendingAttachment[] = [] + const { emitted, uploadFile } = await collectAdditions(file) - for await (const state of adapter.add({ file }) as AsyncGenerator) { - states.push(state) - } - - const ready = states.at(-1) as import('@assistant-ui/react').PendingAttachment & { - path?: string - previewUrl?: string - } - expect(uploadFile).toHaveBeenCalledTimes(1) + expect(readCount).toBe(2) expect(uploadFile).toHaveBeenCalledWith('session-1', 'proof.png', 'dXBsb2Fk', 'image/png') - expect(ready).toMatchObject({ - type: 'file', - name: 'proof.png', + expect(emitted.at(-1)).toMatchObject({ status: { type: 'requires-action', reason: 'composer-send' }, - path: '/uploads/proof.png', + path: '/uploads/file' }) - expect(ready.id).toEqual(expect.any(String)) - expect(ready.previewUrl).toBeUndefined() - - const sent = await adapter.send(ready) - expect(JSON.parse((sent.content[0] as { text: string }).text)).toEqual({ - __attachmentMetadata: { - id: ready.id, - filename: 'proof.png', - mimeType: 'image/png', - size: file.size, - path: '/uploads/proof.png', - }, - }) - - await adapter.remove(ready) - expect(deleteUploadFile).toHaveBeenCalledWith('session-1', '/uploads/proof.png') + expect(emitted.every((attachment) => attachment.previewUrl === undefined)).toBe(true) }) +}) - it('cleans up a successful upload when cancellation occurs during preview generation', async () => { - const preview = stubUploadThenDeferredPreviewReadFailure() - const { createAttachmentAdapter } = await import('./attachmentAdapter') - const uploadFile = vi.fn().mockResolvedValue({ success: true, path: '/uploads/proof.png' }) - const deleteUploadFile = vi.fn().mockResolvedValue({ success: true }) - const adapter = createAttachmentAdapter({ uploadFile, deleteUploadFile } as never, 'session-1') - const file = new File(['proof'], 'proof.png', { type: 'image/png' }) - const iter = adapter.add({ file }) as AsyncGenerator +describe('attachmentAdapter image previews', () => { + it('includes the preview URL in every image upload state', async () => { + const file = new File(['image'], 'photo.png', { type: 'image/png' }) + const readSpy = vi.spyOn(FileReader.prototype, 'readAsDataURL') + const { emitted } = await collectAdditions(file) - const initial = await iter.next() - const uploading = await iter.next() - expect(uploading.value).toMatchObject({ - status: { type: 'running', reason: 'uploading', progress: 50 }, + expect(emitted).toHaveLength(3) + expect(emitted[0]).toMatchObject({ + previewUrl: 'data:image/png;base64,aW1hZ2U=', + status: { type: 'running', progress: 0 } }) - expect((uploading.value as { path?: string }).path).toBeUndefined() + expect(emitted[1]).toMatchObject({ + previewUrl: 'data:image/png;base64,aW1hZ2U=', + status: { type: 'running', progress: 50 } + }) + expect(emitted[2]).toMatchObject({ + previewUrl: 'data:image/png;base64,aW1hZ2U=', + status: { type: 'requires-action' } + }) + expect(readSpy).toHaveBeenCalledTimes(1) + }) - const completion = iter.next() - await preview.previewStarted - expect(uploadFile).toHaveBeenCalledTimes(1) - await adapter.remove(uploading.value) - preview.failPreview() + it('does not generate previews for non-image attachments', async () => { + const file = new File(['notes'], 'notes.txt', { type: 'text/plain' }) + const { emitted } = await collectAdditions(file) - expect(await completion).toEqual({ done: true, value: undefined }) - expect([initial.value, uploading.value]).toEqual([ - expect.objectContaining({ status: { type: 'running', reason: 'uploading', progress: 0 } }), - expect.objectContaining({ status: { type: 'running', reason: 'uploading', progress: 50 } }), - ]) - expect(deleteUploadFile).toHaveBeenCalledTimes(1) - expect(deleteUploadFile).toHaveBeenCalledWith('session-1', '/uploads/proof.png') + expect(emitted).toHaveLength(3) + expect(emitted.every((attachment) => attachment.previewUrl === undefined)).toBe(true) }) }) diff --git a/web/src/lib/attachmentAdapter.ts b/web/src/lib/attachmentAdapter.ts index 9527db26..2c47b9a9 100644 --- a/web/src/lib/attachmentAdapter.ts +++ b/web/src/lib/attachmentAdapter.ts @@ -50,16 +50,26 @@ export function createAttachmentAdapter(api: ApiClient, sessionId: string): Atta const id = randomId() const contentType = file.type || 'application/octet-stream' - yield { - id, - type: 'file', - name: file.name, - contentType, - file, - status: { type: 'running', reason: 'uploading', progress: 0 } - } - try { + let previewUrl: string | undefined + if (isImageMimeType(contentType) && file.size <= MAX_PREVIEW_BYTES) { + try { + previewUrl = await fileToDataUrl(file) + } catch { + // Preview generation is optional; retry the read for the upload payload below. + } + } + + yield { + id, + type: 'file', + name: file.name, + contentType, + file, + status: { type: 'running', reason: 'uploading', progress: 0 }, + previewUrl + } as PendingUploadAttachment + if (cancelledAttachmentIds.has(id)) { return } @@ -76,7 +86,9 @@ export function createAttachmentAdapter(api: ApiClient, sessionId: string): Atta return } - const content = await fileToBase64(file) + const content = previewUrl + ? base64FromDataUrl(previewUrl) + : await fileToBase64(file) if (cancelledAttachmentIds.has(id)) { return } @@ -87,8 +99,9 @@ export function createAttachmentAdapter(api: ApiClient, sessionId: string): Atta name: file.name, contentType, file, - status: { type: 'running', reason: 'uploading', progress: 50 } - } + status: { type: 'running', reason: 'uploading', progress: 50 }, + previewUrl + } as PendingUploadAttachment const result = await api.uploadFile(sessionId, file.name, content, contentType) if (cancelledAttachmentIds.has(id)) { @@ -110,21 +123,6 @@ export function createAttachmentAdapter(api: ApiClient, sessionId: string): Atta return } - // Generate preview URL for images under 5MB - let previewUrl: string | undefined - if (isImageMimeType(contentType) && file.size <= MAX_PREVIEW_BYTES) { - try { - previewUrl = await fileToDataUrl(file) - } catch { - // Preview generation is optional after the upload has succeeded. - } - } - - if (cancelledAttachmentIds.has(id)) { - await deleteUpload(result.path) - return - } - yield { id, type: 'file', @@ -181,20 +179,16 @@ export function createAttachmentAdapter(api: ApiClient, sessionId: string): Atta } async function fileToBase64(file: File): Promise { - return new Promise((resolve, reject) => { - const reader = new FileReader() - reader.onload = () => { - const result = reader.result as string - const base64 = result.split(',')[1] - if (!base64) { - reject(new Error('Failed to read file')) - return - } - resolve(base64) - } - reader.onerror = reject - reader.readAsDataURL(file) - }) + return base64FromDataUrl(await fileToDataUrl(file)) +} + +function base64FromDataUrl(dataUrl: string): string { + const separatorIndex = dataUrl.indexOf(',') + const base64 = separatorIndex >= 0 ? dataUrl.slice(separatorIndex + 1) : '' + if (!base64) { + throw new Error('Failed to read file') + } + return base64 } async function fileToDataUrl(file: File): Promise {