fix(plugin-grid): resolve off-spec rowHeight at the state boundary instead of styling it as medium (#4443)

.changeset/objectgrid-rowheight-boundary-4443.md

@@ -0,0 +1,18 @@
+---
+'@object-ui/plugin-grid': patch
+---
+
+standalone ObjectGrid resolves off-spec `rowHeight` to compact, matching ListView and the spec bridge, instead of silently styling it as medium
+
+One component answered one question two ways. `ObjectGrid` seeded its density state with `schema.rowHeight ?? 'compact'`, so an ABSENT `rowHeight` landed on `compact` while an OFF-SPEC one skipped every arm of the density ternaries and came out at their terminal `else` — the `medium` styling. That is the absent-vs-off-spec split objectui#4440 removed from `ListView`, and it made a standalone grid the third answer to a question the rest of the system had already settled: `@object-ui/core`'s `rowHeightToDensityMode` abstains for an off-spec value, the `@object-ui/react` spec bridge abstains, and `ListView` defaults the abstention to `compact`. Off-spec now renders exactly like absent, everywhere.
+
+Only a standalone grid was affected. When `ListView` owns the grid it overwrites the prop with a value derived from `density.mode`, so nothing off-spec survives that hop.
+
+The narrowing happens at the state boundary, not in the ternaries. `medium` is still a real row height with its own styling arm, and a leaf renderer's terminal `else` is still legitimate styling — what changes is that nothing unrecognized can reach it. Membership is tested against `ROW_HEIGHT_TO_DENSITY_MODE`, so the admitted values keep one definition in the repo and the build fails if the spec grows a sixth row height without teaching the resolver about it. Both entry points go through the resolver: the initial state and the effect that re-syncs when the `rowHeight` prop changes.
+
+Two off-spec spellings behaved differently before this, which the report of the defect did not distinguish, and the boundary fix covers both:
+
+- A plain off-spec value (`'garbage'`) was not a key of the toolbar's row-height icon map either. That map is looked up by the same unvalidated state, so `rowHeightIcons[mode]` was `undefined` and rendering `<RowHeightIcon />` threw `Element type is invalid` — a standalone grid with an off-spec `rowHeight` did not render at all, rather than rendering as `medium`. The toolbar is shown precisely when `schema.rowHeight` is defined, so the crash and the off-spec case coincide exactly.
+- A prototype member (`'toString'`) WAS reachable through that map's prototype chain, resolving to `Object.prototype.toString` — a function, which React accepts as a component — so it survived to the ternaries and rendered as `medium`, the defect as filed. The resolver uses `hasOwnProperty` rather than `in` for this reason, the same reason `@object-ui/core` does.
+
+Both are now inert: the state can only ever hold one of the five admitted row heights, so the icon lookup is total and the ternaries never fall through.

packages/plugin-grid/src/ObjectGrid.tsx

@@ -36,7 +36,7 @@ import {
  RefreshIndicator,
 } from '@object-ui/components';
 import { usePullToRefresh } from '@object-ui/mobile';
-import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, listViewPredicates, isProjectableField, isExpandableFieldType, toFilterNode } from '@object-ui/core';
+import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, columnIdentity, collectPredicateFieldRefs, listViewPredicates, isProjectableField, isExpandableFieldType, toFilterNode, ROW_HEIGHT_TO_DENSITY_MODE } from '@object-ui/core';
 import { usePermissions } from '@object-ui/permissions';
 import { ChevronRight, ChevronDown, ChevronLeft, ChevronsLeft, ChevronsRight, Download, Rows2, Rows3, Rows4, AlignJustify, Type, Hash, Calendar, CheckSquare, User, Tag, Clock, Loader2 } from 'lucide-react';
 import { useRowColor } from './useRowColor';
@@ -325,6 +325,43 @@ function normalizeColumns(
  return columns as string[];
 }
 
