Skip to content

Commit 1499512

Browse files
authored
fix(grid): inline-edit toggle takes effect immediately + staged editor closes on save (#2151)
* fix(grid): keep lookup inline-edit value after clicking another row Two independent defects made an inline-edited lookup/reference value disappear (falling back to the raw id) once the user clicked another row: 1. data-table `startEdit` had no re-entry guard. A lookup option renders in a Portal, but React synthetic events still bubble through the component tree back to the cell onClick, re-invoking startEdit for the same cell; that reset editValue from a not-yet-flushed pendingChanges and clobbered the freshly picked value. Guard against re-entering the same cell. 2. ObjectGrid's column-building paths dropped reference_to (they copied only label/currency/precision/scale/format/options) so the read-only LookupCellRenderer/useLookupName could not resolve the staged id to a display name. Add applyRelationalMeta() and call it in every path to forward reference_to/display_field/id_field/etc. Adds a reproduction test; both fixes are individually load-bearing. Closes #2150 * fix(grid): inline-edit toggle takes effect immediately + staged editor closes on save Bug A: toggling 行内编辑 needed a second click to actually make the grid editable. ListView's memoized grid schema (which sets editable: inlineEdit) omitted inlineEdit from its deps, so it served the stale value for one toggle. Add inlineEdit to the dep array so the toggle propagates on the same interaction. Bug B: after inline-editing a lookup cell and clicking 全部保存, the cell stayed stuck showing the picker widget. Relational editors (lookup/master_detail/ user/owner) stage their value and deliberately keep the editor open; saveRow/ saveBatch cleared pendingChanges but never cleared editingCell, so the persisted cell never reverted to its resolved display value. Close any open editor once the row/batch is saved. Also: ListView refetches on dataSource.onMutation even when an external refreshTrigger is present (inline-edit saves go straight through dataSource.update and only signal via onMutation), and the lookup cell derives its reference key correctly. Regression tests: data-table-inline-edit (E), ListView toggle/mutationRefresh, onMutation, lookupCellReferenceKey. * fix(grid): refresh list after a bulk/row action succeeds (#2159) The bottom BulkActionBar's string bulk actions (e.g. 下推 / 派工) and the row action menu both dispatched via executeAction but never bumped refreshKey, so the grid kept showing stale data after the server mutation completed — and the selection toolbar stayed stuck. Only the delete branch (onBulkDelete) and the rich BulkActionDialog (handleBulkDialogClose) refreshed. ObjectGrid self-fetches on refreshKey and has no onMutation subscription, and custom bulk APIs bypass dataSource.update, so nothing else signals a refetch. On a successful action, reset the selection and bump refreshKey so the list reflects the server state (mirrors the delete + dialog branches). Adds bulkActionRefresh.test.tsx driving the full path (header checkbox → BulkActionBar → dispatchBulkAction → runner handler) against an in-memory fake server, asserting a refetch + selection reset on success and neither on failure. * fix(grid): guard inline-edit re-entry with a ref (flaky CI #2150) The startEdit re-entry guard read the editingCell React state, which can observe a stale value under batching/contention. Picking a lookup/select option (rendered in a Portal) bubbles its click back to the cell onClick, re-invoking startEdit for the same cell; when the guard missed, editValue was reset from not-yet-flushed pendingChanges and the freshly picked value was clobbered — the intermittent 'Dev Admin' failure in CI. Track the active cell in a synchronously-mutated ref and guard on it, so the re-entrant call within the same event is always caught. Keep the ref in sync at every edit-exit site. Also lands the bulkActionDefs-path refresh regression test (#2159). * fix(fields): cache picked lookup option so its label resolves synchronously handleSelect now stores the chosen option in pickerResolvedRecords, so selectedOptions/findOption can resolve the just-picked foreign-key id to its display label without depending on the popover's transient fetchedOptions or an async findOne hydration. Fixes the intermittent CI failure where a freshly-picked lookup value showed no label (#2150).
1 parent 4d617a6 commit 1499512

13 files changed

Lines changed: 953 additions & 19 deletions

File tree

packages/components/src/__tests__/data-table-inline-edit.test.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,4 +179,40 @@ describe('data-table — inline edit is per-row usable', () => {
179179

180180
expect(container.textContent).not.toContain('Save failed');
181181
});
182+
183+
it('E) a staged editor (e.g. a lookup picker) exits edit mode after Save All', async () => {
184+
// Regression: relational pickers (lookup/master_detail/user/owner) keep their
185+
// widget open on pick — they `stage` the value rather than commit-and-close.
186+
// saveBatch cleared pendingChanges but never cleared `editingCell`, so after
187+
// 全部保存 the saved cell stayed stuck showing the picker widget instead of
188+
// reverting to the resolved display value. Save All must close any open editor.
189+
const onBatchSave = vi.fn().mockResolvedValue(undefined);
190+
// A host-injected editor that mimics a lookup: picking a value STAGES it and
191+
// deliberately keeps the editor open (no commit/close).
192+
const renderCellEditor = ({ column, stage }: any) =>
193+
column.accessorKey === 'qty' ? (
194+
<button data-testid="picker" onClick={() => stage('picked')}>
195+
picker
196+
</button>
197+
) : null;
198+
199+
const { container, getByText, getByTestId, queryByTestId } = renderComponent({
200+
...editableSchema,
201+
onBatchSave,
202+
renderCellEditor,
203+
});
204+
205+
const cells = container.querySelectorAll('tbody td');
206+
const qtyCell = cells[2] as HTMLElement; // uses the injected picker editor
207+
fireEvent.click(qtyCell);
208+
209+
// Editor is open; pick a value — it stages and stays open (no close).
210+
fireEvent.click(getByTestId('picker'));
211+
expect(queryByTestId('picker')).toBeInTheDocument();
212+
213+
// Save All persists the staged value AND must exit edit mode.
214+
fireEvent.click(getByText(/Save All/i));
215+
await waitFor(() => expect(onBatchSave).toHaveBeenCalledTimes(1));
216+
await waitFor(() => expect(queryByTestId('picker')).not.toBeInTheDocument());
217+
});
182218
});

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

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,16 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
363363
const [draggedColumn, setDraggedColumn] = useState<number | null>(null);
364364
const [dragOverColumn, setDragOverColumn] = useState<number | null>(null);
365365
const [editingCell, setEditingCell] = useState<{ rowIndex: number; columnKey: string } | null>(null);
366+
// Mirror of `editingCell` that is mutated synchronously, so the `startEdit`
367+
// re-entry guard can't be defeated by a stale closure. A lookup/select option
368+
// renders in a Portal; picking it fires the option's onChange (which stages
369+
// the value) and — because React synthetic events still bubble through the
370+
// component tree — the cell's onClick, re-invoking `startEdit` for the SAME
371+
// cell within one event. Reading `editingCell` state there can observe a stale
372+
// (pre-edit) value under batching/contention, so the guard misses and the
373+
// just-picked value is reset from empty `pendingChanges`. The ref always
374+
// reflects the latest edit target within the same tick, so re-entry is caught.
375+
const editingCellRef = useRef<{ rowIndex: number; columnKey: string } | null>(null);
366376
const [editValue, setEditValue] = useState<any>('');
367377
// Track pending changes for multi-cell editing: rowIndex -> { columnKey -> newValue }
368378
const [pendingChanges, setPendingChanges] = useState<Map<number, Record<string, any>>>(new Map());
@@ -662,10 +672,22 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
662672
// Cell editing handlers
663673
const startEdit = (rowIndex: number, columnKey: string) => {
664674
if (!editable) return;
665-
675+
676+
// Already editing THIS cell — do nothing. Re-entering would reset `editValue`
677+
// from `pendingChanges`, and when a widget-injected editor commits via an
678+
// overlay (a lookup/select popover renders in a Portal, but React events
679+
// still bubble through the component tree to this cell's onClick), that reset
680+
// reads a stale `pendingChanges` — before the just-staged value has flushed —
681+
// and clobbers the freshly picked value. Guard on the synchronous ref (not
682+
// the `editingCell` state, which can read stale under batching/contention —
683+
// the intermittent CI failure #2150) so the re-entrant call is always caught.
684+
const active = editingCellRef.current;
685+
if (active?.rowIndex === rowIndex && active?.columnKey === columnKey) return;
686+
666687
const column = columns.find(col => col.accessorKey === columnKey);
667688
if (column?.editable === false) return;
668-
689+
690+
editingCellRef.current = { rowIndex, columnKey };
669691
setEditingCell({ rowIndex, columnKey });
670692

671693
// Check if there's a pending change for this cell, otherwise use current data value
@@ -703,12 +725,14 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
703725
schema.onCellChange(globalIndex, columnKey, valueToStage, row);
704726
}
705727

