Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-media-picker-broken-image.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@emdash-cms/admin": patch
---

Fixes broken image collapsing media picker container — adds `onError` handler and fallback placeholder so Change/Remove buttons remain accessible when referenced image is missing from storage
88 changes: 64 additions & 24 deletions packages/admin/src/components/BlockKitMediaPickerField.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Button } from "@cloudflare/kumo";
import { useLingui } from "@lingui/react/macro";
import { Image as ImageIcon, X } from "@phosphor-icons/react";
import { Image as ImageIcon, ImageBroken, X } from "@phosphor-icons/react";
import * as React from "react";

import type { MediaItem } from "../lib/api";
Expand Down Expand Up @@ -35,7 +35,12 @@ export function BlockKitMediaPickerField({
}: BlockKitMediaPickerFieldProps) {
const { t } = useLingui();
const [pickerOpen, setPickerOpen] = React.useState(false);
const [imageBroken, setImageBroken] = React.useState(false);
const url = typeof value === "string" && value.length > 0 ? value : "";

React.useEffect(() => {
setImageBroken(false);
}, [url]);
const filter = mimeTypeFilter ?? "image/";
const canPreview = isSafePreviewUrl(url);

Expand All @@ -55,30 +60,65 @@ export function BlockKitMediaPickerField({
<div>
<label className="text-sm font-medium mb-1.5 block">{label}</label>
{canPreview ? (
<div className="relative group">
<img
src={url}
alt=""
className="max-h-40 w-full rounded-md border border-kumo-line object-contain bg-kumo-muted"
referrerPolicy="no-referrer"
loading="lazy"
/>
<div className="absolute top-2 end-2 opacity-0 pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto group-focus-within:opacity-100 group-focus-within:pointer-events-auto transition-opacity flex gap-1">
<Button type="button" size="sm" variant="secondary" onClick={() => setPickerOpen(true)}>
{t`Change`}
</Button>
<Button
type="button"
shape="square"
variant="destructive"
className="h-8 w-8"
onClick={() => onChange(actionId, "")}
aria-label={t`Remove`}
>
<X className="h-4 w-4" />
</Button>
imageBroken ? (
<div className="relative group min-h-20">
<div className="min-h-20 w-full rounded-md border border-kumo-line bg-kumo-muted flex items-center justify-center gap-2 text-kumo-subtle">
<ImageBroken className="h-5 w-5" />
<span className="text-sm">{t`Image not found`}</span>
</div>
<div className="absolute top-2 end-2 opacity-0 pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto group-focus-within:opacity-100 group-focus-within:pointer-events-auto transition-opacity flex gap-1">
<Button
type="button"
size="sm"
variant="secondary"
onClick={() => setPickerOpen(true)}
>
{t`Change`}
</Button>
<Button
type="button"
shape="square"
variant="destructive"
className="h-8 w-8"
onClick={() => onChange(actionId, "")}
aria-label={t`Remove`}
>
<X className="h-4 w-4" />
</Button>
</div>
</div>
) : (
<div className="relative group">
<img
src={url}
alt=""
className="max-h-40 min-h-20 w-full rounded-md border border-kumo-line object-contain bg-kumo-muted"
referrerPolicy="no-referrer"
loading="lazy"
onError={() => setImageBroken(true)}
/>
<div className="absolute top-2 end-2 opacity-0 pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto group-focus-within:opacity-100 group-focus-within:pointer-events-auto transition-opacity flex gap-1">
<Button
type="button"
size="sm"
variant="secondary"
onClick={() => setPickerOpen(true)}
>
{t`Change`}
</Button>
<Button
type="button"
shape="square"
variant="destructive"
className="h-8 w-8"
onClick={() => onChange(actionId, "")}
aria-label={t`Remove`}
>
<X className="h-4 w-4" />
</Button>
</div>
</div>
</div>
)
) : (
<Button
type="button"
Expand Down
79 changes: 62 additions & 17 deletions packages/admin/src/components/ContentEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
ArrowsInSimple,
ArrowsOutSimple,
ArrowSquareOut,
ImageBroken,
} from "@phosphor-icons/react";
import { Link, useNavigate } from "@tanstack/react-router";
import type { Editor } from "@tiptap/react";
Expand Down Expand Up @@ -1574,6 +1575,7 @@ function ImageFieldRenderer({
}: ImageFieldRendererProps) {
const { t } = useLingui();
const [pickerOpen, setPickerOpen] = React.useState(false);
const [imageBroken, setImageBroken] = React.useState(false);
// Normalize value to get display URL (handles both object and legacy string)
// Prefer previewUrl for admin display, fall back to src, then derive from storageKey/id
const displayUrl =
Expand All @@ -1585,6 +1587,10 @@ function ImageFieldRenderer({
? `/_emdash/api/media/file/${typeof value.meta?.storageKey === "string" ? value.meta.storageKey : value.id}`
: undefined);

React.useEffect(() => {
setImageBroken(false);
}, [displayUrl]);

const handleSelect = (item: MediaItem) => {
const isLocalProvider = !item.provider || item.provider === "local";

Expand All @@ -1609,24 +1615,63 @@ function ImageFieldRenderer({
<div id={id}>
<Label>{label}</Label>
{displayUrl ? (
<div className="mt-2 relative group">
<img src={displayUrl} alt="" className="max-h-48 rounded-lg border object-cover" />
<div className="absolute top-2 end-2 opacity-0 group-hover:opacity-100 transition-opacity flex gap-1">
<Button type="button" size="sm" variant="secondary" onClick={() => setPickerOpen(true)}>
{t`Change`}
</Button>
<Button
type="button"
shape="square"
variant="destructive"
className="h-8 w-8"
onClick={handleRemove}
aria-label={t`Remove image`}
>
<X className="h-4 w-4" />
</Button>
imageBroken ? (
<div className="mt-2 relative group">
<div className="min-h-20 rounded-lg border bg-kumo-muted flex items-center justify-center gap-2 text-kumo-subtle">
<ImageBroken className="h-5 w-5" />
<span className="text-sm">{t`Image not found`}</span>
</div>
<div className="absolute top-2 end-2 opacity-0 group-hover:opacity-100 transition-opacity flex gap-1">
<Button
type="button"
size="sm"
variant="secondary"
onClick={() => setPickerOpen(true)}
>
{t`Change`}
</Button>
<Button
type="button"
shape="square"
variant="destructive"
className="h-8 w-8"
onClick={handleRemove}
aria-label={t`Remove image`}
>
<X className="h-4 w-4" />
</Button>
</div>
</div>
</div>
) : (
<div className="mt-2 relative group">
<img
src={displayUrl}
alt=""
className="max-h-48 min-h-20 rounded-lg border object-cover"
onError={() => setImageBroken(true)}
/>
<div className="absolute top-2 end-2 opacity-0 group-hover:opacity-100 transition-opacity flex gap-1">
<Button
type="button"
size="sm"
variant="secondary"
onClick={() => setPickerOpen(true)}
>
{t`Change`}
</Button>
<Button
type="button"
shape="square"
variant="destructive"
className="h-8 w-8"
onClick={handleRemove}
aria-label={t`Remove image`}
>
<X className="h-4 w-4" />
</Button>
</div>
</div>
)
) : (
<Button
type="button"
Expand Down
47 changes: 47 additions & 0 deletions packages/admin/tests/components/BlockKitMediaPickerField.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,53 @@ describe("BlockKitMediaPickerField", () => {
});
});

describe("broken image", () => {
it("shows a broken-image placeholder when the img fails to load", async () => {
const { screen } = await renderField({
value: "/_emdash/api/media/file/missing.png",
});

// Image should render initially
const img = await waitForImg();
expect(img.getAttribute("src")).toBe("/_emdash/api/media/file/missing.png");

// Simulate image load failure
img.dispatchEvent(new Event("error"));

// Broken-image placeholder should appear
await expect.element(screen.getByText("Image not found")).toBeInTheDocument();
});

it("keeps Change and Remove buttons accessible when the image is broken", async () => {
const { screen } = await renderField({
value: "/_emdash/api/media/file/missing.png",
});

const img = await waitForImg();
img.dispatchEvent(new Event("error"));

// Both action buttons must remain in the DOM and accessible
await expect.element(screen.getByLabelText("Remove")).toBeInTheDocument();
await expect.element(screen.getByText("Change")).toBeInTheDocument();
});

it("maintains a minimum height so the container does not collapse", async () => {
const { screen } = await renderField({
value: "/_emdash/api/media/file/missing.png",
});

const img = await waitForImg();
img.dispatchEvent(new Event("error"));

// The broken-image placeholder should be visible (its presence confirms
// the container didn't collapse — a zero-height container can't show text)
await expect.element(screen.getByText("Image not found")).toBeInTheDocument();

// The remove button must remain accessible via label
await expect.element(screen.getByLabelText("Remove")).toBeInTheDocument();
});
});

describe("remove", () => {
it("clears the value when Remove is clicked", async () => {
const onChange = vi.fn();
Expand Down
Loading