Skip to content

Commit 1b26453

Browse files
authored
fix(grid): make inline cell editing usable — stop row-nav on edit, type-aware editors, default persist (#2021)
1 parent 433358a commit 1b26453

3 files changed

Lines changed: 290 additions & 11 deletions

File tree

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* Regression: inline cell editing on an `editable` data-table must be usable
11+
* for per-row data entry (e.g. 生产报工 where each row gets its own actual
12+
* date). Two bugs made it unusable:
13+
*
14+
* A) Clicking an editable cell entered edit mode AND bubbled up to the row's
15+
* onClick → onRowClick, which in ObjectGrid opens the record-detail drawer.
16+
* The edit cell must stopPropagation so the drawer never opens.
17+
* B) The inline editor was a hardcoded text <Input> for every field type, so
18+
* date columns could only be typed by hand. A `type: 'date'` column must
19+
* render a native date picker (<input type="date">).
20+
*/
21+
import { describe, it, expect, vi, beforeAll } from 'vitest';
22+
import { fireEvent } from '@testing-library/react';
23+
import '@testing-library/jest-dom';
24+
import React from 'react';
25+
import { renderComponent } from './test-utils';
26+
27+
beforeAll(async () => {
28+
await import('../renderers');
29+
}, 30000);
30+
31+
const editableSchema = {
32+
type: 'data-table' as const,
33+
editable: true,
34+
singleClickEdit: true,
35+
columns: [
36+
{ header: '工序', accessorKey: 'name', editable: false },
37+
{ header: '实际开始时间', accessorKey: 'actual_start', type: 'date' },
38+
{ header: '报工数量', accessorKey: 'qty', type: 'number' },
39+
],
40+
data: [{ id: '1', name: '将军柱下料', actual_start: '', qty: '' }],
41+
} as any;
42+
43+
describe('data-table — inline edit is per-row usable', () => {
44+
it('A) clicking an editable cell does NOT fire onRowClick (no detail drawer)', () => {
45+
const onRowClick = vi.fn();
46+
const { container } = renderComponent({ ...editableSchema, onRowClick });
47+
48+
const cell = Array.from(container.querySelectorAll('td')).find((td) =>
49+
td.textContent?.includes('将军柱下料')
50+
) as HTMLElement;
51+
// sanity: the readonly name cell is editable:false, so it should still
52+
// behave like a normal row click.
53+
expect(cell).toBeTruthy();
54+
55+
// Click the editable date cell (3rd column has no text yet → locate by index).
56+
const cells = container.querySelectorAll('tbody td');
57+
const dateCell = cells[1] as HTMLElement; // 实际开始时间
58+
fireEvent.click(dateCell);
59+
60+
expect(onRowClick).not.toHaveBeenCalled();
61+
});
62+
63+
it('A2) clicking a readonly (editable:false) cell still fires onRowClick (sanity)', () => {
64+
const onRowClick = vi.fn();
65+
const { container } = renderComponent({ ...editableSchema, onRowClick });
66+
67+
const nameCell = Array.from(container.querySelectorAll('tbody td')).find((td) =>
68+
td.textContent?.includes('将军柱下料')
69+
) as HTMLElement;
70+
fireEvent.click(nameCell);
71+
72+
expect(onRowClick).toHaveBeenCalledTimes(1);
73+
});
74+
75+
it('B) a date column renders a native date picker, not a free-text input', () => {
76+
const { container } = renderComponent(editableSchema);
77+
78+
const cells = container.querySelectorAll('tbody td');
79+
const dateCell = cells[1] as HTMLElement; // 实际开始时间
80+
fireEvent.click(dateCell);
81+
82+
const input = dateCell.querySelector('input') as HTMLInputElement;
83+
expect(input).toBeTruthy();
84+
expect(input.type).toBe('date');
85+
});
86+
87+
it('B2) a number column renders a numeric input', () => {
88+
const { container } = renderComponent(editableSchema);
89+
90+
const cells = container.querySelectorAll('tbody td');
91+
const qtyCell = cells[2] as HTMLElement; // 报工数量
92+
fireEvent.click(qtyCell);
93+
94+
const input = qtyCell.querySelector('input') as HTMLInputElement;
95+
expect(input).toBeTruthy();
96+
expect(input.type).toBe('number');
97+
});
98+
});

packages/components/src/renderers/complex/data-table.tsx

