Skip to content

Commit 43c3996

Browse files
committed
improvement(chat): tighten composer chip markup and tokens
- Remove the chip tooltips: the document card already shows its filename, so the tooltip mostly restated it. - Collapse the single-use height constant and use size-[48px] on the media branch, which was h-[48px] + w-[48px] split across two class strings. - Remove badge moves to --surface-2; --surface-1 sat 8/255 from the chip fill in light mode, reachable on a coarse pointer where the chip's hover-hover fill never applies. Its hover gating now matches the chip's. - py-[7px] so the 32px icon badge fits the 48px box instead of overflowing it. - Trim comments to TSDoc or one-line rationale per the repo rule.
1 parent 14f6225 commit 43c3996

4 files changed

Lines changed: 91 additions & 137 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/components/chat-message-attachments/chat-message-attachments.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ function FileAttachmentPill(props: { mediaType: string; filename: string }) {
66
const Icon = getDocumentIcon(props.mediaType, props.filename)
77
return (
88
<div className='flex max-w-[140px] items-center gap-[5px] rounded-lg bg-[var(--surface-5)] px-[6px] py-[3px]'>
9-
<Icon className='size-[14px] flex-shrink-0 text-[var(--text-icon)]' />
9+
<Icon className='size-[14px] shrink-0 text-[var(--text-icon)]' />
1010
<span className='truncate text-[var(--text-body)] text-xs'>{props.filename}</span>
1111
</div>
1212
)

apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/attached-files-list/attached-files-list.test.tsx

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,19 @@ describe('AttachedFilesList', () => {
5858
})
5959

6060
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.
6461
render([file({ name: 'photo.heic', type: 'image/heic' })])
6562

6663
expect(container.textContent).not.toContain('photo.heic')
6764
expect(container.querySelector('img')).toBeNull()
6865
})
6966

7067
it('caps the card wrapper so a long filename cannot strand the remove badge', () => {
71-
// The badge is positioned against this wrapper. Without a cap here the wrapper
72-
// stretches to the filename's max-content width while the card stays 220px, and
73-
// the badge drifts off to the right of the card.
7468
render([file({ name: '9bacf973-cd64-437b-be12-58be9f2c1a4d-very-long-name.pdf' })])
7569

70+
// jsdom does no layout, so the cap can only be asserted structurally: it has to sit
71+
// on the wrapper the badge is positioned against, not on the button.
7672
const wrapper = container.querySelector('button')?.parentElement
77-
expect(wrapper?.className).toContain('max-w-[min(220px,100%)]')
73+
expect(wrapper?.className).toMatch(/max-w-/)
7874
})
7975

