From b8a62cedf98aac3e5dc8915a33ed234984fea946 Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:27:12 +0100 Subject: [PATCH] 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. --- .../message-input/AttachmentChipList.test.tsx | 102 ++++++++++++++++++ .../message-input/AttachmentChipList.tsx | 49 ++++----- .../specs/tier-a/composer-resilience.spec.ts | 13 ++- 3 files changed, 133 insertions(+), 31 deletions(-) create mode 100644 frontend/src/components/message-input/AttachmentChipList.test.tsx diff --git a/frontend/src/components/message-input/AttachmentChipList.test.tsx b/frontend/src/components/message-input/AttachmentChipList.test.tsx new file mode 100644 index 00000000..a2f7b799 --- /dev/null +++ b/frontend/src/components/message-input/AttachmentChipList.test.tsx @@ -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 => ({ + 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( + {}} + 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(); + }); +}); diff --git a/frontend/src/components/message-input/AttachmentChipList.tsx b/frontend/src/components/message-input/AttachmentChipList.tsx index 7d31dfc9..bda4e01a 100644 --- a/frontend/src/components/message-input/AttachmentChipList.tsx +++ b/frontend/src/components/message-input/AttachmentChipList.tsx @@ -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({ <>
{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 = (
{attachment.status === 'completed' && ( @@ -115,9 +112,15 @@ export default function AttachmentChipList({ )}
- + {attachment.fileName} + {failureReason && ( + {failureReason} + )}
); + if (!failureReason) return chip; + return ( + + {chip} + {failureReason} + + ); })}
- {failures.length > 0 && ( -
- {failures.map((attachment) => ( - - {attachment.fileName}: {attachment.errorMessage} - - ))} -
- )} - {unreadable.length > 0 && (
{ .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("");