mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-25 03:42:06 +00:00
feat(attachments): make the viewer open gate total across every surface (TASK-2431)
DR-16's allowlist gated the image the user CLICKED. That is not a gate.
The timeline built its ←/→ sibling list from every `img[data-attachment-id]`
in the comment body with no MIME consulted at all, while the markdown renderer
emits an `<img>` for any `image/*` — correct for RENDERING, wrong for OPENING.
So a user could open a safe PNG and press Right onto an `image/svg+xml`. The
whole list is now resolved from the CACHED probe metadata and filtered through
`canOpenInViewer`, on both the mouse and the keyboard path; the index is
derived from the clicked attachment's ID rather than its DOM position, because
filtering reindexes everything after the first refusal. An unresolved MIME
fails safe and is retried on a later probe run — no cold-start regression,
since an unprobed thumbnail renders as a placeholder, not an image.
A refused thumbnail is no longer a dead control either: `role="button"`,
`tabindex` and the "View image" name now track the same predicate, so a
filtered-out SVG is not a focus stop whose activation does nothing. That pass
tracks the RENDERED set, not the fetched one — the pane's Activity / Versions
tabs rebuild every comment card without changing `entries`, and the rebuilt
images were left mouse-openable with no keyboard route at all.
The timeline also had NO A→B viewer reset — `lightbox` was cleared on close and
nowhere else — so a workspace switch under the same ref left a viewer up,
rebuilding URLs for the previous workspace's ids. It now clears on a view
change. Both direct mounts are keyed per open, like the bus host's, so the
viewer's untracked capture of its index can never be reused.
`Lightbox` re-states the rule at the point of USE, and FAILS CLOSED: only a
positively allowlisted `mime_type` is viewable, so a null / unresolved one is
not. It is the last thing between a set and a rendered image — the place where
the benefit of the doubt is worth least — and admitting null let an emitter
hand over `[safe, unresolved]` and the user arrow onto the unresolved one. The
producers lose nothing: the strip always has the MIME from its list row, and
the timeline already excludes unresolved entries. The contract for new
producers is therefore to RESOLVE BEFORE EMITTING. The filter is `$derived`
rather than captured, so a record whose MIME resolves to something unsafe
after open, a set replaced under an open viewer, or an entry removed beneath
the position the user navigated to are all re-answered rather than trusted;
the shown index clamps instead of blanking.
`LightboxImage` gains `mime_type`, `filename` and — nullable — `size_bytes`,
`width`, `height`. The dimensions have no reader yet: they land now so phase
3b's pixel-based loading policy need not reopen the event, the host and every
producer. The component's own `{id, alt}` twin is gone; the channel's
declaration is the only one. The strip threads the full row (it had been
dropping `width`/`height` at `StripAttachment` and `size_bytes` at the mapping)
and both producers now pass the invoking element, so focus returns to the tile
rather than relying on the viewer's held-focus fallback.
NOT changed: `isImageMime`. It decides `<img>` vs chip and governs deferred
share-page surfaces; this phase gates the viewer OPEN, not the render.
Tests: a mixed safe/unsafe/unresolved list driven through mouse, keyboard and a
full ←/→ cycle against the REAL viewer (what an arrow key lands on is the
claim); the set changing UNDER an open viewer — resolved-unsafe-after-open,
removed, replaced, appended; and the payload each producer emits, fed unsafe
and unresolved rows rather than only safe ones, since a stub-based payload test
on safe inputs cannot fail when the gate does. One earlier test asserted that
an unresolved MIME OPENS — it pinned the hole open, and is now the test that
it must not. Every guard was mutation-checked; each kills the tests that
cover it.
This commit is contained in:
xarmian
@@ -281,9 +281,19 @@ export interface LightboxImage {
|
||||
* Metadata the viewer may caption with, all NULLABLE for the same reason
|
||||
* the panel's three are: an emitter knows only what its own surface gives
|
||||
* it, and an inline image's HEAD probe may not have completed or may have
|
||||
* failed. Structurally a superset of the `Lightbox` component's own
|
||||
* `LightboxImage` ({id, alt}), so a set built for this channel is passed
|
||||
* straight through to it.
|
||||
* failed, while an upload event carries only four fields
|
||||
* (`UploadedAttachment`).
|
||||
*
|
||||
* `mime_type` is not decoration: it is what lets a CONSUMER re-state the
|
||||
* DR-16 open gate over a whole set rather than trusting the one element
|
||||
* that was clicked (TASK-2431). `width` / `height` are here ahead of any
|
||||
* reader — phase 3b's pixel-based loading policy needs them, and adding
|
||||
* them now costs one nullable field per producer instead of reopening the
|
||||
* event, the host and every producer later.
|
||||
*
|
||||
* This is the ONLY declaration of the shape. `Lightbox.svelte` used to
|
||||
* carry its own `{id, alt}` twin; it now re-exports this one, so the
|
||||
* component's props and the channel's payload cannot drift.
|
||||
*/
|
||||
filename: string | null;
|
||||
mime_type: string | null;
|
||||
|
||||
@@ -7,9 +7,10 @@
|
||||
<script lang="ts">
|
||||
import { untrack } from 'svelte';
|
||||
import { lightboxStubCalls, type LightboxStubCall } from './lightboxStub';
|
||||
import type { LightboxImage } from '$lib/attachments/events';
|
||||
|
||||
interface Props {
|
||||
images: { id: string }[];
|
||||
images: LightboxImage[];
|
||||
index?: number;
|
||||
wsSlug: string;
|
||||
invoker?: HTMLElement | null;
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import type { LightboxImage } from '$lib/attachments/events';
|
||||
|
||||
/**
|
||||
* Recording surface for `LightboxStub.svelte` (TASK-2428).
|
||||
*
|
||||
@@ -7,7 +9,12 @@
|
||||
* proves nothing (Codex round 4 found the click-based version vacuous).
|
||||
*/
|
||||
export interface LightboxStubCall {
|
||||
images: { id: string }[];
|
||||
/**
|
||||
* The FULL records, not `{id}` (TASK-2431): the metadata a producer threads
|
||||
* onto each image — `mime_type` above all — is part of what it must get
|
||||
* right, and a narrower type here would make that unassertable.
|
||||
*/
|
||||
images: LightboxImage[];
|
||||
index: number;
|
||||
wsSlug: string;
|
||||
/** Threaded down by the host since TASK-2429; the viewer owns the restore. */
|
||||
|
||||
@@ -0,0 +1,403 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { flushSync, mount, unmount, tick } from 'svelte';
|
||||
import type {
|
||||
AttachmentListItem,
|
||||
AttachmentListResponse,
|
||||
Comment,
|
||||
TimelineEntry,
|
||||
TimelineResponse,
|
||||
} from '$lib/types';
|
||||
|
||||
/**
|
||||
* WHAT THE PRODUCERS PUT ON EACH IMAGE (PLAN-2392 / TASK-2431).
|
||||
*
|
||||
* The strip and the timeline mount `Lightbox` directly, so what they hand it is
|
||||
* only observable by standing in for the component — hence the stub, and hence
|
||||
* a file of its own (a `vi.mock` is file-scoped, and the behavioural suites for
|
||||
* both components deliberately drive the REAL viewer).
|
||||
*
|
||||
* The claim under test is narrow and unglamorous: every image carries its
|
||||
* `mime_type` and the nullable metadata beside it. `mime_type` is what lets a
|
||||
* consumer re-state the DR-16 gate over a set it did not build, and
|
||||
* `width` / `height` are here for 3b's pixel-based loading policy — no reader
|
||||
* today, which is exactly why a test has to hold them. Dropped fields are
|
||||
* silent: nothing renders them, nothing type-errors (they are nullable), and
|
||||
* the next phase would rediscover the gap by reopening every producer.
|
||||
*/
|
||||
|
||||
vi.mock('$lib/components/common/Lightbox.svelte', async () => ({
|
||||
default: (await import('./fixtures/LightboxStub.svelte')).default,
|
||||
}));
|
||||
|
||||
// ─── strip mocks ────────────────────────────────────────────────────────────
|
||||
|
||||
const listMock = vi.fn<(ws: string, filters: Record<string, unknown>) => Promise<AttachmentListResponse>>();
|
||||
const timelineListMock = vi.fn<(ws: string, slug: string) => Promise<TimelineResponse>>();
|
||||
|
||||
class FakeApiError extends Error {
|
||||
code: string;
|
||||
constructor(code: string) {
|
||||
super(code);
|
||||
this.code = code;
|
||||
}
|
||||
}
|
||||
|
||||
vi.mock('$lib/api/client', () => ({
|
||||
PadApiError: FakeApiError,
|
||||
api: {
|
||||
attachments: {
|
||||
list: (ws: string, filters: Record<string, unknown>) => listMock(ws, filters),
|
||||
downloadUrl: (ws: string, id: string, variant?: string) =>
|
||||
`/api/v1/workspaces/${ws}/attachments/${id}${variant ? `?variant=${variant}` : ''}`,
|
||||
delete: vi.fn(),
|
||||
},
|
||||
timeline: { list: (ws: string, slug: string) => timelineListMock(ws, slug) },
|
||||
comments: { create: vi.fn(), update: vi.fn(), delete: vi.fn() },
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('$lib/stores/toast.svelte', () => ({
|
||||
toastStore: { show: vi.fn() },
|
||||
}));
|
||||
|
||||
vi.mock('$lib/services/sse.svelte', () => ({
|
||||
sseService: { onItemEvent: () => () => {} },
|
||||
}));
|
||||
|
||||
vi.mock('$lib/stores/auth.svelte', () => ({
|
||||
authStore: { userId: 'user-1', user: { id: 'user-1', role: 'member' } },
|
||||
}));
|
||||
|
||||
vi.mock('$lib/stores/workspace.svelte', () => ({
|
||||
workspaceStore: { canEditItem: () => false },
|
||||
}));
|
||||
|
||||
const PNG = '11111111-1111-4111-8111-111111111111';
|
||||
const SVG = '22222222-2222-4222-8222-222222222222';
|
||||
/** Resolves to nothing — the unresolved-MIME case the set must exclude. */
|
||||
const UNPROBED = '33333333-3333-4333-8333-333333333333';
|
||||
|
||||
vi.mock('$lib/components/editor/attachment-metadata', () => ({
|
||||
fetchAttachmentMetadata: (_ws: string, uuid: string) =>
|
||||
Promise.resolve(
|
||||
uuid === PNG
|
||||
? { status: 'ok' as const, mime: 'image/png', size: 4096 }
|
||||
: uuid === SVG
|
||||
? { status: 'ok' as const, mime: 'image/svg+xml', size: 512 }
|
||||
: { status: 'transient' as const }
|
||||
),
|
||||
invalidateAttachmentMetadata: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('$lib/components/CommentEditor.svelte', async () => ({
|
||||
default: (await import('../timeline/fixtures/InertCommentEditor.svelte')).default,
|
||||
}));
|
||||
|
||||
const { lightboxStubCalls } = await import('./fixtures/lightboxStub');
|
||||
const { default: ItemAttachmentStrip } = await import('../items/ItemAttachmentStrip.svelte');
|
||||
const { default: ItemTimeline } = await import('../timeline/ItemTimeline.svelte');
|
||||
|
||||
let target: HTMLElement;
|
||||
let instance: ReturnType<typeof mount> | undefined;
|
||||
|
||||
beforeEach(() => {
|
||||
lightboxStubCalls.length = 0;
|
||||
listMock.mockReset();
|
||||
timelineListMock.mockReset();
|
||||
target = document.body.appendChild(document.createElement('div'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (instance) unmount(instance);
|
||||
instance = undefined;
|
||||
target.remove();
|
||||
});
|
||||
|
||||
async function settle() {
|
||||
for (let i = 0; i < 8; i++) {
|
||||
await tick();
|
||||
flushSync();
|
||||
}
|
||||
}
|
||||
|
||||
describe('strip → viewer payload (TASK-2431)', () => {
|
||||
function row(over: Partial<AttachmentListItem> & { id: string }): AttachmentListItem {
|
||||
return {
|
||||
workspace_id: 'ws-1',
|
||||
uploaded_by: 'u-1',
|
||||
storage_key: `key/${over.id}`,
|
||||
content_hash: `hash-${over.id}`,
|
||||
mime_type: 'image/png',
|
||||
size_bytes: 2048,
|
||||
filename: `${over.id}.png`,
|
||||
created_at: '2026-08-01T00:00:00Z',
|
||||
...over,
|
||||
};
|
||||
}
|
||||
|
||||
it('threads the full row — mime, size and dimensions — onto every image', async () => {
|
||||
listMock.mockResolvedValue({
|
||||
attachments: [row({ id: 'img1', size_bytes: 9001, width: 800, height: 600 })],
|
||||
total: 1,
|
||||
limit: 50,
|
||||
offset: 0,
|
||||
});
|
||||
instance = mount(ItemAttachmentStrip, {
|
||||
target,
|
||||
props: {
|
||||
wsSlug: 'ws',
|
||||
username: 'dave',
|
||||
itemId: 'item-a',
|
||||
canDelete: false,
|
||||
hostToken: 'host-1',
|
||||
},
|
||||
});
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
const tile = target.querySelector<HTMLElement>('.att-tile')!;
|
||||
tile.click();
|
||||
flushSync();
|
||||
|
||||
expect(lightboxStubCalls).toHaveLength(1);
|
||||
expect(lightboxStubCalls[0].images).toEqual([
|
||||
{
|
||||
id: 'img1',
|
||||
alt: 'img1.png',
|
||||
filename: 'img1.png',
|
||||
mime_type: 'image/png',
|
||||
size_bytes: 9001,
|
||||
width: 800,
|
||||
height: 600,
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it('passes the tile itself as the invoker, so focus returns to it', async () => {
|
||||
listMock.mockResolvedValue({
|
||||
attachments: [row({ id: 'img1' })],
|
||||
total: 1,
|
||||
limit: 50,
|
||||
offset: 0,
|
||||
});
|
||||
instance = mount(ItemAttachmentStrip, {
|
||||
target,
|
||||
props: {
|
||||
wsSlug: 'ws',
|
||||
username: 'dave',
|
||||
itemId: 'item-a',
|
||||
canDelete: false,
|
||||
hostToken: 'host-1',
|
||||
},
|
||||
});
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
const tile = target.querySelector<HTMLElement>('.att-tile')!;
|
||||
tile.click();
|
||||
flushSync();
|
||||
|
||||
// The viewer's own fallback ("whatever held focus at open") cannot stand
|
||||
// in for this: a click does not focus a button on every engine, and by
|
||||
// the time the viewer restores, focus may have been anywhere.
|
||||
expect(lightboxStubCalls[0].invoker).toBe(tile);
|
||||
});
|
||||
|
||||
it('emits ONLY allowlisted rows, whatever else the item holds', async () => {
|
||||
// The payload assertions above are all safe inputs, so they cannot catch
|
||||
// a gate regression on their own — this one feeds the producer an SVG, an
|
||||
// undecodable raster and a row with no MIME at all, and pins what comes
|
||||
// out the other side (Codex: stub-based payload tests with only safe
|
||||
// inputs prove nothing about the gate).
|
||||
listMock.mockResolvedValue({
|
||||
attachments: [
|
||||
row({ id: 'svg', mime_type: 'image/svg+xml', filename: 'logo.svg' }),
|
||||
row({ id: 'img1' }),
|
||||
row({ id: 'tiff', mime_type: 'image/tiff', filename: 'scan.tiff' }),
|
||||
row({ id: 'blank', mime_type: '', filename: 'mystery' }),
|
||||
row({ id: 'img2' }),
|
||||
],
|
||||
total: 5,
|
||||
limit: 50,
|
||||
offset: 0,
|
||||
});
|
||||
instance = mount(ItemAttachmentStrip, {
|
||||
target,
|
||||
props: {
|
||||
wsSlug: 'ws',
|
||||
username: 'dave',
|
||||
itemId: 'item-a',
|
||||
canDelete: false,
|
||||
hostToken: 'host-1',
|
||||
},
|
||||
});
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
// Only the two PNGs render as IMAGE tiles (a thumbnail inside the
|
||||
// button); the rest take the file branch, which is the same `.att-tile`
|
||||
// class with an icon and opens the options panel instead.
|
||||
const tiles = Array.from(target.querySelectorAll<HTMLElement>('.att-tile'));
|
||||
expect(tiles).toHaveLength(5);
|
||||
const imageTiles = tiles.filter((t) => t.querySelector('img') !== null);
|
||||
expect(imageTiles).toHaveLength(2);
|
||||
imageTiles[0].click();
|
||||
flushSync();
|
||||
|
||||
expect(lightboxStubCalls).toHaveLength(1);
|
||||
expect(lightboxStubCalls[0].images.map((im) => im.id)).toEqual(['img1', 'img2']);
|
||||
expect(lightboxStubCalls[0].images.every((im) => im.mime_type === 'image/png')).toBe(true);
|
||||
// Opened on the clicked one, at its position in the FILTERED set.
|
||||
expect(lightboxStubCalls[0].index).toBe(0);
|
||||
});
|
||||
|
||||
it('leaves dimensions null when the row has none, rather than inventing them', async () => {
|
||||
listMock.mockResolvedValue({
|
||||
attachments: [row({ id: 'img1', width: null, height: null })],
|
||||
total: 1,
|
||||
limit: 50,
|
||||
offset: 0,
|
||||
});
|
||||
instance = mount(ItemAttachmentStrip, {
|
||||
target,
|
||||
props: {
|
||||
wsSlug: 'ws',
|
||||
username: 'dave',
|
||||
itemId: 'item-a',
|
||||
canDelete: false,
|
||||
hostToken: 'host-1',
|
||||
},
|
||||
});
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
target.querySelector<HTMLElement>('.att-tile')!.click();
|
||||
flushSync();
|
||||
|
||||
expect(lightboxStubCalls[0].images[0].width).toBeNull();
|
||||
expect(lightboxStubCalls[0].images[0].height).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('timeline → viewer payload (TASK-2431)', () => {
|
||||
function timelineResponse(): TimelineResponse {
|
||||
const comment: Comment = {
|
||||
id: 'c1',
|
||||
item_id: 'item-a',
|
||||
workspace_id: 'ws-1',
|
||||
author: 'alice',
|
||||
body: ``,
|
||||
created_by: 'alice',
|
||||
source: 'web',
|
||||
created_at: '2026-01-01T00:00:00Z',
|
||||
updated_at: '2026-01-01T00:00:00Z',
|
||||
};
|
||||
const entry: TimelineEntry = {
|
||||
id: 'e1',
|
||||
kind: 'comment',
|
||||
created_at: comment.created_at,
|
||||
actor: 'alice',
|
||||
source: 'web',
|
||||
comment,
|
||||
};
|
||||
return { entries: [entry], has_more: false };
|
||||
}
|
||||
|
||||
it('emits ONLY resolved, allowlisted images — never an SVG or an unprobed id', async () => {
|
||||
// Same reason as the strip's: a stub fed only safe inputs cannot fail
|
||||
// when the gate does. The body embeds a PNG, an SVG and an id whose probe
|
||||
// never resolves; the SVG renders as an <img> (the renderer emits one for
|
||||
// any image/*, deliberately) and must still be absent from the set.
|
||||
const comment: Comment = {
|
||||
id: 'c1',
|
||||
item_id: 'item-a',
|
||||
workspace_id: 'ws-1',
|
||||
author: 'alice',
|
||||
body: `\n\n\n\n`,
|
||||
created_by: 'alice',
|
||||
source: 'web',
|
||||
created_at: '2026-01-01T00:00:00Z',
|
||||
updated_at: '2026-01-01T00:00:00Z',
|
||||
};
|
||||
timelineListMock.mockResolvedValue({
|
||||
entries: [
|
||||
{
|
||||
id: 'e1',
|
||||
kind: 'comment',
|
||||
created_at: comment.created_at,
|
||||
actor: 'alice',
|
||||
source: 'web',
|
||||
comment,
|
||||
},
|
||||
],
|
||||
has_more: false,
|
||||
});
|
||||
instance = mount(ItemTimeline, {
|
||||
target,
|
||||
props: {
|
||||
wsSlug: 'ws',
|
||||
username: 'alice',
|
||||
itemSlug: 'TASK-1',
|
||||
currentContent: '',
|
||||
itemId: 'item-a',
|
||||
collectionId: 'coll-1',
|
||||
},
|
||||
});
|
||||
await settle();
|
||||
|
||||
const rendered = Array.from(target.querySelectorAll<HTMLElement>('img[data-attachment-id]'));
|
||||
// The SVG IS rendered — if it ever stops being, this test goes vacuous.
|
||||
expect(rendered.map((el) => el.getAttribute('data-attachment-id'))).toContain(SVG);
|
||||
|
||||
const png = rendered.find((el) => el.getAttribute('data-attachment-id') === PNG)!;
|
||||
png.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
|
||||
flushSync();
|
||||
|
||||
expect(lightboxStubCalls).toHaveLength(1);
|
||||
expect(lightboxStubCalls[0].images.map((im) => im.id)).toEqual([PNG]);
|
||||
|
||||
// ...and the SVG opens nothing at all.
|
||||
lightboxStubCalls.length = 0;
|
||||
const svg = rendered.find((el) => el.getAttribute('data-attachment-id') === SVG)!;
|
||||
svg.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
|
||||
flushSync();
|
||||
expect(lightboxStubCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('carries the probed mime and size, nulls what HEAD cannot know, and names the invoker', async () => {
|
||||
timelineListMock.mockResolvedValue(timelineResponse());
|
||||
instance = mount(ItemTimeline, {
|
||||
target,
|
||||
props: {
|
||||
wsSlug: 'ws',
|
||||
username: 'alice',
|
||||
itemSlug: 'TASK-1',
|
||||
currentContent: '',
|
||||
itemId: 'item-a',
|
||||
collectionId: 'coll-1',
|
||||
},
|
||||
});
|
||||
await settle();
|
||||
|
||||
const thumb = target.querySelector<HTMLElement>('img[data-attachment-id]')!;
|
||||
thumb.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
|
||||
flushSync();
|
||||
|
||||
expect(lightboxStubCalls).toHaveLength(1);
|
||||
expect(lightboxStubCalls[0].images).toEqual([
|
||||
{
|
||||
id: PNG,
|
||||
alt: 'a diagram',
|
||||
// The probe deliberately stores no filename (the alt text is the
|
||||
// label), and a HEAD response carries no intrinsic dimensions —
|
||||
// null is the honest answer, not a placeholder.
|
||||
filename: null,
|
||||
mime_type: 'image/png',
|
||||
size_bytes: 4096,
|
||||
width: null,
|
||||
height: null,
|
||||
},
|
||||
]);
|
||||
expect(lightboxStubCalls[0].invoker).toBe(thumb);
|
||||
});
|
||||
});
|
||||
@@ -28,11 +28,28 @@
|
||||
VIEWER_ROOT_CLASS,
|
||||
} from '$lib/a11y/viewerBackdrop';
|
||||
import { pushEscapeHandler, ESCAPE_PRIORITY } from '$lib/stores/escapeStack';
|
||||
|
||||
export interface LightboxImage {
|
||||
id: string;
|
||||
alt: string;
|
||||
}
|
||||
import { canOpenInViewer } from '$lib/attachments/display';
|
||||
/**
|
||||
* ONE definition of what an image in this viewer is (PLAN-2392 / TASK-2431).
|
||||
*
|
||||
* It used to be declared here as `{id, alt}` and again — as a superset — on
|
||||
* the open-viewer channel, with a comment noting the two were structurally
|
||||
* compatible. Two declarations of the same thing drift the moment one gains
|
||||
* a field, which is exactly what TASK-2431 does (`mime_type` is what makes
|
||||
* the DR-16 open gate total, and `size_bytes` / `width` / `height` land now
|
||||
* so phase 3b's pixel-based loading policy need not reopen the event, the
|
||||
* host and every producer). The channel's is now the only one.
|
||||
*
|
||||
* Everything past `id` / `alt` is NULLABLE and this component treats it as
|
||||
* such: an inline image's metadata comes from a HEAD probe that may not have
|
||||
* completed, and an upload event carries only four fields.
|
||||
*
|
||||
* Producers that mount this component directly import the type from the
|
||||
* CHANNEL too, not from here — a `.svelte` module cannot re-export a type,
|
||||
* and a local `interface … extends` would be a second declaration, which is
|
||||
* the drift this consolidation removes.
|
||||
*/
|
||||
import type { LightboxImage } from '$lib/attachments/events';
|
||||
|
||||
interface Props {
|
||||
images: LightboxImage[];
|
||||
@@ -51,11 +68,62 @@
|
||||
|
||||
let { images, index = 0, wsSlug, onClose, invoker = null }: Props = $props();
|
||||
|
||||
/**
|
||||
* THE LAST-MILE GATE (PLAN-2392 DR-16 / TASK-2431).
|
||||
*
|
||||
* Every producer filters its own set before opening — that is where the
|
||||
* gate belongs, because only the producer knows which of its rows are
|
||||
* images at all. This is the same rule re-stated at the point of USE, and it
|
||||
* is not redundant with them:
|
||||
*
|
||||
* - `←/→` page through a set the producer chose ONCE. A producer's filter
|
||||
* is a statement about the moment it built the list; this one has to hold
|
||||
* for every frame the viewer shows.
|
||||
* - the set is `readonly` on the channel but a plain array as a prop, and a
|
||||
* producer that mutates it — or a record inside it — after emitting is
|
||||
* not a hypothesis this component can rule out.
|
||||
* - a producer added later inherits the rule instead of having to know it.
|
||||
*
|
||||
* IT FAILS CLOSED. Only a POSITIVELY allowlisted `mime_type` is viewable; a
|
||||
* null / undefined / unresolved one is not. "Not yet known" is not evidence
|
||||
* that a file is a PNG, and this is the last thing standing between a set
|
||||
* and a rendered image — the place where the benefit of the doubt is worth
|
||||
* least. An earlier revision admitted null on the grounds that an inline
|
||||
* image's probe is often unasked at click time; that reasoning belongs to
|
||||
* the PRODUCER, which can wait for the probe, and it had the effect of
|
||||
* letting an emitter hand over `[safe, unresolved]` and letting the user
|
||||
* arrow onto the unresolved one. The producers that exist today lose
|
||||
* nothing: the strip always has the MIME from its list row, and the timeline
|
||||
* already excludes unresolved entries from the set it builds.
|
||||
*
|
||||
* THE CONTRACT FOR NEW PRODUCERS, therefore: resolve the MIME before you
|
||||
* emit. Passing a possibly-null value is not "let the viewer decide", it is
|
||||
* an image that silently will not open.
|
||||
*
|
||||
* DERIVED, NOT CAPTURED. Every other open-time value here is `untrack`ed
|
||||
* because the props are constant for the instance's life — but that is a
|
||||
* claim about the CURRENT producers, and this filter is exactly the thing
|
||||
* that must not rest on one. As a `$derived` it re-runs when the array is
|
||||
* replaced or when a record's MIME resolves to something unsafe after the
|
||||
* viewer opened, so a stale capture cannot outlive its own truth.
|
||||
*/
|
||||
let viewable = $derived(images.filter((im) => canOpenInViewer(im.mime_type)));
|
||||
|
||||
// Seeded once at mount — the host remounts (null → set) on each open, so
|
||||
// no prop-sync effect is needed. untrack makes the initial-value capture
|
||||
// explicit (props are constant for this component's lifetime).
|
||||
// explicit.
|
||||
//
|
||||
// Resolved through the ID rather than carried across as a number: filtering
|
||||
// reindexes everything after a refusal, so the requested POSITION can name a
|
||||
// different image (or none) in the filtered set. Where the requested image
|
||||
// is the one refused, there is nothing to land on and the first viewable
|
||||
// image is what opens.
|
||||
let current = $state(
|
||||
untrack(() => Math.min(Math.max(index, 0), Math.max(images.length - 1, 0)))
|
||||
untrack(() => {
|
||||
const wanted = images[Math.min(Math.max(index, 0), Math.max(images.length - 1, 0))];
|
||||
const at = wanted ? viewable.findIndex((im) => im.id === wanted.id) : -1;
|
||||
return at < 0 ? 0 : at;
|
||||
})
|
||||
);
|
||||
|
||||
// CAPTURED AT OPEN, never read live (TASK-2429). The pane switches workspace
|
||||
@@ -79,8 +147,20 @@
|
||||
return active && active !== document.body ? (active as HTMLElement) : null;
|
||||
});
|
||||
|
||||
let hasMultiple = $derived(images.length > 1);
|
||||
let img = $derived(images[current]);
|
||||
// EVERYTHING PAST THIS POINT READS `viewable`, NEVER `images` — the nav
|
||||
// wrap-around, the counter and the rendered `<img>` alike. A single read of
|
||||
// the unfiltered prop below would reopen the hole this filter closes.
|
||||
let hasMultiple = $derived(viewable.length > 1);
|
||||
// The position actually shown, clamped. `current` is what the user's ←/→
|
||||
// moved, but the set can SHRINK underneath it — a record whose MIME resolves
|
||||
// to something unsafe after open drops out of `viewable`, and the array can
|
||||
// be replaced outright. Clamping in a derived (rather than writing `current`
|
||||
// from an effect, which would be an effect writing state it reads) keeps the
|
||||
// viewer on a real member instead of blanking or showing `undefined`.
|
||||
let shownIndex = $derived(
|
||||
Math.min(Math.max(current, 0), Math.max(viewable.length - 1, 0))
|
||||
);
|
||||
let img = $derived(viewable[shownIndex]);
|
||||
let src = $derived(img ? attachmentDownloadUrl(openWsSlug, img.id) : '');
|
||||
// The accessible name: the image's own alt where there is one, else a
|
||||
// generic label. Never empty — an unnamed `role="dialog"` is announced as
|
||||
@@ -92,11 +172,18 @@
|
||||
// a flush (CONVE-1688).
|
||||
let rootEl = $state<HTMLElement | null>(null);
|
||||
|
||||
// Stepped from `shownIndex`, not from `current`: after the set shrinks they
|
||||
// differ, and moving from the raw value would jump relative to a position
|
||||
// the user was never on. Both are no-ops on an empty set — reachable only
|
||||
// through the nav controls / arrow keys, which `hasMultiple` already hides
|
||||
// and gates, but written so the modulo can never be `% 0`.
|
||||
function prev() {
|
||||
current = (current - 1 + images.length) % images.length;
|
||||
if (viewable.length === 0) return;
|
||||
current = (shownIndex - 1 + viewable.length) % viewable.length;
|
||||
}
|
||||
function next() {
|
||||
current = (current + 1) % images.length;
|
||||
if (viewable.length === 0) return;
|
||||
current = (shownIndex + 1) % viewable.length;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -307,7 +394,9 @@
|
||||
{/if}
|
||||
|
||||
{#if hasMultiple}
|
||||
<div class="lightbox-counter">{current + 1} / {images.length}</div>
|
||||
<!-- `shownIndex`, so the counter names the image actually on screen even
|
||||
after the set shrank under `current`. -->
|
||||
<div class="lightbox-counter">{shownIndex + 1} / {viewable.length}</div>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { flushSync, mount, unmount } from 'svelte';
|
||||
import Lightbox from './Lightbox.svelte';
|
||||
import type { LightboxImage } from '$lib/attachments/events';
|
||||
import {
|
||||
acquire,
|
||||
hasForeignEscapeOwner,
|
||||
@@ -42,8 +43,29 @@ const realGetClientRects = HTMLElement.prototype.getClientRects;
|
||||
const IMG_A = 'aaaaaaaa-1111-4111-8111-aaaaaaaaaaaa';
|
||||
const IMG_B = 'bbbbbbbb-2222-4222-8222-bbbbbbbbbbbb';
|
||||
|
||||
const IMG_C = 'cccccccc-3333-4333-8333-cccccccccccc';
|
||||
|
||||
/**
|
||||
* A member of the viewer's set. `mime_type` defaults to an ALLOWLISTED type,
|
||||
* because the gate fails closed: a record without one is not viewable, so a
|
||||
* null default would silently empty the viewer for every case in this file
|
||||
* that is about something else. Pass `null` explicitly to test the unresolved
|
||||
* case.
|
||||
*/
|
||||
function image(id: string, alt: string, mime: string | null = 'image/png'): LightboxImage {
|
||||
return {
|
||||
id,
|
||||
alt,
|
||||
filename: null,
|
||||
mime_type: mime,
|
||||
size_bytes: null,
|
||||
width: null,
|
||||
height: null,
|
||||
};
|
||||
}
|
||||
|
||||
interface Props {
|
||||
images: { id: string; alt: string }[];
|
||||
images: LightboxImage[];
|
||||
index?: number;
|
||||
wsSlug: string;
|
||||
onClose: () => void;
|
||||
@@ -53,7 +75,7 @@ interface Props {
|
||||
// Reactive props for the capture-at-open cases ($state may only initialize a
|
||||
// declaration, hence top level).
|
||||
const liveProps = $state<Props>({
|
||||
images: [{ id: IMG_A, alt: 'a diagram' }],
|
||||
images: [image(IMG_A, 'a diagram')],
|
||||
index: 0,
|
||||
wsSlug: 'ws-one',
|
||||
onClose: () => {},
|
||||
@@ -67,7 +89,7 @@ function mountViewer(props: Partial<Props> = {}): ReturnType<typeof mount> {
|
||||
const app = mount(Lightbox, {
|
||||
target: appRoot,
|
||||
props: {
|
||||
images: [{ id: IMG_A, alt: 'a diagram' }],
|
||||
images: [image(IMG_A, 'a diagram')],
|
||||
index: 0,
|
||||
wsSlug: 'ws-one',
|
||||
onClose: () => {},
|
||||
@@ -115,7 +137,7 @@ beforeEach(() => {
|
||||
return [{}] as unknown as DOMRectList;
|
||||
};
|
||||
Object.assign(liveProps, {
|
||||
images: [{ id: IMG_A, alt: 'a diagram' }],
|
||||
images: [image(IMG_A, 'a diagram')],
|
||||
index: 0,
|
||||
wsSlug: 'ws-one',
|
||||
onClose: () => {},
|
||||
@@ -144,7 +166,7 @@ describe('Lightbox — dialog semantics', () => {
|
||||
it('falls back to a generic name when the image has no alt', () => {
|
||||
// An unnamed dialog is announced as nothing at all, so the fallback is
|
||||
// part of the contract rather than a nicety.
|
||||
mountViewer({ images: [{ id: IMG_A, alt: '' }] });
|
||||
mountViewer({ images: [image(IMG_A, '')] });
|
||||
expect(root().getAttribute('aria-label')).toBe('Attachment viewer');
|
||||
});
|
||||
|
||||
@@ -155,8 +177,8 @@ describe('Lightbox — dialog semantics', () => {
|
||||
// addresses surfaces BY NAME) would have nothing to target.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
expect(closeButton().getAttribute('aria-label')).toBe('Close');
|
||||
@@ -171,8 +193,8 @@ describe('Lightbox — dialog semantics', () => {
|
||||
it('names itself after the image CURRENTLY shown, not the one it opened on', () => {
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
expect(root().getAttribute('aria-label')).toBe('first');
|
||||
@@ -182,6 +204,196 @@ describe('Lightbox — dialog semantics', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — the last-mile open gate (TASK-2431)', () => {
|
||||
/**
|
||||
* The producers filter their own sets, and these do not replace that. They
|
||||
* cover the case a producer's filter structurally cannot: the set is
|
||||
* captured at open, and ←/→ page through it for as long as the viewer is
|
||||
* up. Anything that arrives in the array — a stale capture, a future
|
||||
* producer that forgets — must still be unreachable frame by frame.
|
||||
*/
|
||||
function shown(): string {
|
||||
return root().querySelector<HTMLImageElement>('.lightbox-image')?.getAttribute('alt') ?? '';
|
||||
}
|
||||
|
||||
it('never shows a known non-allowlisted type, even when asked to open ON it', () => {
|
||||
mountViewer({
|
||||
images: [image(IMG_A, 'png', 'image/png'), image(IMG_B, 'svg', 'image/svg+xml')],
|
||||
index: 1,
|
||||
});
|
||||
|
||||
// The requested image is refused, so the viewer opens on what is left
|
||||
// rather than on the SVG — and with one member, there is no ←/→ at all.
|
||||
expect(shown()).toBe('png');
|
||||
expect(imageSrc()).toContain(IMG_A);
|
||||
expect(root().querySelector('.lightbox-counter')).toBeNull();
|
||||
expect(root().querySelector('.lightbox-nav')).toBeNull();
|
||||
});
|
||||
|
||||
it('cannot be paged onto one with ←/→', () => {
|
||||
mountViewer({
|
||||
images: [
|
||||
image(IMG_A, 'png', 'image/png'),
|
||||
image(IMG_B, 'svg', 'image/svg+xml'),
|
||||
image(IMG_C, 'jpeg', 'image/jpeg'),
|
||||
],
|
||||
});
|
||||
|
||||
expect(root().querySelector('.lightbox-counter')?.textContent).toBe('1 / 2');
|
||||
|
||||
const seen = [shown()];
|
||||
for (let i = 0; i < 3; i++) {
|
||||
press('ArrowRight');
|
||||
seen.push(shown());
|
||||
}
|
||||
press('ArrowLeft');
|
||||
seen.push(shown());
|
||||
|
||||
expect(seen).toEqual(['png', 'jpeg', 'png', 'jpeg', 'png']);
|
||||
expect(seen).not.toContain('svg');
|
||||
});
|
||||
|
||||
it('keeps the requested image when EARLIER members are filtered out', () => {
|
||||
// The requested image is at position 1 in the given set and position 0
|
||||
// in the filtered one. Carrying the NUMBER across — even clamped to the
|
||||
// filtered length, which is the plausible wrong version — lands on the
|
||||
// image after it. Only resolving by id opens what was asked for.
|
||||
mountViewer({
|
||||
images: [
|
||||
image(IMG_B, 'svg', 'image/svg+xml'),
|
||||
image(IMG_A, 'png', 'image/png'),
|
||||
image(IMG_C, 'jpeg', 'image/jpeg'),
|
||||
],
|
||||
index: 1,
|
||||
});
|
||||
|
||||
expect(shown()).toBe('png');
|
||||
expect(root().querySelector('.lightbox-counter')?.textContent).toBe('1 / 2');
|
||||
});
|
||||
|
||||
it('REFUSES an image whose type is not resolved', () => {
|
||||
// This test previously asserted the opposite, on the reasoning that an
|
||||
// inline image's probe is often unasked at click time. That reasoning
|
||||
// belongs to the PRODUCER — which can wait for the probe — and asserting
|
||||
// it HERE pinned open the exact hole the task exists to close: an emitter
|
||||
// could hand over `[safe, unresolved]` and the user could arrow onto the
|
||||
// unresolved one. This is the last thing between a set and a rendered
|
||||
// image; "not yet known" is not evidence that a file is a PNG.
|
||||
mountViewer({ images: [image(IMG_A, 'unprobed', null)] });
|
||||
expect(root().querySelector('.lightbox-image')).toBeNull();
|
||||
});
|
||||
|
||||
it('cannot be paged onto an unresolved sibling', () => {
|
||||
mountViewer({
|
||||
images: [image(IMG_A, 'png'), image(IMG_B, 'unprobed', null), image(IMG_C, 'jpeg', 'image/jpeg')],
|
||||
});
|
||||
|
||||
expect(root().querySelector('.lightbox-counter')?.textContent).toBe('1 / 2');
|
||||
const seen = [shown()];
|
||||
for (let i = 0; i < 3; i++) {
|
||||
press('ArrowRight');
|
||||
seen.push(shown());
|
||||
}
|
||||
expect(seen).toEqual(['png', 'jpeg', 'png', 'jpeg']);
|
||||
expect(seen).not.toContain('unprobed');
|
||||
});
|
||||
|
||||
it('shows nothing at all rather than one refused image', () => {
|
||||
mountViewer({ images: [image(IMG_B, 'svg', 'image/svg+xml')] });
|
||||
|
||||
expect(root().querySelector('.lightbox-image')).toBeNull();
|
||||
// Still a real dialog with a way out — the failure mode is an empty
|
||||
// viewer, never a rendered one.
|
||||
expect(closeButton()).not.toBeNull();
|
||||
// And the arrows cannot divide by an empty set.
|
||||
press('ArrowRight');
|
||||
press('ArrowLeft');
|
||||
expect(root().querySelector('.lightbox-image')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — the set changing under an OPEN viewer (TASK-2431)', () => {
|
||||
/**
|
||||
* The producers hand over a set once and the viewer pages through it for as
|
||||
* long as it is up, so "was safe when the list was built" is not the claim
|
||||
* that has to hold — "is safe on the frame being shown" is. These drive the
|
||||
* live props (`liveProps`, the reactive object the file already uses for the
|
||||
* capture-at-open cases) rather than remounting, which is the only way to
|
||||
* reach a set that changes under a viewer that is already open.
|
||||
*/
|
||||
function shown(): string {
|
||||
return root().querySelector<HTMLImageElement>('.lightbox-image')?.getAttribute('alt') ?? '';
|
||||
}
|
||||
|
||||
function mountLive() {
|
||||
const app = mount(Lightbox, { target: appRoot, props: liveProps });
|
||||
mounted.push(app);
|
||||
flushSync();
|
||||
return app;
|
||||
}
|
||||
|
||||
it('drops a record whose MIME resolves to something unsafe AFTER open', () => {
|
||||
liveProps.images = [image(IMG_A, 'png'), image(IMG_B, 'later-svg')];
|
||||
mountLive();
|
||||
expect(root().querySelector('.lightbox-counter')?.textContent).toBe('1 / 2');
|
||||
|
||||
// The late answer arrives: what was believed safe is not. A set captured
|
||||
// once would keep paging onto it.
|
||||
liveProps.images = [image(IMG_A, 'png'), image(IMG_B, 'later-svg', 'image/svg+xml')];
|
||||
flushSync();
|
||||
|
||||
expect(shown()).toBe('png');
|
||||
expect(root().querySelector('.lightbox-counter')).toBeNull();
|
||||
press('ArrowRight');
|
||||
expect(shown()).toBe('png');
|
||||
});
|
||||
|
||||
it('keeps showing a real image when the one under it is removed', () => {
|
||||
liveProps.images = [image(IMG_A, 'png'), image(IMG_C, 'jpeg', 'image/jpeg')];
|
||||
mountLive();
|
||||
press('ArrowRight');
|
||||
expect(shown()).toBe('jpeg');
|
||||
|
||||
// The set shrinks beneath the position the user navigated to — a delete
|
||||
// from another surface, a reload. The index must clamp, not blank out or
|
||||
// render `undefined`.
|
||||
liveProps.images = [image(IMG_A, 'png')];
|
||||
flushSync();
|
||||
|
||||
expect(shown()).toBe('png');
|
||||
expect(imageSrc()).toContain(IMG_A);
|
||||
});
|
||||
|
||||
it('cannot be navigated onto an unsafe entry ADDED after open', () => {
|
||||
liveProps.images = [image(IMG_A, 'png')];
|
||||
mountLive();
|
||||
|
||||
liveProps.images = [
|
||||
image(IMG_A, 'png'),
|
||||
image(IMG_B, 'svg', 'image/svg+xml'),
|
||||
image(IMG_C, 'unprobed', null),
|
||||
];
|
||||
flushSync();
|
||||
|
||||
// Both additions are refused, so the viewer is still single-image.
|
||||
expect(root().querySelector('.lightbox-counter')).toBeNull();
|
||||
press('ArrowRight');
|
||||
press('ArrowLeft');
|
||||
expect(shown()).toBe('png');
|
||||
});
|
||||
|
||||
it('empties rather than showing an unsafe replacement', () => {
|
||||
liveProps.images = [image(IMG_A, 'png')];
|
||||
mountLive();
|
||||
expect(shown()).toBe('png');
|
||||
|
||||
liveProps.images = [image(IMG_B, 'svg', 'image/svg+xml')];
|
||||
flushSync();
|
||||
|
||||
expect(root().querySelector('.lightbox-image')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — portal', () => {
|
||||
it('portals to <body> DIRECTLY, not into its mount container', () => {
|
||||
// The structural half of the fixed-overlay contract: with `<body>` as the
|
||||
@@ -231,8 +443,8 @@ describe('Lightbox — workspace captured at open', () => {
|
||||
// all, which would be a different bug. This is the counterweight.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
expect(imageSrc()).toContain(IMG_A);
|
||||
@@ -250,8 +462,8 @@ describe('Lightbox — focus', () => {
|
||||
// LAST candidate.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
const controls = Array.from(root().querySelectorAll('button'));
|
||||
@@ -368,8 +580,8 @@ describe('Lightbox — Tab trap', () => {
|
||||
it('wraps forward off the last focusable', () => {
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
const next = root().querySelector<HTMLButtonElement>('.lightbox-nav.next')!;
|
||||
@@ -382,8 +594,8 @@ describe('Lightbox — Tab trap', () => {
|
||||
it('wraps backward off the first focusable', () => {
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
closeButton().focus();
|
||||
@@ -401,8 +613,8 @@ describe('Lightbox — Tab trap', () => {
|
||||
const outside = appRoot.appendChild(document.createElement('button'));
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
outside.focus();
|
||||
@@ -423,8 +635,8 @@ describe('Lightbox — Tab trap', () => {
|
||||
// the natural order inside the viewer.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
closeButton().focus();
|
||||
@@ -494,8 +706,8 @@ describe('Lightbox — Escape ownership', () => {
|
||||
// Escape branch had.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
expect(imageSrc()).toContain(IMG_A);
|
||||
@@ -559,16 +771,16 @@ describe('Lightbox — only the frontmost viewer acts', () => {
|
||||
mountViewer({
|
||||
onClose: onCloseBack,
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'back-first' },
|
||||
{ id: IMG_B, alt: 'back-second' },
|
||||
image(IMG_A, 'back-first'),
|
||||
image(IMG_B, 'back-second'),
|
||||
],
|
||||
});
|
||||
const back = root();
|
||||
mountViewer({
|
||||
onClose: onCloseFront,
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'front-first' },
|
||||
{ id: IMG_B, alt: 'front-second' },
|
||||
image(IMG_A, 'front-first'),
|
||||
image(IMG_B, 'front-second'),
|
||||
],
|
||||
});
|
||||
const front = root();
|
||||
@@ -655,8 +867,8 @@ describe('Lightbox — a native modal opened OVER the viewer', () => {
|
||||
mockOpenModals([dialog]);
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
inDialog.focus();
|
||||
@@ -670,8 +882,8 @@ describe('Lightbox — a native modal opened OVER the viewer', () => {
|
||||
mockOpenModals([dialog]);
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
|
||||
@@ -685,8 +897,8 @@ describe('Lightbox — a native modal opened OVER the viewer', () => {
|
||||
mockOpenModals([dialog]);
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
image(IMG_A, 'first'),
|
||||
image(IMG_B, 'second'),
|
||||
],
|
||||
});
|
||||
expect(press('ArrowRight')).toBe(false);
|
||||
|
||||
@@ -53,7 +53,7 @@
|
||||
import AttachmentDeleteConfirm, {
|
||||
attachmentDeletePrompt,
|
||||
} from '$lib/components/attachments/AttachmentDeleteConfirm.svelte';
|
||||
import Lightbox, { type LightboxImage } from '$lib/components/common/Lightbox.svelte';
|
||||
import Lightbox from '$lib/components/common/Lightbox.svelte';
|
||||
import { attachmentRefsIn } from '$lib/utils/commentAttachments';
|
||||
import { invalidateAttachmentMetadata } from '$lib/components/editor/attachment-metadata';
|
||||
import { toastStore } from '$lib/stores/toast.svelte';
|
||||
@@ -62,6 +62,10 @@
|
||||
notifyAttachmentPanelOpen,
|
||||
registerAttachmentDeletionListener,
|
||||
registerAttachmentUploadListener,
|
||||
// The viewer's image shape lives on the channel, not on the component
|
||||
// (TASK-2431) — one declaration for the direct mounts and the bus alike.
|
||||
// A type-only import, so the test's module mock is unaffected.
|
||||
type LightboxImage,
|
||||
} from '$lib/attachments/events';
|
||||
import { viewIdentity, createFence, createPaintFence } from '$lib/attachments/viewFence';
|
||||
|
||||
@@ -146,6 +150,16 @@
|
||||
filename: string;
|
||||
mime_type: string;
|
||||
size_bytes: number;
|
||||
/**
|
||||
* Intrinsic pixels, OPTIONAL and nullable (TASK-2431). The list row has
|
||||
* them, an upload event does not (`UploadedAttachment` is four fields),
|
||||
* and no tile reads them — they are carried so the set handed to the
|
||||
* viewer is complete, which is what keeps phase 3b's pixel-based loading
|
||||
* policy from having to reopen every producer. Optional rather than
|
||||
* required precisely so the upload buffer stays assignable to this type.
|
||||
*/
|
||||
width?: number | null;
|
||||
height?: number | null;
|
||||
}
|
||||
|
||||
function toStripAttachment(row: AttachmentListItem): StripAttachment {
|
||||
@@ -154,6 +168,8 @@
|
||||
filename: row.filename,
|
||||
mime_type: row.mime_type,
|
||||
size_bytes: row.size_bytes,
|
||||
width: row.width ?? null,
|
||||
height: row.height ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -164,7 +180,12 @@
|
||||
|
||||
let attachments = $state<StripAttachment[]>([]);
|
||||
let expanded = $state(false);
|
||||
let lightbox = $state<{ images: LightboxImage[]; index: number } | null>(null);
|
||||
let lightbox = $state<{
|
||||
images: LightboxImage[];
|
||||
index: number;
|
||||
/** The tile that opened it — focus goes back here on close (TASK-2431). */
|
||||
invoker: HTMLElement | null;
|
||||
} | null>(null);
|
||||
/**
|
||||
* The delete confirmation currently on screen, if any (PLAN-2392 DR-18 /
|
||||
* TASK-2425). One at a time: opening a second supersedes the first, which
|
||||
@@ -653,16 +674,36 @@
|
||||
// reaches the viewer, because SVG can carry active content. Both this list
|
||||
// and the tile branch below read the same predicate, so a file tile can
|
||||
// never be a member of the lightbox's set.
|
||||
//
|
||||
// The full row is threaded, not just `{id, alt}` (TASK-2431): `mime_type`
|
||||
// so the set carries its own evidence of why each member passed the gate,
|
||||
// and `size_bytes` / `width` / `height` so 3b's loading policy has them.
|
||||
// The strip is the one producer that knows all of it from its list row.
|
||||
let lightboxImages = $derived<LightboxImage[]>(
|
||||
attachments
|
||||
.filter((a) => canOpenInViewer(a.mime_type))
|
||||
.map((a) => ({ id: a.id, alt: displayFilename(a.filename) }))
|
||||
.map((a) => ({
|
||||
id: a.id,
|
||||
alt: displayFilename(a.filename),
|
||||
filename: a.filename || null,
|
||||
mime_type: a.mime_type,
|
||||
size_bytes: a.size_bytes ?? null,
|
||||
width: a.width ?? null,
|
||||
height: a.height ?? null,
|
||||
}))
|
||||
);
|
||||
|
||||
function openLightbox(att: StripAttachment) {
|
||||
/**
|
||||
* `invoker` is the tile's own button — the viewer returns focus to it on
|
||||
* close. It has to be passed rather than inferred: `Lightbox` falls back to
|
||||
* whatever held focus at open, which is right for a click but says nothing
|
||||
* useful when the open came from somewhere else, and the fallback runs
|
||||
* AFTER the viewer's own focus entry in some orders.
|
||||
*/
|
||||
function openLightbox(att: StripAttachment, invoker: HTMLElement | null = null) {
|
||||
const index = lightboxImages.findIndex((img) => img.id === att.id);
|
||||
if (index < 0) return;
|
||||
lightbox = { images: lightboxImages, index };
|
||||
lightbox = { images: lightboxImages, index, invoker };
|
||||
}
|
||||
|
||||
/** What the file IS — the tooltip, and the base of the accessible name. */
|
||||
@@ -924,7 +965,7 @@
|
||||
class="att-tile"
|
||||
title={tileLabel(att)}
|
||||
aria-label={tileActionLabel(att)}
|
||||
onclick={() => openLightbox(att)}
|
||||
onclick={(e) => openLightbox(att, e.currentTarget)}
|
||||
>
|
||||
<img
|
||||
src={api.attachments.downloadUrl(wsSlug, att.id, 'thumb-sm')}
|
||||
@@ -1042,14 +1083,36 @@
|
||||
</Menu>
|
||||
{/if}
|
||||
|
||||
{#if lightbox}
|
||||
<Lightbox
|
||||
images={lightbox.images}
|
||||
index={lightbox.index}
|
||||
{wsSlug}
|
||||
onClose={() => (lightbox = null)}
|
||||
/>
|
||||
{/if}
|
||||
<!--
|
||||
Keyed per open (TASK-2431), the shape `AttachmentViewerHost` uses. `Lightbox`
|
||||
seeds its INDEX once through `untrack`, so replacing `lightbox` while a
|
||||
viewer is already up would reuse the instance and open the new set at the old
|
||||
position. (Its MIME filter is `$derived` and would re-answer on its own —
|
||||
the index is what cannot.) Nothing reaches that state today, the open viewer
|
||||
being inert over everything that could cause it, which is why this belongs in
|
||||
the structure rather than resting on a fact about the current UI.
|
||||
|
||||
Accepted cost, recorded: a keyed block DESTROYS the old instance before
|
||||
creating the new one, so a viewer→viewer swap briefly releases the last
|
||||
backdrop lease (un-inerting the app) and runs the old viewer's focus restore
|
||||
before the new one takes focus. That is the same transient
|
||||
`AttachmentViewerHost` has carried since TASK-2428, it is unreachable from
|
||||
the UI (the open viewer inerts every control that could trigger it), and the
|
||||
alternative — a reused instance showing a stale set — is the worse of the
|
||||
two. Only a viewer→NULL→viewer sequence happens in practice, where the
|
||||
release is meant to happen anyway.
|
||||
-->
|
||||
{#key lightbox}
|
||||
{#if lightbox}
|
||||
<Lightbox
|
||||
images={lightbox.images}
|
||||
index={lightbox.index}
|
||||
{wsSlug}
|
||||
invoker={lightbox.invoker}
|
||||
onClose={() => (lightbox = null)}
|
||||
/>
|
||||
{/if}
|
||||
{/key}
|
||||
|
||||
<style>
|
||||
.attachment-strip {
|
||||
|
||||
@@ -356,6 +356,33 @@ describe('ItemAttachmentStrip', () => {
|
||||
expect(document.querySelector('.lightbox-counter')?.textContent).toBe('2 / 2');
|
||||
});
|
||||
|
||||
it('opens a FRESH viewer when a second tile is activated (TASK-2431)', async () => {
|
||||
// The viewer seeds its index — and its own MIME filter — once at mount,
|
||||
// so the mount must be keyed per open. Without that, a second open reuses
|
||||
// the instance and keeps showing the first image.
|
||||
//
|
||||
// In the app an open viewer inerts the tiles behind it, so this is
|
||||
// insurance rather than a live path; jsdom does not implement inertness,
|
||||
// which is what makes the invariant testable at all.
|
||||
listMock.mockResolvedValue(response([att({ id: 'img1' }), att({ id: 'img2' })]));
|
||||
mountStrip('item-a');
|
||||
await settle();
|
||||
|
||||
tiles()[0].click();
|
||||
flushSync();
|
||||
expect(
|
||||
document.querySelector<HTMLImageElement>('.lightbox-image')?.getAttribute('alt')
|
||||
).toBe('img1.png');
|
||||
|
||||
tiles()[1].click();
|
||||
flushSync();
|
||||
expect(
|
||||
document.querySelector<HTMLImageElement>('.lightbox-image')?.getAttribute('alt')
|
||||
).toBe('img2.png');
|
||||
// ...and exactly one viewer, not two stacked.
|
||||
expect(document.querySelectorAll('.lightbox-backdrop')).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('opens the lightbox at the IMAGE index, not the attachment index', async () => {
|
||||
// Interleaved non-images: a naive `attachments.indexOf(att)` would open
|
||||
// the wrong image, since the lightbox only ever receives image rows.
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
<script lang="ts">
|
||||
import { onDestroy, tick } from 'svelte';
|
||||
import { onDestroy, tick, untrack } from 'svelte';
|
||||
import { api } from '$lib/api/client';
|
||||
import { sseService } from '$lib/services/sse.svelte';
|
||||
import { authStore } from '$lib/stores/auth.svelte';
|
||||
@@ -11,7 +11,10 @@
|
||||
import { attachmentRefsIn } from '$lib/utils/commentAttachments';
|
||||
import { fetchAttachmentMetadata } from '$lib/components/editor/attachment-metadata';
|
||||
import { attachmentDownloadUrl, type AttachmentMeta } from '$lib/markdown/attachments';
|
||||
import Lightbox, { type LightboxImage } from '$lib/components/common/Lightbox.svelte';
|
||||
import { canOpenInViewer } from '$lib/attachments/display';
|
||||
// One declaration of the viewer's image shape, on the channel (TASK-2431).
|
||||
import type { LightboxImage } from '$lib/attachments/events';
|
||||
import Lightbox from '$lib/components/common/Lightbox.svelte';
|
||||
import CommentEditor from '$lib/components/CommentEditor.svelte';
|
||||
|
||||
interface Props {
|
||||
@@ -172,30 +175,100 @@
|
||||
|
||||
// Lightbox state (IDEA-1660). Set when a thumbnail is activated; cleared
|
||||
// on close. Null = closed, so the host remounts fresh on each open.
|
||||
let lightbox: { images: LightboxImage[]; index: number } | null = $state(null);
|
||||
let lightbox: { images: LightboxImage[]; index: number; invoker: HTMLElement | null } | null =
|
||||
$state(null);
|
||||
let entryListEl: HTMLElement | undefined = $state();
|
||||
|
||||
// Open the lightbox for a clicked/activated thumbnail, gathering sibling
|
||||
// attachment images in the same comment/reply body so ←/→ can page them.
|
||||
function openLightboxFromImg(imgEl: HTMLElement) {
|
||||
/**
|
||||
* The DR-16 open gate, applied to ONE rendered thumbnail (TASK-2431).
|
||||
*
|
||||
* Returns the viewer's own record for an `<img data-attachment-id>`, or
|
||||
* null when that image may not be opened. Null is the answer in two
|
||||
* different situations and deliberately does not distinguish them:
|
||||
*
|
||||
* - the MIME is known and is not on the allowlist. `image/svg+xml` is the
|
||||
* one that matters — SVG carries active content, and the markdown
|
||||
* renderer emits an `<img>` for ANY `image/*` (`markdown/attachments.ts`
|
||||
* `isImageMime`), which is the correct decision for RENDERING and the
|
||||
* wrong one for OPENING. That difference is the whole reason this
|
||||
* function exists rather than a `startsWith('image/')` test.
|
||||
* - the MIME is not known yet. Fail SAFE: an unresolved probe is not
|
||||
* evidence that a file is a PNG. It costs nothing, because an image
|
||||
* whose metadata has not resolved is not rendered as an `<img>` at all
|
||||
* (the renderer shows a "missing" placeholder until `attMeta` has the
|
||||
* row), so there is no cold-start case where this refuses something the
|
||||
* user can see and could previously open. A later probe re-runs the
|
||||
* semantics effect below and the thumbnail becomes clickable then.
|
||||
*
|
||||
* Read straight off the CACHED metadata — never a fresh HEAD per click.
|
||||
* `attMeta` is the same map the renderer resolved the image through, so the
|
||||
* gate and the picture on screen are answering from one source.
|
||||
*/
|
||||
function viewerImageFor(el: HTMLElement): LightboxImage | null {
|
||||
const id = el.getAttribute('data-attachment-id') ?? '';
|
||||
if (!id) return null;
|
||||
const meta = attMeta.get(id);
|
||||
if (!meta || !canOpenInViewer(meta.mime_type)) return null;
|
||||
return {
|
||||
id,
|
||||
alt: el.getAttribute('alt') ?? '',
|
||||
// The probe leaves filename empty (see probeAttachment) and HEAD
|
||||
// carries no intrinsic dimensions, so most of these are null here.
|
||||
// That is what nullable means on this type; the strip fills them.
|
||||
filename: meta.filename || null,
|
||||
mime_type: meta.mime_type,
|
||||
size_bytes: meta.size_bytes ?? null,
|
||||
width: meta.width ?? null,
|
||||
height: meta.height ?? null,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Open the viewer on an activated thumbnail, with its siblings in the same
|
||||
* comment/reply body so ←/→ can page them.
|
||||
*
|
||||
* THE WHOLE LIST IS GATED, not just the clicked image (TASK-2431). Before
|
||||
* this, the set was built from every `img[data-attachment-id]` in scope with
|
||||
* no MIME consulted at all, so opening a safe PNG and pressing → could land
|
||||
* on an SVG: gating the click alone is not a gate.
|
||||
*
|
||||
* And the index is derived from the clicked attachment's ID, not from its
|
||||
* position among the DOM elements — filtering reindexes everything after the
|
||||
* first refusal, so a DOM index would silently open the wrong image. When the
|
||||
* clicked image is itself refused it is not in the list, `findIndex` returns
|
||||
* -1, and nothing opens.
|
||||
*/
|
||||
function openLightboxFromImg(imgEl: HTMLElement): boolean {
|
||||
const clickedId = imgEl.getAttribute('data-attachment-id') ?? '';
|
||||
if (!clickedId) return false;
|
||||
const scope = imgEl.closest('.comment-body, .reply-body') ?? imgEl.parentElement;
|
||||
const els = scope
|
||||
? Array.from(scope.querySelectorAll<HTMLElement>('img[data-attachment-id]'))
|
||||
: [imgEl];
|
||||
const list: LightboxImage[] = els
|
||||
.map((el) => ({ id: el.getAttribute('data-attachment-id') ?? '', alt: el.getAttribute('alt') ?? '' }))
|
||||
.filter((x) => x.id !== '');
|
||||
if (list.length === 0) return;
|
||||
lightbox = { images: list, index: Math.max(0, els.indexOf(imgEl)) };
|
||||
const list = els
|
||||
.map((el) => viewerImageFor(el))
|
||||
.filter((x): x is LightboxImage => x !== null);
|
||||
// A body can legitimately embed the same attachment twice; the first
|
||||
// occurrence wins, which shows the image the user asked for.
|
||||
const index = list.findIndex((x) => x.id === clickedId);
|
||||
if (index < 0) return false;
|
||||
lightbox = { images: list, index, invoker: imgEl };
|
||||
return true;
|
||||
}
|
||||
|
||||
// BOTH activation routes go through the one gated opener — a filter applied
|
||||
// to the mouse path only would be no filter at all.
|
||||
//
|
||||
// `preventDefault` is now conditional on actually opening: swallowing the
|
||||
// event for a thumbnail the viewer refuses would leave the click doing
|
||||
// nothing at all, including whatever the surrounding markup would have done
|
||||
// with it.
|
||||
function onThumbClick(e: MouseEvent) {
|
||||
const imgEl = (e.target as HTMLElement | null)?.closest(
|
||||
'img[data-attachment-id]'
|
||||
) as HTMLElement | null;
|
||||
if (!imgEl) return;
|
||||
e.preventDefault();
|
||||
openLightboxFromImg(imgEl);
|
||||
if (openLightboxFromImg(imgEl)) e.preventDefault();
|
||||
}
|
||||
|
||||
function onThumbKeydown(e: KeyboardEvent) {
|
||||
@@ -204,8 +277,9 @@
|
||||
'img[data-attachment-id]'
|
||||
) as HTMLElement | null;
|
||||
if (!imgEl) return;
|
||||
e.preventDefault(); // Space would otherwise scroll the page
|
||||
openLightboxFromImg(imgEl);
|
||||
// Space would otherwise scroll the page — but only suppress it when the
|
||||
// key actually did something.
|
||||
if (openLightboxFromImg(imgEl)) e.preventDefault();
|
||||
}
|
||||
|
||||
// Delegated click + keydown on the entry list (rather than declarative
|
||||
@@ -227,13 +301,43 @@
|
||||
// Depends on BOTH `entries` (new comments) AND `attMeta` (an image only
|
||||
// renders as an <img> once its metadata resolves — before that it's a
|
||||
// "missing" placeholder span — so the pass must re-run on resolution).
|
||||
//
|
||||
// THE SEMANTICS ARE CONDITIONAL ON THE SAME GATE THE OPENER USES
|
||||
// (TASK-2431). Previously every `image/*` thumbnail got `role="button"`, a
|
||||
// tabindex and a "View image" name; with the opener now refusing the ones
|
||||
// outside the allowlist, that would leave an SVG as a focus stop announced
|
||||
// as a button whose activation does nothing — a worse outcome than the hole
|
||||
// it replaces, and the reason this pass has to be able to take semantics
|
||||
// BACK OFF an element (a probe can resolve a MIME that turns a thumbnail
|
||||
// from viewable-by-assumption into refused).
|
||||
$effect(() => {
|
||||
void entries;
|
||||
// `visibleEntries`, NOT `entries`: the pane's Activity / Versions tabs
|
||||
// filter the rendered set without refetching, so flipping away from
|
||||
// comments and back DESTROYS and rebuilds every comment card while
|
||||
// `entries` never changes. Tracking the raw list left those rebuilt
|
||||
// images mouse-openable (the delegated listeners live on the container,
|
||||
// which survives) but with no role, no tabindex and no name — openable
|
||||
// by mouse only, which is the dead-control failure in its other
|
||||
// direction (Codex round 4). Reading the derived also reads `entries`,
|
||||
// so nothing is lost by not naming it too.
|
||||
void visibleEntries;
|
||||
void attMeta;
|
||||
const el = entryListEl;
|
||||
if (!el) return;
|
||||
// The DOM pass is deferred a tick, so it can land after an item /
|
||||
// workspace switch has already replaced what it was written for. Nothing
|
||||
// it does is destructive, but a cancelled continuation is the local house
|
||||
// rule for every await-then-write here (TASK-2112).
|
||||
let cancelled = false;
|
||||
tick().then(() => {
|
||||
if (cancelled) return;
|
||||
for (const img of el.querySelectorAll<HTMLElement>('img[data-attachment-id]')) {
|
||||
if (viewerImageFor(img) === null) {
|
||||
img.removeAttribute('role');
|
||||
img.removeAttribute('tabindex');
|
||||
img.removeAttribute('aria-label');
|
||||
continue;
|
||||
}
|
||||
if (img.getAttribute('role') === 'button') continue;
|
||||
img.setAttribute('role', 'button');
|
||||
img.setAttribute('tabindex', '0');
|
||||
@@ -241,6 +345,9 @@
|
||||
img.setAttribute('aria-label', alt ? `View image: ${alt}` : 'View attachment image');
|
||||
}
|
||||
});
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
});
|
||||
|
||||
// Current user ID for reaction toggle — read from the global auth store.
|
||||
@@ -301,9 +408,44 @@
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The view the currently painted timeline belongs to. Plain `let` + untrack,
|
||||
* NOT `$state`: the effect below both reads and writes it, and a `$state`
|
||||
* read inside its own writing effect self-depends, aborts the flush and
|
||||
* silently strands unrelated reactivity (CONVE-1688). Seeded from the
|
||||
* initial props so a fresh mount is not treated as a switch — there is
|
||||
* nothing to tear down on the first run.
|
||||
*/
|
||||
// The two parts are joined with a separator that cannot occur in a slug,
|
||||
// so no pair of (workspace, item) values can collide into one key.
|
||||
let lastView = untrack(() => `${wsSlug}/${itemSlug}`);
|
||||
|
||||
$effect(() => {
|
||||
void wsSlug;
|
||||
void itemSlug;
|
||||
const ws = wsSlug;
|
||||
const slug = itemSlug;
|
||||
// A→B LIFECYCLE (TASK-2431). This component is reused across an item /
|
||||
// workspace switch (no `{#key}`), and it had NO viewer reset at all —
|
||||
// `lightbox` was cleared on close and nowhere else. So a switch left a
|
||||
// full-screen viewer up over the incoming item, still holding the
|
||||
// previous view's attachment ids while `Lightbox` rebuilt their URLs
|
||||
// from the workspace it captured at open. The strip's reset is the
|
||||
// pattern; this is the same rule stated for this component.
|
||||
//
|
||||
// `attMeta` is deliberately NOT cleared alongside it. It is keyed by a
|
||||
// bare uuid where the shared HEAD cache is keyed `ws:uuid`, which looks
|
||||
// like a workspace-scoped cache leaking across the switch — but an
|
||||
// attachment id is a UUID belonging to exactly one workspace, so an
|
||||
// entry can only ever answer for the id it describes. Clearing it would
|
||||
// buy nothing and cost something real: the probe effect refills it only
|
||||
// when `entries` next changes, so a FAILED load after the switch would
|
||||
// leave every already-rendered image permanently un-openable for the
|
||||
// rest of the mount (Codex round 3).
|
||||
untrack(() => {
|
||||
const view = `${ws}/${slug}`;
|
||||
if (view === lastView) return;
|
||||
lastView = view;
|
||||
lightbox = null;
|
||||
});
|
||||
loadTimeline();
|
||||
});
|
||||
|
||||
@@ -575,14 +717,36 @@
|
||||
{/if}
|
||||
</section>
|
||||
|
||||
{#if lightbox}
|
||||
<Lightbox
|
||||
images={lightbox.images}
|
||||
index={lightbox.index}
|
||||
{wsSlug}
|
||||
onClose={() => (lightbox = null)}
|
||||
/>
|
||||
{/if}
|
||||
<!--
|
||||
Keyed per open (TASK-2431), the shape `AttachmentViewerHost` uses. `Lightbox`
|
||||
seeds its INDEX once through `untrack`, so replacing `lightbox` while a
|
||||
viewer is already up would reuse the instance and open the new set at the old
|
||||
position. (Its MIME filter is `$derived` and would re-answer on its own —
|
||||
the index is what cannot.) Nothing reaches that state today, the open viewer
|
||||
being inert over everything that could cause it, which is why this belongs in
|
||||
the structure rather than resting on a fact about the current UI.
|
||||
|
||||
Accepted cost, recorded: a keyed block DESTROYS the old instance before
|
||||
creating the new one, so a viewer→viewer swap briefly releases the last
|
||||
backdrop lease (un-inerting the app) and runs the old viewer's focus restore
|
||||
before the new one takes focus. That is the same transient
|
||||
`AttachmentViewerHost` has carried since TASK-2428, it is unreachable from
|
||||
the UI (the open viewer inerts every control that could trigger it), and the
|
||||
alternative — a reused instance showing a stale set — is the worse of the
|
||||
two. Only a viewer→NULL→viewer sequence happens in practice, where the
|
||||
release is meant to happen anyway.
|
||||
-->
|
||||
{#key lightbox}
|
||||
{#if lightbox}
|
||||
<Lightbox
|
||||
images={lightbox.images}
|
||||
index={lightbox.index}
|
||||
{wsSlug}
|
||||
invoker={lightbox.invoker}
|
||||
onClose={() => (lightbox = null)}
|
||||
/>
|
||||
{/if}
|
||||
{/key}
|
||||
|
||||
<style>
|
||||
.timeline {
|
||||
|
||||
@@ -0,0 +1,482 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { flushSync, mount, unmount, tick } from 'svelte';
|
||||
import type { Comment, TimelineEntry, TimelineResponse } from '$lib/types';
|
||||
import { __resetViewerBackdropForTests } from '$lib/a11y/viewerBackdrop';
|
||||
import { _resetEscapeStackForTests } from '$lib/stores/escapeStack';
|
||||
|
||||
/**
|
||||
* The timeline's open-the-viewer gate (PLAN-2392 DR-16 / TASK-2431).
|
||||
*
|
||||
* The hole these cover: the sibling list the viewer's ←/→ page through was
|
||||
* built from every `img[data-attachment-id]` in the comment body with NO MIME
|
||||
* consulted, while the markdown renderer emits an `<img>` for any `image/*` —
|
||||
* so opening a safe PNG and pressing → could land on an `image/svg+xml`.
|
||||
* Gating the CLICKED image alone is not a gate, which is why every assertion
|
||||
* here is about the whole list and about both activation routes.
|
||||
*
|
||||
* Deliberately against the REAL `Lightbox` rather than a stub: the claim is
|
||||
* "no unsafe image can be REACHED", and only the real component's ←/→ can
|
||||
* demonstrate where an arrow key actually lands. The rendered `<img>`'s `src`
|
||||
* carries the attachment id, so what the viewer is showing is observable.
|
||||
*
|
||||
* The markdown pipeline is real too — the `<img>` elements under test are the
|
||||
* ones `renderMarkdown` + `sanitizeMarkdownHtml` actually produce.
|
||||
*/
|
||||
|
||||
const PNG_A = '11111111-1111-4111-8111-111111111111';
|
||||
const SVG = '22222222-2222-4222-8222-222222222222';
|
||||
const PNG_B = '33333333-3333-4333-8333-333333333333';
|
||||
const TIFF = '44444444-4444-4444-8444-444444444444';
|
||||
const UNPROBED = '55555555-5555-4555-8555-555555555555';
|
||||
|
||||
/** MIME per attachment id, as the HEAD probe would report it. */
|
||||
const MIMES: Record<string, string> = {
|
||||
[PNG_A]: 'image/png',
|
||||
[SVG]: 'image/svg+xml',
|
||||
[PNG_B]: 'image/jpeg',
|
||||
[TIFF]: 'image/tiff',
|
||||
};
|
||||
|
||||
const timelineListMock = vi.fn<(ws: string, slug: string) => Promise<TimelineResponse>>();
|
||||
|
||||
vi.mock('$lib/api/client', () => ({
|
||||
api: {
|
||||
timeline: {
|
||||
list: (ws: string, slug: string) => timelineListMock(ws, slug),
|
||||
},
|
||||
comments: {
|
||||
create: vi.fn(),
|
||||
update: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
addReaction: vi.fn(),
|
||||
removeReaction: vi.fn(),
|
||||
},
|
||||
attachments: {
|
||||
downloadUrl: (ws: string, id: string, variant?: string) =>
|
||||
`/api/v1/workspaces/${ws}/attachments/${id}${variant ? `?variant=${variant}` : ''}`,
|
||||
},
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock('$lib/services/sse.svelte', () => ({
|
||||
sseService: { onItemEvent: () => () => {} },
|
||||
}));
|
||||
|
||||
vi.mock('$lib/stores/auth.svelte', () => ({
|
||||
authStore: { userId: 'user-1', user: { id: 'user-1', role: 'member' } },
|
||||
}));
|
||||
|
||||
vi.mock('$lib/stores/workspace.svelte', () => ({
|
||||
workspaceStore: { canEditItem: () => false },
|
||||
}));
|
||||
|
||||
// The HEAD probe, answered from MIMES. An id absent from the table resolves as
|
||||
// `transient` — the "not known" case the gate must fail safe on.
|
||||
vi.mock('$lib/components/editor/attachment-metadata', () => ({
|
||||
fetchAttachmentMetadata: (_ws: string, uuid: string) =>
|
||||
Promise.resolve(
|
||||
MIMES[uuid]
|
||||
? { status: 'ok' as const, mime: MIMES[uuid], size: 4096 }
|
||||
: { status: 'transient' as const }
|
||||
),
|
||||
invalidateAttachmentMetadata: vi.fn(),
|
||||
}));
|
||||
|
||||
// Tiptap in jsdom is not what these tests are about; the composer is inert.
|
||||
vi.mock('$lib/components/CommentEditor.svelte', async () => ({
|
||||
default: (await import('./fixtures/InertCommentEditor.svelte')).default,
|
||||
}));
|
||||
|
||||
const { default: ItemTimeline } = await import('./ItemTimeline.svelte');
|
||||
|
||||
function comment(body: string, id = 'c1'): Comment {
|
||||
return {
|
||||
id,
|
||||
item_id: 'item-a',
|
||||
workspace_id: 'ws-1',
|
||||
author: 'alice',
|
||||
body,
|
||||
created_by: 'alice',
|
||||
source: 'web',
|
||||
created_at: '2026-01-01T00:00:00Z',
|
||||
updated_at: '2026-01-01T00:00:00Z',
|
||||
};
|
||||
}
|
||||
|
||||
function entry(c: Comment): TimelineEntry {
|
||||
return {
|
||||
id: `e-${c.id}`,
|
||||
kind: 'comment',
|
||||
created_at: c.created_at,
|
||||
actor: 'alice',
|
||||
source: 'web',
|
||||
comment: c,
|
||||
};
|
||||
}
|
||||
|
||||
/** A body embedding each id as an inline image, in order. */
|
||||
function bodyWith(ids: string[]): string {
|
||||
return ids.map((id, i) => ``).join('\n\n');
|
||||
}
|
||||
|
||||
function respond(ids: string[]): TimelineResponse {
|
||||
return { entries: [entry(comment(bodyWith(ids)))], has_more: false };
|
||||
}
|
||||
|
||||
let host: HTMLElement;
|
||||
let app: Record<string, unknown> | null = null;
|
||||
|
||||
// Reactive props object so a test can flip wsSlug / itemSlug the way ItemDetail
|
||||
// does — the timeline is mounted WITHOUT `{#key}` and is reused across the
|
||||
// switch, which is the whole point of the lifecycle tests below.
|
||||
const props = $state<{
|
||||
wsSlug: string;
|
||||
username: string;
|
||||
itemSlug: string;
|
||||
currentContent: string;
|
||||
itemId: string;
|
||||
collectionId: string;
|
||||
visibleKinds: Array<'comment' | 'activity' | 'version'> | undefined;
|
||||
}>({
|
||||
wsSlug: 'ws',
|
||||
username: 'alice',
|
||||
itemSlug: 'TASK-1',
|
||||
currentContent: '',
|
||||
itemId: 'item-a',
|
||||
collectionId: 'coll-1',
|
||||
visibleKinds: undefined,
|
||||
});
|
||||
|
||||
function resetProps() {
|
||||
props.wsSlug = 'ws';
|
||||
props.username = 'alice';
|
||||
props.itemSlug = 'TASK-1';
|
||||
props.currentContent = '';
|
||||
props.itemId = 'item-a';
|
||||
props.collectionId = 'coll-1';
|
||||
props.visibleKinds = undefined;
|
||||
}
|
||||
|
||||
function render() {
|
||||
app = mount(ItemTimeline, { target: host, props }) as Record<string, unknown>;
|
||||
return app;
|
||||
}
|
||||
|
||||
/**
|
||||
* Let the timeline fetch, probe, render and run its deferred semantics pass.
|
||||
* Several microtask hops: list() → entries → probe → attMeta → re-render →
|
||||
* `tick()` inside the semantics effect.
|
||||
*/
|
||||
async function settle() {
|
||||
for (let i = 0; i < 8; i++) {
|
||||
await tick();
|
||||
flushSync();
|
||||
}
|
||||
}
|
||||
|
||||
function thumbs(): HTMLElement[] {
|
||||
return Array.from(host.querySelectorAll<HTMLElement>('img[data-attachment-id]'));
|
||||
}
|
||||
|
||||
function thumbFor(id: string): HTMLElement {
|
||||
const el = thumbs().find((t) => t.getAttribute('data-attachment-id') === id);
|
||||
if (!el) throw new Error(`no thumbnail rendered for ${id}`);
|
||||
return el;
|
||||
}
|
||||
|
||||
/** The attachment id the open viewer is currently SHOWING, or null if closed. */
|
||||
function viewerShowing(): string | null {
|
||||
const img = document.querySelector<HTMLImageElement>('.lightbox-backdrop .lightbox-image');
|
||||
if (!img) return null;
|
||||
const m = /attachments\/([0-9a-f-]+)/.exec(img.getAttribute('src') ?? '');
|
||||
return m ? m[1] : null;
|
||||
}
|
||||
|
||||
function viewerOpen(): boolean {
|
||||
return document.querySelector('.lightbox-backdrop') !== null;
|
||||
}
|
||||
|
||||
function click(el: HTMLElement) {
|
||||
el.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
|
||||
flushSync();
|
||||
}
|
||||
|
||||
function pressOn(el: HTMLElement, key: string) {
|
||||
el.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true }));
|
||||
flushSync();
|
||||
}
|
||||
|
||||
/** The viewer listens on `window`, so its arrows are driven from there. */
|
||||
function pressGlobal(key: string) {
|
||||
window.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true }));
|
||||
flushSync();
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
host = document.createElement('div');
|
||||
document.body.appendChild(host);
|
||||
resetProps();
|
||||
timelineListMock.mockReset();
|
||||
timelineListMock.mockResolvedValue({ entries: [], has_more: false });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Unmount FIRST: that runs the viewer's own teardown, which is what
|
||||
// unregisters its Escape handler and releases its backdrop lease. Ripping
|
||||
// the portaled node out beforehand would leave both behind.
|
||||
if (app) unmount(app);
|
||||
app = null;
|
||||
host.remove();
|
||||
// The viewer portals to <body>; make sure nothing leaks between tests.
|
||||
document.querySelectorAll('.lightbox-backdrop').forEach((n) => n.remove());
|
||||
// ...and reset the two module-global registries a viewer touches, so a case
|
||||
// that ends with one open cannot colour the next test (the strip suite's
|
||||
// afterEach does the same).
|
||||
__resetViewerBackdropForTests();
|
||||
_resetEscapeStackForTests();
|
||||
});
|
||||
|
||||
describe('ItemTimeline — viewer open gate (TASK-2431)', () => {
|
||||
it('renders an <img> for the SVG — the render decision is not the gate', async () => {
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A, SVG]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
// If this ever stops holding, the tests below are vacuous: they would be
|
||||
// proving that an element nobody renders cannot be clicked.
|
||||
expect(thumbs().map((t) => t.getAttribute('data-attachment-id'))).toEqual([PNG_A, SVG]);
|
||||
});
|
||||
|
||||
it('refuses a click on a non-allowlisted image', async () => {
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A, SVG]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
click(thumbFor(SVG));
|
||||
expect(viewerOpen()).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses Enter and Space on a non-allowlisted image', async () => {
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A, SVG]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
pressOn(thumbFor(SVG), 'Enter');
|
||||
expect(viewerOpen()).toBe(false);
|
||||
pressOn(thumbFor(SVG), ' ');
|
||||
expect(viewerOpen()).toBe(false);
|
||||
});
|
||||
|
||||
it('opens on an allowlisted image, by mouse and by keyboard', async () => {
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A, SVG]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
click(thumbFor(PNG_A));
|
||||
expect(viewerShowing()).toBe(PNG_A);
|
||||
|
||||
// Close and reopen from the keyboard.
|
||||
(document.querySelector('.lightbox-close') as HTMLElement).click();
|
||||
flushSync();
|
||||
expect(viewerOpen()).toBe(false);
|
||||
|
||||
pressOn(thumbFor(PNG_A), 'Enter');
|
||||
expect(viewerShowing()).toBe(PNG_A);
|
||||
});
|
||||
|
||||
it('never pages onto an unsafe sibling with ←/→', async () => {
|
||||
// Mixed: safe, unsafe, safe, undecodable, unprobed.
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A, SVG, PNG_B, TIFF, UNPROBED]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
click(thumbFor(PNG_A));
|
||||
expect(viewerShowing()).toBe(PNG_A);
|
||||
|
||||
// The set is the two allowlisted images ONLY, so → wraps between them
|
||||
// and every position is safe. Walk a full cycle in both directions.
|
||||
const seen: (string | null)[] = [viewerShowing()];
|
||||
for (let i = 0; i < 4; i++) {
|
||||
pressGlobal('ArrowRight');
|
||||
seen.push(viewerShowing());
|
||||
}
|
||||
for (let i = 0; i < 4; i++) {
|
||||
pressGlobal('ArrowLeft');
|
||||
seen.push(viewerShowing());
|
||||
}
|
||||
expect(seen).toEqual([PNG_A, PNG_B, PNG_A, PNG_B, PNG_A, PNG_B, PNG_A, PNG_B, PNG_A]);
|
||||
expect(seen).not.toContain(SVG);
|
||||
expect(seen).not.toContain(TIFF);
|
||||
expect(seen).not.toContain(UNPROBED);
|
||||
|
||||
// ...and the counter agrees the set has two members, not five.
|
||||
expect(document.querySelector('.lightbox-counter')?.textContent).toBe('1 / 2');
|
||||
});
|
||||
|
||||
it('derives the index from the clicked ID, not the DOM position', async () => {
|
||||
// PNG_B is DOM index 2 but list index 1 once the SVG is filtered out.
|
||||
// A position-derived index would open on the wrong image — and with a
|
||||
// leading run of refusals, past the end of the list entirely.
|
||||
timelineListMock.mockResolvedValue(respond([SVG, TIFF, PNG_B, PNG_A]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
click(thumbFor(PNG_B));
|
||||
expect(viewerShowing()).toBe(PNG_B);
|
||||
expect(document.querySelector('.lightbox-counter')?.textContent).toBe('1 / 2');
|
||||
|
||||
pressGlobal('ArrowRight');
|
||||
expect(viewerShowing()).toBe(PNG_A);
|
||||
});
|
||||
|
||||
it('fails safe on a thumbnail whose MIME it holds no answer for', async () => {
|
||||
// The gate's `!meta` branch, tested where it is REACHABLE rather than
|
||||
// through the renderer. Asserting that an unprobed id renders no <img>
|
||||
// would prove nothing about the gate — it is a fact about the markdown
|
||||
// resolver, and it holds with the gate deleted.
|
||||
//
|
||||
// The handlers are delegated over the entry list and act on whatever
|
||||
// `img[data-attachment-id]` is in the DOM, so an element the component
|
||||
// has no metadata for is a real input: a node left over from a body that
|
||||
// re-rendered, or one from a future surface. It must be refused, and it
|
||||
// must not be a member of a NEIGHBOUR's set either.
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
const body = host.querySelector('.comment-body, .reply-body')!;
|
||||
const stray = document.createElement('img');
|
||||
stray.setAttribute('data-attachment-id', UNPROBED);
|
||||
stray.setAttribute('alt', 'unknown');
|
||||
body.appendChild(stray);
|
||||
|
||||
click(stray);
|
||||
expect(viewerOpen()).toBe(false);
|
||||
pressOn(stray, 'Enter');
|
||||
expect(viewerOpen()).toBe(false);
|
||||
|
||||
// ...and the PNG beside it opens a ONE-image viewer: no counter means no
|
||||
// second member, so ← / → cannot page onto the stray.
|
||||
click(thumbFor(PNG_A));
|
||||
expect(viewerShowing()).toBe(PNG_A);
|
||||
expect(document.querySelector('.lightbox-counter')).toBeNull();
|
||||
});
|
||||
|
||||
it('admits an image once a later probe resolves its MIME', async () => {
|
||||
// The other half of failing safe: refusal is provisional, not a latch.
|
||||
MIMES[UNPROBED] = 'image/png';
|
||||
try {
|
||||
timelineListMock.mockResolvedValue(respond([UNPROBED, PNG_A]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
click(thumbFor(UNPROBED));
|
||||
expect(viewerShowing()).toBe(UNPROBED);
|
||||
} finally {
|
||||
delete MIMES[UNPROBED];
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('ItemTimeline — interactive semantics track the gate (TASK-2431)', () => {
|
||||
it('marks allowlisted thumbnails as buttons and leaves refused ones inert', async () => {
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A, SVG, TIFF]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
const png = thumbFor(PNG_A);
|
||||
expect(png.getAttribute('role')).toBe('button');
|
||||
expect(png.getAttribute('tabindex')).toBe('0');
|
||||
expect(png.getAttribute('aria-label')).toContain('View image');
|
||||
|
||||
for (const refused of [thumbFor(SVG), thumbFor(TIFF)]) {
|
||||
// A focus stop announced as a button whose activation does nothing is
|
||||
// worse than the hole it replaces.
|
||||
expect(refused.getAttribute('role')).toBeNull();
|
||||
expect(refused.getAttribute('tabindex')).toBeNull();
|
||||
expect(refused.getAttribute('aria-label')).toBeNull();
|
||||
}
|
||||
});
|
||||
|
||||
it('re-applies them when the kind filter rebuilds the cards', async () => {
|
||||
// The pane's Activity / Versions tabs filter the RENDERED set without
|
||||
// refetching: `entries` is untouched while every comment card is
|
||||
// destroyed and rebuilt. The rebuilt image keeps working by mouse (the
|
||||
// listeners are delegated to the container), so a pass that missed this
|
||||
// left it openable by mouse and unreachable by keyboard.
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A]));
|
||||
props.visibleKinds = ['comment'];
|
||||
render();
|
||||
await settle();
|
||||
expect(thumbFor(PNG_A).getAttribute('role')).toBe('button');
|
||||
|
||||
props.visibleKinds = ['activity'];
|
||||
flushSync();
|
||||
await settle();
|
||||
expect(thumbs()).toHaveLength(0);
|
||||
|
||||
props.visibleKinds = ['comment'];
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
const png = thumbFor(PNG_A);
|
||||
expect(png.getAttribute('role')).toBe('button');
|
||||
expect(png.getAttribute('tabindex')).toBe('0');
|
||||
pressOn(png, 'Enter');
|
||||
expect(viewerShowing()).toBe(PNG_A);
|
||||
});
|
||||
});
|
||||
|
||||
describe('ItemTimeline — A→B viewer lifecycle (TASK-2431)', () => {
|
||||
it('closes an open viewer when the workspace switches under the same ref', async () => {
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A, PNG_B]));
|
||||
props.wsSlug = 'ws-one';
|
||||
render();
|
||||
await settle();
|
||||
|
||||
click(thumbFor(PNG_A));
|
||||
expect(viewerShowing()).toBe(PNG_A);
|
||||
expect(document.querySelector('.lightbox-image')?.getAttribute('src')).toContain('ws-one');
|
||||
|
||||
// Same itemSlug, different workspace — the case the component had no
|
||||
// reset for at all. A viewer left up here keeps showing ws-one's ids
|
||||
// while the timeline underneath is ws-two's.
|
||||
props.wsSlug = 'ws-two';
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
expect(viewerOpen()).toBe(false);
|
||||
});
|
||||
|
||||
it('closes an open viewer when the item switches', async () => {
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A, PNG_B]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
click(thumbFor(PNG_A));
|
||||
expect(viewerOpen()).toBe(true);
|
||||
|
||||
props.itemSlug = 'TASK-2';
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
expect(viewerOpen()).toBe(false);
|
||||
});
|
||||
|
||||
it('leaves an open viewer alone when nothing switched', async () => {
|
||||
timelineListMock.mockResolvedValue(respond([PNG_A, PNG_B]));
|
||||
render();
|
||||
await settle();
|
||||
|
||||
click(thumbFor(PNG_A));
|
||||
expect(viewerOpen()).toBe(true);
|
||||
|
||||
// A prop that is not part of the view identity must not tear the viewer
|
||||
// down — a reset keyed too broadly is its own bug.
|
||||
props.username = 'bob';
|
||||
flushSync();
|
||||
await settle();
|
||||
|
||||
expect(viewerShowing()).toBe(PNG_A);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,17 @@
|
||||
<!--
|
||||
Test double for `CommentEditor` (TASK-2431).
|
||||
|
||||
`ItemTimeline` mounts one as its composer, and it drags in Tiptap — which
|
||||
these tests neither exercise nor need. The timeline's own behaviour under
|
||||
test (which inline images may be handed to the viewer) is entirely in the
|
||||
rendered comment bodies, so the composer is replaced with a marker.
|
||||
|
||||
Accepts and ignores every prop: a Svelte 5 rest-props catch-all, so adding a
|
||||
prop to the real component can never break this file.
|
||||
-->
|
||||
<script lang="ts">
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
let _props: Record<string, unknown> = $props();
|
||||
</script>
|
||||
|
||||
<div class="comment-editor-stub"></div>
|
||||
Reference in New Issue
Block a user