Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions .changeset/6175-column-state-persistence.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
/**
* 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<string, any>) {
const DataTable = ComponentRegistry.get('data-table') as any;
if (!DataTable) throw new Error('data-table not registered');
return render(<DataTable schema={{ ...baseSchema, ...extra }} />);
}

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);
});

/**
* ⚠️ 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 });

fireEvent.mouseDown(resizeHandleFor('Amount'), { clientX: 100 });
fireEvent.mouseUp(document);

expect(onColumnResize).not.toHaveBeenCalled();
});
});
20 changes: 20 additions & 0 deletions packages/components/src/renderers/complex/data-table.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -927,6 +927,12 @@ const DataTableRenderer = ({ schema }: { schema: DataTableSchema }) => {
const resizingColumn = useRef<string | null>(null);
const startX = useRef<number>(0);
const startWidth = useRef<number>(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<number | null>(null);
const editInputRef = useRef<HTMLInputElement>(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
Expand DownExpand Up@@ -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) {
Expand All@@ -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
Expand Down
15 changes: 13 additions & 2 deletions packages/plugin-grid/src/ObjectGrid.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -3087,10 +3087,21 @@ export const ObjectGrid: React.FC<ObjectGridComponentProps> = ({
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,
});
},
};
Expand Down
123 changes: 123 additions & 0 deletions packages/plugin-grid/src/__tests__/columnStatePersistence.test.tsx
Original file line numberDiff line numberDiff line change
@@ -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<string, any>) {
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(
<ActionProvider>
<ObjectGrid schema={schema} onColumnStateChange={onColumnStateChange} />
</ActionProvider>
);
}

/** Drag the resize handle of the header cell whose label is `header`. */
function dragResize(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();
renderGrid(onColumnStateChange);
await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument());

expect(onColumnStateChange).not.toHaveBeenCalled();
expect(localStorage.getItem(STORAGE_KEY)).toBeNull();

dragResize('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']);
});
});
20 changes: 13 additions & 7 deletions packages/types/src/__tests__/zod-mirror-parity.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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'
Expand Down
Loading