728+
editingCellRef.current = null;
706729
setEditingCell(null);
707730
setEditValue('');
708731
};
709732

710733
const cancelEdit = () => {
711734
skipBlurSaveRef.current = true;
735+
editingCellRef.current = null;
712736
setEditingCell(null);
713737
setEditValue('');
714738
};
@@ -757,6 +781,14 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
757781
const newPendingChanges = new Map(pendingChanges);
758782
newPendingChanges.delete(rowIndex);
759783
setPendingChanges(newPendingChanges);
784+
// A staged editor (e.g. a lookup picker, which keeps its widget open on
785+
// pick rather than committing) must exit edit mode once its value is
786+
// persisted — otherwise the saved cell stays stuck showing the editor.
787+
if (editingCell?.rowIndex === rowIndex) {
788+
editingCellRef.current = null;
789+
setEditingCell(null);
790+
setEditValue('');
791+
}
760792
// Saved — drop any prior error for this row, and clear the banner once
761793
// no errored rows remain.
762794
setErroredRows((prev) => {
@@ -807,6 +839,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
807839

808840
// Clear all pending changes
809841
setPendingChanges(new Map());
842+
// Any staged editor left open (e.g. a lookup picker that keeps its widget
843+
// open on pick) must exit edit mode now that every row is persisted —
844+
// otherwise the edited cell stays stuck showing the editor after 全部保存.
845+
editingCellRef.current = null;
846+
setEditingCell(null);
847+
setEditValue('');
810848
// Saved — clear any prior errors.
811849
setErroredRows(new Set());
812850
setSaveError(null);

packages/data-objectstack/src/index.ts

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import { ObjectStackClient, type QueryOptions as ObjectStackQueryOptions } from '@objectstack/client';
1010
import type {
1111
DataSource,
12+
MutationEvent,
1213
QueryParams,
1314
QueryResult,
1415
FileUploadResult,
@@ -403,6 +404,11 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
403404
// so optional collections like sys_presence don't hammer the server with
404405
// failing requests on every record open / panel render.
405406
private missingResources = new Set<string>();
407+
// Subscribers registered via onMutation(). Emitted after each successful
408+
// create/update/delete so data-bound views (ListView, ObjectView, kanban,
409+
// calendar) auto-refresh — the interface ListView relies on to reflect
410+
// inline-edit "Save All" writes without a manual reload.
411+
private mutationListeners = new Set<(event: MutationEvent<T>) => void>();
406412

407413
constructor(config: {
408414
baseUrl: string;
@@ -706,9 +712,37 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
706712
/**
707713
* Create a new record.
708714
*/
715+
/**
716+
* Notify all mutation subscribers. A throwing listener must not break the
717+
* mutation or starve the other subscribers, so each is isolated.
718+
*/
719+
private emitMutation(event: MutationEvent<T>): void {
720+
for (const listener of this.mutationListeners) {
721+
try {
722+
listener(event);
723+
} catch (err) {
724+
console.warn('ObjectStackAdapter: mutation listener error', err);
725+
}
726+
}
727+
}
728+
729+
/**
730+
* Subscribe to create/update/delete events on any resource. Returns an
731+
* unsubscribe function. Data-bound views use this to auto-refresh after a
732+
* mutation (e.g. inline-edit "Save All", which writes through `update` and
733+
* must repaint the list without a manual reload).
734+
*/
735+
onMutation(callback: (event: MutationEvent<T>) => void): () => void {
736+
this.mutationListeners.add(callback);
737+
return () => {
738+
this.mutationListeners.delete(callback);
739+
};
740+
}
741+
709742
async create(resource: string, data: Partial<T>): Promise<T> {
710743
await this.connect();
711744
const result = await this.client.data.create<T>(resource, data);
745+
this.emitMutation({ type: 'create', resource, record: { ...result.record } });
712746
return result.record;
713747
}
714748

@@ -737,6 +771,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
737771
data,
738772
opts?.ifMatch ? { ifMatch: opts.ifMatch } : undefined,
739773
);
774+
this.emitMutation({ type: 'update', resource, id, record: { ...result.record } });
740775
return result.record;
741776
} catch (err) {
742777
throw normaliseClientError(err);
@@ -762,6 +797,9 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
762797
String(id),
763798
opts?.ifMatch ? { ifMatch: opts.ifMatch } : undefined,
764799
);
800+
if (result.deleted) {
801+
this.emitMutation({ type: 'delete', resource, id });
802+
}
765803
return result.deleted;
766804
} catch (err) {
767805
throw normaliseClientError(err);
@@ -792,6 +830,13 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
792830
if (!ids || ids.length === 0) return 0;
793831
const records = ids.map((id) => ({ id: String(id), data: patch as any }));
794832

833+
// Notify subscribers once for the whole batch (not per-id) so a single
834+
// "mark all read"/"archive selected" refreshes bound views exactly once.
835+
const emitBulk = (count: number): number => {
836+
if (count > 0) this.emitMutation({ type: 'update', resource });
837+
return count;
838+
};
839+
795840
// eslint-disable-next-line @typescript-eslint/no-explicit-any
796841
const updateMany = (this.client.data as any).updateMany;
797842
if (typeof updateMany === 'function') {
@@ -800,10 +845,10 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
800845
// The server returns BatchUpdateResponse { succeeded, failed, ... };
801846
// fall back to ids.length on adapters that return a bare array.
802847
if (res && typeof res === 'object' && typeof (res as any).succeeded === 'number') {
803-
return (res as any).succeeded as number;
848+
return emitBulk((res as any).succeeded as number);
804849
}
805-
if (Array.isArray(res)) return (res as any[]).length;
806-
return ids.length;
850+
if (Array.isArray(res)) return emitBulk((res as any[]).length);
851+
return emitBulk(ids.length);
807852
} catch (err) {
808853
throw normaliseClientError(err);
809854
}
@@ -819,7 +864,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
819864
// continueOnError semantics — swallow per-row errors
820865
}
821866
}
822-
return succeeded;
867+
return emitBulk(succeeded);
823868
}
824869

