Skip to content

Commit 6599b4c

Browse files
authored
fix(chat): keep the composer remove badge anchored and align chip tokens (#6365)
* fix(chat): keep the remove badge anchored to the file card The card wrapper had no width cap, so it sized to the filename's max-content width while the card itself capped at 220px. The remove badge is positioned against that wrapper, so a long filename stranded it far to the right of the card it belongs to. Moves the cap onto the wrapper and lets the card fill it. * fix(chat): make attachment tiles read on every surface they render on The sent-message tile went icon-only, which made its fill the whole affordance — and against the workflow chat panel's --surface-1 that fill is ~8/255 away in light mode. Adds the border the user message bubble already pairs with --surface-5 for the same reason. - Restore an accessible name to the sent tiles: an icon-only div with a title attribute announces as nothing. - Step the composer icon badge on hover; the chip's hover fill closed to within 7/255 of it in light mode. - Extension label moves to --text-icon/text-caption; --text-muted was 2.4:1 on this fill in dark mode, well under AA. - Tooltip.Content no longer re-declares the width and truncation it owns — it was truncating the very name it exists to reveal. * improvement(chat): restore sent-attachment filenames and align chip tokens - Revert the sent-message attachments to main's styling: the icon-only tile dropped the filename, leaving no way to tell what was sent. - Radii onto the scale: --radius is 8px, so rounded-[10px] was off-system in both files. Outer surfaces use rounded-lg, the nested icon badge rounded-md. - Pill filename uses the named text-xs rather than an arbitrary text-[11px]. - The remove badge is opaque instead of a translucent scrim, so it reads the same over a light card and over a photo rather than compositing with each. * 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. * fix(chat): keep the remove badge reachable on coarse pointers Gating the reveal on hover-hover alone would hide it from touch entirely, since that variant is fine-pointer only. Instead it is visible by default and only fine pointers get reveal-on-hover, so the badge never depends on an emulated hover. * fix(chat): reveal the remove control on keyboard focus On a fine pointer the badge is transparent until hover, so tabbing to it left a sighted keyboard user unable to see which attachment Enter would remove. The focus-visible chain carries higher specificity than the hide rule, so it wins regardless of source order. * improvement(chat): drop the icon badge's own hover step The chip's hover is the only hover affordance needed. The badge fill is now constant, sitting one step below --surface-6 in light mode so the chip's hover fill cannot close on it — which is what the per-badge step was compensating for. * fix(chat): stop gating the remove badge on a variant that cannot express it hover-hover expands to '@media (hover:hover) and (pointer:fine) { &:hover }', so it binds to the element carrying the class. On the badge that meant every rule required hovering the badge itself, making the whole chain dead CSS — the badge was simply always visible. Rather than rebuild the gating, drop it: an always-visible control is reachable on touch and stays visible while holding keyboard focus, which the reveal-on-hover form could not manage without special cases for both.
1 parent 0601fcd commit 6599b4c

4 files changed

Lines changed: 122 additions & 122 deletions

File tree

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import type { ChatMessageAttachment } from '@/app/workspace/[workspaceId]/home/t
55
function FileAttachmentPill(props: { mediaType: string; filename: string }) {
66
const Icon = getDocumentIcon(props.mediaType, props.filename)
77
return (
8-
<div className='flex max-w-[140px] items-center gap-[5px] rounded-[10px] bg-[var(--surface-5)] px-[6px] py-[3px]'>
9-
<Icon className='size-[14px] flex-shrink-0 text-[var(--text-icon)]' />
10-
<span className='truncate text-[11px] text-[var(--text-body)]'>{props.filename}</span>
8+
<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] shrink-0 text-[var(--text-icon)]' />
10+
<span className='truncate text-[var(--text-body)] text-xs'>{props.filename}</span>
1111
</div>
1212
)
1313
}
@@ -41,7 +41,7 @@ export function ChatMessageAttachments(props: {
4141
return (
4242
<div
4343
key={att.id}
44-
className='relative size-[56px] overflow-hidden rounded-[8px] bg-[var(--surface-5)]'
44+
className='relative size-[56px] overflow-hidden rounded-lg bg-[var(--surface-5)]'
4545
>
4646
<div className='absolute inset-0 flex items-center justify-center text-[var(--text-icon)]'>
4747
<Icon className='size-[18px]' />
@@ -51,14 +51,14 @@ export function ChatMessageAttachments(props: {
5151
muted
5252
playsInline
5353
preload='metadata'
54-
className='relative h-full w-full object-cover'
54+
className='relative size-full object-cover'
5555
/>
5656
</div>
5757
)
5858
}
5959
return (
60-
<div key={att.id} className='size-[56px] overflow-hidden rounded-[8px]'>
61-
<img src={att.previewUrl} alt={att.filename} className='h-full w-full object-cover' />
60+
<div key={att.id} className='size-[56px] overflow-hidden rounded-lg'>
61+
<img src={att.previewUrl} alt={att.filename} className='size-full object-cover' />
6262
</div>
6363
)
6464
})}

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,31 @@ 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

