diff --git a/.changeset/listview-rowheight-vocabulary.md b/.changeset/listview-rowheight-vocabulary.md new file mode 100644 index 000000000..380182b64 --- /dev/null +++ b/.changeset/listview-rowheight-vocabulary.md @@ -0,0 +1,34 @@ +--- +"@object-ui/core": minor +"@object-ui/plugin-list": minor +"@object-ui/app-shell": minor +"@object-ui/types": patch +--- + +refactor(views): ListView resolves density from the spec-canonical `rowHeight` (#2890 scope A step 2) + +Second rename in the ListView vocabulary migration: **`densityMode` → `rowHeight`**, +folded in the same `normalizeListViewSchema` that step 1 introduced. + +Unlike `fields`/`columns` this is not a pure alias — the two vocabularies are +different sizes. The spec has five row heights (`compact`/`short`/`medium`/ +`tall`/`extra_tall`); ListView's toolbar offers three densities +(`compact`/`comfortable`/`spacious`). Both directions now live in one place as +`DENSITY_MODE_TO_ROW_HEIGHT` / `ROW_HEIGHT_TO_DENSITY_MODE`, chosen so a fold +followed by a read is a round trip (`spacious` → `tall` → `spacious`), with the +narrowing collapse (`short` → `compact`, `extra_tall` → `spacious`) stated once +instead of being re-derived per call site. + +Two behavior fixes fall out of it: + +- **Precedence is no longer inverted.** `ListView` read `densityMode` *first*, so + a view carrying both keys rendered the legacy value — backwards from every + other legacy/canonical pair in the schema. The canonical key now wins. +- **The toolbar stops re-seeding the legacy key.** `ObjectView`'s + `onDensityChange` persisted `densityMode` into stored view metadata on every + density toggle, so the legacy vocabulary kept regrowing underneath the + migration. It persists `rowHeight` now. + +`densityMode` stays declared on `ListViewSchema` and in the drift guard's +sanctioned set — stored views carry it and it is still valid input — but it is +input-only. diff --git a/packages/app-shell/src/views/ObjectView.tsx b/packages/app-shell/src/views/ObjectView.tsx index 8bb038499..f2835b317 100644 --- a/packages/app-shell/src/views/ObjectView.tsx +++ b/packages/app-shell/src/views/ObjectView.tsx @@ -11,7 +11,7 @@ import { useMemo, useState, useCallback, useEffect, useRef, lazy, Suspense, type ComponentType } from 'react'; import { useParams, useSearchParams, useNavigate, useLocation } from 'react-router-dom'; -import { resolveFilterPlaceholders, type FilterTokenScope } from '@object-ui/core'; +import { resolveFilterPlaceholders, DENSITY_MODE_TO_ROW_HEIGHT, type FilterTokenScope } from '@object-ui/core'; import { parseUserFilterParams, applyUserFilterParams } from './userFilterUrlState'; import { buildListFilterKey, readListFilterState, writeListFilterState } from './listFilterStorage'; const ObjectChart = lazy(() => @@ -1317,7 +1317,12 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an hiddenFields: (viewDef as any).hiddenFields ?? listSchema.hiddenFields, columnState: (viewDef as any).columnState ?? (listSchema as any).columnState, onDensityChange: (mode) => { - persistViewPatch(viewDef.id, viewDef, { densityMode: mode }); + // Persist the spec-canonical `rowHeight` (#2890). Writing the + // legacy `densityMode` here is what kept re-seeding it into + // stored view metadata after every toolbar toggle. + persistViewPatch(viewDef.id, viewDef, { + rowHeight: DENSITY_MODE_TO_ROW_HEIGHT[mode], + }); }, onSortChange: (sort: any) => { persistViewPatch(viewDef.id, viewDef, { sort }); diff --git a/packages/core/src/utils/__tests__/normalize-list-view.test.ts b/packages/core/src/utils/__tests__/normalize-list-view.test.ts index 0df34d1e2..a170f7d7b 100644 --- a/packages/core/src/utils/__tests__/normalize-list-view.test.ts +++ b/packages/core/src/utils/__tests__/normalize-list-view.test.ts @@ -7,7 +7,11 @@ */ import { describe, it, expect } from 'vitest'; -import { normalizeListViewSchema } from '../normalize-list-view.js'; +import { + normalizeListViewSchema, + DENSITY_MODE_TO_ROW_HEIGHT, + ROW_HEIGHT_TO_DENSITY_MODE, +} from '../normalize-list-view.js'; describe('normalizeListViewSchema (#2890)', () => { describe('fields → columns', () => { @@ -56,6 +60,44 @@ describe('normalizeListViewSchema (#2890)', () => { }); }); + describe('densityMode → rowHeight', () => { + it('folds the legacy `densityMode` into the spec-canonical `rowHeight`', () => { + const out = normalizeListViewSchema({ viewType: 'grid', densityMode: 'spacious' }) as Record; + expect(out.rowHeight).toBe('tall'); + expect('densityMode' in out).toBe(false); + }); + + it('lets the canonical key win when a view carries both', () => { + const out = normalizeListViewSchema({ + viewType: 'grid', + rowHeight: 'extra_tall', + densityMode: 'compact', + }) as Record; + expect(out.rowHeight).toBe('extra_tall'); + expect('densityMode' in out).toBe(false); + }); + + it('leaves an unrecognized density value alone rather than inventing a row height', () => { + const out = normalizeListViewSchema({ viewType: 'grid', densityMode: 'cozy' }) as Record; + expect(out.rowHeight).toBeUndefined(); + expect(out.densityMode).toBe('cozy'); + }); + + it('round-trips every density through the widening and back', () => { + // The fold widens 3 values onto 5 and the renderer narrows them back, so + // a folded view must render the density the author picked. + for (const mode of ['compact', 'comfortable', 'spacious'] as const) { + expect(ROW_HEIGHT_TO_DENSITY_MODE[DENSITY_MODE_TO_ROW_HEIGHT[mode]]).toBe(mode); + } + }); + + it('narrows every spec row height onto a density the toolbar can show', () => { + for (const height of ['compact', 'short', 'medium', 'tall', 'extra_tall'] as const) { + expect(['compact', 'comfortable', 'spacious']).toContain(ROW_HEIGHT_TO_DENSITY_MODE[height]); + } + }); + }); + describe('viewType defaulting', () => { it('defaults a missing view kind to the renderable `grid`', () => { expect(normalizeListViewSchema({ type: 'list-view' })).toEqual({ type: 'list-view', viewType: 'grid' }); diff --git a/packages/core/src/utils/normalize-list-view.ts b/packages/core/src/utils/normalize-list-view.ts index f22130ff3..f33272a0a 100644 --- a/packages/core/src/utils/normalize-list-view.ts +++ b/packages/core/src/utils/normalize-list-view.ts @@ -6,6 +6,53 @@ * LICENSE file in the root directory of this source tree. */ +/** ListView's toolbar density vocabulary — three steps, not the spec's five. */ +export type DensityMode = 'compact' | 'comfortable' | 'spacious'; + +/** The spec's `RowHeightSchema` vocabulary. */ +export type RowHeight = 'compact' | 'short' | 'medium' | 'tall' | 'extra_tall'; + +/** + * `densityMode` → `rowHeight`. The two vocabularies are not the same size: the + * spec has five row heights, ListView's toolbar offers three. Widening maps + * each density onto the spec value the renderer already resolves it back to + * (see {@link ROW_HEIGHT_TO_DENSITY_MODE}), so a fold followed by a read is a + * round trip — `spacious` → `tall` → `spacious`. + */ +export const DENSITY_MODE_TO_ROW_HEIGHT: Record = { + compact: 'compact', + comfortable: 'medium', + spacious: 'tall', +}; + +/** + * `rowHeight` → `densityMode`, the read direction. Narrowing five values onto + * three is lossy by construction: `short` collapses into `compact` and + * `extra_tall` into `spacious`. Exported so the renderer and the persistence + * layer agree on the collapse instead of each hard-coding its own table. + */ +export const ROW_HEIGHT_TO_DENSITY_MODE: Record = { + compact: 'compact', + short: 'compact', + medium: 'comfortable', + tall: 'spacious', + extra_tall: 'spacious', +}; + +/** + * Tolerant reader for {@link ROW_HEIGHT_TO_DENSITY_MODE}. View metadata is + * user-authored, so `rowHeight` is not guaranteed to be one of the five spec + * values — an unknown one lands on `comfortable` rather than rendering an + * undefined density. Keeping the fallback here means every surface collapses + * the same way. + */ +export function rowHeightToDensityMode(rowHeight: unknown): DensityMode { + if (typeof rowHeight === 'string' && rowHeight in ROW_HEIGHT_TO_DENSITY_MODE) { + return ROW_HEIGHT_TO_DENSITY_MODE[rowHeight as RowHeight]; + } + return 'comfortable'; +} + /** * ObjectUI's `list-view` node historically used a different vocabulary from * `@objectstack/spec` for the same concepts (`fields` where the spec says @@ -39,6 +86,7 @@ * * Currently folded: * - `fields` → `columns` (#2890 scope A step 1) + * - `densityMode` → `rowHeight` (#2890 scope A step 2) * - `viewType`: a missing kind, or the view CATEGORY `'list'` that AI-authored * metadata stores and hosts forward verbatim, becomes the renderable `'grid'` * — otherwise it reaches the renderer's typeless default branch and shows as @@ -50,15 +98,23 @@ export function normalizeListViewSchema(schema: T): T { const legacyFields = s.fields; const foldColumns = Array.isArray(legacyFields); + const legacyDensity = s.densityMode; + const foldRowHeight = typeof legacyDensity === 'string' && legacyDensity in DENSITY_MODE_TO_ROW_HEIGHT; const viewType = s.viewType; const defaultViewKind = !viewType || viewType === 'list'; - if (!foldColumns && !defaultViewKind) return schema; + if (!foldColumns && !foldRowHeight && !defaultViewKind) return schema; const next: Record = { ...s }; if (foldColumns) { if (!Array.isArray(next.columns)) next.columns = legacyFields; delete next.fields; } + if (foldRowHeight) { + if (typeof next.rowHeight !== 'string') { + next.rowHeight = DENSITY_MODE_TO_ROW_HEIGHT[legacyDensity as DensityMode]; + } + delete next.densityMode; + } if (defaultViewKind) next.viewType = 'grid'; return next as T; } diff --git a/packages/plugin-list/src/ListView.tsx b/packages/plugin-list/src/ListView.tsx index 136c7c7b8..27a873c79 100644 --- a/packages/plugin-list/src/ListView.tsx +++ b/packages/plugin-list/src/ListView.tsx @@ -19,7 +19,7 @@ import { useDensityMode } from '@object-ui/react'; import type { ListViewSchema } from '@object-ui/types'; import { detectStatusField } from '@object-ui/types'; import { usePullToRefresh } from '@object-ui/mobile'; -import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveCrudAffordances, normalizeListViewSchema } from '@object-ui/core'; +import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveCrudAffordances, normalizeListViewSchema, rowHeightToDensityMode } from '@object-ui/core'; import { useObjectTranslation, useObjectLabel, useSafeFieldLabel, createSafeTranslation } from '@object-ui/i18n'; import { usePermissions } from '@object-ui/permissions'; @@ -676,21 +676,14 @@ export const ListView = React.forwardRef(({ return schema.exportOptions; }, [schema.exportOptions]); - // Density Mode — rowHeight maps to density if densityMode not explicitly set + // Toolbar density, resolved from the spec-canonical `rowHeight` (#2890). The + // legacy `densityMode` is folded into it by `normalizeListViewSchema` above — + // it used to be read FIRST here, so a view carrying both rendered the legacy + // value, backwards from every other pair's canonical-wins precedence. const resolvedDensity = React.useMemo(() => { - if (schema.densityMode) return schema.densityMode; - if (schema.rowHeight) { - const map: Record = { - compact: 'compact', - short: 'compact', - medium: 'comfortable', - tall: 'spacious', - extra_tall: 'spacious', - }; - return map[schema.rowHeight] || 'comfortable'; - } + if (schema.rowHeight) return rowHeightToDensityMode(schema.rowHeight); return 'compact'; - }, [schema.densityMode, schema.rowHeight]); + }, [schema.rowHeight]); const density = useDensityMode(resolvedDensity, { onChange: schema.onDensityChange, }); diff --git a/packages/plugin-list/src/__tests__/ListView.test.tsx b/packages/plugin-list/src/__tests__/ListView.test.tsx index bc1dd2418..e2753fca8 100644 --- a/packages/plugin-list/src/__tests__/ListView.test.tsx +++ b/packages/plugin-list/src/__tests__/ListView.test.tsx @@ -517,20 +517,38 @@ describe('ListView', () => { expect(densityButton).toBeInTheDocument(); }); - it('should prefer densityMode over rowHeight', () => { + it('should prefer the spec-canonical rowHeight over the legacy densityMode', () => { + // #2890: this precedence used to be inverted — the legacy key was read + // first, so a view carrying both rendered the legacy value, backwards from + // every other legacy/canonical pair. const schema: ListViewSchema = { type: 'list-view', objectName: 'contacts', viewType: 'grid', - fields: ['name', 'email'], - rowHeight: 'compact', + columns: ['name', 'email'], + rowHeight: 'tall', + densityMode: 'compact', + showDensity: true, + }; + + renderWithProvider(); + expect(screen.getByLabelText('Density: Spacious')).toBeInTheDocument(); + }); + + it('should still honor a legacy densityMode when no rowHeight is set', () => { + // Stored view metadata carries `densityMode`; `normalizeListViewSchema` + // folds it into `rowHeight` at the component boundary. + const schema: ListViewSchema = { + type: 'list-view', + objectName: 'contacts', + viewType: 'grid', + columns: ['name', 'email'], densityMode: 'spacious', showDensity: true, }; renderWithProvider(); - const densityButton = screen.getByLabelText('Density: Spacious'); - expect(densityButton).toBeInTheDocument(); + expect(screen.getByLabelText('Density: Spacious')).toBeInTheDocument(); }); it('should apply aria attributes to root container', () => { diff --git a/packages/types/src/__tests__/list-view-spec-parity.test.ts b/packages/types/src/__tests__/list-view-spec-parity.test.ts index fff31af3e..46012dd39 100644 --- a/packages/types/src/__tests__/list-view-spec-parity.test.ts +++ b/packages/types/src/__tests__/list-view-spec-parity.test.ts @@ -89,7 +89,9 @@ const SANCTIONED_LOCAL = new Set([ 'showDensity', 'showDescription', 'allowExport', - // legacy density shorthand (spec-canonical: `rowHeight`) + // legacy density shorthand, INPUT-ONLY since #2890: `normalizeListViewSchema` + // (@object-ui/core) folds it into the spec's `rowHeight` at the ListView + // boundary and no renderer reads it. 'densityMode', // legacy row/text coloring shorthand (spec-canonical: `rowColor`) 'color', diff --git a/packages/types/src/zod/objectql.zod.ts b/packages/types/src/zod/objectql.zod.ts index 5c2badacd..f6a2615e2 100644 --- a/packages/types/src/zod/objectql.zod.ts +++ b/packages/types/src/zod/objectql.zod.ts @@ -398,6 +398,11 @@ export const ListViewSchema = BaseSchema showDensity: z.boolean().optional().describe('Show density/row-height button in toolbar'), showDescription: z.boolean().optional().describe('Show field descriptions'), allowExport: z.boolean().optional().describe('Allow data export'), + // Legacy alias for the spec's `rowHeight`, still accepted because stored + // view metadata carries it. NO renderer reads it: `normalizeListViewSchema` + // (`@object-ui/core`) folds it into `rowHeight` at the ListView boundary + // (#2890). Note the vocabularies differ in size — three densities widen + // onto the spec's five row heights. densityMode: z.enum(['compact', 'comfortable', 'spacious']).optional().describe('Density mode'), color: z.string().optional().describe('Color field for row/card coloring'), fieldTextColor: z.string().optional().describe('Field for custom text color'),