Show a failed attachment's reason on hover, not as a red line

The composer showed each failed file's reason as a red line of text under
the chips. A failed file no longer blocks the send, since it is dropped
then, so that line was louder than it needed to be. DESIGN.md also says
status text of a sentence or more must not be a coloured paragraph.

The chip's warning icon already marks the failure. The reason is now a
Tooltip on the failed chip. It falls back to "Failed" when no reason is
known. It is also kept as sr-only text so screen readers still read it.
The native title moves from the chip to the truncating file name, where
DESIGN.md keeps title=, and is left off failed chips so the tooltip
doesn't double up.
This commit is contained in:
arc53-machine committed 2026-09-28 10:27:12 +01:00
1 parent dc5ebf5bb9
commit b8a62cedf9
3 files changed
+133 -31

No files matched your search

@@ -0,0 +1,102 @@
import { act } from 'react';
import { createRoot, type Root } from 'react-dom/client';
vi.mock('react-i18next', () => ({
useTranslation: () => ({ t: (key: string) => key }),
}));
import type { Attachment } from '../../upload/uploadSlice';
import AttachmentChipList from './AttachmentChipList';
Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });
const att = (over: Partial<Attachment> = {}): Attachment => ({
id: 'a1',
fileName: 'scan.pdf',
progress: 0,
status: 'failed',
taskId: '',
errorMessage: 'Upload failed. The file could not be read.',
...over,
});
describe('AttachmentChipList failed chip', () => {
let container: HTMLDivElement;
let root: Root;
beforeEach(() => {
vi.useFakeTimers();
container = document.createElement('div');
document.body.appendChild(container);
root = createRoot(container);
});
afterEach(async () => {
await act(async () => root.unmount());
container.remove();
vi.useRealTimers();
});
const render = async (attachments: Attachment[]) => {
await act(async () => {
root.render(
<AttachmentChipList
attachments={attachments}
draggingId={null}
onRemove={() => {}}
onDragStart={() => {}}
onDragOver={() => {}}
onDropOn={() => {}}
/>,
);
});
};
const tooltip = () =>
document.body.querySelector('[data-slot="tooltip-content"]');
it('shows no inline failure line under the chips', async () => {
await render([att()]);
expect(container.querySelector('[role="alert"]')).toBeNull();
expect(container.querySelector('.text-destructive')).toBeNull();
});
it('shows the reason in a tooltip when the chip is hovered', async () => {
await render([att()]);
expect(tooltip()).toBeNull();
// The remove X is an IconButton with its own tooltip; the chip is the div.
const chip = container.querySelector('div[data-slot="tooltip-trigger"]')!;
await act(async () => {
chip.dispatchEvent(
new PointerEvent('pointermove', {
bubbles: true,
pointerType: 'mouse',
}),
);
vi.advanceTimersByTime(500);
});
expect(tooltip()?.textContent).toContain(
'Upload failed. The file could not be read.',
);
});
it('keeps the reason available to screen readers', async () => {
await render([att()]);
const srOnly = container.querySelector('.sr-only');
expect(srOnly?.textContent).toBe(
'Upload failed. The file could not be read.',
);
});
it('adds no tooltip to a chip that did not fail', async () => {
await render([att({ status: 'completed', errorMessage: undefined })]);
expect(
container.querySelector('div[data-slot="tooltip-trigger"]'),
).toBeNull();
});
});
@@ -3,6 +3,7 @@ import { useTranslation } from 'react-i18next';
import type { Attachment } from '../../upload/uploadSlice';
import { IconButton } from '../ui/icon-button';
import { Tooltip, TooltipContent, TooltipTrigger } from '../ui/tooltip';
import { cn } from '@/lib/utils';
type AttachmentChipListProps = {
@@ -30,12 +31,6 @@ export default function AttachmentChipList({
}: AttachmentChipListProps) {
const { t } = useTranslation();
// A tooltip is the one place a touch user can never look, and this list is
// where a phone picker's unsupported file lands. Show the reason inline,
// as soon as it is known, rather than only once a send is attempted.
const failures = attachments.filter(
(attachment) => attachment.status === 'failed' && attachment.errorMessage,
);
// Not a failure: the file is kept and still sends. It only warns that the
// model picked right now would receive nothing it can read.
const unreadable = modelName
@@ -46,7 +41,14 @@ export default function AttachmentChipList({
<>
<div className="flex flex-wrap gap-1.5 px-2 py-2 sm:gap-2 sm:px-3">
{attachments.map((attachment) => {
return (
const failed = attachment.status === 'failed';
// A failed file never blocks the send (it is dropped then), so its
// reason stays out of the way: the chip's warning icon marks it, and
// the reason is a hover tooltip plus screen-reader text.
const failureReason = failed
? (attachment.errorMessage ?? t('conversation.attachments.failed'))
: null;
const chip = (
<div
key={attachment.id}
draggable={true}
@@ -62,11 +64,6 @@ export default function AttachmentChipList({
: 'opacity-100',
draggingId === attachment.id && 'ring-primary/30 ring-2',
)}
title={
attachment.status === 'failed' && attachment.errorMessage
? `${attachment.fileName}: ${attachment.errorMessage}`
: attachment.fileName
}
>
<div className="bg-primary mr-2 flex size-8 items-center justify-center rounded-md p-1">
{attachment.status === 'completed' && (
@@ -115,9 +112,15 @@ export default function AttachmentChipList({
)}
</div>
<span className="max-w-[120px] truncate font-medium sm:max-w-[150px]">
<span
className="max-w-[120px] truncate font-medium sm:max-w-[150px]"
title={failed ? undefined : attachment.fileName}
>
{attachment.fileName}
</span>
{failureReason && (
<span className="sr-only">{failureReason}</span>
)}
<IconButton
label={t('conversation.attachments.remove')}
@@ -133,22 +136,16 @@ export default function AttachmentChipList({
</IconButton>
</div>
);
if (!failureReason) return chip;
return (
<Tooltip key={attachment.id}>
<TooltipTrigger asChild>{chip}</TooltipTrigger>
<TooltipContent>{failureReason}</TooltipContent>
</Tooltip>
);
})}
</div>
{failures.length > 0 && (
<div
className="text-destructive flex flex-col gap-0.5 px-2 pb-1 text-xs sm:px-3"
role="alert"
>
{failures.map((attachment) => (
<span key={attachment.id}>
{attachment.fileName}: {attachment.errorMessage}
</span>
))}
</div>
)}
{unreadable.length > 0 && (
<div
className="text-warning flex flex-col gap-0.5 px-2 pb-1 text-xs sm:px-3"
@@ -276,10 +276,12 @@ test.describe("tier-a · composer resilience", () => {
.first()
.setInputFiles(SMALL_FIXTURE_PATH);
// The failed chip explains itself before Send.
await expect(page.getByText(/upload failed/i)).toBeVisible({
timeout: 5_000,
});
// The failed chip is marked by its warning icon; the reason is a
// hover tooltip, not a line of text in the composer.
const failedIcon = page.getByLabel("Failed", { exact: true });
await expect(failedIcon).toBeVisible({ timeout: 5_000 });
await failedIcon.hover();
await expect(page.getByRole("tooltip")).toContainText(/upload failed/i);
const question = "does this still send? e2e-failed-attachment";
const textarea = page.locator("#message-input");
@@ -298,7 +300,8 @@ test.describe("tier-a · composer resilience", () => {
// The question is in the thread; the chip and its reason are gone.
await expect(page.getByText(question)).toBeVisible();
await expect(page.getByText(/upload failed/i)).toBeHidden();
await expect(failedIcon).toHaveCount(0);
await expect(page.getByText(/upload failed/i)).toHaveCount(0);
await expect(page.getByText("notes.txt")).toBeHidden();
await expect(textarea).toHaveValue("");