8076
it('drops the image and reveals the type icon when the preview fails to decode', () => {

apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/attached-files-list/attached-files-list.tsx

Lines changed: 79 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,19 @@
11
'use client'
22

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

1010
/**
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".
11+
* Chrome shared by both chip shapes. Not `chipFilledFillTokens` — its dark fill is
12+
* `--surface-4`, which is the composer's own background.
1913
*/
2014
const CHIP_SURFACE =
2115
'relative cursor-pointer rounded-lg border border-[var(--border)] bg-[var(--surface-5)] transition-colors hover-hover:bg-[var(--surface-active)] dark:hover-hover:bg-[var(--surface-6)]'
2216

23-
/** Height lives on the wrapper so both shapes are the same size by construction. */
24-
const CHIP_HEIGHT = 'h-[48px]'
25-
2617
interface AttachedFilesListProps {
2718
attachedFiles: AttachedFile[]
2819
onFileClick: (file: AttachedFile) => void
@@ -36,11 +27,9 @@ interface AttachedFileChipProps {
3627
}
3728

3829
/**
39-
* One attachment.
40-
*
41-
* Media renders as a thumbnail; everything else renders as a labelled card — icon
42-
* badge, filename, file type. A document has no thumbnail worth showing, and the
43-
* filename is the thing worth reading.
30+
* One attachment: media renders as a thumbnail, anything else as a labelled card.
31+
* Shape keys off the media type, not preview presence — a HEIC has no preview until its
32+
* server derivative lands, and flipping shape mid-upload would jump the layout.
4433
*/
4534
const AttachedFileChip = React.memo(function AttachedFileChip({
4635
file,
@@ -49,118 +38,89 @@ const AttachedFileChip = React.memo(function AttachedFileChip({
4938
}: AttachedFileChipProps) {
5039
const Icon = getDocumentIcon(file.type, file.name)
5140
const isVideo = file.type.startsWith('video/')
52-
// Keyed off the type, not the presence of a preview: a HEIC has no preview until its
53-
// upload finishes, and flipping shape mid-upload would jump the layout.
5441
const isMedia = isVideo || file.type.startsWith('image/')
5542
const extension = getFileExtension(file.name)
5643
const [previewFailed, setPreviewFailed] = useState(false)
5744

5845
return (
59-
<Tooltip.Root>
60-
{/* Both the size and the width cap live here, not on the button: this wrapper
61-
anchors the remove badge, so sizing it to the button's uncapped max-content
62-
width would strand the badge far to the right of a long filename. */}
63-
<div
46+
/* Owns the width cap: it anchors the remove badge, which a max-content button would strand. */
47+
<div
48+
className={cn(
49+
'group relative',
50+
isMedia ? 'size-[48px] shrink-0' : 'h-[48px] min-w-0 max-w-[min(220px,100%)]'
51+
)}
52+
>
53+
<button
54+
type='button'
6455
className={cn(
65-
'group relative',
66-
CHIP_HEIGHT,
67-
isMedia ? 'w-[48px] shrink-0' : 'min-w-0 max-w-[min(220px,100%)]'
56+
CHIP_SURFACE,
57+
'size-full',
58+
isMedia ? 'overflow-hidden' : 'flex items-center gap-2 py-[7px] pr-5 pl-2'
6859
)}
60+
onClick={() => onFileClick(file)}
6961
>
70-
<Tooltip.Trigger asChild>
71-
<button
72-
type='button'
73-
className={cn(
74-
CHIP_SURFACE,
75-
'size-full',
76-
isMedia
77-
? 'overflow-hidden'
78-
: // `pr-5` reserves room for the remove badge so it never sits over the
79-
// filename.
80-
'flex items-center gap-2 py-2 pr-5 pl-2'
81-
)}
82-
onClick={() => onFileClick(file)}
83-
>
84-
{isMedia ? (
85-
<>
86-
<span className='absolute inset-0 flex items-center justify-center text-[var(--text-icon)]'>
87-
<Icon className='size-[18px]' />
88-
</span>
89-
{file.previewUrl &&
90-
!previewFailed &&
91-
(isVideo ? (
92-
<video
93-
src={file.previewUrl}
94-
muted
95-
playsInline
96-
preload='metadata'
97-
className='relative size-full object-cover'
98-
/>
99-
) : (
100-
<img
101-
src={file.previewUrl}
102-
alt={file.name}
103-
// A HEIC whose server-side transcode failed comes back as bytes the
104-
// browser still cannot decode. Dropping the image reveals the type
105-
// icon beneath instead of a broken glyph.
106-
onError={() => setPreviewFailed(true)}
107-
className='relative size-full object-cover'
108-
/>
109-
))}
110-
</>
111-
) : (
112-
<>
113-
{/* Steps again on hover: the chip's own hover fill closes to within
114-
7/255 of this badge in light mode, which would erase it during the
115-
one interaction where it is being looked at. */}
116-
<span className='flex size-[32px] shrink-0 items-center justify-center rounded-md bg-[var(--surface-6)] text-[var(--text-icon)] transition-colors group-hover:bg-[var(--surface-7)] dark:bg-[var(--surface-3)] dark:group-hover:bg-[var(--surface-3)]'>
117-
<Icon className='size-[16px]' />
118-
</span>
119-
<span className='flex min-w-0 flex-col items-start'>
120-
<span className='w-full truncate text-[var(--text-body)] text-small leading-tight'>
121-
{file.name}
122-
</span>
123-
{/* The name truncates from the tail, so the extension is often not
124-
readable from it — this is the format, not a restatement.
125-
`--text-icon`, not `--text-muted`: muted lands at 2.4:1 on this
126-
fill in dark mode, well under AA. */}
127-
{extension && (
128-
<span className='text-[var(--text-icon)] text-caption uppercase leading-tight'>
129-
{extension}
130-
</span>
131-
)}
132-
</span>
133-
</>
134-
)}
135-
{file.uploading && (
136-
<span className='absolute inset-0 flex items-center justify-center rounded-[inherit] bg-[var(--surface-5)]/70 dark:bg-[var(--surface-4)]/70'>
137-
<Loader className='size-[14px] text-[var(--text-icon)]' animate />
62+
{isMedia ? (
63+
<>
64+
<span className='absolute inset-0 flex items-center justify-center text-[var(--text-icon)]'>
65+
<Icon className='size-[18px]' />
66+
</span>
67+
{file.previewUrl &&
68+
!previewFailed &&
69+
(isVideo ? (
70+
<video
71+
src={file.previewUrl}
72+
muted
73+
playsInline
74+
preload='metadata'
75+
className='relative size-full object-cover'
76+
/>
77+
) : (
78+
<img
79+
src={file.previewUrl}
80+
alt={file.name}
81+
onError={() => setPreviewFailed(true)}
82+
className='relative size-full object-cover'
83+
/>
84+
))}
85+
</>
86+
) : (
87+
<>
88+
{/* Hover steps too: --surface-active is within 7/255 of --surface-6 in light mode. */}
89+
<span className='flex size-[32px] shrink-0 items-center justify-center rounded-md bg-[var(--surface-6)] text-[var(--text-icon)] transition-colors hover-hover:group-hover:bg-[var(--surface-7)] dark:bg-[var(--surface-3)]'>
90+
<Icon className='size-[16px]' />
91+
</span>
92+
<span className='flex min-w-0 flex-col items-start'>
93+
<span className='w-full truncate text-[var(--text-body)] text-small leading-tight'>
94+
{file.name}
13895
</span>
139-
)}
140-
</button>
141-
</Tooltip.Trigger>
142-
{!file.uploading && (
143-
<button
144-
type='button'
145-
onClick={(e) => {
146-
e.stopPropagation()
147-
onRemoveFile(file.id)
148-
}}
149-
aria-label={`Remove ${file.name}`}
150-
// Opaque, not a translucent scrim: a semi-transparent fill composites with
151-
// whatever sits under it, so the same badge reads differently over a light
152-
// card than over a photo. An opaque surface plus a border keeps the glyph
153-
// contrast fixed and gives the badge an edge against any thumbnail.
154-
className='absolute top-[2px] right-[2px] flex size-[16px] items-center justify-center rounded-full border border-[var(--border)] bg-[var(--surface-1)] text-[var(--text-body)] opacity-0 transition-opacity group-hover:opacity-100'
155-
>
156-
<X className='size-[9px]' />
157-
</button>
96+
{extension && (
97+
<span className='text-[var(--text-icon)] text-caption uppercase leading-tight'>
98+
{extension}
99+
</span>
100+
)}
101+
</span>
102+
</>
103+
)}
104+
{file.uploading && (
105+
<span className='absolute inset-0 flex items-center justify-center rounded-[inherit] bg-[var(--surface-5)]/70 dark:bg-[var(--surface-4)]/70'>
106+
<Loader className='size-[14px] text-[var(--text-icon)]' animate />
107+
</span>
158108
)}
159-
</div>
160-
{/* No width or truncation here — Tooltip.Content already caps and wraps, and this
161-
exists precisely to reveal the name the card truncated. */}
162-
<Tooltip.Content>{file.name}</Tooltip.Content>
163-
</Tooltip.Root>
109+
</button>
110+
{!file.uploading && (
111+
<button
112+
type='button'
113+
onClick={(e) => {
114+
e.stopPropagation()
115+
onRemoveFile(file.id)
116+
}}
117+
aria-label={`Remove ${file.name}`}
118+
className='absolute top-[2px] right-[2px] flex size-[16px] items-center justify-center rounded-full border border-[var(--border)] bg-[var(--surface-2)] text-[var(--text-body)] opacity-0 transition-opacity group-hover:opacity-100'
119+
>
120+
<X className='size-[10px]' />
121+
</button>
122+
)}
123+
</div>
164124
)
165125
})
166126

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/user-input/hooks/use-file-attachments.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ export function useFileAttachments(props: UseFileAttachmentsProps) {
159159
if (files.length === 0) return
160160

161161
const placeholders: AttachedFile[] = files.map((file) => {
162-
// Resolve once: the browser reports `application/octet-stream` (or nothing) for
163-
// plenty of files, and both the chip and the preview decision key off the type.
162+
/** Resolved once: browsers report `application/octet-stream` (or nothing) for
163+
* plenty of files, and both the chip and the preview decision key off it. */
164164
const type = resolveFileType(file)
165165
return {
166166
id: generateId(),
@@ -211,10 +211,9 @@ export function useFileAttachments(props: UseFileAttachmentsProps) {
211211
path: result.path,
212212
key: result.key,
213213
uploading: false,
214-
// A format the browser cannot decode has no local preview; now that
215-
// the bytes are stored, the serve route can hand back a renderable
216-
// derivative. Anything already previewing keeps its blob URL rather
217-
// than paying a round trip for a thumbnail it can draw locally.
214+
/** A format the browser cannot decode has no local preview; the
215+
* stored bytes now have a renderable derivative. Anything already
216+
* previewing keeps its blob URL. */
218217
previewUrl:
219218
f.previewUrl ??
220219
getMothershipAttachmentPreviewUrl({
@@ -352,12 +351,11 @@ export function useFileAttachments(props: UseFileAttachmentsProps) {
352351
}, [])
353352

354353
/**
355-
* Replaces the current attached files with a given set.
356-
* Cleans up preview URLs from the prior set before replacing.
354+
* Replaces the current attached files with a given set, revoking the prior set's
355+
* preview URLs first. Revoked outside the updater, which must stay pure — React
356+
* double-invokes updaters in StrictMode and may replay them.
357357
*/
358358
const restoreAttachedFiles = useCallback((files: AttachedFile[]) => {
359-
// Revoked outside the updater: React double-invokes updaters in StrictMode and may
360-
// replay them, so they have to stay pure.
361359
attachedFilesRef.current.forEach((f) => revokePreviewUrl(f.previewUrl))
362360
setAttachedFiles(files)
363361
}, [])

0 commit comments

Comments
 (0)