Lines changed: 125 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,45 @@ import {
6161

6262
type SortDirection = 'asc' | 'desc' | null;
6363

64+
/**
65+
* Inline-edit helpers: convert a stored cell value to the string a native
66+
* `<input type="date">` / `<input type="datetime-local">` expects, and back.
67+
*
68+
* Native date inputs require `yyyy-MM-dd`; datetime-local requires
69+
* `yyyy-MM-ddTHH:mm`. We pad to the LOCAL wall-clock so the picker shows the
70+
* same day the user sees, then convert back on change. A `date` field stays a
71+
* plain `yyyy-MM-dd` string; a `datetime` field round-trips through an ISO
72+
* string (matching how display/format code already treats ISO datetimes).
73+
*/
74+
function pad2(n: number): string {
75+
return String(n).padStart(2, '0');
76+
}
77+
78+
function toDateInputValue(value: unknown): string {
79+
if (value == null || value === '') return '';
80+
// A bare yyyy-MM-dd (or its leading slice of an ISO string) is already in the
81+
// exact shape the native control wants. Pass it through verbatim — parsing it
82+
// through `new Date()` would interpret it as UTC midnight and can shift the
83+
// displayed day by one in negative-offset timezones.
84+
if (typeof value === 'string') {
85+
const m = value.match(/^(\d{4}-\d{2}-\d{2})/);
86+
if (m) return m[1];
87+
}
88+
const d = value instanceof Date ? value : new Date(String(value));
89+
if (Number.isNaN(d.getTime())) return '';
90+
return `${d.getFullYear()}-${pad2(d.getMonth() + 1)}-${pad2(d.getDate())}`;
91+
}
92+
93+
function toDateTimeInputValue(value: unknown): string {
94+
if (value == null || value === '') return '';
95+
const d = value instanceof Date ? value : new Date(String(value));
96+
if (Number.isNaN(d.getTime())) return '';
97+
return `${d.getFullYear()}-${pad2(d.getMonth() + 1)}-${pad2(d.getDate())}T${pad2(d.getHours())}:${pad2(d.getMinutes())}`;
98+
}
99+
100+
// Field types that should edit as a numeric `<Input type="number">`.
101+
const NUMERIC_EDIT_TYPES = new Set(['number', 'currency', 'percent', 'int', 'integer', 'float', 'double']);
102+
64103
// Default English fallback translations for the data table
65104
const TABLE_DEFAULT_TRANSLATIONS: Record<string, string> = {
66105
'table.rowsPerPage': 'Rows per page',
@@ -1062,19 +1101,96 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
10621101
maxWidth: columnWidth,
10631102
...(isFrozen && { left: frozenOffset }),
10641103
}}
1065-
onDoubleClick={() => isEditable && !singleClickEdit && startEdit(rowIndex, col.accessorKey)}
1066-
onClick={() => isEditable && singleClickEdit && startEdit(rowIndex, col.accessorKey)}
1104+
onDoubleClick={(e) => {
1105+
// Entering edit mode must NOT also fire the row's
1106+
// onRowClick (record-detail drawer). The row heuristic
1107+
// can't see the editor yet (the <input> only renders
1108+
// next frame), so stop propagation here explicitly.
1109+
if (isEditable && !singleClickEdit) {
1110+
e.stopPropagation();
1111+
startEdit(rowIndex, col.accessorKey);
1112+
}
1113+
}}
1114+
onClick={(e) => {
1115+
if (isEditable && singleClickEdit) {
1116+
e.stopPropagation();
1117+
startEdit(rowIndex, col.accessorKey);
1118+
}
1119+
}}
10671120
onKeyDown={(e) => handleCellKeyDown(e, rowIndex, col.accessorKey)}
10681121
tabIndex={0}
10691122
>
10701123
{isEditing ? (
1071-
<Input
1072-
ref={editInputRef}
1073-
value={editValue}
1074-
onChange={(e) => setEditValue(e.target.value)}
1075-
onKeyDown={handleEditKeyDown}
1076-
className="h-8 px-2 py-1"
1077-
/>
1124+
(() => {
1125+
// Type-aware inline editor. `col.type` is forwarded
1126+
// from ObjectGrid's column inference. Keep this a small,
1127+
// readable switch that's easy to extend.
1128+
const editType = (col as any).type as string | undefined;
1129+
1130+
if (editType === 'date') {
1131+
return (
1132+
<Input
1133+
ref={editInputRef}
1134+
type="date"
1135+
value={toDateInputValue(editValue)}
1136+
// Store a plain yyyy-MM-dd string — matches how
1137+
// date fields are displayed/persisted elsewhere.
1138+
onChange={(e) => setEditValue(e.target.value)}
1139+
onKeyDown={handleEditKeyDown}
1140+
className="h-8 px-2 py-1"
1141+
/>
1142+
);
1143+
}
1144+
1145+
if (editType === 'datetime' || editType === 'datetime-local') {
1146+
return (
1147+
<Input
1148+
ref={editInputRef}
1149+
type="datetime-local"
1150+
value={toDateTimeInputValue(editValue)}
1151+
// The native control yields a local `yyyy-MM-ddTHH:mm`;
1152+
// store back as an ISO string so display/format code
1153+
// (formatCellValue) renders it consistently.
1154+
onChange={(e) => {
1155+
const v = e.target.value;
1156+
const d = v ? new Date(v) : null;
1157+
setEditValue(d && !Number.isNaN(d.getTime()) ? d.toISOString() : v);
1158+
}}
1159+
onKeyDown={handleEditKeyDown}
1160+
className="h-8 px-2 py-1"
1161+
/>
1162+
);
1163+
}
1164+
1165+
if (editType && NUMERIC_EDIT_TYPES.has(editType)) {
1166+
return (
1167+
<Input
1168+
ref={editInputRef}
1169+
type="number"
1170+
value={editValue ?? ''}
1171+
onChange={(e) => setEditValue(e.target.value)}
1172+
onKeyDown={handleEditKeyDown}
1173+
className="h-8 px-2 py-1"
1174+
/>
1175+
);
1176+
}
1177+
1178+
// NOTE: `select`/`boolean` editors are intentionally not
1179+
// wired here — the data-table column does not currently
1180+
// carry option metadata. Extension point: when `col.options`
1181+
// is forwarded from ObjectGrid, render a <Select>/checkbox.
1182+
1183+
// Fallback: plain text input (original behavior).
1184+
return (
1185+
<Input
1186+
ref={editInputRef}
1187+
value={editValue}
1188+
onChange={(e) => setEditValue(e.target.value)}
1189+
onKeyDown={handleEditKeyDown}
1190+
className="h-8 px-2 py-1"
1191+
/>
1192+
);
1193+
})()
10781194
) : (
10791195
<div className="truncate w-full" title={cellValue != null && typeof cellValue !== 'object' ? String(cellValue) : undefined}>
10801196
{typeof col.cell === 'function'

packages/plugin-grid/src/ObjectGrid.tsx

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,9 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
849849

850850
return {
851851
...col,
852+
// Forward the resolved type so the inline editor (data-table) can
853+
// pick a type-aware control (date picker, number, ...).
854+
type: col.type ?? inferredType,
852855
...(schema.showColumnTypeIcons && { headerIcon: getTypeIcon(inferredType) }),
853856
cell: (value: any) => <CellRenderer value={value} field={fieldMeta as any} />,
854857
};
@@ -1010,6 +1013,10 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
10101013
return {
10111014
header,
10121015
accessorKey: col.field,
1016+
// Forward the resolved (base) field type so the inline editor can
1017+
// pick a type-aware control. Use baseInferredType (date/number/...)
1018+
// rather than the renderer type so e.g. `date` stays `date`.
1019+
...(baseInferredType && { type: baseInferredType }),
10131020
...(schema.showColumnTypeIcons && { headerIcon: getTypeIcon(inferredType) }),
10141021
...(!isEssential && { className: 'hidden sm:table-cell' }),
10151022
...(col.width && { width: col.width }),
@@ -1090,6 +1097,8 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
10901097
return {
10911098
header,
10921099
accessorKey: fieldName,
1100+
// Forward the resolved field type for the type-aware inline editor.
1101+
...(resolvedType && { type: resolvedType }),
10931102
...(schema.showColumnTypeIcons && resolvedType && { headerIcon: getTypeIcon(resolvedType) }),
10941103
...(inferredAlign && { align: inferredAlign }),
10951104
...(cellRenderer && { cell: cellRenderer }),
@@ -1133,6 +1142,8 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
11331142
return {
11341143
header,
11351144
accessorKey: fieldName,
1145+
// Forward the resolved field type for the type-aware inline editor.
1146+
...(resolvedType && { type: resolvedType }),
11361147
...(schema.showColumnTypeIcons && resolvedType && { headerIcon: getTypeIcon(resolvedType) }),
11371148
...(inferredAlign && { align: inferredAlign }),
11381149
...(CellRenderer && { cell: (value: any) => <CellRenderer value={value} field={fieldMeta as any} /> }),
@@ -1197,6 +1208,8 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
11971208
generatedColumns.push({
11981209
header: schema.objectName ? resolveFieldLabel(schema.objectName, fieldName, field.label || fieldName) : field.label || fieldName,
11991210
accessorKey: fieldName,
1211+
// Forward the field type for the type-aware inline editor.
1212+
...(field.type && { type: field.type }),
12001213
...(numericTypes.includes(field.type) && { align: 'right' }),
12011214
cell: (value: any) => <CellRenderer value={value} field={fieldForCell} />,
12021215
sortable: field.sortable !== false,
@@ -1544,6 +1557,56 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
15441557
}
15451558
};
15461559

1560+
// Default inline-edit persistence.
1561+
//
1562+
// When a consumer wires `onRowSave`/`onBatchSave` (React host), we defer to it.
1563+
// But a declaratively-configured `editable: true` view has no host wiring — so
1564+
// "Save All" would otherwise just clear pending changes without writing to the
1565+
// backend. Supply a default that persists through the grid's `dataSource`, then
1566+
// refresh so the grid reflects persisted values. Throwing on failure is
1567+
// important: DataTable's saveRow/saveBatch keep pending changes when the save
1568+
// promise rejects, so a failed write doesn't silently lose the user's edits.
1569+
const resolveRecordId = (row: any): string | number | undefined =>
1570+
row?._id ?? row?.id;
1571+
1572+
const defaultRowSave = async (
1573+
_rowIndex: number,
1574+
changes: Record<string, any>,
1575+
row: any,
1576+
): Promise<void> => {
1577+
if (!dataSource || !objectName) {
1578+
throw new Error('Cannot persist inline edit: no dataSource/objectName configured on the grid.');
1579+
}
1580+
const id = resolveRecordId(row);
1581+
if (id === undefined || id === null) {
1582+
throw new Error('Cannot persist inline edit: row has no id/_id.');
1583+
}
1584+
await dataSource.update(objectName, id, changes);
1585+
// Refresh so the grid shows the persisted values.
1586+
setRefreshKey(k => k + 1);
1587+
};
1588+
1589+
const defaultBatchSave = async (
1590+
changes: Array<{ rowIndex: number; changes: Record<string, any>; row: any }>,
1591+
): Promise<void> => {
1592+
if (!dataSource || !objectName) {
1593+
throw new Error('Cannot persist inline edits: no dataSource/objectName configured on the grid.');
1594+
}
1595+
// Update each modified row. The DataSource `bulk`/`bulkUpdate` primitives
1596+
// apply a single uniform patch across many ids, which does NOT fit per-row
1597+
// edits (each row has its own field changes), so issue one update per row.
1598+
await Promise.all(
1599+
changes.map(({ changes: rowChanges, row }) => {
1600+
const id = resolveRecordId(row);
1601+
if (id === undefined || id === null) {
1602+
throw new Error('Cannot persist inline edit: row has no id/_id.');
1603+
}
1604+
return dataSource.update(objectName, id, rowChanges);
1605+
}),
1606+
);
1607+
setRefreshKey(k => k + 1);
1608+
};
1609+
15471610
// Determine pagination settings (support both new and legacy formats)
15481611
const paginationEnabled = schema.pagination !== undefined
15491612
? true
@@ -1639,8 +1702,10 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
16391702
},
16401703
onRowClick: navigation.handleClick,
16411704
onCellChange: onCellChange,
1642-
onRowSave: onRowSave,
1643-
onBatchSave: onBatchSave,
1705+
// Install a dataSource-backed default only when the consumer did NOT wire
1706+
// its own handler, so declarative `editable: true` views still persist.
1707+
onRowSave: onRowSave ?? defaultRowSave,
1708+
onBatchSave: onBatchSave ?? defaultBatchSave,
16441709
onColumnResize: (columnKey: string, width: number) => {
16451710
saveColumnState({
16461711
...columnState,

0 commit comments

Comments
 (0)