Skip to content

Commit 8694f55

Browse files
authored
fix(chat): render HEIC attachments and restyle composer file chips (#6361)
* fix(chat): render HEIC attachments and restyle composer file chips The composer previewed every attachment through URL.createObjectURL of the raw bytes. No browser decodes HEVC-coded HEIF, so a HEIC showed a broken glyph, and the upload-completion handler never replaced that blob URL — so it stayed broken even once a derivative was available. - Skip the blob for HEIC/HEIF and pick up the serve URL (preview=1) once the upload lands, so the server derivative renders. - Fall back to the type icon if the image still fails to decode. - Documents render as labelled cards (icon, name, type) instead of a 9px extension caption; media keeps a thumbnail. - Fix a blob-URL leak: the unmount cleanup closed over the first render's empty array and revoked nothing. * fix(chat): make composer chips read against the composer shell The composer is --white in light and --surface-4 in dark. The chip reused chipFilledFillTokens (--surface-5 / dark:--surface-4), which assumes a page background, so in dark mode the chip fill matched its own container exactly and only the border showed. Same for the remove badge, which sits on the shell and was 5/255 from it. - Chip fills --surface-5 in both themes and hover steps away from the shell in each theme's 'raised' direction. - Remove badge uses --surface-6, readable on white and on --surface-4. - Cap the document card at min(220px,100%) so a long filename truncates on a narrow viewport instead of overflowing the composer. * chore(chat): use the absolute alias for the chip test import
1 parent e1f2bf8 commit 8694f55

3 files changed

Lines changed: 303 additions & 114 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { createRoot, type Root } from 'react-dom/client'
6+
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
7+
import { AttachedFilesList } from '@/app/workspace/[workspaceId]/home/components/user-input/components/attached-files-list/attached-files-list'
8+
import type { AttachedFile } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/user-input/hooks/use-file-attachments'
9+
10+
function file(overrides: Partial<AttachedFile>): AttachedFile {
11+
return {
12+
id: 'f1',
13+
name: 'report.pdf',
14+
size: 1024,
15+
type: 'application/pdf',
16+
path: '',
17+
uploading: false,
18+
...overrides,
19+
}
20+
}
21+
22+
let container: HTMLDivElement
23+
let root: Root
24+
25+
beforeEach(() => {
26+
globalThis.IS_REACT_ACT_ENVIRONMENT = true
27+
container = document.createElement('div')
28+
document.body.appendChild(container)
29+
root = createRoot(container)
30+
})
31+
32+
afterEach(() => {
33+
act(() => root.unmount())
34+
container.remove()
35+
})
36+
37+
function render(files: AttachedFile[]) {
38+
act(() => {
39+
root.render(
40+
<AttachedFilesList attachedFiles={files} onFileClick={() => {}} onRemoveFile={() => {}} />
41+
)
42+
})
43+
}
44+
45+
describe('AttachedFilesList', () => {
46+
it('renders a document as a labelled card showing the filename', () => {
47+
render([file({})])
48+
49+
expect(container.textContent).toContain('report.pdf')
50+
expect(container.querySelector('img')).toBeNull()
51+
})
52+
53+
it('renders an image with a preview as a thumbnail, not a filename card', () => {
54+
render([file({ name: 'photo.png', type: 'image/png', previewUrl: 'blob:xyz' })])
55+
56+
expect(container.querySelector('img')?.getAttribute('src')).toBe('blob:xyz')
57+
expect(container.textContent).not.toContain('photo.png')
58+
})
59+
60+
it('keeps a HEIC on the thumbnail shape while it has no preview yet', () => {
61+
// The shape is keyed off the type, not the preview: a HEIC gets its preview only
62+
// once the server derivative exists, and switching shape mid-upload would jump the
63+
// layout. It must not fall back to the document card.
64+
render([file({ name: 'photo.heic', type: 'image/heic' })])
65+
66+
expect(container.textContent).not.toContain('photo.heic')
67+
expect(container.querySelector('img')).toBeNull()
68+
})
69+
70+
it('drops the image and reveals the type icon when the preview fails to decode', () => {
71+
render([file({ name: 'photo.heic', type: 'image/heic', previewUrl: '/api/files/serve/x' })])
72+
73+
const img = container.querySelector('img')
74+
expect(img).not.toBeNull()
75+
76+
act(() => {
77+
img?.dispatchEvent(new Event('error'))
78+
})
79+
80+
expect(container.querySelector('img')).toBeNull()
81+
expect(container.querySelector('svg')).not.toBeNull()
82+
})
83+
})
Lines changed: 141 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,97 +1,165 @@
11
'use client'
22

3-
import React from 'react'
4-
import { Loader, Tooltip } from '@sim/emcn'
3+
import React, { useState } from 'react'
4+
import { cn, Loader, Tooltip } from '@sim/emcn'
55
import { X } from '@sim/emcn/icons'
66
import { getDocumentIcon } from '@/components/icons/document-icons'
7+
import { getFileExtension } from '@/lib/uploads/utils/file-utils'
78
import type { AttachedFile } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/user-input/hooks/use-file-attachments'
89

10+
/**
11+
* Chrome shared by both chip shapes. Both stand 48px tall so a row mixing thumbnails
12+
* and documents sits on one baseline.
13+
*
14+
* Deliberately NOT `chipFilledFillTokens` (`--surface-5` / `dark:--surface-4`): that
15+
* pair assumes a page background, but this chip sits inside the composer, which is
16+
* already `--surface-4` in dark mode — reusing it would make the chip invisible against
17+
* its own container. `--surface-5` steps away from the composer in both themes, and
18+
* hover steps further away in the direction each theme reads as "raised".
19+
*/
20+
const CHIP_SURFACE =
21+
'relative h-[48px] cursor-pointer rounded-[10px] border border-[var(--border)] bg-[var(--surface-5)] transition-colors hover-hover:bg-[var(--surface-active)] dark:hover-hover:bg-[var(--surface-6)]'
22+
923
interface AttachedFilesListProps {
1024
attachedFiles: AttachedFile[]
1125
onFileClick: (file: AttachedFile) => void
1226
onRemoveFile: (id: string) => void
1327
}
1428

15-
export const AttachedFilesList = React.memo(function AttachedFilesList({
16-
attachedFiles,
29+
interface AttachedFileChipProps {
30+
file: AttachedFile
31+
onFileClick: (file: AttachedFile) => void
32+
onRemoveFile: (id: string) => void
33+
}
34+
35+
/**
36+
* One attachment.
37+
*
38+
* Media renders as a thumbnail; everything else renders as a labelled card — icon
39+
* badge, filename, file type. A document has no thumbnail worth showing, and the
40+
* filename is the thing worth reading.
41+
*/
42+
const AttachedFileChip = React.memo(function AttachedFileChip({
43+
file,
1744
onFileClick,
1845
onRemoveFile,
19-
}: AttachedFilesListProps) {
20-
if (attachedFiles.length === 0) return null
46+
}: AttachedFileChipProps) {
47+
const Icon = getDocumentIcon(file.type, file.name)
48+
const isVideo = file.type.startsWith('video/')
49+
// Keyed off the type, not the presence of a preview: a HEIC has no preview until its
50+
// upload finishes, and flipping shape mid-upload would jump the layout.
51+
const isMedia = isVideo || file.type.startsWith('image/')
52+
const extension = getFileExtension(file.name)
53+
const [previewFailed, setPreviewFailed] = useState(false)
2154

2255
return (
23-
<div className='mb-1.5 flex flex-wrap gap-1.5'>
24-
{attachedFiles.map((file) => {
25-
const isVideo = file.type.startsWith('video/')
26-
const hasPreview = Boolean(file.previewUrl)
27-
return (
28-
<Tooltip.Root key={file.id}>
29-
<div className='group relative size-[56px] flex-shrink-0'>
30-
<Tooltip.Trigger asChild>
31-
<button
32-
type='button'
33-
className='relative h-full w-full cursor-pointer overflow-hidden rounded-[8px] border border-[var(--border-1)] bg-[var(--surface-5)] p-0 hover:bg-[var(--surface-4)]'
34-
onClick={() => onFileClick(file)}
35-
>
36-
{hasPreview && isVideo ? (
37-
<>
38-
<div className='absolute inset-0 flex items-center justify-center text-[var(--text-icon)]'>
39-
{(() => {
40-
const Icon = getDocumentIcon(file.type, file.name)
41-
return <Icon className='size-[18px]' />
42-
})()}
43-
</div>
44-
<video
45-
src={file.previewUrl}
46-
muted
47-
playsInline
48-
preload='metadata'
49-
className='relative h-full w-full object-cover'
50-
/>
51-
</>
52-
) : hasPreview ? (
56+
<Tooltip.Root>
57+
<div className={cn('group relative', isMedia ? 'flex-shrink-0' : 'min-w-0')}>
58+
<Tooltip.Trigger asChild>
59+
<button
60+
type='button'
61+
className={cn(
62+
CHIP_SURFACE,
63+
isMedia
64+
? 'w-[48px] overflow-hidden'
65+
: // Capped at 220px but never wider than the composer, so a long filename
66+
// truncates on a narrow viewport instead of overflowing the shell.
67+
'flex max-w-[min(220px,100%)] items-center gap-2 py-2 pr-3 pl-2'
68+
)}
69+
onClick={() => onFileClick(file)}
70+
>
71+
{isMedia ? (
72+
<>
73+
<span className='absolute inset-0 flex items-center justify-center text-[var(--text-icon)]'>
74+
<Icon className='size-[18px]' />
75+
</span>
76+
{file.previewUrl &&
77+
!previewFailed &&
78+
(isVideo ? (
79+
<video
80+
src={file.previewUrl}
81+
muted
82+
playsInline
83+
preload='metadata'
84+
className='relative size-full object-cover'
85+
/>
86+
) : (
5387
<img
5488
src={file.previewUrl}
5589
alt={file.name}
56-
className='h-full w-full object-cover'
90+
// A HEIC whose server-side transcode failed comes back as bytes the
91+
// browser still cannot decode. Dropping the image reveals the type
92+
// icon beneath instead of a broken glyph.
93+
onError={() => setPreviewFailed(true)}
94+
className='relative size-full object-cover'
5795
/>
58-
) : (
59-
<div className='flex h-full w-full flex-col items-center justify-center gap-0.5 text-[var(--text-icon)]'>
60-
{(() => {
61-
const Icon = getDocumentIcon(file.type, file.name)
62-
return <Icon className='size-[18px]' />
63-
})()}
64-
<span className='max-w-[48px] truncate px-[2px] text-[9px] text-[var(--text-muted)]'>
65-
{file.name.split('.').pop()}
66-
</span>
67-
</div>
96+
))}
97+
</>
98+
) : (
99+
<>
100+
<span className='flex size-[32px] shrink-0 items-center justify-center rounded-[8px] bg-[var(--surface-6)] text-[var(--text-icon)] dark:bg-[var(--surface-3)]'>
101+
<Icon className='size-[16px]' />
102+
</span>
103+
<span className='flex min-w-0 flex-col items-start'>
104+
<span className='w-full truncate text-[var(--text-body)] text-small'>
105+
{file.name}
106+
</span>
107+
{/* The name truncates, so the extension is genuinely not readable from
108+
it — this is the format, not a restatement of the label. */}
109+
{extension && (
110+
<span className='text-[var(--text-muted)] text-xs uppercase'>{extension}</span>
68111
)}
69-
{file.uploading && (
70-
<div className='absolute inset-0 flex items-center justify-center bg-black/50'>
71-
<Loader className='size-[14px] text-white' animate />
72-
</div>
73-
)}
74-
</button>
75-
</Tooltip.Trigger>
76-
{!file.uploading && (
77-
<button
78-
type='button'
79-
onClick={(e) => {
80-
e.stopPropagation()
81-
onRemoveFile(file.id)
82-
}}
83-
className='absolute top-[2px] right-[2px] flex size-[16px] items-center justify-center rounded-full bg-black/60 opacity-0 group-hover:opacity-100'
84-
>
85-
<X className='size-[10px] text-white' />
86-
</button>
87-
)}
88-
</div>
89-
<Tooltip.Content side='top'>
90-
<p className='max-w-[200px] truncate'>{file.name}</p>
91-
</Tooltip.Content>
92-
</Tooltip.Root>
93-
)
94-
})}
112+
</span>
113+
</>
114+
)}
115+
{file.uploading && (
116+
<span className='absolute inset-0 flex items-center justify-center rounded-[inherit] bg-[var(--surface-5)]/70 dark:bg-[var(--surface-4)]/70'>
117+
<Loader className='size-[14px] text-[var(--text-icon)]' animate />
118+
</span>
119+
)}
120+
</button>
121+
</Tooltip.Trigger>
122+
{!file.uploading && (
123+
<button
124+
type='button'
125+
onClick={(e) => {
126+
e.stopPropagation()
127+
onRemoveFile(file.id)
128+
}}
129+
aria-label={`Remove ${file.name}`}
130+
// Overhangs the chip by 5px, which the composer's `py-2` absorbs. `--surface-6`
131+
// (not the chip's own fill) because this badge sits on the composer shell —
132+
// white in light mode, `--surface-4` in dark — and must read against both.
133+
className='-top-[5px] -right-[5px] absolute flex size-[16px] items-center justify-center rounded-full border border-[var(--border)] bg-[var(--surface-6)] text-[var(--text-icon)] opacity-0 transition-opacity group-hover:opacity-100'
134+
>
135+
<X className='size-[9px]' />
136+
</button>
137+
)}
138+
</div>
139+
<Tooltip.Content side='top'>
140+
<p className='max-w-[200px] truncate'>{file.name}</p>
141+
</Tooltip.Content>
142+
</Tooltip.Root>
143+
)
144+
})
145+
146+
export const AttachedFilesList = React.memo(function AttachedFilesList({
147+
attachedFiles,
148+
onFileClick,
149+
onRemoveFile,
150+
}: AttachedFilesListProps) {
151+
if (attachedFiles.length === 0) return null
152+
153+
return (
154+
<div className='mb-1.5 flex flex-wrap items-center gap-1.5'>
155+
{attachedFiles.map((file) => (
156+
<AttachedFileChip
157+
key={file.id}
158+
file={file}
159+
onFileClick={onFileClick}
160+
onRemoveFile={onRemoveFile}
161+
/>
162+
))}
95163
</div>
96164
)
97165
})

0 commit comments

Comments
 (0)