mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-24 19:32:10 +00:00
Merge branch 'task/2423-details-panel' into feat/attachment-options-panel
This commit is contained in:
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<AttachmentIconId, string> = {
|
||||
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:
|
||||
|
||||
@@ -0,0 +1,607 @@
|
||||
<!--
|
||||
AttachmentDetailsPanel — what a file IS, and what you can do with it
|
||||
(PLAN-2392 DR-2 / DR-6 / DR-10 / DR-13 / DR-18, TASK-2423).
|
||||
|
||||
Tapping a file used to do the most destructive-adjacent thing available:
|
||||
a strip tile was a bare `<a download>`, so one tap put the file in your
|
||||
Downloads folder with no way to see what it was first. This panel is what
|
||||
a tap opens instead — the metadata, then the actions.
|
||||
|
||||
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, portal placement and the sheet's
|
||||
focus handling are the app's existing ones rather than a second
|
||||
implementation of each.
|
||||
|
||||
THE ACTIONS ARE NOT DEFINED HERE. They come from
|
||||
`$lib/attachments/actions` and are rendered from the descriptor list
|
||||
(DR-5) — this component chooses between the anchor and button branches of
|
||||
`MenuItem` on the descriptor's own `element` discriminant, and never calls
|
||||
`run()` on an anchor (the browser performs those; calling both would fire
|
||||
the action twice). Adding an action means adding a descriptor, not editing
|
||||
this file.
|
||||
|
||||
IT OPENS IMMEDIATELY AND COMPLETES THE METADATA AFTER (DR-2, DR-10). The
|
||||
open event's `filename` / `mime_type` / `size_bytes` are nullable by
|
||||
contract: a chip NodeView knows only what its options give it and fills
|
||||
these from an asynchronous HEAD probe that may be incomplete or failed.
|
||||
Awaiting that before opening would make a tap feel broken on a slow
|
||||
connection, so the panel paints what it was handed and fetches the rest
|
||||
itself. The three states are distinguishable, deliberately:
|
||||
|
||||
- `ok` — gaps filled in place.
|
||||
- `missing` — the row is gone (404). AUTHORITATIVE: the panel says so
|
||||
and every action goes inert, rather than offering a
|
||||
Download that will fail.
|
||||
- `transient` — an inline, retryable error BESIDE the row it already
|
||||
knows. Never a blank sheet. Retry goes through
|
||||
`revalidateAttachmentMetadata`, which invalidates before
|
||||
refetching — a plain refetch would replay the cached
|
||||
failure and look broken (DR-10).
|
||||
|
||||
THE DELETE CONFIRMATION IS AN IN-APP DRILL-DOWN (DR-18), copying the item
|
||||
menu's shape exactly: the prompt is `role="presentation"` with an
|
||||
`aria-describedby` back-reference from the destructive row, Cancel comes
|
||||
FIRST so the focus handoff can never land Enter on Delete, and the
|
||||
contextual "still used in this item's content" warning is carried through
|
||||
from the strip. It is wired as the descriptor's `confirmDelete` promise
|
||||
rather than as a bespoke delete path, so the descriptor's own
|
||||
identity-snapshot and permission re-check across the confirmation stay in
|
||||
force.
|
||||
|
||||
SWITCH-SAFETY. The host swaps this component's props from one attachment
|
||||
to another without a `{#key}` remount (a second tap while the panel is
|
||||
open), so every await-then-write is fenced through
|
||||
`$lib/attachments/viewFence` against the (workspace, attachment) pair —
|
||||
the same bug class as the strip's. Read that module's header for why there
|
||||
are three fences.
|
||||
|
||||
NOT HERE: `state_generation` and Undo. Delete behaves exactly like today's
|
||||
tile delete; the generation token and the Undo toast land across all three
|
||||
entry points at once in PLAN-2411 (DR-19).
|
||||
-->
|
||||
<script lang="ts">
|
||||
import { untrack } from 'svelte';
|
||||
import Menu from '$lib/components/common/Menu.svelte';
|
||||
import MenuItem from '$lib/components/common/MenuItem.svelte';
|
||||
import AttachmentIcon from '$lib/attachments/icons/AttachmentIcon.svelte';
|
||||
import {
|
||||
attachmentActionsFor,
|
||||
type AttachmentActionContext,
|
||||
type ButtonAttachmentAction,
|
||||
} from '$lib/attachments/actions';
|
||||
import {
|
||||
describeAttachmentType,
|
||||
formatBytes,
|
||||
iconForAttachment,
|
||||
} from '$lib/attachments/display';
|
||||
import { api } from '$lib/api/client';
|
||||
import {
|
||||
fetchAttachmentMetadata,
|
||||
revalidateAttachmentMetadata,
|
||||
} from '$lib/components/editor/attachment-metadata';
|
||||
import { attachmentRefsIn } from '$lib/utils/commentAttachments';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
import { createFence, createPaintFence, viewIdentity } from '$lib/attachments/viewFence';
|
||||
|
||||
interface Props {
|
||||
open: boolean;
|
||||
wsSlug: string;
|
||||
attachmentId: string;
|
||||
/**
|
||||
* Seed metadata from the open event. All three are NULLABLE by
|
||||
* contract (DR-2) — the strip populates them from its list row, a chip
|
||||
* may have none of them yet.
|
||||
*/
|
||||
filename: string | null;
|
||||
mimeType: string | null;
|
||||
sizeBytes: number | null;
|
||||
/** Element the panel positions against and returns focus to. */
|
||||
anchor: HTMLElement | null;
|
||||
/**
|
||||
* Supplied by the HOST from its own `computeMutationsEnabled(canEdit,
|
||||
* peeking)` — never by the emitting surface, which has no mutation
|
||||
* context (DR-8). Delete is absent-as-disabled without it.
|
||||
*/
|
||||
mutationsEnabled: boolean;
|
||||
/** Persisted item body, for the "still used here" delete warning. */
|
||||
itemContent?: string | null;
|
||||
/**
|
||||
* The editor's LIVE markdown. `itemContent` lags by design (saved on
|
||||
* flush, not per keystroke), so an image inserted seconds ago wouldn't
|
||||
* trip the warning for exactly the attachment a user is most likely to
|
||||
* delete by mistake. Consulted at confirm time only.
|
||||
*/
|
||||
liveContent?: (() => string | null) | null;
|
||||
/**
|
||||
* Bumped by the host to force a fresh metadata read — parent-item
|
||||
* restore revalidates rather than assuming the prior state holds
|
||||
* (DR-14).
|
||||
*/
|
||||
revalidateToken?: number;
|
||||
onclose: () => void;
|
||||
onDeleted?: (attachmentId: string) => void;
|
||||
}
|
||||
|
||||
let {
|
||||
open,
|
||||
wsSlug,
|
||||
attachmentId,
|
||||
filename,
|
||||
mimeType,
|
||||
sizeBytes,
|
||||
anchor,
|
||||
mutationsEnabled,
|
||||
itemContent = null,
|
||||
liveContent = null,
|
||||
revalidateToken = 0,
|
||||
onclose,
|
||||
onDeleted,
|
||||
}: Props = $props();
|
||||
|
||||
const uid = $props.id();
|
||||
const promptId = `attachment-delete-note-${uid}`;
|
||||
|
||||
// What the server told us, filling the gaps in what the event carried.
|
||||
let fetchedMime = $state<string | null>(null);
|
||||
let fetchedSize = $state<number | null>(null);
|
||||
let loading = $state(false);
|
||||
/** 404 — authoritative. Actions go inert. */
|
||||
let missing = $state(false);
|
||||
/** Non-404 failure — inline, retryable, alongside what we already know. */
|
||||
let loadFailed = $state(false);
|
||||
let view = $state<'root' | 'delete'>('root');
|
||||
let busy = $state(false);
|
||||
let actionError = $state<string | null>(null);
|
||||
let deletePrompt = $state('');
|
||||
/** Bumped by Retry; drives the loader effect's forced-revalidate path. */
|
||||
let forceReload = $state(0);
|
||||
|
||||
// --- fences (see $lib/attachments/viewFence) ------------------------------
|
||||
// The identity of what this panel is showing. The PAIR, not the id alone:
|
||||
// the workspace half is what a hand-rolled fence keeps forgetting.
|
||||
const identity = viewIdentity(() => ({ ws: wsSlug, att: attachmentId }));
|
||||
// 1. Request fence — restarted per metadata read, so a Retry supersedes
|
||||
// its own predecessor and only the newest response may write.
|
||||
const loadFence = createFence(identity);
|
||||
// 2. View fence — invalidated only when the panel really changes subject,
|
||||
// so an in-flight delete of the attachment still on screen can still
|
||||
// reconcile even after a Retry reloaded its metadata.
|
||||
const viewFence = createFence(identity);
|
||||
// 3. Paint fence — "does the control the user clicked belong to what is on
|
||||
// screen?" Checked at ENTRY by every control, because the other two run
|
||||
// after an await and no fence can unsend a request.
|
||||
const paint = createPaintFence(identity);
|
||||
|
||||
// Plain `let`, never $state: read and written only inside effects, and a
|
||||
// $state here would make the effect below depend on what it writes
|
||||
// (CONVE-1688 — the self-write loop that silently aborts the flush).
|
||||
let paintedKey: string | null = null;
|
||||
/**
|
||||
* The reload stamp this component has already acted on — the host's
|
||||
* revalidate signal and the local Retry counter together. Seeded from the
|
||||
* incoming prop so a host that has already bumped its counter (an earlier
|
||||
* restore, before this panel existed) doesn't read as a pending reload on
|
||||
* the first render.
|
||||
*/
|
||||
let seenReload = untrack(() => `${revalidateToken}:0`);
|
||||
/** Resolver for the in-app confirmation currently on screen, if any. */
|
||||
let pendingConfirm: ((confirmed: boolean) => void) | null = null;
|
||||
/**
|
||||
* Counts confirmed deletes. `run()` resolves the same way whether the row
|
||||
* was deleted or the user CANCELLED the confirmation, so closing on a
|
||||
* resolved delete would dismiss the panel out from under a cancel — this is
|
||||
* how the two are told apart.
|
||||
*/
|
||||
let deleteSignal = 0;
|
||||
|
||||
const displayName = $derived(filename?.trim() || 'Attachment');
|
||||
// The event's value wins when it has one — it came from a list row, which
|
||||
// is at least as good as a HEAD and is available before any fetch.
|
||||
const mime = $derived(mimeType || fetchedMime || '');
|
||||
const size = $derived(sizeBytes ?? fetchedSize);
|
||||
const iconId = $derived(iconForAttachment(mime || null, filename));
|
||||
const typeLabel = $derived(describeAttachmentType(mime || null, filename));
|
||||
// Always says at least the type — a panel that opened on a chip with no
|
||||
// metadata at all still has to describe SOMETHING while the HEAD is in
|
||||
// flight, and "Reading details…" replaces only the part that is genuinely
|
||||
// unknown rather than the whole line (DR-10).
|
||||
const metaLine = $derived(
|
||||
missing
|
||||
? 'No longer available'
|
||||
: [
|
||||
typeLabel,
|
||||
size !== null && size !== undefined
|
||||
? formatBytes(size)
|
||||
: loading
|
||||
? 'Reading details…'
|
||||
: null,
|
||||
]
|
||||
.filter(Boolean)
|
||||
.join(' · ')
|
||||
);
|
||||
/**
|
||||
* The panel's accessible name carries filename, type and size (DR-12) —
|
||||
* and the FULL filename, unelided: truncation is a visual affordance, never
|
||||
* an information loss (DR-13).
|
||||
*/
|
||||
const panelLabel = $derived(`Options for ${displayName}, ${metaLine}`);
|
||||
|
||||
/**
|
||||
* The action context. Built with GETTERS rather than as a snapshot object:
|
||||
* the delete descriptor deliberately re-reads `mutationsEnabled` and the
|
||||
* attachment identity on the far side of the confirmation, and a frozen
|
||||
* object would make those re-checks read the values as they were when the
|
||||
* confirmation opened — exactly the staleness they exist to catch.
|
||||
*/
|
||||
const ctx: AttachmentActionContext = {
|
||||
get workspaceSlug() {
|
||||
return wsSlug;
|
||||
},
|
||||
get attachment() {
|
||||
return { id: attachmentId, filename: filename ?? '', mime_type: mime };
|
||||
},
|
||||
get mutationsEnabled() {
|
||||
// A row the server says is gone is not deletable, and offering it
|
||||
// would produce a 404 the user can do nothing about.
|
||||
return mutationsEnabled && !missing;
|
||||
},
|
||||
confirmDelete: () => confirmDelete(),
|
||||
onDeleted: (id) => {
|
||||
deleteSignal += 1;
|
||||
onDeleted?.(id);
|
||||
},
|
||||
onCopied: () => toastStore.show('Link copied to clipboard', 'success'),
|
||||
};
|
||||
|
||||
const actions = $derived(attachmentActionsFor(ctx));
|
||||
|
||||
function downloadUrl(uuid: string, variant?: 'thumb-sm' | 'thumb-md' | 'original'): string {
|
||||
return api.attachments.downloadUrl(wsSlug, uuid, variant);
|
||||
}
|
||||
|
||||
/**
|
||||
* Load whatever the event didn't carry, and re-read on demand.
|
||||
*
|
||||
* Reads only props + the fence identity in tracked scope; every piece of
|
||||
* state it writes (`loading`, `missing`, `fetched*`) is read in the markup
|
||||
* and in `untrack`ed blocks only, so the effect cannot self-invalidate.
|
||||
*/
|
||||
$effect(() => {
|
||||
const req = loadFence.restart();
|
||||
const isOpen = open;
|
||||
const seedMime = mimeType;
|
||||
const seedSize = sizeBytes;
|
||||
const reloadStamp = `${revalidateToken}:${forceReload}`;
|
||||
|
||||
let forced = false;
|
||||
untrack(() => {
|
||||
// A genuine subject change: drop everything the previous attachment
|
||||
// left behind, stop any in-flight continuation from reconciling, and
|
||||
// abandon a confirmation that was up for a file the user is no longer
|
||||
// looking at.
|
||||
if (req.key !== paintedKey) {
|
||||
paintedKey = req.key;
|
||||
viewFence.invalidate();
|
||||
settleConfirm(false);
|
||||
fetchedMime = null;
|
||||
fetchedSize = null;
|
||||
missing = false;
|
||||
loadFailed = false;
|
||||
busy = false;
|
||||
actionError = null;
|
||||
}
|
||||
// Whatever this run paints belongs to this (workspace, attachment).
|
||||
// An un-addressable token records nothing, which correctly stops the
|
||||
// panel's controls claiming the previous subject.
|
||||
paint.record(req);
|
||||
if (reloadStamp !== seenReload) {
|
||||
seenReload = reloadStamp;
|
||||
forced = true;
|
||||
}
|
||||
});
|
||||
|
||||
if (!isOpen || req.key === null) return;
|
||||
// Nothing to complete: the strip's entry point always has all three.
|
||||
if (!forced && seedMime && seedSize !== null && seedSize !== undefined) return;
|
||||
|
||||
loading = true;
|
||||
loadFailed = false;
|
||||
void (async () => {
|
||||
// The workspace comes off the TOKEN, not the live prop: the request
|
||||
// must name the workspace it was issued for even if the panel has
|
||||
// since moved on.
|
||||
const result = forced
|
||||
? await revalidateAttachmentMetadata(req.value.ws, req.value.att, downloadUrl)
|
||||
: await fetchAttachmentMetadata(req.value.ws, req.value.att, downloadUrl);
|
||||
if (req.stale()) return;
|
||||
loading = false;
|
||||
if (result.status === 'ok') {
|
||||
fetchedMime = result.mime;
|
||||
fetchedSize = result.size;
|
||||
missing = false;
|
||||
loadFailed = false;
|
||||
} else if (result.status === 'missing') {
|
||||
// Authoritative. Latch it — the actions go inert below.
|
||||
missing = true;
|
||||
loadFailed = false;
|
||||
} else {
|
||||
// Says nothing about whether the row exists: keep showing what we
|
||||
// have and stay retryable.
|
||||
loadFailed = true;
|
||||
}
|
||||
})();
|
||||
});
|
||||
|
||||
function retry() {
|
||||
// ENTRY fence: the clicked row was painted for `paint`'s identity, and
|
||||
// the live props may already name a different attachment.
|
||||
if (!paint.isCurrent()) return;
|
||||
loadFailed = false;
|
||||
// Goes through the loader effect's revalidate path rather than fetching
|
||||
// here, so a user Retry and the host's restore signal (DR-14) are ONE
|
||||
// code path — and both therefore invalidate before refetching, which is
|
||||
// the whole point of Retry (DR-10).
|
||||
forceReload += 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Ids referenced by THIS item's body. A hit means deleting leaves a
|
||||
* missing-attachment placeholder in the content, which the user deserves
|
||||
* to know before confirming. Read at confirm time (not derived) so it sees
|
||||
* unflushed editor edits.
|
||||
*/
|
||||
function referencedHere(): boolean {
|
||||
let live: string | null = null;
|
||||
try {
|
||||
live = liveContent?.() ?? null;
|
||||
} catch {
|
||||
live = null;
|
||||
}
|
||||
return new Set(attachmentRefsIn(live ?? itemContent ?? '')).has(attachmentId);
|
||||
}
|
||||
|
||||
/**
|
||||
* The confirmation, as a promise the delete descriptor awaits. The
|
||||
* descriptor snapshots identity BEFORE this and re-checks permission after
|
||||
* it resolves, which is the whole reason it is wired this way rather than
|
||||
* as a bespoke "confirm, then call the API" path here.
|
||||
*
|
||||
* The "not referenced here" arm deliberately does NOT claim the attachment
|
||||
* is unused, and stays hedged word-for-word with the strip's. Two
|
||||
* independent reasons: a reference can live in another item's content, in
|
||||
* fields JSON, or in any comment — the server's scan covers all three and
|
||||
* none of it is visible client-side — and the body this checks is the
|
||||
* HOST's, which is not necessarily the attachment's parent item. The
|
||||
* open-panel event's `itemId` is ROUTING, not ownership: a chip in a reused
|
||||
* comment composer's unsubmitted draft correctly routes to the host in
|
||||
* front of the user even after an item switch.
|
||||
*/
|
||||
function confirmDelete(): Promise<boolean> {
|
||||
deletePrompt = referencedHere()
|
||||
? `Delete ${displayName}? It's still used in this item's content — deleting it will leave a "missing attachment" placeholder where it appears.`
|
||||
: `Delete ${displayName}? It isn't referenced in this item's content, but it may still be referenced by another item or a comment. This cannot be undone.`;
|
||||
return new Promise<boolean>((resolve) => {
|
||||
// Supersede any confirmation already up — two open at once would
|
||||
// leave one resolver dangling forever.
|
||||
pendingConfirm?.(false);
|
||||
pendingConfirm = resolve;
|
||||
view = 'delete';
|
||||
});
|
||||
}
|
||||
|
||||
function settleConfirm(confirmed: boolean) {
|
||||
const resolve = pendingConfirm;
|
||||
pendingConfirm = null;
|
||||
view = 'root';
|
||||
resolve?.(confirmed);
|
||||
}
|
||||
|
||||
async function runAction(action: ButtonAttachmentAction) {
|
||||
if (!paint.isCurrent()) return;
|
||||
if (!action.enabled(ctx)) return;
|
||||
// Fence 2: a subject change mid-action must not write this action's
|
||||
// outcome onto a DIFFERENT attachment's panel. The request itself still
|
||||
// lands — it targets an id, not a view.
|
||||
const token = viewFence.begin();
|
||||
const deletesBefore = deleteSignal;
|
||||
actionError = null;
|
||||
busy = true;
|
||||
try {
|
||||
await action.run(ctx);
|
||||
if (token.stale()) return;
|
||||
// Only when the row actually went: `run()` also resolves when the
|
||||
// user cancelled the confirmation, and closing then would dismiss
|
||||
// the panel out from under a Cancel.
|
||||
if (deleteSignal !== deletesBefore) onclose();
|
||||
} catch (err) {
|
||||
if (token.stale()) return;
|
||||
actionError = err instanceof Error ? err.message : `Couldn't ${action.label.toLowerCase()}`;
|
||||
} finally {
|
||||
if (!token.stale()) busy = false;
|
||||
}
|
||||
}
|
||||
|
||||
function handleClose() {
|
||||
// A confirmation still on screen when the panel closes is a rejection:
|
||||
// leaving the promise unresolved would strand the descriptor's `await`
|
||||
// forever.
|
||||
settleConfirm(false);
|
||||
onclose();
|
||||
}
|
||||
|
||||
/**
|
||||
* Anchor rows navigate/download by their DEFAULT ACTION, so the close is
|
||||
* deferred to a macrotask. Closing synchronously detaches the `<a>` during
|
||||
* its own click handler, and a detached anchor's navigation is cancelled in
|
||||
* some browsers — the download would silently not happen.
|
||||
*/
|
||||
function closeAfterNavigation() {
|
||||
setTimeout(() => handleClose(), 0);
|
||||
}
|
||||
</script>
|
||||
|
||||
<Menu
|
||||
{open}
|
||||
onclose={handleClose}
|
||||
trigger={anchor ?? undefined}
|
||||
mode="portal"
|
||||
width={272}
|
||||
sheetOnMobile
|
||||
sheetTitle={displayName}
|
||||
ariaLabel={panelLabel}
|
||||
focusKey={`${attachmentId}:${view}`}
|
||||
>
|
||||
{#if view === 'root'}
|
||||
<!-- Header. `role="presentation"`, like the item menu's confirm note:
|
||||
a role="menu" owns menuitem / separator / group children, and this
|
||||
says explicitly that the header is none of them. -->
|
||||
<div class="ap-header" role="presentation">
|
||||
<span class="ap-icon" aria-hidden="true"><AttachmentIcon id={iconId} size={22} /></span>
|
||||
<span class="ap-head-text">
|
||||
<!-- The full name stays in `title` and in the panel's accessible
|
||||
name; the ellipsis is visual only (DR-13). -->
|
||||
<span class="ap-name" title={displayName}>{displayName}</span>
|
||||
<span class="ap-meta" class:ap-meta-missing={missing} title={mime || undefined}>
|
||||
{metaLine}
|
||||
</span>
|
||||
</span>
|
||||
</div>
|
||||
|
||||
{#if missing}
|
||||
<div class="ap-note ap-note-missing" role="presentation">
|
||||
This file is no longer available. It may have been deleted.
|
||||
</div>
|
||||
{:else if loadFailed}
|
||||
<!-- Beside what we already know, never instead of it (DR-10). -->
|
||||
<div class="ap-note ap-note-error" role="presentation">Couldn't load the file details.</div>
|
||||
<MenuItem icon="↻" onclick={retry}>Retry</MenuItem>
|
||||
{/if}
|
||||
|
||||
{#if actionError}
|
||||
<div class="ap-note ap-note-error" role="presentation">{actionError}</div>
|
||||
{/if}
|
||||
|
||||
<div class="menu-divider" role="separator"></div>
|
||||
|
||||
{#each actions as action (action.id)}
|
||||
{#if action.element === 'anchor'}
|
||||
<MenuItem
|
||||
icon={action.icon}
|
||||
href={action.href(ctx)}
|
||||
download={action.download?.(ctx)}
|
||||
target={action.target}
|
||||
rel={action.rel}
|
||||
disabled={!action.enabled(ctx) || missing}
|
||||
onclick={closeAfterNavigation}
|
||||
>
|
||||
{action.label}
|
||||
</MenuItem>
|
||||
{:else}
|
||||
<MenuItem
|
||||
icon={action.icon}
|
||||
danger={action.danger}
|
||||
disabled={!action.enabled(ctx) || missing || busy}
|
||||
onclick={() => runAction(action)}
|
||||
>
|
||||
{busy && action.id === 'delete' ? 'Deleting…' : action.label}
|
||||
</MenuItem>
|
||||
{/if}
|
||||
{/each}
|
||||
{:else}
|
||||
<!--
|
||||
Delete confirmation as a drill-down sub-view (DR-18), the same shape
|
||||
as the item menu's: the prompt is presentational, so it is never
|
||||
announced on its own — hence the aria-describedby back-reference
|
||||
from the destructive row — and Cancel comes FIRST so the focusKey
|
||||
handoff can never land Enter on Delete.
|
||||
-->
|
||||
<div class="ap-note ap-note-warn" role="presentation" id={promptId}>{deletePrompt}</div>
|
||||
<MenuItem icon="‹" onclick={() => settleConfirm(false)}>Cancel</MenuItem>
|
||||
<div class="menu-divider" role="separator"></div>
|
||||
<MenuItem icon="🗑" danger describedBy={promptId} onclick={() => settleConfirm(true)}>
|
||||
Delete file
|
||||
</MenuItem>
|
||||
{/if}
|
||||
</Menu>
|
||||
|
||||
<style>
|
||||
/* Every rule uses LOGICAL properties (DR-13): the panel has to survive a
|
||||
200-character filename and an RTL locale without pushing its actions
|
||||
off-screen. */
|
||||
.ap-header {
|
||||
display: flex;
|
||||
align-items: flex-start;
|
||||
gap: 9px;
|
||||
padding-block: 7px 8px;
|
||||
padding-inline: 9px;
|
||||
}
|
||||
|
||||
.ap-icon {
|
||||
flex: 0 0 auto;
|
||||
color: var(--text-secondary);
|
||||
margin-block-start: 1px;
|
||||
}
|
||||
|
||||
/* min-width: 0 on every flex child holding the filename, or the ellipsis
|
||||
below never engages and the row grows instead. */
|
||||
.ap-head-text {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 2px;
|
||||
min-width: 0;
|
||||
flex: 1 1 auto;
|
||||
}
|
||||
|
||||
.ap-name {
|
||||
min-width: 0;
|
||||
font-size: 13px;
|
||||
font-weight: 600;
|
||||
color: var(--text-primary);
|
||||
white-space: nowrap;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
}
|
||||
|
||||
.ap-meta {
|
||||
min-width: 0;
|
||||
font-size: 11.5px;
|
||||
color: var(--text-muted);
|
||||
white-space: nowrap;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
}
|
||||
|
||||
.ap-meta-missing {
|
||||
color: var(--accent-orange);
|
||||
}
|
||||
|
||||
.ap-note {
|
||||
padding-block: 4px 6px;
|
||||
padding-inline: 9px;
|
||||
font-size: 12px;
|
||||
line-height: 1.35;
|
||||
/* Wraps rather than ellipsizes: the delete prompt carries the filename
|
||||
and must stay readable in full. */
|
||||
overflow-wrap: anywhere;
|
||||
}
|
||||
|
||||
.ap-note-warn {
|
||||
font-weight: 500;
|
||||
color: var(--accent-orange);
|
||||
}
|
||||
|
||||
.ap-note-error {
|
||||
color: var(--accent-red);
|
||||
}
|
||||
|
||||
.ap-note-missing {
|
||||
color: var(--text-muted);
|
||||
}
|
||||
|
||||
.menu-divider {
|
||||
border-block-start: 1px solid var(--border-subtle);
|
||||
margin-block: 5px;
|
||||
margin-inline: 4px;
|
||||
}
|
||||
</style>
|
||||
@@ -0,0 +1,143 @@
|
||||
<!--
|
||||
AttachmentPanelHost — the ONE consumer of the open-panel channel for one
|
||||
`ItemDetail` mount (PLAN-2392 DR-8 / DR-14, TASK-2423).
|
||||
|
||||
The emitters — strip tiles, editor chip NodeViews, comment-composer chips —
|
||||
cannot mount a Svelte component (a Tiptap NodeView is imperative DOM), so
|
||||
they signal through the module-global bus in `$lib/attachments/events`.
|
||||
Something has to own the panel on the other side. That owner is
|
||||
`ItemDetail`, which mounts exactly one of these.
|
||||
|
||||
WHY A SEPARATE COMPONENT rather than a block inside `ItemDetail`: the
|
||||
addressing rule below is the load-bearing part of DR-8 and has to be
|
||||
testable with TWO hosts mounted at once, which is what the pane host
|
||||
actually does at runtime (a master pane plus a peeked pane). Folded into a
|
||||
6,000-line component it would be unreachable by any test. `ItemDetail` is
|
||||
still the host in every sense that matters — it mints the token, it supplies
|
||||
the permission, and it is the only mount site.
|
||||
|
||||
ADDRESSING. A host consumes an event only when BOTH `itemId` and
|
||||
`hostToken` are its own (`isAttachmentPanelEventForHost`). Matching on the
|
||||
item alone is not enough — both panes can show the same item — and matching
|
||||
on the token alone is not enough either, since a host must not open a panel
|
||||
for an attachment belonging to a different item.
|
||||
|
||||
PERMISSION NEVER TRAVELS ON THE EVENT. `mutationsEnabled` is the host's own
|
||||
`computeMutationsEnabled(canEdit, peeking)`. Not the NodeView's (it has no
|
||||
mutation context at all), and not `ItemTimeline`'s `canEdit`, which ignores
|
||||
`peeking` and would let a peeked pane mutate.
|
||||
|
||||
PARENT LIFECYCLE (DR-14). Attachment GET/HEAD rejects an archived parent
|
||||
with a generic 404, while `ItemDetail` keeps the archived item and its
|
||||
attachment surfaces mounted — so an open panel would keep offering an Open
|
||||
and a Download that now fail. Archiving therefore CLOSES the panel, and
|
||||
restoring REVALIDATES it rather than assuming the previous state still
|
||||
holds. Both arrive here declaratively as `parentArchived`, which the host
|
||||
derives from the item it already refetches on the SSE lifecycle events.
|
||||
-->
|
||||
<script lang="ts">
|
||||
import { untrack } from 'svelte';
|
||||
import AttachmentDetailsPanel from './AttachmentDetailsPanel.svelte';
|
||||
import {
|
||||
isAttachmentPanelEventForHost,
|
||||
registerAttachmentPanelListener,
|
||||
type AttachmentPanelOpenEvent,
|
||||
} from '$lib/attachments/events';
|
||||
|
||||
interface Props {
|
||||
wsSlug: string;
|
||||
/** Parent item UUID. Null/undefined while the item is loading or mid-switch. */
|
||||
itemId: string | null | undefined;
|
||||
/** This `ItemDetail` mount's identity on the bus. */
|
||||
hostToken: string;
|
||||
/** The host's own mutation gate — `canEdit && !peeking`. */
|
||||
mutationsEnabled: boolean;
|
||||
/** Persisted item body, for the delete confirmation's contextual warning. */
|
||||
itemContent?: string | null;
|
||||
/** Accessor for the editor's live markdown; see the panel's prop docs. */
|
||||
liveContent?: (() => string | null) | null;
|
||||
/** Whether the parent item is currently archived (DR-14). */
|
||||
parentArchived?: boolean;
|
||||
}
|
||||
|
||||
let {
|
||||
wsSlug,
|
||||
itemId,
|
||||
hostToken,
|
||||
mutationsEnabled,
|
||||
itemContent = null,
|
||||
liveContent = null,
|
||||
parentArchived = false,
|
||||
}: Props = $props();
|
||||
|
||||
let request = $state<AttachmentPanelOpenEvent | null>(null);
|
||||
let revalidateToken = $state(0);
|
||||
|
||||
// Plain `let`, not $state: written and read only inside `untrack`ed effect
|
||||
// bodies. As $state they would make each effect depend on what it writes,
|
||||
// which aborts the flush and strands unrelated reactivity (CONVE-1688).
|
||||
// Seeded from the initial prop DELIBERATELY (hence `untrack`): a host that
|
||||
// mounts on an already-archived item has nothing to close and nothing to
|
||||
// revalidate — only a TRANSITION is a lifecycle event.
|
||||
let wasArchived = untrack(() => parentArchived === true);
|
||||
let lastItemId = '';
|
||||
|
||||
// Subscribe once. `itemId` / `hostToken` are read inside the callback at
|
||||
// EMIT time, so the comparison always uses the host's current address —
|
||||
// deliberately not captured, since this component (like `ItemDetail`) can
|
||||
// outlive an A→B item switch.
|
||||
$effect(() => {
|
||||
return registerAttachmentPanelListener((event) => {
|
||||
if (!isAttachmentPanelEventForHost(event, { itemId, hostToken })) return;
|
||||
request = event;
|
||||
});
|
||||
});
|
||||
|
||||
// A→B item switch: the open panel belongs to the item that is no longer on
|
||||
// screen, and its Delete would be permissioned by the NEW item's gate.
|
||||
$effect(() => {
|
||||
const id = itemId ?? '';
|
||||
untrack(() => {
|
||||
if (id === lastItemId) return;
|
||||
lastItemId = id;
|
||||
request = null;
|
||||
});
|
||||
});
|
||||
|
||||
// Archive closes; restore revalidates (DR-14).
|
||||
$effect(() => {
|
||||
const archived = parentArchived === true;
|
||||
untrack(() => {
|
||||
if (archived === wasArchived) return;
|
||||
wasArchived = archived;
|
||||
if (archived) request = null;
|
||||
else revalidateToken += 1;
|
||||
});
|
||||
});
|
||||
</script>
|
||||
|
||||
<!--
|
||||
The `request?.` guards are load-bearing, not defensive noise: props are
|
||||
getters the child reads LAZILY, and a delete's own continuation reads them
|
||||
again (through the panel's view fence) after `onDeleted` has already nulled
|
||||
`request` — a bare `request.attachmentId` throws there, on the success path.
|
||||
Reading through to an empty id is the right answer for that read: an
|
||||
un-addressable identity fails the fence, which is precisely what should
|
||||
happen to a continuation whose panel is gone.
|
||||
-->
|
||||
{#if request}
|
||||
<AttachmentDetailsPanel
|
||||
open={true}
|
||||
{wsSlug}
|
||||
attachmentId={request?.attachmentId ?? ''}
|
||||
filename={request?.filename ?? null}
|
||||
mimeType={request?.mime_type ?? null}
|
||||
sizeBytes={request?.size_bytes ?? null}
|
||||
anchor={request?.anchor ?? null}
|
||||
{mutationsEnabled}
|
||||
{itemContent}
|
||||
{liveContent}
|
||||
{revalidateToken}
|
||||
onclose={() => (request = null)}
|
||||
/>
|
||||
{/if}
|
||||
@@ -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<void>>();
|
||||
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<AttachmentMetadataResult>>();
|
||||
const revalidateMetaMock = vi.fn<() => Promise<AttachmentMetadataResult>>();
|
||||
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<typeof import('$lib/attachments/events')>();
|
||||
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<Parameters<typeof notifyAttachmentPanelOpen>[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 <body>, so queries are document-wide by necessity. */
|
||||
function panel(): HTMLElement | null {
|
||||
return document.querySelector<HTMLElement>('[role="menu"]');
|
||||
}
|
||||
|
||||
function rows(): HTMLElement[] {
|
||||
return Array.from(document.querySelectorAll<HTMLElement>('[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<HostProps>({
|
||||
wsSlug: 'ws',
|
||||
itemId: 'item-a',
|
||||
hostToken: 'host-1',
|
||||
mutationsEnabled: true,
|
||||
itemContent: null,
|
||||
liveContent: null,
|
||||
parentArchived: false,
|
||||
});
|
||||
const propsB = $state<HostProps>({
|
||||
wsSlug: 'ws',
|
||||
itemId: 'item-a',
|
||||
hostToken: 'host-2',
|
||||
mutationsEnabled: false,
|
||||
itemContent: null,
|
||||
liveContent: null,
|
||||
parentArchived: false,
|
||||
});
|
||||
|
||||
describe('AttachmentPanelHost', () => {
|
||||
let target: HTMLElement;
|
||||
const mounted: ReturnType<typeof mount>[] = [];
|
||||
|
||||
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
|
||||
// <a download>/<a target> 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<AttachmentMetadataResult>((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  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 `;
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -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}
|
||||
/>
|
||||
|
||||
<!-- The attachment options panel's host (PLAN-2392 DR-8 / TASK-2423).
|
||||
ONE per ItemDetail mount, and the only consumer of the open-panel
|
||||
channel for this mount's token — the panel it opens is
|
||||
permissioned by THIS host's `mutationsEnabled`, never by the
|
||||
emitting strip tile / editor chip, and never by the timeline's
|
||||
`canEdit` (which ignores `peeking`).
|
||||
Sits beside the strip, OUTSIDE the {#key itemSlug} block, for the
|
||||
same reason: it must not remount on an A→B switch — it closes the
|
||||
panel itself when `itemId` changes.
|
||||
parentArchived (DR-14): an archived parent's attachment fetch
|
||||
404s, so an open panel would keep offering an Open and a Download
|
||||
that both fail. `isArchived` follows the SSE item_archived /
|
||||
item_restored refetches above, so archive closes the panel and
|
||||
restore revalidates it. -->
|
||||
<AttachmentPanelHost
|
||||
{wsSlug}
|
||||
itemId={itemMatchesRef ? item?.id : null}
|
||||
hostToken={attachmentHostToken}
|
||||
{mutationsEnabled}
|
||||
itemContent={itemMatchesRef ? item?.content : null}
|
||||
liveContent={liveEditorMarkdown}
|
||||
parentArchived={itemMatchesRef && isArchived}
|
||||
/>
|
||||
|
||||
<!-- Content editor — OUTSIDE the {#key itemSlug} above: the collab
|
||||
|
||||
Reference in New Issue
Block a user