From eadb56497f7db38b2a9569aeca3dd97a28f8f334 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 01:23:41 +0000 Subject: [PATCH 1/2] fix(components,plugin-grid): fire column-state persistence on resize and reorder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DataTable` declared `onColumnResize` and never invoked it, and `ObjectGrid` emitted `onColumnReorder` while the renderer invokes `onColumnsReorder`. Both were the only call sites of `saveColumnState`, so a user's column drag was never written — not to localStorage, not through `onColumnStateChange`. Part of #6175 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L --- ...data-table-column-resize-callback.test.tsx | 87 +++++++++++++ .../src/renderers/complex/data-table.tsx | 20 +++ packages/plugin-grid/src/ObjectGrid.tsx | 15 ++- .../__tests__/columnStatePersistence.test.tsx | 123 ++++++++++++++++++ 4 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 packages/components/src/renderers/complex/__tests__/data-table-column-resize-callback.test.tsx create mode 100644 packages/plugin-grid/src/__tests__/columnStatePersistence.test.tsx diff --git a/packages/components/src/renderers/complex/__tests__/data-table-column-resize-callback.test.tsx b/packages/components/src/renderers/complex/__tests__/data-table-column-resize-callback.test.tsx new file mode 100644 index 0000000000..c7a577514e --- /dev/null +++ b/packages/components/src/renderers/complex/__tests__/data-table-column-resize-callback.test.tsx @@ -0,0 +1,87 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * `DataTable` reports a settled column resize through `onColumnResize` + * (objectui#6175). + * + * `DataTableSchema` has declared `onColumnResize?: (columnKey, width) => void` + * (`data-display.ts:791`) all along, and this renderer invoked it NOWHERE — the + * resize drag mutated local `columnWidths` state and stopped there. ObjectGrid's + * `saveColumnState` hung off exactly that key, so no user-dragged column width + * ever reached `localStorage` or the host's `dataSource.updateViewConfig`. + * + * ⚠️ These assertions observe the CALL, not a rendered width: the drag already + * applied the width to local state before the fix, so anything that only inspects + * the DOM after a drag passes on the UNFIXED renderer too. + * + * Spies are created per case — no module-level mock carrying one case's calls + * into the next. + */ +import { describe, it, expect, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; +import { ComponentRegistry } from '@object-ui/core'; +import '../data-table'; + +const baseSchema = { + columns: [ + { header: 'Name', accessorKey: 'name' }, + { header: 'Amount', accessorKey: 'amount' }, + ], + data: [{ name: 'Alice', amount: 100 }], + pagination: false, + searchable: false, +}; + +function renderTable(extra: Record) { + const DataTable = ComponentRegistry.get('data-table') as any; + if (!DataTable) throw new Error('data-table not registered'); + return render(); +} + +function resizeHandleFor(header: string): HTMLElement { + const th = screen.getByText(header).closest('th') as HTMLElement; + expect(th).toBeTruthy(); + const handle = th.querySelector('.cursor-col-resize') as HTMLElement; + expect(handle).toBeTruthy(); + return handle; +} + +describe('data-table reports column resizes to the host', () => { + it('fires onColumnResize once at mouseup, with the column key and settled width', () => { + const onColumnResize = vi.fn(); + renderTable({ resizableColumns: true, onColumnResize }); + + fireEvent.mouseDown(resizeHandleFor('Name'), { clientX: 100 }); + expect(onColumnResize).not.toHaveBeenCalled(); + + // Two moves in ONE drag: the host turns this callback into a persisted write + // to shared view config, so it must report the settled value once rather than + // stream every intermediate width. + fireEvent.mouseMove(document, { clientX: 200 }); + fireEvent.mouseMove(document, { clientX: 260 }); + expect(onColumnResize).not.toHaveBeenCalled(); + + fireEvent.mouseUp(document); + + expect(onColumnResize).toHaveBeenCalledTimes(1); + expect(onColumnResize).toHaveBeenCalledWith('name', 160); + }); + + it('does not fire when the drag never moved', () => { + const onColumnResize = vi.fn(); + renderTable({ resizableColumns: true, onColumnResize }); + + fireEvent.mouseDown(resizeHandleFor('Amount'), { clientX: 100 }); + fireEvent.mouseUp(document); + + expect(onColumnResize).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/components/src/renderers/complex/data-table.tsx b/packages/components/src/renderers/complex/data-table.tsx index d09817f9cb..f7abeceaa6 100644 --- a/packages/components/src/renderers/complex/data-table.tsx +++ b/packages/components/src/renderers/complex/data-table.tsx @@ -927,6 +927,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { const resizingColumn = useRef(null); const startX = useRef(0); const startWidth = useRef(0); + /** + * Final width produced by the in-flight drag, so `handleResizeEnd` can report + * it once. It has to be a ref, not state: the document-level listeners are one + * render's closures and cannot observe a later `columnWidths` update. + */ + const lastResizeWidth = useRef(null); const editInputRef = useRef(null); // When an edit ends via Enter (already saved) or Escape (cancelled), the // input also blurs. This flag tells the blur handler not to save again so we @@ -1282,6 +1288,7 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { resizingColumn.current = columnKey; startX.current = e.clientX; + lastResizeWidth.current = null; const headerCell = (e.target as HTMLElement).closest('th'); if (headerCell) { @@ -1302,12 +1309,25 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => { ...prev, [resizingColumn.current!]: newWidth })); + lastResizeWidth.current = newWidth; }; const handleResizeEnd = () => { + const resizedColumn = resizingColumn.current; + const finalWidth = lastResizeWidth.current; resizingColumn.current = null; + lastResizeWidth.current = null; document.removeEventListener('mousemove', handleResizeMove); document.removeEventListener('mouseup', handleResizeEnd); + // objectui#6175: report the SETTLED width, once, at mouseup — never per + // mousemove. `onColumnResize` was declared here and invoked nowhere, which + // is why ObjectGrid's `saveColumnState` never ran and column widths never + // persisted. The host turns this into a real write (localStorage plus + // `onColumnStateChange` -> `dataSource.updateViewConfig`), so a per-move + // callback would be a write storm on shared view config. + if (resizedColumn && finalWidth != null) { + schema.onColumnResize?.(resizedColumn, finalWidth); + } }; // Column reordering handlers diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index e48c003c0e..7c211254c0 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -3087,10 +3087,21 @@ export const ObjectGrid: React.FC = ({ widths: { ...columnState.widths, [columnKey]: width }, }); }, - onColumnReorder: (newOrder: string[]) => { + // objectui#6175: the renderer invokes `onColumnsReorder` (with the `s`) — + // `data-table.tsx:handleColumnDrop` — and has never invoked the singular + // `onColumnReorder` this used to emit, so reorders were never persisted. + // Producer-side fix (AGENTS #0.1: fix the producer, don't teach the renderer + // a second spelling), which retires nothing: `onColumnReorder` stays declared + // on `DataTableSchema` and stays unwired, exactly as the RuntimeOnlyDeclared + // ledger records it. Which of the two declared spellings survives is still an + // open ruling and is deliberately NOT settled here. + onColumnsReorder: (newColumns: any[]) => { + const order = newColumns + .map((c) => c?.accessorKey) + .filter((key): key is string => typeof key === 'string' && key.length > 0); saveColumnState({ ...columnState, - order: newOrder, + order, }); }, }; diff --git a/packages/plugin-grid/src/__tests__/columnStatePersistence.test.tsx b/packages/plugin-grid/src/__tests__/columnStatePersistence.test.tsx new file mode 100644 index 0000000000..9301e13498 --- /dev/null +++ b/packages/plugin-grid/src/__tests__/columnStatePersistence.test.tsx @@ -0,0 +1,123 @@ +/** + * ObjectGrid column-state persistence — the OUTBOUND half (objectui#6175). + * + * The inbound half (seed `columnState` from the host / localStorage, re-sync on + * external change) already worked and is pinned by `gridNonAuthorKeys.test.tsx`. + * The outbound half was DEAD: `saveColumnState` (`ObjectGrid.tsx:661`) had exactly + * two call sites — `onColumnResize` and `onColumnReorder` on the synthesised + * `dataTableSchema` — and `data-table.tsx` invoked NEITHER, so a user's drag was + * never written anywhere. + * + * ⚠️ These tests deliberately observe the WRITE, never the read-back. A test that + * seeds a width and re-reads it passes with the outbound half still dead, because + * the inbound half is what answers it. Each case therefore asserts on BOTH outbound + * channels of `saveColumnState`: + * 1. `localStorage.setItem(columnStorageKey, …)` — the per-browser fallback, and + * 2. the `onColumnStateChange` prop — the host channel that `ObjectView` turns + * into `dataSource.updateViewConfig`. + * + * Every spy and the storage are recreated per case (no module-level mock object + * carrying one case's write into the next). + */ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, fireEvent } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import React from 'react'; + +import { ObjectGrid } from '../ObjectGrid'; +import { __clearRecordCrudVerdictCache } from '../hooks/useRecordCrudVerdicts'; +import { installExplainDouble } from './explainDouble'; +import { registerAllFields } from '@object-ui/fields'; +import { ActionProvider } from '@object-ui/react'; + +registerAllFields(); + +const rows = [ + { id: '1', name: 'Alice', amount: 100 }, + { id: '2', name: 'Bob', amount: 200 }, +]; + +beforeEach(() => { + __clearRecordCrudVerdictCache(); + installExplainDouble(); + localStorage.clear(); +}); +afterEach(() => { vi.unstubAllGlobals(); localStorage.clear(); }); + +const STORAGE_KEY = 'grid-columns-test_object'; + +function renderGrid(onColumnStateChange: (s: any) => void, opts?: Record) { + const schema: any = { + type: 'object-grid', + objectName: 'test_object', + columns: [ + { field: 'name', label: 'Name' }, + { field: 'amount', label: 'Amount', type: 'number' }, + ], + data: { provider: 'value', items: rows }, + ...opts, + }; + return render( + + + + ); +} + +/** Drag the resize handle of the header cell whose label is `header`. */ +function dragResize(container: HTMLElement, header: string, byPx: number) { + const th = screen.getByText(header).closest('th') as HTMLElement; + expect(th).toBeTruthy(); + const handle = th.querySelector('.cursor-col-resize') as HTMLElement; + expect(handle).toBeTruthy(); + fireEvent.mouseDown(handle, { clientX: 100 }); + fireEvent.mouseMove(document, { clientX: 100 + byPx }); + fireEvent.mouseUp(document); +} + +describe('ObjectGrid column-state persistence (outbound half)', () => { + it('writes the new width to localStorage AND notifies the host when a column is resized', async () => { + const onColumnStateChange = vi.fn(); + const { container } = renderGrid(onColumnStateChange); + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + + expect(onColumnStateChange).not.toHaveBeenCalled(); + expect(localStorage.getItem(STORAGE_KEY)).toBeNull(); + + dragResize(container, 'Name', 160); + + // Channel 1 — the host callback that reaches dataSource.updateViewConfig. + await waitFor(() => expect(onColumnStateChange).toHaveBeenCalled()); + const notified = onColumnStateChange.mock.calls.at(-1)![0]; + expect(notified.widths).toEqual({ name: 160 }); + + // Channel 2 — the per-browser fallback. + const stored = localStorage.getItem(STORAGE_KEY); + expect(stored).not.toBeNull(); + expect(JSON.parse(stored!).widths).toEqual({ name: 160 }); + }); + + it('writes the new order to localStorage AND notifies the host when columns are reordered', async () => { + const onColumnStateChange = vi.fn(); + renderGrid(onColumnStateChange, { reorderableColumns: true }); + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + + expect(onColumnStateChange).not.toHaveBeenCalled(); + expect(localStorage.getItem(STORAGE_KEY)).toBeNull(); + + const nameTh = screen.getByText('Name').closest('th') as HTMLElement; + const amountTh = screen.getByText('Amount').closest('th') as HTMLElement; + const dataTransfer = { effectAllowed: '', dropEffect: '' }; + fireEvent.dragStart(nameTh, { dataTransfer }); + fireEvent.dragOver(amountTh, { dataTransfer }); + fireEvent.drop(amountTh, { dataTransfer }); + + await waitFor(() => expect(onColumnStateChange).toHaveBeenCalled()); + const notified = onColumnStateChange.mock.calls.at(-1)![0]; + expect(notified.order).toEqual(['amount', 'name']); + + const stored = localStorage.getItem(STORAGE_KEY); + expect(stored).not.toBeNull(); + expect(JSON.parse(stored!).order).toEqual(['amount', 'name']); + }); +}); From a31843d79d7ff07f9d264c4a379527c3103f4dcc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 01:47:43 +0000 Subject: [PATCH 2/2] test(types): correct the RuntimeOnlyDeclared note now that persistence fires The ledger note described objectui#6175 as unfixed and named `onColumnResize` as read nowhere. Both are now false. The two-spelling ruling stays open and is restated as the deliberate residue it is; no ledger membership changes. Part of #6175 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L --- .changeset/6175-column-state-persistence.md | 37 +++++++++++++++++++ ...data-table-column-resize-callback.test.tsx | 9 +++++ .../__tests__/columnStatePersistence.test.tsx | 6 +-- .../src/__tests__/zod-mirror-parity.test.ts | 20 ++++++---- 4 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 .changeset/6175-column-state-persistence.md diff --git a/.changeset/6175-column-state-persistence.md b/.changeset/6175-column-state-persistence.md new file mode 100644 index 0000000000..ae287843cf --- /dev/null +++ b/.changeset/6175-column-state-persistence.md @@ -0,0 +1,37 @@ +--- +'@object-ui/components': patch +'@object-ui/plugin-grid': patch +--- + +Column width and order that a user drags in `ObjectGrid` now actually persist +(objectui#6175). Both halves of `saveColumnState`'s only two call sites were dead, so a +drag was written nowhere — not to `localStorage`, not through `onColumnStateChange` to the +host's `dataSource.updateViewConfig`. The saved state was read back correctly forever; it +was simply never written. + +Two independent breaks, one per package: + +- **`@object-ui/components`** — `DataTableSchema` has declared + `onColumnResize?: (columnKey, width) => void` all along, and `data-table.tsx` invoked it + **nowhere**: the resize drag updated the table's local `columnWidths` state and stopped + there. It now reports the settled width once, at `mouseup`. Once, deliberately — the host + turns this callback into a write to shared view config, so a per-`mousemove` callback + would be a write storm. +- **`@object-ui/plugin-grid`** — `ObjectGrid` emitted `onColumnReorder` (singular) while the + renderer invokes the near-duplicate `onColumnsReorder` (with the `s`), a different declared + key with a different signature. The producer now emits the spelling the renderer actually + invokes, mapping the reported `TableColumn[]` to the `accessorKey` order `columnState` + stores. + +**Nothing is retired.** Both spellings remain declared on `DataTableSchema`; +`onColumnReorder` stays declared and stays unwired, exactly as the `RuntimeOnlyDeclared` +ledger in `zod-mirror-parity.test.ts` records it. Which of the two survives is a +declared-surface ruling that stays open and is deliberately not settled here. + +⚠️ Behavioural note for hosts: `onColumnStateChange` now fires where it previously never +did, which means `dataSource.updateViewConfig` is now reached on a column drag. That call +was unreachable by this path before, so any permission gate on that write now sees traffic +it never saw. + +The renderer's resize/reorder gestures, the inbound seeding of `columnState`, and the +declared surface are all unchanged. diff --git a/packages/components/src/renderers/complex/__tests__/data-table-column-resize-callback.test.tsx b/packages/components/src/renderers/complex/__tests__/data-table-column-resize-callback.test.tsx index c7a577514e..e120e8a944 100644 --- a/packages/components/src/renderers/complex/__tests__/data-table-column-resize-callback.test.tsx +++ b/packages/components/src/renderers/complex/__tests__/data-table-column-resize-callback.test.tsx @@ -75,6 +75,15 @@ describe('data-table reports column resizes to the host', () => { expect(onColumnResize).toHaveBeenCalledWith('name', 160); }); + /** + * ⚠️ Measured: this case is the ONE assertion here that still PASSES on a + * revert of the fix — the unfixed renderer fires nothing at all, so "did not + * fire" is trivially true there. It is a guard against over-firing (a future + * per-mousemove or per-mouseup-without-drag callback would redden it), NOT a + * pin of the fix. The three cases that DO distinguish the two states of the + * world are the one above and both cases in + * `plugin-grid/src/__tests__/columnStatePersistence.test.tsx`. + */ it('does not fire when the drag never moved', () => { const onColumnResize = vi.fn(); renderTable({ resizableColumns: true, onColumnResize }); diff --git a/packages/plugin-grid/src/__tests__/columnStatePersistence.test.tsx b/packages/plugin-grid/src/__tests__/columnStatePersistence.test.tsx index 9301e13498..7c9174c794 100644 --- a/packages/plugin-grid/src/__tests__/columnStatePersistence.test.tsx +++ b/packages/plugin-grid/src/__tests__/columnStatePersistence.test.tsx @@ -65,7 +65,7 @@ function renderGrid(onColumnStateChange: (s: any) => void, opts?: Record { it('writes the new width to localStorage AND notifies the host when a column is resized', async () => { const onColumnStateChange = vi.fn(); - const { container } = renderGrid(onColumnStateChange); + renderGrid(onColumnStateChange); await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); expect(onColumnStateChange).not.toHaveBeenCalled(); expect(localStorage.getItem(STORAGE_KEY)).toBeNull(); - dragResize(container, 'Name', 160); + dragResize('Name', 160); // Channel 1 — the host callback that reaches dataSource.updateViewConfig. await waitFor(() => expect(onColumnStateChange).toHaveBeenCalled()); diff --git a/packages/types/src/__tests__/zod-mirror-parity.test.ts b/packages/types/src/__tests__/zod-mirror-parity.test.ts index f1038610fb..38797ec5b9 100644 --- a/packages/types/src/__tests__/zod-mirror-parity.test.ts +++ b/packages/types/src/__tests__/zod-mirror-parity.test.ts @@ -985,13 +985,19 @@ interface UnmirroredDeclared { interface RuntimeOnlyDeclared { /** * 12 of `DataTableSchema`'s former 29. OVERSIGHT group — this mirror already - * declares four callbacks. ⚠️ Two of these are additionally read NOWHERE: - * `onColumnResize`/`onColumnReorder` are the only call sites of `saveColumnState` - * and `data-table.tsx` invokes neither, so column-state persistence never fires - * (objectui#6175, filed separately — NOT fixed by this reclassification). And - * `onColumnReorder` is a near-duplicate of the MIRRORED `onColumnsReorder`: one - * event, two declared spellings, different signatures. Which spelling survives is - * an open ruling and is deliberately not settled here. + * declares four callbacks. ⚠️ `onColumnReorder` is still read NOWHERE, and + * deliberately so. objectui#6175 repaired the persistence half this entry used to + * describe: `onColumnResize` is now invoked by `data-table.tsx` (at the end of a + * resize drag), and ObjectGrid now emits the MIRRORED near-duplicate + * `onColumnsReorder` — the spelling the renderer already invoked — instead of the + * singular `onColumnReorder` it used to write and nothing read. So column-state + * persistence DOES fire now, and `onColumnReorder` is left declared, mirrored by + * nothing, and wired to nothing. + * + * That residue is the open question, not an oversight: one event, two declared + * spellings, different signatures. objectui#6175 wired persistence WITHOUT + * retiring anything, because retiring either spelling is a declared-surface change + * and that ruling is still OPEN. Nothing about this entry's membership changed. */ 'data-display.zod.ts#DataTableSchema': | 'onAddRecord' | 'onBatchSave' | 'onCellChange' | 'onColumnReorder' | 'onColumnResize'