825870
/**
@@ -837,17 +882,23 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
837882
if (!ids || ids.length === 0) return 0;
838883
const strIds = ids.map((id) => String(id));
839884

885+
// Notify subscribers once for the whole batch (see bulkUpdate).
886+
const emitBulk = (count: number): number => {
887+
if (count > 0) this.emitMutation({ type: 'delete', resource });
888+
return count;
889+
};
890+
840891
// eslint-disable-next-line @typescript-eslint/no-explicit-any
841892
const deleteMany = (this.client.data as any).deleteMany;
842893
if (typeof deleteMany === 'function') {
843894
try {
844895
const res = await deleteMany(resource, strIds, { continueOnError: true });
845896
if (res && typeof res === 'object' && typeof (res as any).succeeded === 'number') {
846-
return (res as any).succeeded as number;
897+
return emitBulk((res as any).succeeded as number);
847898
}
848-
if (Array.isArray(res)) return (res as any[]).length;
899+
if (Array.isArray(res)) return emitBulk((res as any[]).length);
849900
// deleteMany historically returns void on success — assume all hit.
850-
return strIds.length;
901+
return emitBulk(strIds.length);
851902
} catch (err) {
852903
throw normaliseClientError(err);
853904
}
@@ -863,7 +914,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
863914
// continueOnError semantics — swallow per-row errors
864915
}
865916
}
866-
return succeeded;
917+
return emitBulk(succeeded);
867918
}
868919

869920
/**

0 commit comments

Comments
 (0)