-
Notifications
You must be signed in to change notification settings - Fork 926
fix(admin): render file field with media picker instead of text input #719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
67d7af1
b9b8654
1d7cffc
aa44e7f
7633080
8ec439f
94b2914
ac67adf
625f709
2d7417b
2359129
193424c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| --- | ||
| "@emdash-cms/admin": patch | ||
| "emdash": patch | ||
| --- | ||
|
|
||
| Fixes the `file` field type rendering as a plain text input in the content editor. Adds a `FileFieldRenderer` that opens the media picker (with mime filter disabled) so any file type can be attached. Also adds a `hideUrlInput` prop to `MediaPickerModal` so non-image pickers can hide the image-specific "Insert from URL" input. | ||
|
|
||
| Aligns the Zod schema and generated TypeScript types for `image` and `file` fields with the shape the admin actually stores: `provider?`, `meta?` (for both), and `previewUrl?` (for image). Previously these fields were stripped on validation and missing from generated types, so site code could not reliably resolve local media URLs from `meta.storageKey`. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,6 +17,7 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Eye, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Image as ImageIcon, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MagnifyingGlass, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Paperclip, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| X, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Trash, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ArrowsInSimple, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -37,8 +38,9 @@ import type { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "../lib/api"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { getPreviewUrl, getDraftStatus } from "../lib/api"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { fromDatetimeLocalInputValue, toDatetimeLocalInputValue } from "../lib/datetime-local.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { formatFileSize, getFileIcon } from "../lib/media-utils"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { usePluginAdmins } from "../lib/plugin-context.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { contentUrl } from "../lib/url.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { contentUrl, isSafeUrl } from "../lib/url.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { cn, slugify } from "../lib/utils"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ArrowPrev } from "./ArrowIcons.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { BlockKitFieldWidget } from "./BlockKitFieldWidget.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1342,6 +1344,24 @@ function FieldRenderer({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case "file": { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // value is either a FileFieldValue object or undefined. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // The file field type was unusable before this PR (rendered as a text input | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // that produced raw strings nobody could meaningfully save), so there is no | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // "legacy string" data to preserve here. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const fileValue = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value != null && typeof value === "object" ? (value as FileFieldValue) : undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <FileFieldRenderer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| id={id} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| label={label} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value={fileValue} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onChange={handleChange} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required={field.required} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1354
to
+1360
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | |
| <FileFieldRenderer | |
| id={id} | |
| label={label} | |
| value={fileValue} | |
| onChange={handleChange} | |
| required={field.required} | |
| const legacyFileUrl = typeof value === "string" ? value : undefined; | |
| const resolvedLegacyFileUrl = legacyFileUrl | |
| ? isSafeUrl(legacyFileUrl) | |
| ? legacyFileUrl | |
| : contentUrl(legacyFileUrl) | |
| : undefined; | |
| const legacyFileName = | |
| legacyFileUrl?.split("/").filter(Boolean).pop() ?? legacyFileUrl ?? ""; | |
| if (resolvedLegacyFileUrl) { | |
| return ( | |
| <fieldset> | |
| <Label className={labelClass}>{label}</Label> | |
| <div className="mt-2 flex items-center justify-between gap-3 rounded-md border p-3"> | |
| <div className="flex min-w-0 items-center gap-2"> | |
| <Paperclip className="size-4 shrink-0 text-muted-foreground" /> | |
| <a | |
| href={resolvedLegacyFileUrl} | |
| target="_blank" | |
| rel="noreferrer" | |
| className="flex min-w-0 items-center gap-1 truncate text-sm text-blue-600 hover:underline" | |
| > | |
| <span className="truncate">{legacyFileName}</span> | |
| <ArrowSquareOut className="size-4 shrink-0" /> | |
| </a> | |
| </div> | |
| <Button type="button" onClick={() => handleChange(undefined)}> | |
| <X className="size-4" /> | |
| </Button> | |
| </div> | |
| </fieldset> | |
| ); | |
| } | |
| return ( | |
| <FileFieldRenderer | |
| id={id} | |
| label={label} | |
| value={fileValue} | |
| onChange={handleChange} | |
| required={field.required && !legacyFileUrl} |
Copilot
AI
Apr 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileFieldRenderer only builds a link for local files when meta.storageKey is present, and ignores value.src entirely for local providers. Since the file schema/type allows src? and meta?, a valid local file value without meta.storageKey (or one that only has a local src like /_emdash/api/media/file/...) will render without a download link. Consider falling back to value.src when it’s an internal relative media URL, and/or falling back to value.id (matching the existing ImageFieldRenderer behavior) so local files remain clickable even when meta.storageKey is missing.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ | |
| import { Button, Dialog, Input, Label, Loader } from "@cloudflare/kumo"; | ||
| import { plural } from "@lingui/core/macro"; | ||
| import { useLingui } from "@lingui/react/macro"; | ||
| import { Upload, Image, Check, Globe, MagnifyingGlass } from "@phosphor-icons/react"; | ||
| import { Upload, Image, Check, Globe, MagnifyingGlass, Paperclip } from "@phosphor-icons/react"; | ||
| import { X } from "@phosphor-icons/react"; | ||
| import { useQuery, useMutation, useQueryClient } from "@tanstack/react-query"; | ||
| import * as React from "react"; | ||
|
|
@@ -42,6 +42,18 @@ export interface MediaPickerModalProps { | |
| /** Filter by mime type prefix, e.g. "image/" */ | ||
| mimeTypeFilter?: string; | ||
| title?: string; | ||
| /** | ||
| * Hide the "Insert from URL" input. Defaults to false. | ||
| * The URL input probes image dimensions and is only meaningful for image pickers, | ||
| * so non-image pickers (e.g. generic file pickers) should hide it. | ||
| */ | ||
| hideUrlInput?: boolean; | ||
|
Comment on lines
+45
to
+50
|
||
| /** | ||
| * What kind of media this picker is for. Drives user-facing copy | ||
| * (default title, empty-state message, upload button label, empty-state icon). | ||
| * Defaults to "image" — set to "file" for generic file pickers. | ||
| */ | ||
| mediaKind?: "image" | "file"; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -66,9 +78,17 @@ export function MediaPickerModal({ | |
| onSelect, | ||
| mimeTypeFilter = "image/", | ||
| title: providedTitle, | ||
| hideUrlInput = false, | ||
| mediaKind = "image", | ||
| }: MediaPickerModalProps) { | ||
| const { t } = useLingui(); | ||
| const title = providedTitle ?? t`Select Image`; | ||
| const isFileKind = mediaKind === "file"; | ||
| const title = providedTitle ?? (isFileKind ? t`Select File` : t`Select Image`); | ||
| const emptyStateUploadHint = isFileKind | ||
| ? t`Upload a file to get started` | ||
| : t`Upload an image to get started`; | ||
| const emptyStateUploadCta = isFileKind ? t`Upload File` : t`Upload Image`; | ||
| const EmptyStateIcon = isFileKind ? Paperclip : Image; | ||
| const queryClient = useQueryClient(); | ||
| const [selectedItem, setSelectedItem] = React.useState<SelectedMedia | null>(null); | ||
| const [activeProvider, setActiveProvider] = React.useState<string>("local"); | ||
|
|
@@ -362,41 +382,45 @@ export function MediaPickerModal({ | |
| /> | ||
| </div> | ||
|
|
||
| {/* URL Input */} | ||
| <div className="border-b pb-4"> | ||
| <Label>{t`Insert from URL`}</Label> | ||
| <div className="flex gap-2 mt-1.5"> | ||
| <div className="flex-1 relative"> | ||
| <Globe className="absolute start-3 top-1/2 -translate-y-1/2 h-4 w-4 text-kumo-subtle" /> | ||
| <Input | ||
| type="url" | ||
| placeholder="https://example.com/image.jpg" | ||
| aria-label={t`Image URL`} | ||
| value={imageUrl} | ||
| onChange={(e) => { | ||
| setImageUrl(e.target.value); | ||
| setUrlError(null); | ||
| }} | ||
| onKeyDown={handleUrlKeyDown} | ||
| className="ps-9" | ||
| /> | ||
| {/* URL Input (image pickers only — probes image dimensions) */} | ||
| {!hideUrlInput && ( | ||
| <> | ||
| <div className="border-b pb-4"> | ||
| <Label>{t`Insert from URL`}</Label> | ||
| <div className="flex gap-2 mt-1.5"> | ||
| <div className="flex-1 relative"> | ||
| <Globe className="absolute start-3 top-1/2 -translate-y-1/2 h-4 w-4 text-kumo-subtle" /> | ||
| <Input | ||
| type="url" | ||
| placeholder="https://example.com/image.jpg" | ||
| aria-label={t`Image URL`} | ||
| value={imageUrl} | ||
| onChange={(e) => { | ||
| setImageUrl(e.target.value); | ||
| setUrlError(null); | ||
| }} | ||
| onKeyDown={handleUrlKeyDown} | ||
| className="ps-9" | ||
| /> | ||
| </div> | ||
| <Button onClick={handleUrlSubmit} disabled={!imageUrl.trim() || isProbing}> | ||
| {isProbing ? <Loader size="sm" /> : t`Insert`} | ||
| </Button> | ||
| </div> | ||
| {urlError && <p className="text-sm text-kumo-danger mt-1">{urlError}</p>} | ||
| </div> | ||
| <Button onClick={handleUrlSubmit} disabled={!imageUrl.trim() || isProbing}> | ||
| {isProbing ? <Loader size="sm" /> : t`Insert`} | ||
| </Button> | ||
| </div> | ||
| {urlError && <p className="text-sm text-kumo-danger mt-1">{urlError}</p>} | ||
| </div> | ||
|
|
||
| {/* Divider with "or" */} | ||
| <div className="relative py-2"> | ||
| <div className="absolute inset-0 flex items-center"> | ||
| <span className="w-full border-t" /> | ||
| </div> | ||
| <div className="relative flex justify-center text-xs uppercase"> | ||
| <span className="bg-kumo-base px-2 text-kumo-subtle">{t`or choose from library`}</span> | ||
| </div> | ||
| </div> | ||
| {/* Divider with "or" */} | ||
| <div className="relative py-2"> | ||
| <div className="absolute inset-0 flex items-center"> | ||
| <span className="w-full border-t" /> | ||
| </div> | ||
| <div className="relative flex justify-center text-xs uppercase"> | ||
| <span className="bg-kumo-base px-2 text-kumo-subtle">{t`or choose from library`}</span> | ||
| </div> | ||
| </div> | ||
| </> | ||
| )} | ||
|
|
||
| {/* Provider Tabs */} | ||
| {providerTabs.length > 1 && ( | ||
|
|
@@ -487,13 +511,13 @@ export function MediaPickerModal({ | |
| </div> | ||
| ) : items.length === 0 ? ( | ||
| <div className="flex flex-col items-center justify-center h-full text-center p-8"> | ||
| <Image className="h-12 w-12 text-kumo-subtle mb-4" aria-hidden="true" /> | ||
| <EmptyStateIcon className="h-12 w-12 text-kumo-subtle mb-4" aria-hidden="true" /> | ||
| <h3 className="text-lg font-medium">{t`No media found`}</h3> | ||
| <p className="text-sm text-kumo-subtle mt-1"> | ||
| {canSearch && searchQuery | ||
| ? t`Try a different search term` | ||
| : canUpload | ||
| ? t`Upload an image to get started` | ||
| ? emptyStateUploadHint | ||
| : t`No media available from this provider`} | ||
| </p> | ||
| {canUpload && !searchQuery && ( | ||
|
|
@@ -502,7 +526,7 @@ export function MediaPickerModal({ | |
| icon={<Upload />} | ||
| onClick={() => fileInputRef.current?.click()} | ||
| > | ||
| {t`Upload Image`} | ||
| {emptyStateUploadCta} | ||
| </Button> | ||
| )} | ||
| </div> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs fixing: The comment says
valuemay be "a legacy string URL", but the very next line casts any string toundefinedbefore passing it toFileFieldRenderer. This makes existing string data invisible in the UI — a regression from the previous behavior where it at least rendered in a text input. For parity withImageFieldRenderer(which acceptsstring | undefinedand renders legacy URLs), consider passing rawvaluetoFileFieldRendererand handling strings there (e.g., showing them as a plain link with a clear button), or update the comment to remove the legacy-string claim if dropping support is intentional.