67+
it('caps the card wrapper so a long filename cannot strand the remove badge', () => {
68+
render([file({ name: '9bacf973-cd64-437b-be12-58be9f2c1a4d-very-long-name.pdf' })])
69+
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.
72+
const wrapper = container.querySelector('button')?.parentElement
73+
expect(wrapper?.className).toMatch(/max-w-/)
74+
})
75+
76+
it('keeps the remove control visible rather than gating it on hover', () => {
77+
render([file({})])
78+
79+
// A reveal-on-hover badge is unreachable on touch and invisible while it holds
80+
// keyboard focus, so it must not be opacity-gated at all.
81+
const remove = container.querySelector('button[aria-label^="Remove"]')
82+
expect(remove).not.toBeNull()
83+
expect(remove?.className).not.toMatch(/opacity-0/)
84+
})
85+
7086
it('drops the image and reveals the type icon when the preview fails to decode', () => {
7187
render([file({ name: 'photo.heic', type: 'image/heic', previewUrl: '/api/files/serve/x' })])
7288

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

Lines changed: 88 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,18 @@
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 =
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)]'
15+
'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

2317
interface AttachedFilesListProps {
2418
attachedFiles: AttachedFile[]
@@ -33,11 +27,9 @@ interface AttachedFileChipProps {
3327
}
3428

3529
/**
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.
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.
4133
*/
4234
const AttachedFileChip = React.memo(function AttachedFileChip({
4335
file,
@@ -46,100 +38,94 @@ const AttachedFileChip = React.memo(function AttachedFileChip({
4638
}: AttachedFileChipProps) {
4739
const Icon = getDocumentIcon(file.type, file.name)
4840
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.
5141
const isMedia = isVideo || file.type.startsWith('image/')
5242
const extension = getFileExtension(file.name)
5343
const [previewFailed, setPreviewFailed] = useState(false)
5444

5545
return (
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-
) : (
87-
<img
88-
src={file.previewUrl}
89-
alt={file.name}
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'
95-
/>
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>
111-
)}
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 />
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'
55+
className={cn(
56+
CHIP_SURFACE,
57+
'size-full',
58+
isMedia ? 'overflow-hidden' : 'flex items-center gap-2 py-[7px] pr-5 pl-2'
59+
)}
60+
onClick={() => onFileClick(file)}
61+
>
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+
{/* Fill is constant — the chip's own hover is the only hover affordance. It
89+
sits a step below `--surface-6` in light mode so the chip's hover fill
90+
(`--surface-active`) cannot close on it. */}
91+
<span className='flex size-[32px] shrink-0 items-center justify-center rounded-md bg-[var(--surface-7)] text-[var(--text-icon)] dark:bg-[var(--surface-3)]'>
92+
<Icon className='size-[16px]' />
93+
</span>
94+
<span className='flex min-w-0 flex-col items-start'>
95+
<span className='w-full truncate text-[var(--text-body)] text-small leading-tight'>
96+
{file.name}
11897
</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>
98+
{extension && (
99+
<span className='text-[var(--text-icon)] text-caption uppercase leading-tight'>
100+
{extension}
101+
</span>
102+
)}
103+
</span>
104+
</>
105+
)}
106+
{file.uploading && (
107+
<span className='absolute inset-0 flex items-center justify-center rounded-[inherit] bg-[var(--surface-5)]/70 dark:bg-[var(--surface-4)]/70'>
108+
<Loader className='size-[14px] text-[var(--text-icon)]' animate />
109+
</span>
137110
)}
138-
</div>
139-
<Tooltip.Content side='top'>
140-
<p className='max-w-[200px] truncate'>{file.name}</p>
141-
</Tooltip.Content>
142-
</Tooltip.Root>
111+
</button>
112+
{!file.uploading && (
113+
<button
114+
type='button'
115+
onClick={(e) => {
116+
e.stopPropagation()
117+
onRemoveFile(file.id)
118+
}}
119+
aria-label={`Remove ${file.name}`}
120+
/* Always visible: reveal-on-hover would hide it from touch and from keyboard
121+
focus, and `hover-hover` cannot express "while the chip is hovered" from
122+
here anyway — it carries its own `&:hover`, so it binds to this element. */
123+
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)]'
124+
>
125+
<X className='size-[10px]' />
126+
</button>
127+
)}
128+
</div>
143129
)
144130
})
145131

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)