From 5a24c45c3c10009bc0aaa071a12f2bc50bb647ee Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 14:14:37 +0000 Subject: [PATCH] fix(grid): stamp the persisted column width as `width`, not `size` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ungrouped `persistedColumns` map wrote the restored width onto the column as `size`. Nothing downstream reads a column-level `size`: `TableColumn` declares `width`, and `data-table` resolves a column's width at all four of its sites as `columnWidths[accessorKey] || col.width || autoSizedWidths[key]`. The user's resize was persisted, reported to the host and read back correctly, then discarded at the last hop — so the width was never restored on reload and the column fell back to the char-estimate auto width. The grouped path in the same component reads the same `columnState.widths` and has always stamped `width`; it worked. That asymmetry is what identifies `width` as the fix rather than a second key taught to `data-table`. `TableColumn` is not touched — the consumer's declaration is the correct one. The map callback is typed as `ObjectGridColumn` instead of `(col: any)`, so a stray `size` here is now a compile error: the cast is what let the wrong key cross a boundary that has declared the right one since objectui#6004. New pin covers the INBOUND half — a persisted width seeded through both the localStorage and the host `columnState` channel, asserted at the rendered header cell, plus the grouped path as a control. The pre-existing suite pins only the outbound half, which passes on the broken code because the write is exactly what was wrong. Card: objectui#6457 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .changeset/6457-persisted-column-width-key.md | 31 ++++ packages/plugin-grid/src/ObjectGrid.tsx | 26 +++- .../columnWidthInbound-6457.test.tsx | 136 ++++++++++++++++++ 3 files changed, 190 insertions(+), 3 deletions(-) create mode 100644 .changeset/6457-persisted-column-width-key.md create mode 100644 packages/plugin-grid/src/__tests__/columnWidthInbound-6457.test.tsx diff --git a/.changeset/6457-persisted-column-width-key.md b/.changeset/6457-persisted-column-width-key.md new file mode 100644 index 0000000000..ba3c1dcde7 --- /dev/null +++ b/.changeset/6457-persisted-column-width-key.md @@ -0,0 +1,31 @@ +--- +'@object-ui/plugin-grid': patch +--- + +ObjectGrid now restores a persisted column width on the ungrouped path — the stamp key was +`size`, which nothing downstream reads (objectui#6457). + +Resize a grid column and reload: the width came back. It was written to `localStorage` +(and reported to the host, which persists it through `dataSource.updateViewConfig`), read +back into `columnState.widths`, and stamped onto the column — as `size`. `TableColumn` +declares `width`, and `data-table` resolves a column's width at all four of its sites as +`columnWidths[accessorKey] || col.width || autoSizedWidths[accessorKey]`; it reads no +column-level `size` anywhere, and ObjectGrid never passes a `columnWidths` prop down. So +the round trip completed and was discarded at the last hop, and the column fell back to +the char-estimate auto width. The `persistedColumns` map now stamps `width`. + +The correct key was not a judgement call: the **grouped** path in the same component reads +the same `columnState.widths` and has always stamped `width`, and it worked. One path was +out of step with its sibling — so this restores a convention rather than teaching +`data-table` a second spelling. `TableColumn` is not edited: the consumer's declaration +was the correct one. Precedence is unchanged and needs no change — a persisted width still +loses to an in-session resize and still beats auto-sizing. + +Two things stop it recurring. The map's callback is no longer `(col: any)`: typed as +`ObjectGridColumn` (`TableColumn & …`, declared since objectui#6004), a stray `size` here +is now a compile error instead of a silent, user-visible drop — the `any` was what let the +wrong key cross a boundary that had already declared the right one. And the new pin is the +**inbound** half — a persisted width seeded through both channels, asserted at the rendered +header cell. The pre-existing suite asserted only the outbound half, which passes on the +broken code, because the write is exactly what was wrong; that is the measured reason this +shipped. diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx index 1488f7b1ee..56416cfbde 100644 --- a/packages/plugin-grid/src/ObjectGrid.tsx +++ b/packages/plugin-grid/src/ObjectGrid.tsx @@ -2468,12 +2468,32 @@ export const ObjectGrid: React.FC = ({ // Apply persisted column order and widths let persistedColumns = [...columns]; - // Apply saved widths + // Apply saved widths. + // + // ⭐ THE KEY IS `width` (objectui#6457). This stamp used to write `size`, + // and `size` is a key nothing downstream consumes: `TableColumn` + // (`@object-ui/types` `data-display.ts`) declares `width`, and `data-table` + // resolves a column's width at all four of its sites as + // `columnWidths[accessorKey] || col.width || autoSizedWidths[accessorKey]` + // — zero column-level `size` reads. So a user's resize was written to + // localStorage, read back into `columnState.widths`, stamped onto the column + // here, and then dropped at the last hop; the width was never restored on the + // ungrouped path. + // + // The grouped path is the control that identified `width` as the right fix + // rather than teaching `data-table` a second key: `groupedColumnWidths` below + // reads the SAME `columnState.widths` and stamps `width`, and it works. + // + // ⛔ Do not re-widen this callback to `(col: any)`. That cast is what let + // the wrong key through a boundary which has DECLARED the right one since + // objectui#6004 — `ObjectGridColumn` is `TableColumn & …`, so with the + // callback typed, a stray `size` here is a compile error instead of a silent, + // user-visible drop. Typed, this defect class cannot come back by hand. if (columnState.widths) { - persistedColumns = persistedColumns.map((col: any) => { + persistedColumns = persistedColumns.map((col): ObjectGridColumn => { const savedWidth = columnState.widths?.[col.accessorKey]; if (savedWidth) { - return { ...col, size: savedWidth }; + return { ...col, width: savedWidth }; } return col; }); diff --git a/packages/plugin-grid/src/__tests__/columnWidthInbound-6457.test.tsx b/packages/plugin-grid/src/__tests__/columnWidthInbound-6457.test.tsx new file mode 100644 index 0000000000..9d3fb709ab --- /dev/null +++ b/packages/plugin-grid/src/__tests__/columnWidthInbound-6457.test.tsx @@ -0,0 +1,136 @@ +/** + * 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. + */ + +/** + * ObjectGrid persisted column width — the INBOUND half (objectui#6457). + * + * ⚠️ This file exists because the OUTBOUND half was already pinned and the bug + * shipped anyway. `columnStatePersistence.test.tsx` asserts that a resize is + * written to localStorage and reported to the host, and its own docblock warns + * that it "deliberately observe[s] the WRITE, never the read-back". That is + * exactly the wrong half for this defect: the write was correct, and the value + * came back correctly — `persistedColumns` then stamped it onto the column as + * `size`, a key nothing downstream reads. `TableColumn` declares `width`, and + * `data-table` resolves every column's width as + * `columnWidths[accessorKey] || col.width || autoSizedWidths[accessorKey]`. + * So an outbound assertion PASSES on the broken code — the write is what was + * wrong — and the user-visible symptom (resize a column, reload, the width is + * gone) was invisible to the suite. + * + * ⭐ Every case here therefore starts from a value that is ALREADY persisted + * and ends at the RENDERED column: `style.width` on the header cell in the + * DOM, which is the last hop the key has to survive. Nothing here observes the + * write. + * + * The seeded widths (321, 277) are deliberately unreachable by `data-table`'s + * auto-size heuristic, which only ever yields `min(400, max(80, maxLen*8+48))` + * — i.e. 80, 400, or a value ≡ 48 (mod 8). Neither 321−48=273 nor 277−48=229 + * is divisible by 8, so a matching assertion cannot be satisfied by the + * fallback that runs when the persisted width is dropped. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { render, screen, waitFor } 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 STORAGE_KEY = 'grid-columns-test_object'; + +const rows = [ + { id: '1', name: 'Alice', amount: 100, category: 'West' }, + { id: '2', name: 'Bob', amount: 200, category: 'West' }, +]; + +beforeEach(() => { + __clearRecordCrudVerdictCache(); + installExplainDouble(); + localStorage.clear(); +}); +afterEach(() => { vi.unstubAllGlobals(); localStorage.clear(); }); + +function renderGrid(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( + + + + ); +} + +/** + * The rendered width of a column, read off the header cell in the DOM — the + * far end of the round trip. `getAllByText` because grouped mode renders one + * sub-table (and therefore one header row) per group. + */ +function renderedWidths(header: string): string[] { + return screen.getAllByText(header) + .map(el => el.closest('th') as HTMLElement) + .filter(Boolean) + .map(th => th.style.width); +} + +describe('ObjectGrid persisted column width (inbound half)', () => { + it('applies a width persisted in localStorage to the rendered column', async () => { + localStorage.setItem(STORAGE_KEY, JSON.stringify({ widths: { name: 321 } })); + + renderGrid(); + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + + // The seeded column reaches the DOM with the persisted width. + expect(renderedWidths('Name')).toEqual(['321px']); + + // …and only that column: the unseeded sibling still auto-sizes, so the + // assertion above cannot be satisfied by a blanket width applied to every + // header cell. + const amount = renderedWidths('Amount'); + expect(amount).toHaveLength(1); + expect(amount[0]).not.toBe('321px'); + expect(amount[0]).toMatch(/^\d+px$/); + }); + + it('applies a width handed in by the host via columnState to the rendered column', async () => { + // The product path: ObjectView reads the saved layout off the view def and + // passes it down, rather than relying on the per-browser localStorage copy. + renderGrid({ columnState: { widths: { name: 321, amount: 277 } } }); + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + + expect(renderedWidths('Name')).toEqual(['321px']); + expect(renderedWidths('Amount')).toEqual(['277px']); + }); + + it('applies a persisted width in grouped mode too (the sibling path that always worked)', async () => { + // The control that identified `width` as the correct key: the grouped path + // reads the SAME `columnState.widths` and has always stamped `width`. It is + // pinned here so the two paths cannot drift apart again in either + // direction. + localStorage.setItem(STORAGE_KEY, JSON.stringify({ widths: { name: 321 } })); + + renderGrid({ grouping: { fields: [{ field: 'category' }] } }); + await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument()); + + const widths = renderedWidths('Name'); + expect(widths.length).toBeGreaterThan(0); + for (const w of widths) expect(w).toBe('321px'); + }); +});