From b8909befdf46714cbb48ee24b595fef4e0c5f156 Mon Sep 17 00:00:00 2001 From: xarmian Date: Tue, 4 Aug 2026 02:44:57 +0000 Subject: [PATCH] feat(attachments): an options panel for files (TASK-2423) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tapping a file used to do the most destructive-adjacent thing available: a strip tile was a bare ``, so one tap put the file in your Downloads folder with no way to see what it was first. This is what a tap opens instead — `AttachmentDetailsPanel`, plus the one host that owns it. PLAN-2392 phase 2, wave B. Nothing routes into the panel yet; the strip's tiles and the editor's chips start emitting the open event in TASK-2424, so the panel is driven here through its host and the events bus. Presentation is the existing `Menu` with `sheetOnMobile` — a popover on desktop, a BottomSheet at the mobile breakpoint (DR-6). No new overlay primitive, so ESC ordering, outside-click, placement and the sheet's focus handling are the app's existing ones rather than second implementations. The actions are NOT defined here: they are rendered from the shared descriptor list (DR-5), choosing between MenuItem's anchor and button branches on the descriptor's own `element` discriminant and never calling `run()` on an anchor. Adding an action stays a one-descriptor change. It opens IMMEDIATELY and completes the metadata after (DR-2, DR-10). The event's filename / mime / size are nullable by contract, so the panel paints what it was handed and fetches the rest itself: `ok` fills the gaps, `missing` (404) latches an authoritative "no longer available" with every action inert, and `transient` shows an inline error beside the row it already knows, with a Retry that goes through `revalidateAttachmentMetadata` — a plain refetch would replay the cached failure and look broken. Delete is an in-app drill-down sub-view (DR-18), the item menu's shape exactly: prompt as `role="presentation"` with an aria-describedby back-reference, Cancel FIRST, destructive row last, and the strip's contextual "still used in this item's content" warning carried through (read at confirm time from the LIVE editor markdown, since the persisted body lags). It is wired as the delete descriptor's `confirmDelete` promise rather than as a bespoke path, so the descriptor's identity snapshot and permission re-check across the confirmation stay in force. The unreferenced arm stays hedged: this can only speak for the HOST's content, and the event's `itemId` is routing, not ownership. The host is `ItemDetail`, through a small `AttachmentPanelHost` it mounts beside the strip. It consumes an event only when BOTH `itemId` and `hostToken` are its own (DR-8), and supplies `mutationsEnabled` itself — never the NodeView's (it has no mutation context) and never the timeline's `canEdit` (which ignores `peeking` and would let a peeked pane mutate). The host is a component rather than a block inside ItemDetail because the addressing rule has to be testable with two hosts mounted at once, which is what the pane host does at runtime. Parent lifecycle (DR-14): an archived parent's attachment fetch returns a generic 404, so archive CLOSES the panel and restore REVALIDATES it rather than assuming the previous state holds. The strip sits outside ItemDetail's keyed lifecycle block, so this is added, not inherited; it arrives declaratively as `parentArchived`, following the item ItemDetail already refetches on the SSE lifecycle events. Long filenames and RTL are handled with logical properties throughout, `min-width: 0` on every flex child holding the name, and the full unelided filename in both `title` and the panel's accessible name (DR-13). No `state_generation` and no Undo (DR-19) — Delete behaves exactly like today's tile Delete; PLAN-2411 adds the generation token and the Undo toast to all three entry points at once. Also here: - `describeAttachmentType` in the shared display helpers, built on `iconForAttachment` so the words and the icon beside them cannot disagree about what a file is. - `liveEditorMarkdown` extracted in ItemDetail — the strip and the panel now read the live body through one accessor instead of two copies. Tested through the host (20 jsdom cases): addressing with two hosts mounted, open-with-partial-then-complete, all three metadata arms, Retry's invalidate-before-refetch, host-supplied permission for peeked vs master, the full confirm/cancel/failure delete paths, both warning arms, archive- closes / restore-revalidates, item switch, and re-targeting in place. Focus entry and return, background inertness, real placement, the sheet swap and Enter/Space activation are browser-only and belong to phase 3d. --- web/src/lib/attachments/display.test.ts | 39 ++ web/src/lib/attachments/display.ts | 40 ++ .../attachments/AttachmentDetailsPanel.svelte | 607 ++++++++++++++++++ .../attachments/AttachmentPanelHost.svelte | 143 +++++ .../AttachmentPanelHost.svelte.test.ts | 511 +++++++++++++++ .../lib/components/items/ItemDetail.svelte | 60 +- 6 files changed, 1387 insertions(+), 13 deletions(-) create mode 100644 web/src/lib/components/attachments/AttachmentDetailsPanel.svelte create mode 100644 web/src/lib/components/attachments/AttachmentPanelHost.svelte create mode 100644 web/src/lib/components/attachments/AttachmentPanelHost.svelte.test.ts diff --git a/web/src/lib/attachments/display.test.ts b/web/src/lib/attachments/display.test.ts index 4fb427d4..f83c702e 100644 --- a/web/src/lib/attachments/display.test.ts +++ b/web/src/lib/attachments/display.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { canBrowserPreview, canOpenInViewer, + describeAttachmentType, formatBytes, iconForAttachment, isImage @@ -277,3 +278,41 @@ describe('canBrowserPreview — PLAN-2392 DR-5', () => { } }); }); + +/** + * The panel's type line (PLAN-2392 DR-2 / TASK-2423). Built on + * `iconForAttachment` so the words and the icon beside them can never + * disagree about what a file is. + */ +describe('describeAttachmentType', () => { + it('drops an extension that merely repeats the family', () => { + expect(describeAttachmentType('application/pdf', 'spec.pdf')).toBe('PDF'); + }); + + it('keeps an extension that names the specific format', () => { + expect(describeAttachmentType('image/png', 'shot.png')).toBe('PNG image'); + expect( + describeAttachmentType( + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + 'budget.xlsx' + ) + ).toBe('XLSX spreadsheet'); + expect(describeAttachmentType('application/zip', 'logs.zip')).toBe('ZIP archive'); + }); + + it('falls back to the family alone without a usable filename', () => { + expect(describeAttachmentType('application/pdf', null)).toBe('PDF'); + expect(describeAttachmentType('image/webp', 'no-extension')).toBe('Image'); + }); + + it('never returns an empty string — a panel with nothing known still says something', () => { + // The chip entry point can open the panel with no MIME and no filename + // at all; an empty answer would render as a stray separator. + expect(describeAttachmentType(null, null)).toBe('File'); + expect(describeAttachmentType('', '')).toBe('File'); + }); + + it('reads the filename when the stored MIME is uselessly generic', () => { + expect(describeAttachmentType('application/octet-stream', 'notes.md')).toBe('MD text'); + }); +}); diff --git a/web/src/lib/attachments/display.ts b/web/src/lib/attachments/display.ts index 3bfa9572..9b587a3d 100644 --- a/web/src/lib/attachments/display.ts +++ b/web/src/lib/attachments/display.ts @@ -222,6 +222,46 @@ export function isImage(mime: string): boolean { return mime.startsWith('image/'); } +/** Human label per icon family, for the "what IS this file" line. */ +const FAMILY_LABELS: Record = { + image: 'Image', + video: 'Video', + audio: 'Audio', + document: 'Document', + spreadsheet: 'Spreadsheet', + presentation: 'Presentation', + pdf: 'PDF', + archive: 'Archive', + text: 'Text', + generic: 'File', +}; + +/** + * A short, human file-type description — "PDF", "PNG image", "XLSX + * spreadsheet", "File" (PLAN-2392 DR-2 / DR-18). + * + * Built on `iconForAttachment` on purpose: the icon and the words beside it + * must never disagree about what a file is, and reading the family from the + * same mapper is the only way to guarantee that. The raw MIME is deliberately + * NOT what surfaces show — `application/vnd.openxmlformats-officedocument. + * spreadsheetml.sheet` is not a type a human reads — but it stays available + * to call sites for a `title`. + * + * The extension is dropped when it merely repeats the family ("PDF · PDF") + * and kept when it adds the specific format ("PNG image"). With neither a + * usable MIME nor an extension the answer is the family fallback, "File" — + * never an empty string, so the line never renders as a stray separator. + */ +export function describeAttachmentType( + mime: string | null | undefined, + filename?: string | null, +): string { + const family = FAMILY_LABELS[iconForAttachment(mime, filename)]; + const ext = extensionOf(filename).toUpperCase(); + if (!ext || ext === family.toUpperCase()) return family; + return `${ext} ${family.toLowerCase()}`; +} + /** * The exact raster types the in-app image viewer may open (PLAN-2392 * DR-16). Deliberately an allowlist and NOT an `image/` prefix test: diff --git a/web/src/lib/components/attachments/AttachmentDetailsPanel.svelte b/web/src/lib/components/attachments/AttachmentDetailsPanel.svelte new file mode 100644 index 00000000..695a2f36 --- /dev/null +++ b/web/src/lib/components/attachments/AttachmentDetailsPanel.svelte @@ -0,0 +1,607 @@ + + + + + {#if view === 'root'} + + + + {#if missing} + + {:else if loadFailed} + + + Retry + {/if} + + {#if actionError} + + {/if} + + + + {#each actions as action (action.id)} + {#if action.element === 'anchor'} + + {action.label} + + {:else} + runAction(action)} + > + {busy && action.id === 'delete' ? 'Deleting…' : action.label} + + {/if} + {/each} + {:else} + + + settleConfirm(false)}>Cancel + + settleConfirm(true)}> + Delete file + + {/if} + + + diff --git a/web/src/lib/components/attachments/AttachmentPanelHost.svelte b/web/src/lib/components/attachments/AttachmentPanelHost.svelte new file mode 100644 index 00000000..9259cefb --- /dev/null +++ b/web/src/lib/components/attachments/AttachmentPanelHost.svelte @@ -0,0 +1,143 @@ + + + + +{#if request} + (request = null)} + /> +{/if} diff --git a/web/src/lib/components/attachments/AttachmentPanelHost.svelte.test.ts b/web/src/lib/components/attachments/AttachmentPanelHost.svelte.test.ts new file mode 100644 index 00000000..144b8c8d --- /dev/null +++ b/web/src/lib/components/attachments/AttachmentPanelHost.svelte.test.ts @@ -0,0 +1,511 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { flushSync, mount, unmount } from 'svelte'; +import type { AttachmentMetadataResult } from '$lib/components/editor/attachment-metadata'; + +// TASK-2423. The options panel is exercised THROUGH its host, because the host +// is where the two rules that matter live: an event is consumed only when both +// `itemId` and `hostToken` are this host's own (DR-8), and the permission the +// panel's Delete uses comes from the host rather than from the emitting +// surface. Nothing routes INTO the panel yet — the strip's tiles and the +// editor's chips start emitting in the next task — so these tests emit on the +// bus directly, which is also the only way to drive a NodeView-originated open. +// +// What jsdom CANNOT prove here, and is therefore phase 3d's browser suite: +// focus entry/return, background inertness, the desktop popover's real +// placement, the mobile sheet swap, and Enter/Space activation of the rows. + +const deleteMock = vi.fn<(ws: string, id: string) => Promise>(); +const toastMock = vi.fn<(message: string, kind?: string) => void>(); + +class FakeApiError extends Error { + code: string; + constructor(code: string) { + super(code); + this.code = code; + } +} + +vi.mock('$lib/api/client', () => ({ + PadApiError: FakeApiError, + api: { + attachments: { + downloadUrl: (ws: string, id: string, variant?: string) => + `/api/v1/workspaces/${ws}/attachments/${id}${variant ? `?variant=${variant}` : ''}`, + delete: (ws: string, id: string) => deleteMock(ws, id), + }, + }, +})); + +// The metadata cache is mocked so a test can hand back each arm of the typed +// result (DR-10) and, crucially, assert WHICH entry point was used: Retry must +// go through `revalidate*` (invalidate-then-fetch), because a plain refetch +// replays the cached failure and looks broken. +const fetchMetaMock = vi.fn<() => Promise>(); +const revalidateMetaMock = vi.fn<() => Promise>(); +const invalidateMetaMock = vi.fn<(ws: string, id: string) => void>(); +vi.mock('$lib/components/editor/attachment-metadata', () => ({ + fetchAttachmentMetadata: () => fetchMetaMock(), + revalidateAttachmentMetadata: () => revalidateMetaMock(), + invalidateAttachmentMetadata: (ws: string, id: string) => invalidateMetaMock(ws, id), +})); + +vi.mock('$lib/stores/toast.svelte', () => ({ + toastStore: { show: (message: string, kind?: string) => toastMock(message, kind) }, +})); + +// The events bus stays REAL — addressing is the thing under test — with only +// the deletion broadcast wrapped so a test can assert the panel announces +// exactly as the strip's tile does. +const announceMock = vi.fn<(ws: string, id: string) => void>(); +vi.mock('$lib/attachments/events', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + announceAttachmentDeleted: (ws: string, id: string) => announceMock(ws, id), + }; +}); + +const { notifyAttachmentPanelOpen } = await import('$lib/attachments/events'); +const { default: AttachmentPanelHost } = await import('./AttachmentPanelHost.svelte'); + +interface HostProps { + wsSlug: string; + itemId: string | null; + hostToken: string; + mutationsEnabled: boolean; + itemContent: string | null; + liveContent: (() => string | null) | null; + parentArchived: boolean; +} + +// A canonical UUID: `attachmentRefsIn` only recognizes the 36-char form, so +// the "still used in this item's content" warning is only exercised with a +// real id. +const ATT_ID = '11111111-2222-4333-8444-555555555555'; +const ATT_ID_2 = '99999999-8888-4777-8666-555555555555'; + +function openEvent(overrides: Partial[0]> = {}) { + return { + attachmentId: ATT_ID, + itemId: 'item-a', + hostToken: 'host-1', + anchor: null, + filename: 'spec.pdf', + mime_type: 'application/pdf', + size_bytes: 1536, + ...overrides, + }; +} + +/** Rows are portaled to , so queries are document-wide by necessity. */ +function panel(): HTMLElement | null { + return document.querySelector('[role="menu"]'); +} + +function rows(): HTMLElement[] { + return Array.from(document.querySelectorAll('[role="menu"] [role="menuitem"]')); +} + +/** By VISIBLE label — MenuItem's icon span is part of `textContent`. */ +function row(label: string): HTMLElement | undefined { + return rows().find((el) => el.querySelector('.mi-label')?.textContent?.trim() === label); +} + +async function settle() { + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + flushSync(); +} + +// Reactive props objects, declared at the top level because `$state(...)` may +// only initialize a declaration. Two of them: the pane host runs a master and a +// peeked ItemDetail at once, and that concurrency is exactly what DR-8's +// addressing exists for. +const propsA = $state({ + wsSlug: 'ws', + itemId: 'item-a', + hostToken: 'host-1', + mutationsEnabled: true, + itemContent: null, + liveContent: null, + parentArchived: false, +}); +const propsB = $state({ + wsSlug: 'ws', + itemId: 'item-a', + hostToken: 'host-2', + mutationsEnabled: false, + itemContent: null, + liveContent: null, + parentArchived: false, +}); + +describe('AttachmentPanelHost', () => { + let target: HTMLElement; + const mounted: ReturnType[] = []; + + beforeEach(() => { + deleteMock.mockReset(); + deleteMock.mockResolvedValue(undefined); + toastMock.mockReset(); + announceMock.mockReset(); + fetchMetaMock.mockReset(); + fetchMetaMock.mockResolvedValue({ status: 'ok', mime: 'application/pdf', size: 2048 }); + revalidateMetaMock.mockReset(); + revalidateMetaMock.mockResolvedValue({ status: 'ok', mime: 'application/pdf', size: 2048 }); + invalidateMetaMock.mockReset(); + Object.assign(propsA, { + wsSlug: 'ws', + itemId: 'item-a', + hostToken: 'host-1', + mutationsEnabled: true, + itemContent: null, + liveContent: null, + parentArchived: false, + }); + Object.assign(propsB, { + wsSlug: 'ws', + itemId: 'item-a', + hostToken: 'host-2', + mutationsEnabled: false, + itemContent: null, + liveContent: null, + parentArchived: false, + }); + target = document.body.appendChild(document.createElement('div')); + }); + + afterEach(() => { + while (mounted.length) unmount(mounted.pop()!); + target.remove(); + }); + + function mountHost(props: HostProps) { + mounted.push(mount(AttachmentPanelHost, { target, props })); + flushSync(); + } + + it('opens for an event addressed to it', () => { + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + flushSync(); + + expect(panel()).not.toBeNull(); + expect(panel()?.textContent).toContain('spec.pdf'); + }); + + it('ignores an event addressed to the OTHER host, with both mounted', () => { + mountHost(propsA); + mountHost(propsB); + + // Same item, other host token: only one panel may open, and it must be + // the addressed one. Matching on itemId alone would open two. + notifyAttachmentPanelOpen(openEvent({ hostToken: 'host-2' })); + flushSync(); + + const panels = document.querySelectorAll('[role="menu"]'); + expect(panels).toHaveLength(1); + // host-2 is the peeked pane in this fixture (mutationsEnabled false), so + // the panel that opened must be the one WITHOUT a live Delete. + expect((row('Delete') as HTMLButtonElement | undefined)?.disabled).toBe(true); + }); + + it('ignores an event for a different item on its own token', () => { + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent({ itemId: 'item-b' })); + flushSync(); + + expect(panel()).toBeNull(); + }); + + it('renders the actions from the shared descriptor list, honouring element', async () => { + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + + // PDF previews natively, so Open applies; both anchors are real + // / elements, not buttons that navigate. + const open = row('Open in new tab'); + expect(open?.tagName).toBe('A'); + expect(open?.getAttribute('target')).toBe('_blank'); + expect(open?.getAttribute('rel')).toBe('noopener noreferrer'); + const download = row('Download'); + expect(download?.tagName).toBe('A'); + expect(download?.getAttribute('href')).toBe(`/api/v1/workspaces/ws/attachments/${ATT_ID}`); + expect(download?.getAttribute('download')).toBe('spec.pdf'); + expect(row('Copy workspace link')?.tagName).toBe('BUTTON'); + expect(row('Delete')?.tagName).toBe('BUTTON'); + }); + + it('omits Open for a type the browser cannot preview', async () => { + mountHost(propsA); + notifyAttachmentPanelOpen( + openEvent({ mime_type: 'application/zip', filename: 'logs.zip' }) + ); + await settle(); + + // Absent, never disabled: a greyed Open implies a preview Pad could + // give and won't. + expect(row('Open in new tab')).toBeUndefined(); + expect(row('Download')).toBeDefined(); + expect(panel()?.textContent).toContain('ZIP archive'); + }); + + it('opens IMMEDIATELY with partial metadata, then completes it', async () => { + let resolveMeta!: (r: AttachmentMetadataResult) => void; + fetchMetaMock.mockReturnValue( + new Promise((r) => (resolveMeta = r)) + ); + mountHost(propsA); + // A chip's HEAD probe may not have completed: all three fields null. + notifyAttachmentPanelOpen( + openEvent({ filename: null, mime_type: null, size_bytes: null }) + ); + flushSync(); + + // Painted before the fetch settles — never a blank sheet, never a wait. + expect(panel()).not.toBeNull(); + expect(panel()?.textContent).toContain('Attachment'); + expect(panel()?.textContent).toContain('Reading details…'); + + resolveMeta({ status: 'ok', mime: 'application/pdf', size: 1024 }); + await settle(); + expect(panel()?.textContent).toContain('PDF'); + expect(panel()?.textContent).toContain('1.0 KB'); + }); + + it('does not fetch when the event carried all three fields', async () => { + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + + // The strip's entry point always has them from its list row. + expect(fetchMetaMock).not.toHaveBeenCalled(); + expect(panel()?.textContent).toContain('1.5 KB'); + }); + + it('shows an inline retryable error on a transient failure, keeping what it knows', async () => { + fetchMetaMock.mockResolvedValue({ status: 'transient' }); + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent({ mime_type: null, size_bytes: null })); + await settle(); + + expect(panel()?.textContent).toContain("Couldn't load the file details."); + // Beside the row it already knows, not instead of it. + expect(panel()?.textContent).toContain('spec.pdf'); + // Actions stay live: transient says NOTHING about whether the row exists. + expect((row('Download') as HTMLElement).tagName).toBe('A'); + + revalidateMetaMock.mockResolvedValue({ status: 'ok', mime: 'application/pdf', size: 4096 }); + row('Retry')!.click(); + await settle(); + + // Retry INVALIDATES before refetching — a plain refetch would replay the + // cached failure (DR-10). + expect(revalidateMetaMock).toHaveBeenCalledTimes(1); + expect(panel()?.textContent).not.toContain("Couldn't load the file details."); + expect(panel()?.textContent).toContain('4.0 KB'); + }); + + it('latches an authoritative missing state and makes every action inert', async () => { + fetchMetaMock.mockResolvedValue({ status: 'missing' }); + mountHost(propsA); + // Size unknown, so the panel fetches; the MIME is known, so Open is in + // the rendered set and its inertness is observable too. + notifyAttachmentPanelOpen(openEvent({ size_bytes: null })); + await settle(); + + expect(panel()?.textContent).toContain('This file is no longer available.'); + expect(panel()?.textContent).toContain('No longer available'); + // A disabled anchor is not a thing, so MenuItem falls back to a disabled + // button — the row is inert AND skipped by the menu's keyboard walk. + for (const label of ['Open in new tab', 'Download', 'Copy workspace link', 'Delete']) { + const el = row(label) as HTMLButtonElement | undefined; + expect(el?.tagName).toBe('BUTTON'); + expect(el?.disabled).toBe(true); + } + }); + + it('takes Delete permission from the HOST: peeked pane cannot, master can', async () => { + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + expect((row('Delete') as HTMLButtonElement).disabled).toBe(false); + + // Peek freezes this side; the event said nothing about permission and + // must not be able to. + propsA.mutationsEnabled = false; + flushSync(); + expect((row('Delete') as HTMLButtonElement).disabled).toBe(true); + }); + + it('deletes through an in-app drill-down confirmation, Cancel first', async () => { + propsA.itemContent = `body with ![x](pad-attachment:${ATT_ID}) inline`; + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + + row('Delete')!.click(); + await settle(); + + // The confirmation is a sub-view of the panel, not a window.confirm. + const prompt = document.querySelector('.ap-note-warn'); + expect(prompt?.getAttribute('role')).toBe('presentation'); + expect(prompt?.textContent).toContain("still used in this item's content"); + const confirmRows = rows(); + const labelOf = (el: HTMLElement) => el.querySelector('.mi-label')?.textContent?.trim(); + expect(labelOf(confirmRows[0])).toBe('Cancel'); + expect(labelOf(confirmRows[confirmRows.length - 1])).toBe('Delete file'); + // The destructive row points back at the prompt, which is otherwise + // never announced. + expect(confirmRows[confirmRows.length - 1].getAttribute('aria-describedby')).toBe( + prompt?.id + ); + expect(deleteMock).not.toHaveBeenCalled(); + + row('Delete file')!.click(); + await settle(); + + expect(deleteMock).toHaveBeenCalledWith('ws', ATT_ID); + // Exactly what the tile does, so the strip and the editor reconcile. + expect(announceMock).toHaveBeenCalledWith('ws', ATT_ID); + expect(panel()).toBeNull(); + }); + + it('warns honestly when the attachment is not referenced in this body', async () => { + propsA.itemContent = 'nothing here'; + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + + row('Delete')!.click(); + await settle(); + expect(document.querySelector('.ap-note-warn')?.textContent).toContain( + "isn't referenced in this item's content" + ); + }); + + it('reads the LIVE editor markdown for the in-use warning, not just the saved body', async () => { + // The persisted body lags the editor, so an image inserted seconds ago + // would otherwise slip past the warning. + propsA.itemContent = 'nothing here'; + propsA.liveContent = () => `just pasted ![x](pad-attachment:${ATT_ID})`; + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + + row('Delete')!.click(); + await settle(); + expect(document.querySelector('.ap-note-warn')?.textContent).toContain( + "still used in this item's content" + ); + }); + + it('cancelling the confirmation sends no request and returns to the actions', async () => { + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + + row('Delete')!.click(); + await settle(); + row('Cancel')!.click(); + await settle(); + + expect(deleteMock).not.toHaveBeenCalled(); + expect(row('Download')).toBeDefined(); + expect(panel()).not.toBeNull(); + }); + + it('surfaces a failed delete inline and leaves the panel open', async () => { + deleteMock.mockRejectedValue(new Error('Network unreachable')); + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + + row('Delete')!.click(); + await settle(); + row('Delete file')!.click(); + await settle(); + + expect(panel()?.textContent).toContain('Network unreachable'); + expect(announceMock).not.toHaveBeenCalled(); + }); + + it('closes an open panel when the parent item is archived (DR-14)', async () => { + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + expect(panel()).not.toBeNull(); + + // An archived parent's attachment fetch returns a generic 404, so an + // open panel would keep offering an Open and a Download that both fail. + // The strip sits outside ItemDetail's keyed lifecycle block, so this is + // added, not inherited. + propsA.parentArchived = true; + await settle(); + expect(panel()).toBeNull(); + }); + + it('revalidates an open panel when the parent item is restored (DR-14)', async () => { + // Opened while the parent was already archived: the fetch 404s, so the + // panel latches the authoritative missing state. + propsA.parentArchived = true; + fetchMetaMock.mockResolvedValue({ status: 'missing' }); + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent({ mime_type: null, size_bytes: null })); + await settle(); + expect(panel()?.textContent).toContain('This file is no longer available.'); + + // Restore does NOT assume the previous state still holds — it re-reads + // through the invalidating path, and the panel comes back to life. + revalidateMetaMock.mockResolvedValue({ status: 'ok', mime: 'application/pdf', size: 1536 }); + propsA.parentArchived = false; + await settle(); + expect(revalidateMetaMock).toHaveBeenCalledTimes(1); + expect(panel()?.textContent).not.toContain('This file is no longer available.'); + expect((row('Download') as HTMLElement).tagName).toBe('A'); + }); + + it('closes when the host switches item', async () => { + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent()); + await settle(); + expect(panel()).not.toBeNull(); + + propsA.itemId = 'item-b'; + await settle(); + expect(panel()).toBeNull(); + }); + + it('re-targets in place when a second attachment is opened, dropping the first state', async () => { + fetchMetaMock.mockResolvedValue({ status: 'missing' }); + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent({ mime_type: null, size_bytes: null })); + await settle(); + expect(panel()?.textContent).toContain('This file is no longer available.'); + + // The panel is NOT re-keyed per attachment, so the previous subject's + // latched state has to be cleared explicitly. + notifyAttachmentPanelOpen( + openEvent({ attachmentId: ATT_ID_2, filename: 'notes.txt', mime_type: 'text/plain', size_bytes: 12 }) + ); + await settle(); + expect(panel()?.textContent).not.toContain('This file is no longer available.'); + expect(panel()?.textContent).toContain('notes.txt'); + expect((row('Download') as HTMLElement).getAttribute('href')).toBe( + `/api/v1/workspaces/ws/attachments/${ATT_ID_2}` + ); + }); + + it('keeps the full filename in the accessible name while the visible row truncates', async () => { + const long = `${'a'.repeat(200)}.pdf`; + mountHost(propsA); + notifyAttachmentPanelOpen(openEvent({ filename: long })); + await settle(); + + // Truncation is a visual affordance, never an information loss (DR-13). + expect(panel()?.getAttribute('aria-label')).toContain(long); + expect(document.querySelector('.ap-name')?.getAttribute('title')).toBe(long); + }); +}); diff --git a/web/src/lib/components/items/ItemDetail.svelte b/web/src/lib/components/items/ItemDetail.svelte index 26dc11a9..48bce3bd 100644 --- a/web/src/lib/components/items/ItemDetail.svelte +++ b/web/src/lib/components/items/ItemDetail.svelte @@ -42,6 +42,7 @@ import ShareDialog from '$lib/components/ShareDialog.svelte'; import CopyItemDialog from '$lib/components/items/CopyItemDialog.svelte'; import ItemAttachmentStrip from '$lib/components/items/ItemAttachmentStrip.svelte'; + import AttachmentPanelHost from '$lib/components/attachments/AttachmentPanelHost.svelte'; import { createAttachmentHostToken } from '$lib/attachments/events'; import { copyToClipboard } from '$lib/utils/clipboard'; import { repairDeadItemLastRoute } from '$lib/collections/paneUrlParams'; @@ -789,6 +790,27 @@ // half of the address changes with the item; the token does not, and does // not need to — the pair is what disambiguates.) const attachmentHostToken = createAttachmentHostToken(); + + /** + * The editor's LIVE markdown, or null when there is no live editor to + * read. Consumed by every attachment surface that warns "this file is + * still used in this item's content" — the strip's tile delete and the + * options panel's (TASK-2423). + * + * The persisted `item.content` lags the editor by design (written on + * flush, not per keystroke), so an image inserted moments ago wouldn't + * trip that warning for exactly the attachment a user is most likely to + * delete by mistake. Callers fall back to `item.content` when this + * returns null. + */ + function liveEditorMarkdown(): string | null { + if (!editorInstance || editorInstance.isDestroyed) return null; + try { + return (editorInstance.storage as any).markdown?.getMarkdown?.() ?? null; + } catch { + return null; + } + } $effect(() => { if (wsSlug && collSlug && itemSlug) { loadData(); @@ -5032,19 +5054,31 @@ hostToken={attachmentHostToken} canDelete={mutationsEnabled} itemContent={itemMatchesRef ? item?.content : null} - liveContent={() => { - // The persisted item.content lags the editor by design (it's - // written on flush, not per keystroke), so an image inserted - // moments ago wouldn't trip the "still used" warning. Read the - // live editor when it's genuinely alive; the strip falls back - // to item.content otherwise. - if (!editorInstance || editorInstance.isDestroyed) return null; - try { - return (editorInstance.storage as any).markdown?.getMarkdown?.() ?? null; - } catch { - return null; - } - }} + liveContent={liveEditorMarkdown} + /> + + +