Skip to content

Commit faad45e

Browse files
baozhoutaoclaude
andauthored
fix(fields): consistent image-field rendering + click-to-zoom (#2836) (#2837)
Resolve a bare `sys_file` id to the stable download URL so edit-form and inline-edit thumbnails no longer render broken; route file-backed types through their upload widgets in inline edit instead of a raw-URL text input; localize ImageField strings; add an ImageLightbox for click-to-enlarge with single/multiple support. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 553443e commit faad45e

10 files changed

Lines changed: 406 additions & 60 deletions

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
"@object-ui/fields": patch
3+
"@object-ui/plugin-detail": patch
4+
"@object-ui/i18n": patch
5+
---
6+
7+
fix(fields): render `image` fields consistently and add click-to-zoom (#2836)
8+
9+
An `image` field rendered differently — and wrongly — on three surfaces:
10+
11+
- **Edit form showed broken thumbnails.** A record read back its `image` value
12+
as a bare `sys_file` id (the reference form), but `readFileValue` returned an
13+
id with no URL — the comment assumed the read path expands it, which the
14+
edit-form data path does not. The result was `<img src="">`. `file-value` now
15+
derives the stable download URL (`/api/v1/storage/files/:id`, which
16+
302-redirects to a signed URL and works directly as `<img src>`) for a bare
17+
id or an id-only object, so every widget and cell renderer resolves one.
18+
- **Inline edit leaked the raw storage URL.** `InlineFieldInput` had no branch
19+
for file-backed types and fell through to a plain text input showing
20+
`/api/v1/storage/files/…`. It now renders the same upload widgets the form
21+
uses (`image`/`avatar`/`signature`/`file`/`video`/`audio`).
22+
- **Hard-coded English.** `ImageField`'s upload/crop/remove/alt strings now go
23+
through `t('fields.image.*')` (en + zh added).
24+
25+
Also adds an `ImageLightbox` — click a read-only thumbnail (detail or list cell)
26+
to open a full-screen preview; multiple images get prev/next navigation, a
27+
position counter and arrow-key support, a single image just the image. In a
28+
grid cell the click is `stopPropagation`-guarded so enlarging doesn't also open
29+
the row.

packages/fields/src/index.tsx

Lines changed: 75 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,8 @@ import { PercentField } from './widgets/PercentField';
303303
import { PasswordField } from './widgets/PasswordField';
304304
import { FileField } from './widgets/FileField';
305305
import { ImageField } from './widgets/ImageField';
306+
import { ImageLightbox } from './widgets/ImageLightbox';
307+
import { readFileValues } from './widgets/file-value';
306308
import { LocationField } from './widgets/LocationField';
307309
import { FormulaField } from './widgets/FormulaField';
308310
import { SummaryField } from './widgets/SummaryField';
@@ -1268,50 +1270,88 @@ export function FileCellRenderer({ value, field }: CellRendererProps): React.Rea
12681270
}
12691271

12701272
/**
1271-
* Image field cell renderer (with thumbnails)
1273+
* Image field cell renderer (with thumbnails + click-to-zoom).
1274+
*
1275+
* An image value may be a plain URL string, an object ({ url | src | href … }),
1276+
* a bare `sys_file` id, or an array of any of those. Normalising through
1277+
* `readFileValues` (which resolves a bare id to its stable download URL) means
1278+
* a string-URL field, a CDN link, and an unexpanded reference all render a
1279+
* thumbnail instead of a broken `<img src="">` placeholder.
1280+
*
1281+
* Clicking a thumbnail opens a full-screen lightbox (single or gallery). The
1282+
* click is `stopPropagation`-guarded so, inside a grid row, enlarging an image
1283+
* doesn't also trigger row navigation.
12721284
*/
12731285
export function ImageCellRenderer({ value }: CellRendererProps): React.ReactElement {
1274-
if (!value) return <EmptyValue />;
1286+
const { t } = useObjectTranslation();
1287+
const [lightboxIndex, setLightboxIndex] = React.useState<number | null>(null);
1288+
1289+
const imgs = React.useMemo(
1290+
() =>
1291+
readFileValues(value, 'Image')
1292+
.filter((v) => v.url)
1293+
.map((v) => ({ url: v.url as string, name: v.name })),
1294+
[value],
1295+
);
12751296

1276-
// An image value may be a plain URL string, an object ({ url | src | href }),
1277-
// or an array of either. Normalize so a string-URL field (common — e.g. a
1278-
// `cover` seeded with a CDN link) renders a thumbnail instead of a broken
1279-
// <img src=""> placeholder.
1280-
const urlOf = (v: any): string =>
1281-
typeof v === 'string' ? v : (v?.url || v?.src || v?.href || v?.thumbnailUrl || '');
1282-
const nameOf = (v: any, fallback: string): string =>
1283-
(v && typeof v === 'object' && (v.name || v.original_name)) || fallback;
1297+
if (!value || imgs.length === 0) return <EmptyValue />;
12841298

1285-
if (Array.isArray(value)) {
1286-
const imgs = value.map((v) => ({ url: urlOf(v), name: nameOf(v, 'Image') })).filter((i) => i.url);
1287-
if (imgs.length === 0) return <EmptyValue />;
1299+
const imageAlt = (idx: number, name?: string) =>
1300+
name || t('fields.image.imageAlt', { index: idx + 1 });
1301+
const open = (idx: number) => (e: React.MouseEvent) => {
1302+
e.stopPropagation();
1303+
setLightboxIndex(idx);
1304+
};
1305+
const multiple = imgs.length > 1;
1306+
1307+
const lightbox = lightboxIndex !== null && (
1308+
<ImageLightbox
1309+
images={imgs}
1310+
index={lightboxIndex}
1311+
open
1312+
onOpenChange={(o) => !o && setLightboxIndex(null)}
1313+
onIndexChange={setLightboxIndex}
1314+
/>
1315+
);
1316+
1317+
if (multiple) {
12881318
return (
1289-
<div className="flex -space-x-2">
1290-
{imgs.slice(0, 3).map((img, idx) => (
1291-
<img
1292-
key={idx}
1293-
src={img.url}
1294-
alt={img.name || `Image ${idx + 1}`}
1295-
className="size-8 rounded-md border-2 border-white object-cover"
1296-
/>
1297-
))}
1298-
{imgs.length > 3 && (
1299-
<div className="size-8 rounded-md border-2 border-white bg-gray-100 flex items-center justify-center text-xs font-medium text-gray-600">
1300-
+{imgs.length - 3}
1301-
</div>
1302-
)}
1303-
</div>
1319+
<>
1320+
<div className="flex -space-x-2">
1321+
{imgs.slice(0, 3).map((img, idx) => (
1322+
<img
1323+
key={idx}
1324+
src={img.url}
1325+
alt={imageAlt(idx, img.name)}
1326+
onClick={open(idx)}
1327+
className="size-8 cursor-zoom-in rounded-md border-2 border-background object-cover transition-transform hover:scale-110 hover:z-10"
1328+
/>
1329+
))}
1330+
{imgs.length > 3 && (
1331+
<button
1332+
type="button"
1333+
onClick={open(3)}
1334+
className="size-8 rounded-md border-2 border-background bg-muted flex items-center justify-center text-xs font-medium text-muted-foreground hover:bg-muted/80"
1335+
>
1336+
+{imgs.length - 3}
1337+
</button>
1338+
)}
1339+
</div>
1340+
{lightbox}
1341+
</>
13041342
);
13051343
}
13061344

1307-
const url = urlOf(value);
1308-
if (!url) return <EmptyValue />;
13091345
return (
1310-
<img
1311-
src={url}
1312-
alt={nameOf(value, 'Image')}
1313-
className="size-10 rounded-md object-cover"
1314-
/>
1346+
<>
1347+
<img
1348+
src={imgs[0].url}
1349+
alt={imageAlt(0, imgs[0].name)}
1350+
onClick={open(0)}
1351+
className="size-10 cursor-zoom-in rounded-md object-cover transition-transform hover:scale-105"
1352+
/>
1353+
{lightbox}
1354+
</>
13151355
);
13161356
}
13171357

packages/fields/src/widgets/ImageField.tsx

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import React, { useRef, useState, useCallback, lazy, Suspense } from 'react';
22
import { Button, EmptyValue } from '@object-ui/components';
33
import { useUpload } from '@object-ui/providers';
4-
import { Upload, X, Image as ImageIcon, Crop as CropIcon, Loader2 } from 'lucide-react';
4+
import { useObjectTranslation } from '@object-ui/i18n';
5+
import { X, Image as ImageIcon, Crop as CropIcon, Loader2 } from 'lucide-react';
56
import { FieldWidgetProps } from './types';
7+
import { ImageLightbox } from './ImageLightbox';
68
import { useUploadingSignal } from './useUploadingSignal';
79
import {
810
fileValueForSubmit,
@@ -32,7 +34,9 @@ export function ImageField({ value, onChange, field, readonly, onUploadingChange
3234
*/
3335
const cropEnabled = imageField?.crop !== false;
3436
const [cropTarget, setCropTarget] = useState<{ index: number; src: string; name: string } | null>(null);
37+
const [lightboxIndex, setLightboxIndex] = useState<number | null>(null);
3538
const { upload } = useUpload();
39+
const { t } = useObjectTranslation();
3640
const [uploading, setUploading] = useState(false);
3741
// Display details of just-uploaded images, keyed by their new `sys_file` id.
3842
// Submitting the reference form means the field value no longer carries the
@@ -83,20 +87,41 @@ export function ImageField({ value, onChange, field, readonly, onUploadingChange
8387
[views],
8488
);
8589

90+
const lightboxImages = views.filter((v) => v.url).map((v) => ({ url: v.url as string, name: v.name }));
91+
8692
if (readonly) {
87-
if (!value) return <EmptyValue />;
93+
if (!value || lightboxImages.length === 0) return <EmptyValue />;
8894

8995
return (
90-
<div className="flex flex-wrap gap-2">
91-
{views.map((img, idx) => (
92-
<img
93-
key={idx}
94-
src={img.url || ''}
95-
alt={img.name || `Image ${idx + 1}`}
96-
className="size-20 rounded-md object-cover border border-gray-200"
96+
<>
97+
<div className="flex flex-wrap gap-2">
98+
{lightboxImages.map((img, idx) => (
99+
<button
100+
key={idx}
101+
type="button"
102+
onClick={() => setLightboxIndex(idx)}
103+
className="group relative overflow-hidden rounded-md border border-border focus:outline-none focus:ring-2 focus:ring-ring"
104+
aria-label={t('fields.image.enlarge', { name: img.name || t('fields.image.imageAlt', { index: idx + 1 }) })}
105+
>
106+
<img
107+
src={img.url}
108+
alt={img.name || t('fields.image.imageAlt', { index: idx + 1 })}
109+
className="size-20 object-cover transition-transform duration-150 group-hover:scale-105"
110+
/>
111+
<span className="pointer-events-none absolute inset-0 bg-black/0 transition-colors group-hover:bg-black/10" />
112+
</button>
113+
))}
114+
</div>
115+
{lightboxIndex !== null && (
116+
<ImageLightbox
117+
images={lightboxImages}
118+
index={lightboxIndex}
119+
open
120+
onOpenChange={(o) => !o && setLightboxIndex(null)}
121+
onIndexChange={setLightboxIndex}
97122
/>
98-
))}
99-
</div>
123+
)}
124+
</>
100125
);
101126
}
102127

@@ -153,7 +178,7 @@ export function ImageField({ value, onChange, field, readonly, onUploadingChange
153178
<div key={idx} className="relative group">
154179
<img
155180
src={img.url || ''}
156-
alt={img.name || `Image ${idx + 1}`}
181+
alt={img.name || t('fields.image.imageAlt', { index: idx + 1 })}
157182
className="size-20 rounded-md object-cover border border-gray-200"
158183
/>
159184
<div className="absolute top-1 right-1 flex gap-1 opacity-0 group-hover:opacity-100 transition-opacity">
@@ -164,7 +189,7 @@ export function ImageField({ value, onChange, field, readonly, onUploadingChange
164189
size="sm"
165190
onClick={() => openCropper(idx)}
166191
className="h-6 w-6 p-0"
167-
aria-label={`Crop image ${idx + 1}`}
192+
aria-label={t('fields.image.crop', { index: idx + 1 })}
168193
data-testid={`image-field-crop-${idx}`}
169194
>
170195
<CropIcon className="size-3" />
@@ -176,6 +201,7 @@ export function ImageField({ value, onChange, field, readonly, onUploadingChange
176201
size="sm"
177202
onClick={() => handleRemove(idx)}
178203
className="h-6 w-6 p-0"
204+
aria-label={t('fields.image.remove', { index: idx + 1 })}
179205
>
180206
<X className="size-3" />
181207
</Button>
@@ -198,7 +224,11 @@ export function ImageField({ value, onChange, field, readonly, onUploadingChange
198224
) : (
199225
<ImageIcon className="size-4 mr-2" />
200226
)}
201-
{uploading ? 'Uploading…' : images.length > 0 ? 'Add More Images' : 'Upload Image'}
227+
{uploading
228+
? t('fields.image.uploading')
229+
: images.length > 0
230+
? t('fields.image.addMore')
231+
: t('fields.image.upload')}
202232
</Button>
203233
</div>
204234

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { render, screen, fireEvent } from '@testing-library/react';
2+
import { describe, it, expect } from 'vitest';
3+
import { I18nProvider } from '@object-ui/i18n';
4+
import { ImageLightbox, type ImageLightboxProps } from './ImageLightbox';
5+
6+
const imgs = [
7+
{ url: '/api/v1/storage/files/a', name: 'a.png' },
8+
{ url: '/api/v1/storage/files/b', name: 'b.png' },
9+
{ url: '/api/v1/storage/files/c', name: 'c.png' },
10+
];
11+
12+
const renderLightbox = (props: ImageLightboxProps) =>
13+
render(
14+
<I18nProvider config={{ defaultLanguage: 'en', detectBrowserLanguage: false }}>
15+
<ImageLightbox {...props} />
16+
</I18nProvider>,
17+
);
18+
19+
describe('ImageLightbox', () => {
20+
it('renders the image at the requested index', () => {
21+
renderLightbox({ images: imgs, index: 1, open: true, onOpenChange: () => {} });
22+
const img = screen.getByRole('img') as HTMLImageElement;
23+
expect(img.src).toContain('/api/v1/storage/files/b');
24+
});
25+
26+
it('shows navigation and a position counter for multiple images', () => {
27+
renderLightbox({ images: imgs, index: 0, open: true, onOpenChange: () => {} });
28+
expect(screen.getByText('1 / 3')).toBeInTheDocument();
29+
expect(screen.getByLabelText('Next image')).toBeInTheDocument();
30+
expect(screen.getByLabelText('Previous image')).toBeInTheDocument();
31+
});
32+
33+
it('omits navigation for a single image', () => {
34+
renderLightbox({ images: [imgs[0]], index: 0, open: true, onOpenChange: () => {} });
35+
expect(screen.queryByLabelText('Next image')).not.toBeInTheDocument();
36+
expect(screen.queryByText('1 / 1')).not.toBeInTheDocument();
37+
});
38+
39+
it('advances to the next image and wraps around', () => {
40+
const seen: number[] = [];
41+
renderLightbox({ images: imgs, index: 2, open: true, onOpenChange: () => {}, onIndexChange: (i) => seen.push(i) });
42+
fireEvent.click(screen.getByLabelText('Next image'));
43+
expect(seen).toEqual([0]); // wrapped 2 -> 0
44+
});
45+
46+
it('steps to the previous image and wraps around', () => {
47+
const seen: number[] = [];
48+
renderLightbox({ images: imgs, index: 0, open: true, onOpenChange: () => {}, onIndexChange: (i) => seen.push(i) });
49+
fireEvent.click(screen.getByLabelText('Previous image'));
50+
expect(seen).toEqual([2]); // wrapped 0 -> 2
51+
});
52+
});

0 commit comments

Comments
 (0)