+/** The row heights this grid styles — the five `RowHeight` values the spec admits. */
+type RowHeightMode = 'compact' | 'short' | 'medium' | 'tall' | 'extra_tall';
+
+/**
+ * The ONE answer this component gives for a `rowHeight` it does not recognize
+ * (objectui#4443).
+ *
+ * The seed used to be `schema.rowHeight ?? 'compact'`, which made the component
+ * answer the same question two ways: an ABSENT `rowHeight` landed on `compact`,
+ * an OFF-SPEC one fell through the density ternaries below to their terminal
+ * `else` — the `medium` styling. That is the absent-vs-off-spec split #4440
+ * removed from `ListView`, and a third answer to a question `@object-ui/core`
+ * (`rowHeightToDensityMode`, which abstains) and the `@object-ui/react` spec
+ * bridge (#4352, which abstains) had already settled. One metadata-driven
+ * system, one answer: off-spec renders exactly like absent.
+ *
+ * The ternary chains are deliberately NOT touched — `medium` is a real value
+ * with its own arm, and a leaf renderer's terminal `else` is legitimate styling.
+ * Narrowing happens here, at the boundary, so nothing off-spec ever reaches it.
+ *
+ * Membership is tested against `ROW_HEIGHT_TO_DENSITY_MODE` rather than a local
+ * list so the admitted values have one definition in the repo; that table is
+ * typed `Record<RowHeight, DensityMode>`, so the build fails if the spec grows a
+ * sixth row height and this resolver is not taught about it.
+ *
+ * `hasOwnProperty`, not `in`: `in` walks the prototype chain, so `'toString'`
+ * would come back admitted. That is not hypothetical here — the toolbar's icon
+ * map is looked up by the same key, and a prototype member reached
+ * `Object.prototype.toString` and rendered it as a React component, while a
+ * plain off-spec value produced `undefined` and threw outright.
+ */
+function resolveRowHeightMode(rowHeight: unknown): RowHeightMode {
+ if (typeof rowHeight !== 'string') return 'compact';
+ if (!Object.prototype.hasOwnProperty.call(ROW_HEIGHT_TO_DENSITY_MODE, rowHeight)) return 'compact';
+ return rowHeight as RowHeightMode;
+}
+
 export const ObjectGrid: React.FC<ObjectGridProps> = ({
  schema,
  dataSource,
@@ -368,7 +405,7 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
  const [showExport, setShowExport] = useState(false);
  const [exportBusy, setExportBusy] = useState(false);
  const [exportError, setExportError] = useState<string | null>(null);
- const [rowHeightMode, setRowHeightMode] = useState<'compact' | 'short' | 'medium' | 'tall' | 'extra_tall'>(schema.rowHeight ?? 'compact');
+ const [rowHeightMode, setRowHeightMode] = useState<RowHeightMode>(resolveRowHeightMode(schema.rowHeight));
  const [selectedRows, setSelectedRows] = useState<any[]>([]);
  const [selectAllMatching, setSelectAllMatching] = useState(false);
  // Bumped to tell the underlying table to drop its internal checkbox selection.
@@ -389,10 +426,15 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
  (schema.pagination as any)?.pageSize ?? schema.pageSize ?? 10,
  );
 
- // Sync internal rowHeightMode when schema.rowHeight prop changes (e.g., parent ListView density toggle)
+ // Sync internal rowHeightMode when schema.rowHeight prop changes (e.g., parent ListView density toggle).
+ // Routed through the same resolver as the seed above: this is the component's
+ // second entry point for an author-supplied `rowHeight`, and one resolver at
+ // every entry is what keeps the answer single (objectui#4443).
  React.useEffect(() => {
- if (schema.rowHeight && schema.rowHeight !== rowHeightMode) {
- setRowHeightMode(schema.rowHeight);
+ if (!schema.rowHeight) return;
+ const next = resolveRowHeightMode(schema.rowHeight);
+ if (next !== rowHeightMode) {
+ setRowHeightMode(next);
  }
  // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [schema.rowHeight]);

packages/plugin-grid/src/__tests__/rowHeightOffSpecBoundary.test.tsx

@@ -0,0 +1,213 @@
+/**
+ * Off-spec `rowHeight` resolves at the state boundary (#4443).
+ *
+ * `ObjectGrid` seeds `rowHeightMode` from `schema.rowHeight`. Before #4443 the
+ * seed was `schema.rowHeight ?? 'compact'`, so the component answered the same
+ * question two different ways: an ABSENT `rowHeight` landed on `compact`, while
+ * an OFF-SPEC one fell through the rendering ternaries to their terminal
+ * `else` — the `medium` styling. That is the absent-vs-off-spec split #4440
+ * removed from `ListView`, and the third answer to a question `@object-ui/core`
+ * (`rowHeightToDensityMode` abstains) and the `@object-ui/react` spec bridge had
+ * already agreed on.
+ *
+ * These tests pin the boundary, not the ternaries: `medium` stays a real value
+ * with its own styling arm, and the terminal `else` stays that arm. What must
+ * hold is that nothing off-spec ever reaches it.
+ *
+ * Two off-spec spellings are covered because they FAIL DIFFERENTLY before the
+ * fix, which the issue text did not distinguish:
+ * - a plain off-spec value (`'garbage'`) is not a key of the toolbar's icon
+ * map either, so `rowHeightIcons[mode]` is `undefined` and rendering
+ * `<RowHeightIcon />` throws — the standalone grid does not render AT ALL;
+ * - a prototype member (`'toString'`) IS reachable through the icon map's
+ * prototype chain, so it survives to the ternary and renders as `medium` —
+ * the defect exactly as filed. Same reason `core` uses `hasOwnProperty`
+ * rather than `in` (see `normalize-list-view.ts`).
+ */
+import { describe, it, expect } from 'vitest';
+import { render, screen, waitFor } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import React from 'react';
+
+import { ObjectGrid } from '../ObjectGrid';
+import { registerAllFields } from '@object-ui/fields';
+import { ActionProvider } from '@object-ui/react';
+
+registerAllFields();
+
+const rows = [
+ { id: '1', name: 'Alice' },
+ { id: '2', name: 'Bob' },
+];
+
+function makeSchema(opts?: Record<string, any>) {
+ return {
+ type: 'object-grid',
+ objectName: 'test_object',
+ columns: [{ field: 'name', label: 'Name' }],
+ data: { provider: 'value', items: rows },
+ ...opts,
+ } as any;
+}
+
+function renderGrid(opts?: Record<string, any>) {
+ return render(
+ <ActionProvider>
+ <ObjectGrid schema={makeSchema(opts)} />
+ </ActionProvider>,
+ );
+}
+
+/**
+ * The two independent copies of the density ternary, as they reach the DOM:
+ *
+ * - COLUMN_COPY (`ObjectGrid.tsx` ~1833, `rowHeightCellClass`) is attached per
+ * column and is the only copy carrying the `h-*` row-height floor. It lands
+ * on the data columns.
+ * - TABLE_COPY (`ObjectGrid.tsx` ~2370, `dataTableSchema.cellClassName`) is the
+ * table-level default for columns that declare none — the narrow row-number
+ * column — and carries padding/leading only, no `h-*`.
+ *
+ * They are therefore separately observable, and each case below asserts both.
+ */
+const COLUMN_COPY: Record<string, string[]> = {
+ compact: ['px-3', 'py-1', 'h-9', 'text-[13px]', 'leading-tight'],
+ short: ['px-3', 'py-1', 'h-9', 'text-[13px]', 'leading-normal'],
+ medium: ['px-3', 'py-1.5', 'h-11', 'text-[13px]', 'leading-normal'],
+ tall: ['px-3', 'py-2.5', 'h-14', 'text-sm'],
+ extra_tall: ['px-3', 'py-3.5', 'h-16', 'text-sm', 'leading-relaxed'],
+};
+
+const TABLE_COPY: Record<string, string[]> = {
+ compact: ['px-3', 'py-1', 'text-[13px]', 'leading-tight'],
+ short: ['px-3', 'py-1', 'text-[13px]', 'leading-normal'],
+ medium: ['px-3', 'py-1.5', 'text-[13px]', 'leading-normal'],
+ tall: ['px-3', 'py-2.5', 'text-sm'],
+ extra_tall: ['px-3', 'py-3.5', 'text-sm', 'leading-relaxed'],
+};
+
+/** Every `h-*` floor the column copy can emit — exactly one may be present. */
+const ALL_HEIGHTS = ['h-9', 'h-11', 'h-14', 'h-16'];
+/** Every vertical padding the table copy can emit — exactly one may be present. */
+const ALL_PADDINGS = ['py-1', 'py-1.5', 'py-2.5', 'py-3.5'];
+
+async function renderAndSettle(opts?: Record<string, any>) {
+ const utils = renderGrid(opts);
+ await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument());
+ return utils;
+}
+
+function cellsOf(container: HTMLElement) {
+ const row = container.querySelector('tbody tr');
+ expect(row, 'expected a rendered body row').toBeTruthy();
+ const tds = Array.from(row!.querySelectorAll('td'));
+ const dataCell = tds.find((td) => !td.classList.contains('w-10'));
+ const plainCell = tds.find((td) => td.classList.contains('w-10'));
+ expect(dataCell, 'expected a data column cell (column-level cellClassName)').toBeTruthy();
+ expect(plainCell, 'expected a row-number cell (table-level cellClassName)').toBeTruthy();
+ return { dataCell: dataCell!, plainCell: plainCell! };
+}
+
+/** Assert BOTH ternary copies resolved to `mode`. */
+function expectDensity(container: HTMLElement, mode: keyof typeof COLUMN_COPY) {
+ const { dataCell, plainCell } = cellsOf(container);
+
+ // Copy 1 — per-column class, the one with the `h-*` floor.
+ for (const token of COLUMN_COPY[mode]) {
+ expect(
+ dataCell.classList.contains(token),
+ `data cell should carry ${mode} token "${token}" (column-level copy), got "${dataCell.className}"`,
+ ).toBe(true);
+ }
+ const heights = ALL_HEIGHTS.filter((h) => dataCell.classList.contains(h));
+ expect(heights, `data cell should carry exactly the ${mode} row-height floor`).toEqual(
+ ALL_HEIGHTS.filter((h) => COLUMN_COPY[mode].includes(h)),
+ );
+
+ // Copy 2 — table-level class on the column that declares none.
+ for (const token of TABLE_COPY[mode]) {
+ expect(
+ plainCell.classList.contains(token),
+ `row-number cell should carry ${mode} token "${token}" (table-level copy), got "${plainCell.className}"`,
+ ).toBe(true);
+ }
+ const paddings = ALL_PADDINGS.filter((p) => plainCell.classList.contains(p));
+ expect(paddings, `row-number cell should carry exactly the ${mode} padding`).toEqual(
+ ALL_PADDINGS.filter((p) => TABLE_COPY[mode].includes(p)),
+ );
+}
+
+describe('ObjectGrid rowHeight — off-spec resolves at the state boundary (#4443)', () => {
+ // ---------------------------------------------------------------------
+ // The direction that must NOT change.
+ // ---------------------------------------------------------------------
+ it('absent rowHeight stays compact', async () => {
+ const { container } = await renderAndSettle();
+ expectDensity(container, 'compact');
+ });
+
+ // ---------------------------------------------------------------------
+ // The five admitted values keep their own styling — the ternary chain is
+ // untouched and `medium` remains a real value, not just a fallback.
+ // ---------------------------------------------------------------------
+ it.each(['compact', 'short', 'medium', 'tall', 'extra_tall'] as const)(
+ 'in-spec rowHeight %s maps to its own styling',
+ async (mode) => {
+ const { container } = await renderAndSettle({ rowHeight: mode });
+ expectDensity(container, mode);
+ },
+ );
+
+ // ---------------------------------------------------------------------
+ // Off-spec input. Before the fix these fall through to the terminal else.
+ // ---------------------------------------------------------------------
+ it('off-spec rowHeight renders as compact, not medium (toolbar suppressed)', async () => {
+ // `hideRowHeightToggle` keeps the icon-map crash (below) out of the way so
+ // this case observes the STYLING ternaries for a plain off-spec value.
+ const { container } = await renderAndSettle({ rowHeight: 'garbage', hideRowHeightToggle: true });
+ expectDensity(container, 'compact');
+ });
+
+ it('off-spec rowHeight leaves a standalone grid renderable, and compact', async () => {
+ // Standalone: `showRowHeightToggle` is true whenever `schema.rowHeight` is
+ // defined, so the toolbar looks the off-spec value up in its icon map.
+ const { container } = await renderAndSettle({ rowHeight: 'garbage' });
+ expectDensity(container, 'compact');
+ expect(screen.getByTitle('Row height: compact')).toBeInTheDocument();
+ });
+
+ it('a prototype-member spelling resolves to compact, not through the prototype chain', async () => {
+ const { container } = await renderAndSettle({ rowHeight: 'toString' });
+ expectDensity(container, 'compact');
+ // Pins the resolved STATE, not just the styling: before the fix this read
+ // "Row height: toString".
+ expect(screen.getByTitle('Row height: compact')).toBeInTheDocument();
+ });
+
+ it('a non-string rowHeight resolves to compact', async () => {
+ const { container } = await renderAndSettle({ rowHeight: 42, hideRowHeightToggle: true });
+ expectDensity(container, 'compact');
+ });
+
+ // ---------------------------------------------------------------------
+ // The second entry point: the effect that re-syncs state when the prop
+ // changes (e.g. a parent ListView's density toggle). One resolver, one
+ // answer, at every entry.
+ // ---------------------------------------------------------------------
+ it('re-syncing to an off-spec rowHeight resolves to compact too', async () => {
+ const { container, rerender } = render(
+ <ActionProvider>
+ <ObjectGrid schema={makeSchema({ rowHeight: 'tall', hideRowHeightToggle: true })} />
+ </ActionProvider>,
+ );
+ await waitFor(() => expect(screen.getByText('Alice')).toBeInTheDocument());
+ expectDensity(container, 'tall');
+
+ rerender(
+ <ActionProvider>
+ <ObjectGrid schema={makeSchema({ rowHeight: 'garbage', hideRowHeightToggle: true })} />
+ </ActionProvider>,
+ );
+ await waitFor(() => expectDensity(container, 'compact'));
+ });
+});