diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7545745c..8cc21af7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,7 +18,7 @@ jobs: # Required by cli/src/utils/windowsProcessTree.pwsh.test.ts — image # inspect alone does not pull; silent skip was fake B1 coverage (#1911). - run: docker pull mcr.microsoft.com/powershell:latest - - run: bun run test:e2e -- terminal-wrap-fidelity.spec.ts composer-copy.spec.ts session-list-scroll.spec.ts cold-initial-tail.spec.ts scrollbar-auto-hide.spec.ts attachment-removal.spec.ts history-navigation-layout.spec.ts share-turn.spec.ts + - run: bun run test:e2e -- terminal-wrap-fidelity.spec.ts composer-copy.spec.ts session-list-scroll.spec.ts cold-initial-tail.spec.ts scrollbar-auto-hide.spec.ts attachment-removal.spec.ts history-navigation-layout.spec.ts share-turn.spec.ts composer-attachment-controls.spec.ts - run: bun run test # Serial runner-integration suite: starts real detached runner/session diff --git a/e2e/composer-attachment-controls.spec.ts b/e2e/composer-attachment-controls.spec.ts new file mode 100644 index 00000000..d6777405 --- /dev/null +++ b/e2e/composer-attachment-controls.spec.ts @@ -0,0 +1,99 @@ +import { expect, test, type Locator, type Page } from '@playwright/test' + +const items = (page: Page) => page.locator('[data-hapi-composer-attachment-id]') +const handles = (page: Page) => page.getByTestId('attachment-drag-handle') +const item = (page: Page, name: string) => page.locator(`[data-hapi-composer-attachment-id="${name}"]`) +test.use({ serviceWorkers: 'block' }) + +async function screenshot(page: Page, path: (name: string) => string, name: string) { + const composer = page.getByRole('region', { name: 'Composer', exact: true }) + await composer.screenshot({ path: path(`${name}.png`) }) + await page.evaluate(() => { document.documentElement.dataset.theme = 'dark' }) + await composer.screenshot({ path: path(`${name}-dark.png`) }) + await page.evaluate(() => { delete document.documentElement.dataset.theme }) +} + +for (const touch of [false, true]) { + test.describe(touch ? 'touch' : 'desktop', () => { + test.use({ viewport: { width: touch ? 390 : 1000, height: 800 }, hasTouch: touch }) + const activate = (locator: Locator) => touch ? locator.tap() : locator.click() + + test.beforeEach(async ({ page }) => { + await page.goto('/e2e-fixtures/composer-image-drag-fixture.html', { waitUntil: 'domcontentloaded' }) + }) + + for (const [kind, name] of [['image', 'first.png'], ['file', 'notes.txt']] as const) { + test(`single ${kind} has no drag handle and can still be removed`, async ({ page }, testInfo) => { + await activate(page.getByRole('button', { name: `Add ${kind}`, exact: true })) + const attachment = item(page, name) + await expect(attachment).toBeVisible() + await attachment.hover() + await expect(handles(page)).toHaveCount(0) + if (kind === 'file') { + const spacing = await attachment.evaluate((element) => { + const name = element.querySelector('span')!.getBoundingClientRect() + const removeIcon = element.querySelector('button svg')!.getBoundingClientRect() + const chip = element.getBoundingClientRect() + // The 12px icon's stroked X ends 2px inside its SVG viewport. + return { left: name.left - chip.left, right: chip.right - removeIcon.right + 2 } + }) + expect(spacing.left).toBeCloseTo(spacing.right, 1) + } + const remove = attachment.getByRole('button', { name: 'Remove attachment' }) + await expect(remove).toHaveCSS('opacity', '1') + await remove.focus() + await expect(handles(page)).toHaveCount(0) + await screenshot(page, (name) => testInfo.outputPath(name), `single-${kind}`) + if (kind === 'image') { + await activate(attachment.getByTitle('Click to zoom')) + await expect(page.getByRole('dialog', { name })).toBeVisible() + await activate(page.getByRole('dialog').getByTitle('Close', { exact: true })) + } + await attachment.hover() + await activate(remove) + await expect(items(page)).toHaveCount(0) + }) + } + + test('updates handles at 1 → 2 → 1, preserving the mixed order and draft', async ({ page }, testInfo) => { + await activate(page.getByRole('button', { name: 'Add image', exact: true })) + await expect(items(page)).toHaveCount(1) + await expect(handles(page)).toHaveCount(0) + await page.getByRole('textbox', { name: 'Draft' }).fill('Keep this draft') + await activate(page.getByRole('button', { name: 'Add file', exact: true })) + await expect(handles(page)).toHaveCount(2) + await item(page, 'first.png').hover() + await expect(item(page, 'first.png').getByTestId('attachment-drag-handle')).toHaveCSS('opacity', '1') + await screenshot(page, (name) => testInfo.outputPath(name), 'mixed-attachments') + await item(page, 'notes.txt').getByTestId('attachment-drag-handle').press('Home') + await expect(items(page).first()).toHaveAttribute('data-hapi-composer-attachment-id', 'notes.txt') + await item(page, 'first.png').hover() + await activate(item(page, 'first.png').getByRole('button', { name: 'Remove attachment' })) + await expect(handles(page)).toHaveCount(0) + await expect(item(page, 'notes.txt')).toBeVisible() + await expect(page.getByRole('textbox', { name: 'Draft' })).toHaveValue('Keep this draft') + }) + + test('two images retain their handles and hide them when one is removed', async ({ page }) => { + await activate(page.getByRole('button', { name: 'Load attachments', exact: true })) + await expect(items(page)).toHaveCount(3) + await activate(item(page, 'notes.txt').getByRole('button', { name: 'Remove attachment' })) + await expect(handles(page)).toHaveCount(2) + await item(page, 'first.png').hover() + await activate(item(page, 'first.png').getByRole('button', { name: 'Remove attachment' })) + await expect(handles(page)).toHaveCount(0) + await expect(item(page, 'second.png')).toBeVisible() + }) + + test('two files retain sorting and hide the remaining handle after deletion', async ({ page }) => { + await activate(page.getByRole('button', { name: 'Add file', exact: true })) + await activate(page.getByRole('button', { name: 'Add second file', exact: true })) + await expect(handles(page)).toHaveCount(2) + await item(page, 'other.txt').getByTestId('attachment-drag-handle').press('Home') + await expect(items(page).first()).toHaveAttribute('data-hapi-composer-attachment-id', 'other.txt') + await activate(item(page, 'other.txt').getByRole('button', { name: 'Remove attachment' })) + await expect(handles(page)).toHaveCount(0) + await expect(item(page, 'notes.txt')).toBeVisible() + }) + }) +} diff --git a/web/e2e-fixtures/composer-image-drag-fixture.tsx b/web/e2e-fixtures/composer-image-drag-fixture.tsx index 33d34c70..e1e39d38 100644 --- a/web/e2e-fixtures/composer-image-drag-fixture.tsx +++ b/web/e2e-fixtures/composer-image-drag-fixture.tsx @@ -51,6 +51,9 @@ function App() { const runtime = useLocalRuntime(model, { adapters: { attachments: adapter } }) const [loaded, setLoaded] = useState(false) const [loading, setLoading] = useState(false) + const addAttachment = (name: string) => runtime.thread.composer.addAttachment(new File(['fixture'], name, { + type: name.endsWith('.png') ? 'image/png' : 'text/plain', + })) const loadAttachments = async () => { setLoading(true) try { @@ -70,6 +73,9 @@ function App() { + + +
{ const dragHandle = screen.getByTestId('attachment-drag-handle') const removeButton = screen.getByRole('button', { name: 'Remove attachment' }) - expect(dragHandle.parentElement).toHaveClass('gap-1.5', 'px-2') + expect(dragHandle.parentElement).toHaveClass('gap-1.5', 'pl-2', 'pr-2') for (const control of [dragHandle, removeButton]) { expect(control).toHaveClass('hapi-composer-attachment-file-control', 'h-6', 'w-6', '-mx-1', 'items-center') diff --git a/web/src/components/AssistantChat/AttachmentItem.tsx b/web/src/components/AssistantChat/AttachmentItem.tsx index 0c5d51c4..a13465c9 100644 --- a/web/src/components/AssistantChat/AttachmentItem.tsx +++ b/web/src/components/AssistantChat/AttachmentItem.tsx @@ -158,7 +158,7 @@ export function AttachmentItem(props: { dragHandleProps?: AttachmentDragHandlePr return ( diff --git a/web/src/components/AssistantChat/SortableComposerAttachments.preview.test.tsx b/web/src/components/AssistantChat/SortableComposerAttachments.preview.test.tsx index 3b454b4a..c35b856a 100644 --- a/web/src/components/AssistantChat/SortableComposerAttachments.preview.test.tsx +++ b/web/src/components/AssistantChat/SortableComposerAttachments.preview.test.tsx @@ -7,11 +7,12 @@ import { reconcileAttachmentOrder } from '@/lib/attachmentOrder' const previewUrl = 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jP1sAAAAASUVORK5CYII=' const model: ChatModelAdapter = { async *run() {} } const attachmentAdapter: AttachmentAdapter = { - accept: 'image/*', + accept: 'image/*,text/plain', async add({ file }) { return { - id: file.name, name: file.name, type: 'image', contentType: file.type, file, - status: { type: 'requires-action', reason: 'composer-send' }, previewUrl, + id: file.name, name: file.name, type: file.type.startsWith('image/') ? 'image' : 'file', contentType: file.type, file, + status: { type: 'requires-action', reason: 'composer-send' }, + ...(file.type.startsWith('image/') ? { previewUrl } : {}), } }, async remove() {}, @@ -54,6 +55,52 @@ async function setup() { } describe('sortable composer image preview with the real attachment runtime', () => { + it.each([ + ['only.png', 'image/png'], + ['only.txt', 'text/plain'], + ])('shows drag handles only while %s has another attachment to reorder', async (name, type) => { + const runtimeRef: HarnessProps['runtimeRef'] = {} + const { container } = render() + const composer = runtimeRef.current!.thread.composer + await act(async () => { + await composer.addAttachment(new File(['attachment'], name, { type })) + }) + expect(screen.queryByTestId('attachment-drag-handle')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: 'Remove attachment' })).toBeInTheDocument() + + await act(async () => { + await composer.addAttachment(new File(['second'], 'second.txt', { type: 'text/plain' })) + }) + expect(screen.getAllByTestId('attachment-drag-handle')).toHaveLength(2) + expect(screen.getAllByRole('button', { name: 'Remove attachment' })).toHaveLength(2) + + const second = container.querySelector('[data-hapi-composer-attachment-id="second.txt"]')! + await act(async () => fireEvent.click(within(second as HTMLElement).getByRole('button', { name: 'Remove attachment' }))) + expect(screen.queryByTestId('attachment-drag-handle')).not.toBeInTheDocument() + expect(composer.getState().attachments.map((attachment) => attachment.name)).toEqual([name]) + await act(async () => fireEvent.click(screen.getByRole('button', { name: 'Remove attachment' }))) + expect(composer.getState().attachments).toHaveLength(0) + }) + + it('keeps an open single-image viewer and its zoom as the attachment count changes', async () => { + const runtimeRef: HarnessProps['runtimeRef'] = {} + const { container } = render() + const composer = runtimeRef.current!.thread.composer + await act(async () => composer.addAttachment(new File(['image'], 'first.png', { type: 'image/png' }))) + fireEvent.click(screen.getByTitle('Click to zoom')) + fireEvent.click(within(screen.getByRole('dialog')).getByTitle('Zoom in')) + const dialog = screen.getByRole('dialog', { name: 'first.png' }) + + await act(async () => composer.addAttachment(new File(['second'], 'second.txt', { type: 'text/plain' }))) + expect(screen.getByRole('dialog', { name: 'first.png' })).toBe(dialog) + expect(within(dialog).getByTitle('Reset zoom')).toHaveTextContent('125%') + const second = container.querySelector('[data-hapi-composer-attachment-id="second.txt"]')! + await act(async () => fireEvent.click(within(second as HTMLElement).getByRole('button', { name: 'Remove attachment' }))) + expect(screen.getByRole('dialog', { name: 'first.png' })).toBe(dialog) + expect(within(dialog).getByTitle('Reset zoom')).toHaveTextContent('125%') + expect(screen.queryByTestId('attachment-drag-handle')).not.toBeInTheDocument() + }) + it('keeps the active image and zoom across draft and parent updates', async () => { const { rerender, runtimeRef } = await setup() fireEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Next image' })) diff --git a/web/src/components/AssistantChat/SortableComposerAttachments.tsx b/web/src/components/AssistantChat/SortableComposerAttachments.tsx index 652f6d74..a87bee19 100644 --- a/web/src/components/AssistantChat/SortableComposerAttachments.tsx +++ b/web/src/components/AssistantChat/SortableComposerAttachments.tsx @@ -272,7 +272,7 @@ export function SortableComposerAttachments(props: { attachment={attachment} index={index} isDragging={draggingId === id} - disabled={disabled} + disabled={disabled || attachments.length < 2} onPointerDown={handlePointerDown} onKeyDown={handleKeyDown} onContextMenu={handleContextMenu}