From 714701e00bac25cb4976f9ba3e5d51b01095d846 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 7 Jun 2026 15:40:54 +0800 Subject: [PATCH] feat(grid): spreadsheet-style line-item editor (computed cells, ghost row, keyboard nav) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rework GridField's editable grid mode into a real enterprise line-item editor (the QuickBooks / Stripe / NetSuite pattern), generalised so every inline grid benefits — not just invoices: - Computed read-only columns: a child field carrying an arithmetic `expression` (e.g. amount = quantity × unit_price) renders read-only, recomputes live as its inputs change, and writes the result back into the row so it persists and the running total reflects it. A tiny safe arithmetic evaluator (no eval) over `+ - * / %`, parens and `record.` refs powers it. - Always-present trailing "ghost" row: start-with-one + auto-append. Typing in the ghost materialises a real row (index-stable so focus/caret survive), so you keep entering lines without clicking "Add". - Borderless click-to-focus cells + role-based column widths (description flexes, qty/price/amount stay narrow) — reads as a table, not a wall of boxes. - Keyboard navigation: Enter / ArrowUp / ArrowDown move between rows in the same column; Tab falls through the natural order into the ghost row. - Per-row "expand to full form" is gated: shown in form mode (it is the editor) and in grid mode only when the grid omits fields — no redundant expand on a thin line like an invoice row. - deriveColumns surfaces `expression` as a computed column; the running-total column now prefers the computed/last-currency column over the first numeric. - Blank/ghost rows are filtered from the persisted batch (isBlankRow), so an untouched trailing line never saves as an empty child. Live e2e: New Invoice grid computes amount live, persists it, rolls up the total, and skips the blank ghost line. Dropped row-expand.spec (its grid-mode-expand-on-a-thin-line scenario is intentionally gone; per-row form editing stays covered by the form-mode master-detail spec). Co-Authored-By: Claude Opus 4.8 --- .changeset/line-grid-spreadsheet-editor.md | 15 + e2e/live/form-view-subforms.spec.ts | 29 +- e2e/live/row-expand.spec.ts | 43 -- .../fields/src/widgets/GridField.test.tsx | 64 ++- packages/fields/src/widgets/GridField.tsx | 473 +++++++++++++----- packages/plugin-form/src/MasterDetailForm.tsx | 9 +- .../src/deriveMasterDetail.test.ts | 23 +- .../plugin-form/src/deriveMasterDetail.ts | 34 +- .../plugin-form/src/masterDetailTx.test.ts | 25 +- packages/plugin-form/src/masterDetailTx.ts | 24 +- 10 files changed, 550 insertions(+), 189 deletions(-) create mode 100644 .changeset/line-grid-spreadsheet-editor.md delete mode 100644 e2e/live/row-expand.spec.ts diff --git a/.changeset/line-grid-spreadsheet-editor.md b/.changeset/line-grid-spreadsheet-editor.md new file mode 100644 index 000000000..1c3bbd99c --- /dev/null +++ b/.changeset/line-grid-spreadsheet-editor.md @@ -0,0 +1,15 @@ +--- +"@object-ui/fields": minor +"@object-ui/plugin-form": minor +--- + +Spreadsheet-style line-item grid editor. + +`GridField`'s editable grid mode is reworked into an enterprise line-item editor (the QuickBooks / Stripe / NetSuite pattern), generalised across every inline grid: + +- **Computed read-only columns** — a child field with an arithmetic `expression` (e.g. `amount = quantity * unit_price`) renders read-only, recomputes live as its inputs change, and writes the result back into the row so it persists and the running total reflects it. A small safe arithmetic evaluator (`+ - * / %`, parens, `record.` refs; no `eval`) powers it. +- **Trailing "ghost" row** — start-with-one + auto-append: typing in the ghost materialises a real row (index-stable, so focus/caret survive), so you keep entering lines without clicking "Add". +- **Borderless click-to-focus cells** + role-based column widths (description flexes; qty/price/amount stay narrow). +- **Keyboard navigation** — Enter / ArrowUp / ArrowDown move between rows in the same column. +- Per-row "expand to full form" is gated to grids that omit fields (no redundant expand on thin lines). +- `deriveColumns` surfaces a field `expression` as a computed column; the running-total column prefers the computed/last-currency column. Blank/ghost rows are filtered from the persisted batch (`isBlankRow`). diff --git a/e2e/live/form-view-subforms.spec.ts b/e2e/live/form-view-subforms.spec.ts index f1ae6a06b..f65eb964c 100644 --- a/e2e/live/form-view-subforms.spec.ts +++ b/e2e/live/form-view-subforms.spec.ts @@ -1,13 +1,19 @@ import { test, expect } from '@playwright/test'; -import { selectOption, fillLookup, addLineItem } from './helpers'; +import { selectOption, fillLookup } from './helpers'; /** * Tier 0 live e2e: an object's standard New/Edit modal renders inline child * collections derived from the DATA MODEL (no view config, no bespoke page). * `showcase_invoice_line.invoice` declares `inlineEdit: 'grid'`, so every - * standard Invoice form auto-renders a "Line Items" grid; "New Invoice" opens - * a master-detail modal that submits the header + its lines in one atomic - * /api/v1/batch. (An explicit `form.subforms` would override the derived one.) + * standard Invoice form auto-renders a spreadsheet-style "Line Items" grid; + * "New Invoice" opens a master-detail modal that submits the header + its + * lines in one atomic /api/v1/batch. + * + * This also exercises the modern grid mechanics: + * • the always-present trailing "ghost" row — you type straight into it, no + * "Add line" click, and it auto-appends the next blank line; + * • the computed read-only `amount = quantity × unit_price`, recomputed live + * and persisted in the batch (so the parent total rolls it up server-side). */ test('New modal renders relationship-derived subforms and submits an atomic batch', async ({ page }) => { const batches: any[] = []; @@ -31,8 +37,14 @@ test('New modal renders relationship-derived subforms and submits an at await selectOption(page, 'status', 'draft'); await expect(dialog.getByText('Northwind', { exact: false }).first()).toBeVisible(); - const row = await addLineItem(page); - await row.getByRole('textbox').first().fill('Widget A'); + // Type straight into the ghost row — no "Add line" click needed. + const li = dialog.getByTestId('line-items'); + await li.locator('input[aria-label="Product"]').first().fill('Widget A'); + await li.locator('input[aria-label="Qty"]').first().fill('2'); + await li.locator('input[aria-label="Unit Price"]').first().fill('50'); + + // The computed Amount column shows the live line total (read-only). + await expect(li.locator('[data-computed="amount"]').first()).toContainText('100'); await Promise.all([ page.waitForRequest((r) => r.url().includes('/api/v1/batch'), { timeout: 15_000 }).catch(() => null), @@ -47,4 +59,9 @@ test('New modal renders relationship-derived subforms and submits an at expect(ops[0].data.status).toBe('draft'); const child = ops.find((o: any) => o.object === 'showcase_invoice_line'); expect(child?.data?.invoice).toEqual({ $ref: 0 }); + expect(child?.data?.product).toBe('Widget A'); + // The client-computed amount (2 × 50) is persisted, so the server total rolls it up. + expect(Number(child?.data?.amount)).toBe(100); + // 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/e2e/live/row-expand.spec.ts b/e2e/live/row-expand.spec.ts deleted file mode 100644 index 7556b58ba..000000000 --- a/e2e/live/row-expand.spec.ts +++ /dev/null @@ -1,43 +0,0 @@ -import { test, expect } from '@playwright/test'; - -/** - * Live e2e for the master-detail inline grid's per-row "expand to full form" - * (the mainstream hybrid: a quick grid + a rich per-row form). Adding a line - * then clicking its expand button reveals the child's COMPLETE form inline, - * with the parent relationship FK excluded. Applying writes the values back - * into the grid row — no separate backend write; the atomic batch still - * persists everything on the parent Save. - * - * The editor is rendered inline (not a portaled drawer) precisely so it stays - * interactive + accessible when this form is itself inside the create-record - * modal (a nested portaled overlay would inherit the host modal's - * pointer-events / aria-hidden lock and be unclickable). - */ -test('a grid row expands into a full inline form and writes values back', async ({ page }) => { - await page.goto('/apps/showcase_app/showcase_invoice'); - await page.getByRole('button', { name: /^New$/i }).first().click(); - - const dialog = page.getByRole('dialog').first(); - await expect(dialog.getByTestId('md-form-submit')).toBeVisible(); - - // Add a line item, then expand it into the full form. - await dialog.getByTestId('line-items-add').click(); - await dialog.getByTestId('line-items-expand-0').click(); - - const editor = page.getByTestId('md-row-form'); - await expect(editor).toBeVisible(); - - // The full form includes the line's business fields (Product) and excludes - // the parent relationship FK (Invoice). - await expect(editor.getByText('Product', { exact: false }).first()).toBeVisible(); - await expect(editor.getByText(/^Invoice$/)).toHaveCount(0); - - // Fill the required fields (Product + Qty) in the full form, then Apply. - await editor.locator('input[name="product"]').fill('Widget Deluxe'); - await editor.locator('input[name="quantity"]').fill('3'); - await editor.getByRole('button', { name: 'Apply', exact: true }).click(); - - // Editor closes and the value is written back into the grid row. - await expect(editor).toBeHidden(); - await expect(dialog.getByTestId('line-items').getByRole('textbox').first()).toHaveValue('Widget Deluxe'); -}); diff --git a/packages/fields/src/widgets/GridField.test.tsx b/packages/fields/src/widgets/GridField.test.tsx index 07f51ce15..1e50d0a59 100644 --- a/packages/fields/src/widgets/GridField.test.tsx +++ b/packages/fields/src/widgets/GridField.test.tsx @@ -100,17 +100,77 @@ describe('GridField / LineItemsField — editable line items', () => { it('editing a text cell emits the raw string', () => { const onChange = vi.fn(); render(); - fireEvent.change(screen.getByLabelText('Description'), { target: { value: 'Taxi' } }); + // [0] = the data row ([1] would be the always-present trailing ghost row). + fireEvent.change(screen.getAllByLabelText('Description')[0], { target: { value: 'Taxi' } }); expect(onChange).toHaveBeenCalledWith([{ description: 'Taxi', amount: null }]); }); it('editing a currency cell coerces to a number', () => { const onChange = vi.fn(); render(); - fireEvent.change(screen.getByLabelText('Amount'), { target: { value: '42.5' } }); + fireEvent.change(screen.getAllByLabelText('Amount')[0], { target: { value: '42.5' } }); expect(onChange).toHaveBeenCalledWith([{ description: 'Taxi', amount: 42.5 }]); }); + describe('trailing ghost row (start-with-one + auto-append)', () => { + it('renders a trailing empty row so an empty grid still has one input line', () => { + render( {}} field={field} />); + // No "No items" empty-state in grid mode — the ghost row IS the first line. + expect(screen.getByText('Description')).toBeTruthy(); + expect(screen.getAllByLabelText('Description')).toHaveLength(1); // just the ghost + }); + + it('typing in the ghost row materialises a new row (no Add click needed)', () => { + const onChange = vi.fn(); + render(); + const inputs = screen.getAllByLabelText('Description'); + expect(inputs).toHaveLength(2); // data row + ghost + fireEvent.change(inputs[1], { target: { value: 'B' } }); // type in the ghost + expect(onChange).toHaveBeenCalledWith([ + { description: 'A', amount: 1 }, + { description: 'B', amount: null }, + ]); + }); + }); + + describe('computed columns (amount = qty × unit_price)', () => { + const computedField = { + columns: [ + { field: 'product', label: 'Product', type: 'text' as const }, + { field: 'quantity', label: 'Qty', type: 'number' as const }, + { field: 'unit_price', label: 'Unit Price', type: 'currency' as const }, + { field: 'amount', label: 'Amount', type: 'currency' as const, computed: true, expr: 'record.quantity * record.unit_price', scale: 2 }, + ], + total_field: 'amount', + } as any; + + it('renders a computed column read-only (no input) and recomputes on edit', () => { + const onChange = vi.fn(); + render(); + // Amount is display-only — there is no editable Amount cell. + expect(screen.queryByLabelText('Amount')).toBeNull(); + // Editing quantity recomputes amount in the emitted row. + fireEvent.change(screen.getAllByLabelText('Qty')[0], { target: { value: '4' } }); + expect(onChange).toHaveBeenCalledWith([{ product: 'Widget', quantity: 4, unit_price: 10, amount: 40 }]); + }); + + it('shows a dash for a computed cell whose inputs are blank', () => { + render( {}} field={computedField} />); + // The computed amount cell reads "—" until its inputs exist. + expect(screen.getAllByText('—').length).toBeGreaterThan(0); + }); + }); + + describe('keyboard navigation', () => { + it('Enter moves focus to the same column in the next row', () => { + render( {}} field={field} />); + const row0 = screen.getAllByLabelText('Description')[0]; + row0.focus(); + fireEvent.keyDown(row0, { key: 'Enter' }); + expect(document.activeElement).toBe(screen.getAllByLabelText('Description')[1]); + }); + }); + it('removing a row emits the array without it', () => { const onChange = vi.fn(); render( diff --git a/packages/fields/src/widgets/GridField.tsx b/packages/fields/src/widgets/GridField.tsx index a25ade1db..8d4a2a141 100644 --- a/packages/fields/src/widgets/GridField.tsx +++ b/packages/fields/src/widgets/GridField.tsx @@ -1,4 +1,4 @@ -import React, { useCallback } from 'react'; +import React, { useCallback, useRef } from 'react'; import { FieldWidgetProps } from './types'; import { cn, @@ -57,6 +57,20 @@ export interface GridColumn { * reachable. Required columns are never default-hidden. */ defaultHidden?: boolean; + /** + * A computed (read-only) column whose value is derived live from sibling + * cells via {@link expr} — e.g. an invoice line's `amount = quantity * + * unit_price`. The grid renders it read-only, recomputes it as the row's + * inputs change, and writes the result back into the row so it persists + * (and any running total reflects it). The classic spreadsheet pattern used + * by QuickBooks / Stripe / NetSuite line grids — nobody types the amount. + */ + computed?: boolean; + /** Arithmetic expression for a {@link computed} column. Supports `+ - * / %`, + * parentheses, numeric literals and field refs (`record.qty` or bare `qty`). */ + expr?: string; + /** Decimal places to round a computed numeric/currency result to. */ + scale?: number; } type Row = Record; @@ -75,6 +89,126 @@ const MIN_WIDTH_BY_TYPE: Record = { }; const minWidthFor = (c: GridColumn): number => c.width ?? MIN_WIDTH_BY_TYPE[c.type ?? 'text'] ?? 132; +/** + * Per-column sizing. Text columns flex to absorb slack (so the description + * never gets crushed), while numeric / select / date / lookup columns get a + * fixed sensible width — the role-based sizing every enterprise line grid uses + * (Qty stays narrow, Description stays wide). Authors can still pin `width`. + */ +function widthStyle(c: GridColumn): React.CSSProperties { + if (c.width) return { width: c.width, minWidth: c.width }; + if ((c.type ?? 'text') === 'text') return { minWidth: 180 }; // flex, no fixed width + const w = MIN_WIDTH_BY_TYPE[c.type ?? 'text'] ?? 132; + return { width: w, minWidth: w }; +} + +// ── Safe arithmetic evaluator for computed columns ────────────────────────── +// A tiny recursive-descent parser over `+ - * / %`, parentheses, numeric +// literals and field references (`record.qty` or bare `qty`). Deliberately NOT +// eval()/Function() — only arithmetic, no code execution. Returns a finite +// number, or null when the expression is unparseable or any referenced cell is +// blank/non-numeric (so a computed amount reads "—" until its inputs exist). +type Tok = { t: 'num' | 'id' | 'op' | 'lp' | 'rp'; v: string }; + +function tokenize(s: string): Tok[] | null { + const toks: Tok[] = []; + let i = 0; + while (i < s.length) { + const c = s[i]; + if (c === ' ' || c === '\t' || c === '\n') { i++; continue; } + if ((c >= '0' && c <= '9') || c === '.') { + let j = i + 1; + while (j < s.length && ((s[j] >= '0' && s[j] <= '9') || s[j] === '.')) j++; + toks.push({ t: 'num', v: s.slice(i, j) }); i = j; continue; + } + if (/[a-zA-Z_]/.test(c)) { + let j = i + 1; + while (j < s.length && /[a-zA-Z0-9_.]/.test(s[j])) j++; + toks.push({ t: 'id', v: s.slice(i, j) }); i = j; continue; + } + if (c === '+' || c === '-' || c === '*' || c === '/' || c === '%') { toks.push({ t: 'op', v: c }); i++; continue; } + if (c === '(') { toks.push({ t: 'lp', v: c }); i++; continue; } + if (c === ')') { toks.push({ t: 'rp', v: c }); i++; continue; } + return null; // unsupported character → bail (treat as non-computable) + } + return toks; +} + +/** Resolve an identifier (`record.quantity` / `quantity`) to a numeric cell. */ +function resolveRef(id: string, row: Row): number | null { + const field = id.startsWith('record.') ? id.slice('record.'.length) : id; + const raw = row?.[field]; + if (raw === null || raw === undefined || raw === '') return null; + const n = Number(raw); + return Number.isFinite(n) ? n : null; +} + +/** Evaluate an arithmetic `expr` against `row`. null when blank/unparseable. */ +export function evalArith(expr: string, row: Row): number | null { + const toks = tokenize(expr); + if (!toks || toks.length === 0) return null; + let pos = 0; + let bad = false; + const peek = () => toks[pos]; + const next = () => toks[pos++]; + + function factor(): number { + const tk = peek(); + if (!tk) { bad = true; return NaN; } + if (tk.t === 'op' && tk.v === '-') { next(); return -factor(); } + if (tk.t === 'op' && tk.v === '+') { next(); return factor(); } + if (tk.t === 'num') { next(); return Number(tk.v); } + if (tk.t === 'id') { next(); const v = resolveRef(tk.v, row); if (v === null) { bad = true; return NaN; } return v; } + if (tk.t === 'lp') { + next(); + const v = expression(); + const close = next(); + if (!close || close.t !== 'rp') bad = true; + return v; + } + bad = true; return NaN; + } + function term(): number { + let v = factor(); + while (peek() && peek().t === 'op' && (peek().v === '*' || peek().v === '/' || peek().v === '%')) { + const op = next().v; + const r = factor(); + v = op === '*' ? v * r : op === '/' ? v / r : v % r; + } + return v; + } + function expression(): number { + let v = term(); + while (peek() && peek().t === 'op' && (peek().v === '+' || peek().v === '-')) { + const op = next().v; + const r = term(); + v = op === '+' ? v + r : v - r; + } + return v; + } + const result = expression(); + if (bad || pos !== toks.length || !Number.isFinite(result)) return null; + return result; +} + +/** + * Recompute every {@link GridColumn.computed} cell in `row` from its sibling + * inputs, returning a new row. Called after each edit so computed columns and + * the running total stay live, and so the computed value persists in the batch. + */ +export function computeRow(columns: GridColumn[], row: Row): Row { + const computedCols = columns.filter((c) => c.computed && c.expr); + if (computedCols.length === 0) return row; + const next = { ...row }; + for (const c of computedCols) { + const v = evalArith(c.expr!, next); + if (v === null) { next[c.field] = null; continue; } + const scale = c.scale ?? (c.type === 'currency' ? 2 : undefined); + next[c.field] = scale != null ? Number(v.toFixed(scale)) : v; + } + return next; +} + /** Read-only display text for a cell in list mode (select → option label, * currency/number → formatted, empty → em dash). Lookups render separately. */ function displayText(c: GridColumn, value: any): string { @@ -174,27 +308,74 @@ export function GridField({ [onChange], ); - /** Set a cell to an already-typed value (lookup ids, etc.) without coercion. */ - const setCellValue = useCallback( + const blankRow = useCallback((): Row => { + const blank: Row = {}; + for (const c of columns) blank[c.field] = null; + return blank; + }, [columns]); + + /** + * Apply a single cell change. `rowIdx === rows.length` targets the trailing + * "ghost" row (always-present empty line) — editing it materialises a new + * real row, so the user never has to click "Add line" to keep entering. + * Computed columns are recomputed for the touched row on every change. + */ + const applyCell = useCallback( (rowIdx: number, field: string, value: any) => { - emit(rows.map((r, i) => (i === rowIdx ? { ...r, [field]: value } : r))); + const isGhost = rowIdx >= rows.length; + if (isGhost) { + if (maxRows != null && rows.length >= maxRows) return; + emit([...rows, computeRow(columns, { ...blankRow(), [field]: value })]); + return; + } + emit(rows.map((r, i) => (i === rowIdx ? computeRow(columns, { ...r, [field]: value }) : r))); }, - [rows, emit], + [rows, columns, maxRows, blankRow, emit], + ); + + /** Set a cell to an already-typed value (lookup ids, etc.) without coercion. */ + const setCellValue = useCallback( + (rowIdx: number, field: string, value: any) => applyCell(rowIdx, field, value), + [applyCell], ); const setCell = useCallback( (rowIdx: number, col: GridColumn, raw: string) => { - setCellValue(rowIdx, col.field, coerce(col.type, raw)); + applyCell(rowIdx, col.field, coerce(col.type, raw)); }, - [setCellValue], + [applyCell], ); const addRow = useCallback(() => { if (maxRows != null && rows.length >= maxRows) return; - const blank: Row = {}; - for (const c of columns) blank[c.field] = null; - emit([...rows, blank]); - }, [rows, columns, maxRows, emit]); + emit([...rows, blankRow()]); + }, [rows, blankRow, maxRows, emit]); + + // Keyboard navigation across cells (spreadsheet-style): the table is a focus + // grid. Enter / ArrowUp / ArrowDown move between rows in the same column; + // Tab / Shift-Tab fall through to the browser's natural row-major order + // (and into the ever-present ghost row, so tabbing past the last cell starts + // a new line). Cells carry data-cell="row-col" so we can target neighbours. + const gridRef = useRef(null); + const focusCell = useCallback((rowIdx: number, colIdx: number) => { + const el = gridRef.current?.querySelector(`[data-cell="${rowIdx}-${colIdx}"]`); + if (el) { + el.focus(); + if (el instanceof HTMLInputElement) el.select(); + } + }, []); + const onCellKeyDown = useCallback( + (e: React.KeyboardEvent, rowIdx: number, colIdx: number) => { + if (e.key === 'ArrowDown' || e.key === 'Enter') { + e.preventDefault(); + focusCell(rowIdx + 1, colIdx); + } else if (e.key === 'ArrowUp') { + e.preventDefault(); + if (rowIdx > 0) focusCell(rowIdx - 1, colIdx); + } + }, + [focusCell], + ); const removeRow = useCallback( (rowIdx: number) => { @@ -346,12 +527,97 @@ export function GridField({ } // ── Editable rendering ────────────────────────────────────────────────────── + // A trailing "ghost" empty row is always present in grid mode (start-with-one + // + auto-append): editing it materialises a real row, so the user keeps + // entering lines without clicking "Add". Index-based keys keep the input + // mounted across materialisation, so focus/caret survive the transition. + const hasGhost = !isList && allowAdd && (maxRows == null || rows.length < maxRows); + const displayRows: Row[] = hasGhost ? [...rows, blankRow()] : rows; + + /** Cell content: read-only display (list mode / computed columns) or an + * editable borderless control (spreadsheet feel). */ + const renderCellInput = (c: GridColumn, colIdx: number, rowIdx: number, row: Row) => { + const val = row?.[c.field]; + // List (form-factor) mode → read-only at-a-glance display. + if (isList) { + if (c.type === 'lookup' && val != null && val !== '') { + return ( + {}} readonly + field={{ reference: c.reference, display_field: c.displayField, id_field: c.idField } as any} /> + ); + } + return ( + + {displayText(c, val)} + + ); + } + // Computed column → read-only, recomputed live, formatted. + if (c.computed) { + return ( + + {displayText(c, val)} + + ); + } + if (c.type === 'lookup') { + return ( + setCellValue(rowIdx, c.field, v)} + field={{ reference: c.reference, display_field: c.displayField, id_field: c.idField, multiple: c.multiple, options: c.options, placeholder: '—' } as any} + disabled={disabled} + /> + ); + } + if (c.type === 'select') { + return ( + + ); + } + return ( +
+ {c.type === 'currency' && ( + {c.prefix || '¥'} + )} + onCellKeyDown(e, rowIdx, colIdx)} + className={cn( + 'h-8 rounded-none border-0 bg-transparent px-2 shadow-none focus-visible:ring-1 focus-visible:ring-ring/60', + c.type === 'currency' && 'pl-6', + isNumeric(c.type) && 'text-right tabular-nums', + )} + type={c.type === 'date' ? 'date' : isNumeric(c.type) ? 'number' : 'text'} + step={isNumeric(c.type) ? c.step ?? 'any' : undefined} + aria-label={c.label || c.field} + value={val != null ? String(val) : ''} + onChange={(e) => setCell(rowIdx, c, e.target.value)} + disabled={disabled} + /> +
+ ); + }; + return (
{columnChooser &&
{columnChooser}
}
- - +
+ {showLineNumbers && ( @@ -360,143 +626,84 @@ export function GridField({ ))} - {hasRowActions && - {rows.length === 0 ? ( + {isList && rows.length === 0 ? ( ) : ( - rows.map((row, rowIdx) => ( - - {showLineNumbers && ( - - )} - {columns.map((c) => ( - - ))} - {hasRowActions && ( - - )} - - )) + displayRows.map((row, rowIdx) => { + const isGhost = hasGhost && rowIdx === rows.length; + return ( + + {showLineNumbers && ( + + )} + {columns.map((c, colIdx) => ( + + ))} + {hasRowActions && ( + + )} + + ); + }) )} {showTotal && ( diff --git a/packages/plugin-form/src/MasterDetailForm.tsx b/packages/plugin-form/src/MasterDetailForm.tsx index 9bec0b146..24ff21928 100644 --- a/packages/plugin-form/src/MasterDetailForm.tsx +++ b/packages/plugin-form/src/MasterDetailForm.tsx @@ -444,7 +444,14 @@ export const MasterDetailForm: React.FC = ({ setRows(i, rows)} - onRowExpand={(rowIdx) => setExpanded({ detailIdx: i, rowIdx })} + // Per-row "expand to full form" is offered when it adds something: + // always in form mode (it IS the editor), and in grid mode only + // when the full form has fields the grid omits. A thin grid whose + // columns already cover every field (e.g. invoice lines) shows no + // redundant expand button. + {...((d.inlineMode === 'form' || (d.formFields?.length ?? 0) > (d.columns?.length ?? 0)) + ? { onRowExpand: (rowIdx: number) => setExpanded({ detailIdx: i, rowIdx }) } + : {})} displayMode={d.inlineMode === 'form' ? 'list' : 'grid'} {...(d.inlineMode === 'form' ? { onAdd: () => addRowViaForm(i) } : {})} field={ diff --git a/packages/plugin-form/src/deriveMasterDetail.test.ts b/packages/plugin-form/src/deriveMasterDetail.test.ts index b6ad70e5f..aaadcebe5 100644 --- a/packages/plugin-form/src/deriveMasterDetail.test.ts +++ b/packages/plugin-form/src/deriveMasterDetail.test.ts @@ -214,7 +214,28 @@ describe('deriveDetail', () => { const d = deriveDetail('showcase_task', taskSchema, 'showcase_project'); expect(d.relationshipField).toBe('project'); expect(d.columns.map((c) => c.field)).toContain('estimate_hours'); - expect(d.amountField).toBe('estimate_hours'); // first numeric/currency column + // The running total prefers the (last) currency column over a raw number + // like hours — a line-grid footer is almost always a money total. + expect(d.amountField).toBe('budget'); + }); + + it('maps a field expression to a read-only computed column and totals it', () => { + const lineSchema = { + fields: { + invoice: { type: 'master_detail', reference: 'inv' }, + product: { type: 'text', label: 'Product', required: true }, + quantity: { type: 'number', label: 'Qty', required: true }, + unit_price: { type: 'currency', label: 'Unit Price' }, + // Normalized CEL envelope, as the server serves it. + amount: { type: 'currency', label: 'Amount', scale: 2, expression: { dialect: 'cel', source: 'record.quantity * record.unit_price' } }, + }, + }; + const d = deriveDetail('inv_line', lineSchema, 'inv'); + const amountCol = d.columns.find((c) => c.field === 'amount')!; + expect(amountCol.computed).toBe(true); + expect(amountCol.expr).toBe('record.quantity * record.unit_price'); + expect(amountCol.required).toBe(false); // computed → never user-required + expect(d.amountField).toBe('amount'); // running total prefers the computed line total }); it('honors explicit overrides over derived values', () => { diff --git a/packages/plugin-form/src/deriveMasterDetail.ts b/packages/plugin-form/src/deriveMasterDetail.ts index 3c2e2e057..9db3fbdbc 100644 --- a/packages/plugin-form/src/deriveMasterDetail.ts +++ b/packages/plugin-form/src/deriveMasterDetail.ts @@ -183,6 +183,16 @@ export function deriveColumns( col.reference = d?.reference; col.displayField = d?.display_field || d?.reference_field; } + // A field carrying an arithmetic `expression` (e.g. amount = quantity * + // unit_price) becomes a live read-only computed column. The expression may + // be a bare string or the normalized CEL envelope `{ dialect, source }`. + const expr = typeof d?.expression === 'string' ? d.expression : d?.expression?.source; + if (expr && typeof expr === 'string') { + col.computed = true; + col.expr = expr; + col.required = false; // computed → never user-entered, so never required + if (typeof d?.scale === 'number') col.scale = d.scale; + } cols.push(col); } const maxColumns = opts.maxColumns ?? DEFAULT_MAX_INLINE_COLUMNS; @@ -252,6 +262,28 @@ export function resolveInlineMode( return 'grid'; } +/** Field names that read as a per-line money total (summed into the footer). */ +const AMOUNT_LIKE_FIELDS = ['amount', 'total', 'subtotal', 'line_total', 'line_amount', 'net_amount']; + +/** + * Choose which numeric column feeds the running total. The line total is, in + * order of preference: a computed numeric column (e.g. amount = qty × price), + * an `amount`/`total`-named numeric column, the last currency column, then the + * last numeric column. Preferring the LAST currency over the first stops a + * grid from accidentally summing `quantity` or `unit_price`. + */ +function pickAmountField(columns: GridColumn[]): string | undefined { + const numeric = columns.filter((c) => c.type === 'number' || c.type === 'currency'); + if (numeric.length === 0) return undefined; + const computed = numeric.find((c) => c.computed); + if (computed) return computed.field; + const named = numeric.find((c) => AMOUNT_LIKE_FIELDS.includes(c.field)); + if (named) return named.field; + const lastCurrency = [...numeric].reverse().find((c) => c.type === 'currency'); + if (lastCurrency) return lastCurrency.field; + return numeric[numeric.length - 1].field; +} + export interface DerivedDetail { childObject: string; relationshipField: string; @@ -284,7 +316,7 @@ export function deriveDetail( ); } const columns = override.columns?.length ? override.columns : deriveColumns(childSchema, { relationshipField }); - const amountField = override.amountField || columns.find((c) => c.type === 'number' || c.type === 'currency')?.field; + const amountField = override.amountField || pickAmountField(columns); const formFields = deriveFormFields(childSchema, { relationshipField }); // Resolve mode from the explicit override, else the relationship field's // `inlineEdit` value, else the smart default from the child's shape. diff --git a/packages/plugin-form/src/masterDetailTx.test.ts b/packages/plugin-form/src/masterDetailTx.test.ts index fbd05e655..56ace8035 100644 --- a/packages/plugin-form/src/masterDetailTx.test.ts +++ b/packages/plugin-form/src/masterDetailTx.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; -import { diffRows, sumRows, applyDetail, idOf, buildMasterDetailBatch, buildMasterDetailEditBatch } from './masterDetailTx'; +import { diffRows, sumRows, applyDetail, idOf, isBlankRow, buildMasterDetailBatch, buildMasterDetailEditBatch } from './masterDetailTx'; describe('buildMasterDetailBatch — atomic master-detail ops', () => { it('puts the parent first and references it via $ref:0 on each child FK', () => { @@ -20,6 +20,29 @@ describe('buildMasterDetailBatch — atomic master-detail ops', () => { expect(ops).toHaveLength(1); expect(ops[0].object).toBe('p'); }); + + it('skips blank/ghost rows so an untouched trailing line never persists', () => { + const ops = buildMasterDetailBatch( + 'inv', + { name: 'INV-1' }, + [{ childObject: 'inv_line', relationshipField: 'invoice', rows: [ + { product: 'Widget', quantity: 2, amount: 20 }, + { product: null, quantity: null, amount: null }, // ghost — must be dropped + ] }], + ); + expect(ops).toHaveLength(2); // parent + the one real line only + expect(ops[1].data).toMatchObject({ product: 'Widget', invoice: { $ref: 0 } }); + }); +}); + +describe('isBlankRow', () => { + it('treats a row as blank when only the FK / id keys carry values', () => { + expect(isBlankRow({ product: null, amount: null, invoice: { $ref: 0 } }, 'invoice')).toBe(true); + expect(isBlankRow({ id: 'x', product: '', amount: null }, 'invoice')).toBe(true); + }); + it('is not blank when any business field has a value', () => { + expect(isBlankRow({ product: 'Widget', amount: null }, 'invoice')).toBe(false); + }); }); describe('buildMasterDetailEditBatch — atomic master-detail edit ops', () => { diff --git a/packages/plugin-form/src/masterDetailTx.ts b/packages/plugin-form/src/masterDetailTx.ts index 1f638912f..5fd267885 100644 --- a/packages/plugin-form/src/masterDetailTx.ts +++ b/packages/plugin-form/src/masterDetailTx.ts @@ -49,6 +49,23 @@ export interface BatchOp { id?: string; } +const ID_KEYS = new Set(['id', '_id', 'recordId']); + +/** + * A row that carries no user input — every field is blank once the back- + * reference FK and id keys are set aside. Computed cells (e.g. amount) read + * null while their inputs are blank, so they don't count as input. Used to + * drop the always-present trailing "ghost" line (and any row the user cleared) + * so it never persists as an empty child record. + */ +export function isBlankRow(row: Record, relationshipField?: string): boolean { + if (!row) return true; + return Object.entries(row).every(([k, v]) => { + if (k === relationshipField || ID_KEYS.has(k)) return true; + return v === null || v === undefined || v === ''; + }); +} + /** * Build cross-object batch operations for an ATOMIC master-detail create: * the parent at index 0, then each child with its relationship FK set to @@ -63,6 +80,7 @@ export function buildMasterDetailBatch( const ops: BatchOp[] = [{ object: parentObject, action: 'create', data: parentData }]; for (const d of details) { for (const row of d.rows) { + if (isBlankRow(row, d.relationshipField)) continue; // skip the ghost/empty line ops.push({ object: d.childObject, action: 'create', data: { ...row, [d.relationshipField]: { $ref: 0 } } }); } } @@ -87,6 +105,7 @@ export function buildMasterDetailEditBatch( const withFk = (d.rows || []).map((r) => ({ ...r, [d.relationshipField]: parentId })); const { toCreate, toUpdate, toDelete } = diffRows(d.original || [], withFk); for (const row of toCreate) { + if (isBlankRow(row, d.relationshipField)) continue; // skip the ghost/empty line // Strip any client-only id so the server generates one. const { id: _omit, _id: _omit2, recordId: _omit3, ...clean } = row as any; ops.push({ object: d.childObject, action: 'create', data: clean }); @@ -182,7 +201,10 @@ export async function applyDetail( opts: ApplyDetailOptions, ): Promise { const created: Array<{ object: string; id: string }> = []; - const withFk = opts.rows.map((r) => ({ ...r, [opts.relationshipField]: parentId })); + const withFk = opts.rows + // Drop blank/ghost lines that were never filled in (keep existing rows). + .filter((r) => idOf(r) || !isBlankRow(r, opts.relationshipField)) + .map((r) => ({ ...r, [opts.relationshipField]: parentId })); if (opts.original !== undefined) { const { toCreate, toUpdate, toDelete } = diffRows(opts.original, withFk);
# {c.label || c.field} - {c.required && *} + {c.required && !c.computed && *} } + {hasRowActions && }
- No items yet — click “{cfg.add_label || 'Add line'}” to begin. + No items yet — click “{cfg.add_label || 'Add'}” to begin.
- {rowIdx + 1} - - {isList ? ( - c.type === 'lookup' && row[c.field] != null && row[c.field] !== '' ? ( - {}} - readonly - field={{ reference: c.reference, display_field: c.displayField, id_field: c.idField } as any} - /> - ) : ( - - {displayText(c, row[c.field])} - - ) - ) : c.type === 'lookup' ? ( - setCellValue(rowIdx, c.field, v)} - field={ - { - reference: c.reference, - display_field: c.displayField, - id_field: c.idField, - multiple: c.multiple, - options: c.options, - placeholder: '—', - } as any - } - disabled={disabled} - /> - ) : c.type === 'select' ? ( - - ) : ( -
- {c.type === 'currency' && ( - - {c.prefix || '¥'} - - )} - setCell(rowIdx, c, e.target.value)} - disabled={disabled} - /> -
- )} - 		- {showExpand && ( - - )} - {allowDelete && ( - - )} -
+ {rowIdx + 1} + + {renderCellInput(c, colIdx, rowIdx, row)} + + {!isGhost && showExpand && ( + + )} + {!isGhost && allowDelete && ( + + )} +