From 7452719cfa35a8a5101fadeab578e73041c94be8 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 7 Jun 2026 16:42:48 +0800 Subject: [PATCH] feat(master-detail): de-frame line-item section, compact lookup cells, persist line order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UI/UX polish for the subform entry experience plus the wrap-up items: - De-frame: the line-item section was wrapped in a Card (border + p-6) around a grid that already has its own border — a wasteful double frame. Render it as a light label + the grid's bordered table so it uses the full width. - Compact lookup: LookupField gains a `compact` mode (grid cells) — the selected value shows inline in a borderless single-line trigger instead of a chip stacked above a "Select…" button. - Persist drag order: deriveMasterDetail detects a sort field (position/sort_order/…), excludes it from columns + row form, and threads it as the grid's `sort_field`, so reorder stamps row[position]=index and survives reload. Live e2e asserts the stamped position. - Fix the long-standing flaky ListView unit test: with no filters the empty state is the first-run copy ("Nothing here yet"), not "No items found". Co-Authored-By: Claude Opus 4.8 --- .changeset/subform-ui-polish.md | 10 +++++ e2e/live/form-view-subforms.spec.ts | 1 + packages/fields/src/widgets/GridField.tsx | 1 + packages/fields/src/widgets/LookupField.tsx | 40 +++++++++++++------ packages/plugin-form/src/MasterDetailForm.tsx | 20 ++++++---- .../src/deriveMasterDetail.test.ts | 16 ++++++++ .../plugin-form/src/deriveMasterDetail.ts | 14 +++++-- .../src/__tests__/ListView.test.tsx | 5 ++- 8 files changed, 82 insertions(+), 25 deletions(-) create mode 100644 .changeset/subform-ui-polish.md diff --git a/.changeset/subform-ui-polish.md b/.changeset/subform-ui-polish.md new file mode 100644 index 000000000..eb27ab75f --- /dev/null +++ b/.changeset/subform-ui-polish.md @@ -0,0 +1,10 @@ +--- +"@object-ui/fields": minor +"@object-ui/plugin-form": minor +--- + +Master-detail entry: lighter layout, compact lookup cells, persisted line order. + +- **De-framed line-item section** — the subform no longer double-frames the grid in a `Card` (border + `p-6`); it renders as a light label + the grid's own bordered table, reclaiming the width the line table needs. +- **Compact lookup cells** — `LookupField` gains a `compact` mode (used by grid cells): the selected value shows inline in a borderless single-line trigger instead of a chip stacked above a separate "Select…" button. +- **Persisted drag-reorder** — `deriveMasterDetail` detects a sort field (`position`/`sort_order`/…), excludes it from the editable columns/row-form, and threads it as the grid's `sort_field` so reordering stamps `row[position] = index` and survives a reload. diff --git a/e2e/live/form-view-subforms.spec.ts b/e2e/live/form-view-subforms.spec.ts index 74511bcab..4f2a95b52 100644 --- a/e2e/live/form-view-subforms.spec.ts +++ b/e2e/live/form-view-subforms.spec.ts @@ -76,6 +76,7 @@ test('New modal renders relationship-derived subforms and submits an at expect(child?.data?.description).toBe('Standard widget'); // auto-filled expect(Number(child?.data?.unit_price)).toBe(29.99); // auto-filled expect(Number(child?.data?.amount)).toBe(59.98); // computed (2 × 29.99) + expect(Number(child?.data?.position)).toBe(0); // sort position stamped from row order // The empty ghost line must NOT have been persisted as a blank child. expect(ops.filter((o: any) => o.object === 'showcase_invoice_line')).toHaveLength(1); }); diff --git a/packages/fields/src/widgets/GridField.tsx b/packages/fields/src/widgets/GridField.tsx index af0d5c157..dd5ede7f4 100644 --- a/packages/fields/src/widgets/GridField.tsx +++ b/packages/fields/src/widgets/GridField.tsx @@ -652,6 +652,7 @@ export function GridField({ value={val} onChange={(v: any) => setCellValue(rowIdx, c.field, v)} onSelectRecord={(rec: any) => applyLookupSelection(rowIdx, c, rec)} + compact field={{ reference: c.reference, display_field: c.displayField, id_field: c.idField, multiple: c.multiple, options: c.options, placeholder: '—' } as any} disabled={disabled} /> diff --git a/packages/fields/src/widgets/LookupField.tsx b/packages/fields/src/widgets/LookupField.tsx index d6613121b..2f10a3cfa 100644 --- a/packages/fields/src/widgets/LookupField.tsx +++ b/packages/fields/src/widgets/LookupField.tsx @@ -1,5 +1,6 @@ import React, { useState, useEffect, useCallback, useRef, useContext, useMemo } from 'react'; -import { Button, +import { cn, + Button, Input, Badge, Popover, @@ -565,14 +566,20 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel ); } + // Compact mode (e.g. inside a line-item grid cell): show the selected value + // INSIDE a borderless trigger on a single line — no chip stacked above a + // separate "Select…" button (which double-stacks and wastes the row height). + const compact = !!(props as any).compact; + const singleSelectedLabel = selectedOptions[0]?.label || selectedOptions[0]?.[displayField]; + return ( -
- {/* Selected values display */} - {selectedOptions.length > 0 && ( +
+ {/* Selected values display (full mode only — compact shows it in-trigger) */} + {selectedOptions.length > 0 && !compact && (
{selectedOptions.map((opt, idx) => ( - @@ -594,9 +601,12 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
!dependenciesMissing && setIsOpen(o)}> - diff --git a/packages/plugin-form/src/MasterDetailForm.tsx b/packages/plugin-form/src/MasterDetailForm.tsx index 46a7cd96a..0550a5736 100644 --- a/packages/plugin-form/src/MasterDetailForm.tsx +++ b/packages/plugin-form/src/MasterDetailForm.tsx @@ -51,6 +51,9 @@ export interface MasterDetailDetailConfig { inlineMode?: InlineMode; /** Numeric child column to sum, e.g. 'amount'. */ amountField?: string; + /** Child field holding the line sort position — stamped on drag-reorder so + * order persists. Auto-derived from a `position`/`sort_order`/… field. */ + sortField?: string; /** Parent field to receive the rolled-up sum, e.g. 'total_amount'. */ totalField?: string; /** Section title. */ @@ -136,6 +139,7 @@ export const MasterDetailForm: React.FC = ({ formFields: d.formFields ?? derived.formFields, inlineMode: d.inlineMode ?? derived.mode, amountField: d.amountField ?? derived.amountField, + sortField: d.sortField ?? derived.sortField, }; } catch { return d; // leave as-is; the grid card will show a config hint @@ -475,13 +479,13 @@ export const MasterDetailForm: React.FC = ({
- {/* 2) Line items below the header */} + {/* 2) Line items below the header. Rendered as a light section (label + + the grid's own bordered table) rather than a heavy Card — a Card here + would double-frame the grid and its p-6 padding wastes the width the + line table needs. */} {details.map((d, i) => ( - - - {d.title || 'Line Items'} - - +
+

{d.title || 'Line Items'}

{!d.columns?.length ? (

Loading columns…

) : ( @@ -504,6 +508,7 @@ export const MasterDetailForm: React.FC = ({ // Show the per-grid running total whenever an amount column is // set — unless the document totals stack below subsumes it. total_field: showTaxStack ? undefined : (d.amountField || (d.totalField ? 'amount' : undefined)), + sort_field: d.sortField, min_rows: d.minRows, max_rows: d.maxRows, add_label: d.inlineMode === 'form' ? (d.addLabel || 'Add') : d.addLabel, @@ -511,8 +516,7 @@ export const MasterDetailForm: React.FC = ({ } /> )} - - +
))} {/* Document totals stack (Subtotal / Tax / Total) — the right-aligned block diff --git a/packages/plugin-form/src/deriveMasterDetail.test.ts b/packages/plugin-form/src/deriveMasterDetail.test.ts index aaadcebe5..a430d9027 100644 --- a/packages/plugin-form/src/deriveMasterDetail.test.ts +++ b/packages/plugin-form/src/deriveMasterDetail.test.ts @@ -238,6 +238,22 @@ describe('deriveDetail', () => { expect(d.amountField).toBe('amount'); // running total prefers the computed line total }); + it('detects a sort/position field, excludes it from columns, and reports it as sortField', () => { + const lineSchema = { + fields: { + invoice: { type: 'master_detail', reference: 'inv' }, + position: { type: 'number', label: 'Position' }, + product: { type: 'text', label: 'Product', required: true }, + quantity: { type: 'number', label: 'Qty', required: true }, + }, + }; + const d = deriveDetail('inv_line', lineSchema, 'inv'); + expect(d.sortField).toBe('position'); + expect(d.columns.map((c) => c.field)).not.toContain('position'); // not user-edited + expect(d.formFields).not.toContain('position'); + expect(d.columns.map((c) => c.field)).toEqual(['product', 'quantity']); + }); + it('honors explicit overrides over derived values', () => { const d = deriveDetail('showcase_task', taskSchema, 'showcase_project', { relationshipField: 'project', diff --git a/packages/plugin-form/src/deriveMasterDetail.ts b/packages/plugin-form/src/deriveMasterDetail.ts index 9db3fbdbc..7a2cbacdf 100644 --- a/packages/plugin-form/src/deriveMasterDetail.ts +++ b/packages/plugin-form/src/deriveMasterDetail.ts @@ -29,6 +29,10 @@ const SYSTEM_FIELDS = new Set([ 'organization_id', 'tenant_id', 'space', 'owner', ]); +/** Field names that hold a line's sort position — excluded from the editable + * columns and the row form (the grid stamps them on drag-reorder instead). */ +const SORT_FIELD_NAMES = new Set(['position', 'sort_order', 'sequence', 'line_no', 'line_number', 'sort']); + /** Field types that are not directly editable in a line-item grid. */ const NON_EDITABLE_TYPES = new Set([ 'formula', 'summary', 'rollup', 'autonumber', 'auto_number', @@ -168,7 +172,7 @@ export function deriveColumns( const cols: GridColumn[] = []; for (const [name, def] of Object.entries(fields)) { const d = def as any; - if (SYSTEM_FIELDS.has(name) || exclude.has(name)) continue; + if (SYSTEM_FIELDS.has(name) || exclude.has(name) || SORT_FIELD_NAMES.has(name)) continue; if (d?.system || d?.readonly || d?.hidden) continue; if (NON_EDITABLE_TYPES.has(d?.type)) continue; const col: GridColumn = { @@ -220,7 +224,7 @@ export function deriveFormFields( const out: string[] = []; for (const [name, def] of Object.entries(fields)) { const d = def as any; - if (SYSTEM_FIELDS.has(name) || exclude.has(name)) continue; + if (SYSTEM_FIELDS.has(name) || exclude.has(name) || SORT_FIELD_NAMES.has(name)) continue; if (d?.system || d?.hidden) continue; if (NON_INPUT_TYPES.has(d?.type)) continue; out.push(name); @@ -294,6 +298,9 @@ export interface DerivedDetail { mode: InlineMode; /** First numeric column, used as the running-total source when none is set. */ amountField?: string; + /** Child field holding the line sort position, if any — the grid stamps it on + * drag-reorder so order persists (excluded from the editable columns). */ + sortField?: string; } /** @@ -322,5 +329,6 @@ export function deriveDetail( // `inlineEdit` value, else the smart default from the child's shape. const inlineEdit = override.inlineEdit ?? (childSchema?.fields as any)?.[relationshipField]?.inlineEdit; const mode = resolveInlineMode(childSchema, inlineEdit, { relationshipField }); - return { childObject, relationshipField, columns, formFields, mode, amountField }; + const sortField = Object.keys(childSchema?.fields ?? {}).find((n) => SORT_FIELD_NAMES.has(n)); + return { childObject, relationshipField, columns, formFields, mode, amountField, sortField }; } diff --git a/packages/plugin-list/src/__tests__/ListView.test.tsx b/packages/plugin-list/src/__tests__/ListView.test.tsx index 1630c29d9..c45226133 100644 --- a/packages/plugin-list/src/__tests__/ListView.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.test.tsx @@ -232,7 +232,10 @@ describe('ListView', () => { await vi.waitFor(() => { expect(screen.getByTestId('empty-state')).toBeInTheDocument(); }); - expect(screen.getByText('No items found')).toBeInTheDocument(); + // With no filters/search this is the FIRST-RUN empty state ("Nothing here + // yet"), which invites the user to create — "No items found" is reserved + // for the filtered/no-matches case (see the hasActiveQuery branch). + expect(screen.getByText('Nothing here yet')).toBeInTheDocument(); }); it('should show custom empty state when configured', async () => {