Skip to content

Commit 9d4a429

Browse files
os-zhuangclaude
andauthored
fix(form+detail): single-file children stay inline grids; drop non-spec attachment (#2654, #2655) (#2656)
* fix(form+detail): single-file children stay inline grids; drop non-spec attachment (#2654, #2655) Follow-ups to the upload-in-grid work (#2360). #2654 — file/image/avatar render a compact upload cell in the grid now, so a child object with a single such field no longer flips the smart inlineEdit default to a per-row form. resolveInlineMode splits FORM_ONLY_TYPES: truly form-only types (textarea/richtext/html/markdown/json/location/address) still tip to form alone, while file-family types only tip when several rich fields pile up (RICH_FIELD_FORM_THRESHOLD, default 2). Explicit inlineEdit still wins. #2655 — attachment is not a @objectstack/spec field type (spec media types are file/image/avatar/video/audio), so the renderer no longer models it: removed from fieldTypeToColumnType, the inline-mode heuristic, and RelatedList's auto-column SKIP_TYPES. Contract-first cleanup (AGENTS.md #0.1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5tsBSEhsDZmdcHBvfymYC * test(e2e): live spec for file upload in the inline line-item grid (#2360) Codifies the dogfood pass that shipped #2360 as a repeatable live e2e next to the sibling showcase_invoice specs: the Receipt file field auto-derives a grid column, the cell is a real input[type=file] upload control (not a text input), uploading shows a chip, and the resolved file object persists on the line in the atomic /api/v1/batch. Live-only (needs the storage service); runs under `pnpm test:e2e:live`, not the mocked PR e2e job. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H5tsBSEhsDZmdcHBvfymYC --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent be0712c commit 9d4a429

6 files changed

Lines changed: 182 additions & 10 deletions

File tree

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
"@object-ui/plugin-form": minor
3+
"@object-ui/plugin-detail": patch
4+
---
5+
6+
fix(form+detail): keep single-file children as inline grids; drop non-spec `attachment` handling
7+
8+
Two follow-ups to the upload-in-grid work (objectui#2360):
9+
10+
- **#2654** — Now that `file`/`image`/`avatar` fields render a compact upload
11+
cell in the line-item grid, a child object with a *single* such field no
12+
longer flips the smart `inlineEdit` default to a per-row form. `resolveInlineMode`
13+
splits the old `FORM_ONLY_TYPES`: truly form-only types (textarea / richtext /
14+
html / markdown / json / location / address) still tip to `form` on their own,
15+
while file-family types only tip when several rich fields pile up
16+
(`RICH_FIELD_FORM_THRESHOLD`, default 2). An explicit `inlineEdit` always wins.
17+
18+
- **#2655**`attachment` is not a `@objectstack/spec` field type (the spec
19+
media types are file/image/avatar/video/audio), so the renderer no longer
20+
models it: removed from `fieldTypeToColumnType`, the inline-mode heuristic, and
21+
`RelatedList`'s auto-column `SKIP_TYPES`. Contract-first cleanup — the renderer
22+
stops fossilizing a phantom type (AGENTS.md #0.1).

content/docs/fields/grid.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,13 @@ When grid columns are auto-derived from a child object's schema (master-detail
7979
subforms), `file` / `image` / `avatar` fields map to `file` columns
8080
automatically — image-flavoured fields default to `accept: ['image/*']`.
8181

82+
Because file/image/avatar now render in-grid, a child object with a *single*
83+
such field keeps the inline **grid** form factor by default (the smart
84+
`inlineEdit` heuristic no longer forces a per-row form for one attachment
85+
column). Only a truly form-only field (textarea / rich text / JSON / location)
86+
or **several** rich fields tips the default to the per-row form; an explicit
87+
`inlineEdit: 'grid' | 'form'` always wins.
88+
8289
## Data Format
8390

8491
Grid data is stored as an array of objects:

e2e/live/grid-file-upload.spec.ts

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import { test, expect } from '@playwright/test';
2+
3+
/**
4+
* Tier 0 live e2e: a `file` field on a master-detail child renders a real
5+
* UPLOAD control inside the inline line-item grid — not a degraded text input —
6+
* and the uploaded file persists on the line in the atomic /api/v1/batch
7+
* (objectui#2360). `showcase_invoice_line.receipt` is a `Field.file()`, so every
8+
* standard Invoice form's "Line Items" grid gets a per-row Receipt upload cell,
9+
* auto-derived from the data model (no columns config).
10+
*
11+
* This codifies the manual dogfood pass that shipped #2360:
12+
* • the Receipt column auto-derives into the grid (file fields are no longer
13+
* dropped from auto-columns);
14+
* • the cell is a genuine `input[type=file]` upload control (compact button +
15+
* removable chip), never a text `<Input>`;
16+
* • picking a file uploads it through the console's UploadProvider adapter and
17+
* shows a chip with the file name;
18+
* • submit carries the resolved file object ({ name, url, … }) on the line.
19+
*
20+
* Live-only (needs the storage service + a real backend); runs under
21+
* `pnpm test:e2e:live`, not the mocked PR e2e job.
22+
*/
23+
24+
// A 1×1 PNG — smallest valid image payload for the upload round-trip.
25+
const PNG = Buffer.from(
26+
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/q842iQAAAABJRU5ErkJggg==',
27+
'base64',
28+
);
29+
30+
test('inline line-item grid uploads a per-row file and persists it in the batch', async ({ page }) => {
31+
const batches: any[] = [];
32+
page.on('request', (r) => {
33+
if (r.method() === 'POST' && r.url().includes('/api/v1/batch')) {
34+
try { batches.push(r.postDataJSON()); } catch { /* ignore */ }
35+
}
36+
});
37+
38+
await page.goto('/apps/showcase_app/showcase_invoice');
39+
await page.getByRole('button', { name: /^New$/i }).first().click();
40+
41+
const dialog = page.getByRole('dialog');
42+
await expect(dialog.getByTestId('md-form-submit')).toBeVisible();
43+
await expect(dialog.getByText('Line Items', { exact: false })).toBeVisible();
44+
45+
const li = dialog.getByTestId('line-items');
46+
// The child schema loads async — wait for real headers, then assert Receipt.
47+
await li.locator('th', { hasText: 'Product' }).first().waitFor();
48+
await expect(li.locator('th', { hasText: 'Receipt' })).toHaveCount(1);
49+
50+
// The Receipt cell is a genuine upload control, NOT a text input (#2360 core).
51+
const fileInput = li.locator('input[type="file"]').first();
52+
await expect(fileInput).toHaveCount(1);
53+
await expect(li.locator('input[type="text"][aria-label="Receipt"]')).toHaveCount(0);
54+
55+
// Materialise the ghost row via a product pick (auto-fills description/price),
56+
// then give the line a quantity so it's a valid billable line.
57+
await li.getByTestId('lookup-trigger').first().click();
58+
const option = page.getByRole('option', { name: /Widget A/i }).first();
59+
await option.waitFor({ state: 'visible' });
60+
await option.click();
61+
await li.locator('input[aria-label="Qty"]').first().fill('1');
62+
63+
// Upload into the row's Receipt cell → a removable chip with the file name.
64+
await fileInput.setInputFiles({ name: 'receipt.png', mimeType: 'image/png', buffer: PNG });
65+
await expect(li.getByTestId('file-cell-chip').first()).toContainText('receipt.png');
66+
67+
// Header fields, then submit and capture the atomic batch.
68+
const name = `INV-${Date.now()}`;
69+
await dialog.locator('input[name="name"]').fill(name);
70+
await dialog.getByTestId('lookup-trigger-account').first().click();
71+
const acct = page.getByRole('option').first();
72+
await acct.waitFor({ state: 'visible' });
73+
await acct.click();
74+
await dialog.getByTestId('select-trigger-status').first().click();
75+
await page.getByTestId('select-option-draft').first().click();
76+
77+
await Promise.all([
78+
page.waitForRequest((r) => r.url().includes('/api/v1/batch'), { timeout: 15_000 }).catch(() => null),
79+
dialog.getByTestId('md-form-submit').click(),
80+
]);
81+
await page.waitForTimeout(500);
82+
83+
expect(batches.length).toBeGreaterThan(0);
84+
const ops = batches[0].operations;
85+
const child = ops.find((o: any) => o.object === 'showcase_invoice_line');
86+
expect(child).toBeTruthy();
87+
// The uploaded file resolved to a stored object, not a blob/text placeholder.
88+
const receipt = child?.data?.receipt;
89+
expect(receipt).toBeTruthy();
90+
expect(receipt.name || receipt.original_name).toContain('receipt');
91+
expect(String(receipt.url)).toMatch(/^https?:\/\//);
92+
});

packages/plugin-detail/src/RelatedList.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -621,8 +621,9 @@ export const RelatedList: React.FC<RelatedListProps> = ({
621621
// file/image are NOT skipped: they have dedicated cell renderers (name
622622
// chip / thumbnail), and dropping them hid business columns like a line's
623623
// receipt attachment (objectui#2360). Only types with no useful tabular
624-
// rendering stay excluded.
625-
const SKIP_TYPES = new Set(['attachment', 'rich_text', 'html', 'json']);
624+
// rendering stay excluded. (`attachment` is intentionally absent — it is not
625+
// a `@objectstack/spec` field type, so the renderer does not model it, #2655.)
626+
const SKIP_TYPES = new Set(['rich_text', 'html', 'json']);
626627
const PRIORITY_NAMES = [
627628
'name',
628629
'full_name',

packages/plugin-form/src/deriveMasterDetail.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,19 @@ describe('fieldTypeToColumnType', () => {
2626
expect(fieldTypeToColumnType('master_detail')).toBe('lookup');
2727
expect(fieldTypeToColumnType('email')).toBe('text');
2828
});
29+
30+
it('maps the spec file-family media types to a file column', () => {
31+
expect(fieldTypeToColumnType('file')).toBe('file');
32+
expect(fieldTypeToColumnType('image')).toBe('file');
33+
expect(fieldTypeToColumnType('avatar')).toBe('file');
34+
});
35+
36+
// #2655: `attachment` is not a `@objectstack/spec` field type (media types are
37+
// file/image/avatar/video/audio), so the renderer does not model it — it falls
38+
// through to the plain-text default rather than being special-cased to file.
39+
it('does not special-case the non-spec `attachment` type', () => {
40+
expect(fieldTypeToColumnType('attachment')).toBe('text');
41+
});
2942
});
3043

3144
describe('findRelationshipField', () => {
@@ -317,6 +330,25 @@ describe('resolveInlineMode (grid vs form)', () => {
317330
it('smart default: many business fields → form', () => {
318331
expect(resolveInlineMode(wide, true, { relationshipField: 'parent' })).toBe('form'); // 9 fields > 8
319332
});
333+
334+
// #2654: file-family fields render a compact upload cell in the grid now, so a
335+
// LONE one no longer forces the per-row form ("attach a receipt per line").
336+
it('smart default: a single file/image field stays a grid', () => {
337+
const oneFile = { fields: { name: { type: 'text' }, receipt: { type: 'file' }, parent: { type: 'master_detail', reference: 'p' } } };
338+
const oneImage = { fields: { name: { type: 'text' }, photo: { type: 'image' }, parent: { type: 'master_detail', reference: 'p' } } };
339+
expect(resolveInlineMode(oneFile, true, { relationshipField: 'parent' })).toBe('grid');
340+
expect(resolveInlineMode(oneImage, true, { relationshipField: 'parent' })).toBe('grid');
341+
});
342+
343+
it('smart default: several rich file fields (≥2) tip to form', () => {
344+
const twoFiles = { fields: { receipt: { type: 'file' }, photo: { type: 'image' }, parent: { type: 'master_detail', reference: 'p' } } };
345+
expect(resolveInlineMode(twoFiles, true, { relationshipField: 'parent' })).toBe('form');
346+
});
347+
348+
it('smart default: a file field alongside a truly form-only field → form', () => {
349+
const mixed = { fields: { receipt: { type: 'file' }, notes: { type: 'textarea' }, parent: { type: 'master_detail', reference: 'p' } } };
350+
expect(resolveInlineMode(mixed, true, { relationshipField: 'parent' })).toBe('form');
351+
});
320352
});
321353

322354
describe('deriveDetail', () => {

packages/plugin-form/src/deriveMasterDetail.ts

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ export function fieldTypeToColumnType(type: string | undefined): GridColumn['typ
6868
case 'file':
6969
case 'image':
7070
case 'avatar':
71-
case 'attachment':
7271
return 'file';
7372
default:
7473
return 'text';
@@ -310,22 +309,36 @@ export function deriveFormFields(
310309
/** Inline-edit form factor. */
311310
export type InlineMode = 'grid' | 'form';
312311

313-
/** Rich / form-only field types that read poorly in a narrow grid cell — their
314-
* presence on a child tips the smart default toward the per-row `form`. */
312+
/** Field types that read poorly in ANY grid cell — a SINGLE one on the child
313+
* tips the smart default toward the per-row `form`. (`attachment` is absent by
314+
* design: it is not a `@objectstack/spec` field type — the spec media types are
315+
* file/image/avatar/video/audio — so the renderer does not model it, #2655.) */
315316
const FORM_ONLY_TYPES = new Set([
316317
'textarea', 'richtext', 'html', 'markdown', 'rich-text',
317-
'file', 'image', 'avatar', 'attachment', 'json', 'location', 'address',
318+
'json', 'location', 'address',
318319
]);
319320

321+
/** File-family types that now render a compact upload cell in the grid (#2360).
322+
* A LONE one no longer forces a per-row form — "attach a receipt per line"
323+
* stays a grid. They still count toward the rich-field tally, so several piling
324+
* up (a cramped row) tips to `form` via {@link RICH_FIELD_FORM_THRESHOLD}. */
325+
const GRID_CAPABLE_RICH_TYPES = new Set(['file', 'image', 'avatar']);
326+
320327
/** Above this many editable business fields, the grid gets cramped → `form`. */
321328
export const SMART_FORM_FIELD_THRESHOLD = 8;
322329

330+
/** When a child has at least this many rich fields (form-only + grid-capable
331+
* file-family combined), the row is too busy for a grid → `form`. Set to 2 so a
332+
* single file/image column stays a grid but a pile of them prefers the form. */
333+
export const RICH_FIELD_FORM_THRESHOLD = 2;
334+
323335
/**
324336
* Resolve the inline-edit form factor for a child collection.
325337
* - explicit `'grid'` / `'form'` win;
326338
* - otherwise (`true` / undefined) pick by the child's shape: a `form` when it
327-
* has rich/form-only fields or more than {@link SMART_FORM_FIELD_THRESHOLD}
328-
* editable business fields, else a `grid`.
339+
* has a truly form-only field, several rich fields
340+
* ({@link RICH_FIELD_FORM_THRESHOLD}), or more than
341+
* {@link SMART_FORM_FIELD_THRESHOLD} editable business fields; else a `grid`.
329342
*/
330343
export function resolveInlineMode(
331344
childSchema: ObjectSchemaLike | undefined,
@@ -335,8 +348,13 @@ export function resolveInlineMode(
335348
if (inlineEdit === 'grid' || inlineEdit === 'form') return inlineEdit;
336349
const fields = (childSchema?.fields ?? {}) as Record<string, any>;
337350
const names = deriveFormFields(childSchema, { relationshipField: opts.relationshipField });
338-
const hasRich = names.some((n) => FORM_ONLY_TYPES.has(fields[n]?.type));
339-
if (hasRich) return 'form';
351+
// A single truly-form-only field (textarea/richtext/json/…) tips to form.
352+
const hasFormOnly = names.some((n) => FORM_ONLY_TYPES.has(fields[n]?.type));
353+
if (hasFormOnly) return 'form';
354+
// File-family fields render in-grid now, so a lone one stays a grid; only a
355+
// cluster of rich fields (≥ RICH_FIELD_FORM_THRESHOLD) tips to form (#2654).
356+
const richCount = names.filter((n) => GRID_CAPABLE_RICH_TYPES.has(fields[n]?.type)).length;
357+
if (richCount >= RICH_FIELD_FORM_THRESHOLD) return 'form';
340358
if (names.length > SMART_FORM_FIELD_THRESHOLD) return 'form';
341359
return 'grid';
342360
}

0 commit comments

Comments
 (0)