Skip to content

Commit f05b84e

Browse files
os-zhuangclaude
andauthored
refactor(views): ListView resolves density from the spec-canonical rowHeight (#2890) (#2933)
Second rename in the ListView vocabulary migration: `densityMode` → `rowHeight`, folded in the same `normalizeListViewSchema` step 1 introduced. Not a pure alias — the vocabularies are different sizes. The spec has five row heights, ListView's toolbar offers three densities. Both directions now live in one place (`DENSITY_MODE_TO_ROW_HEIGHT` / `ROW_HEIGHT_TO_DENSITY_MODE`, plus the tolerant `rowHeightToDensityMode` reader), chosen so a fold followed by a read is a round trip and the lossy narrowing is stated once. Two behavior fixes fall out: - 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. The canonical key now wins. - The toolbar stops re-seeding the legacy key: ObjectView's `onDensityChange` persisted `densityMode` into stored view metadata on every toggle, so the legacy vocabulary kept regrowing underneath the migration. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent cd09a7b commit f05b84e

8 files changed

Lines changed: 179 additions & 24 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
"@object-ui/core": minor
3+
"@object-ui/plugin-list": minor
4+
"@object-ui/app-shell": minor
5+
"@object-ui/types": patch
6+
---
7+
8+
refactor(views): ListView resolves density from the spec-canonical `rowHeight` (#2890 scope A step 2)
9+
10+
Second rename in the ListView vocabulary migration: **`densityMode``rowHeight`**,
11+
folded in the same `normalizeListViewSchema` that step 1 introduced.
12+
13+
Unlike `fields`/`columns` this is not a pure alias — the two vocabularies are
14+
different sizes. The spec has five row heights (`compact`/`short`/`medium`/
15+
`tall`/`extra_tall`); ListView's toolbar offers three densities
16+
(`compact`/`comfortable`/`spacious`). Both directions now live in one place as
17+
`DENSITY_MODE_TO_ROW_HEIGHT` / `ROW_HEIGHT_TO_DENSITY_MODE`, chosen so a fold
18+
followed by a read is a round trip (`spacious``tall``spacious`), with the
19+
narrowing collapse (`short``compact`, `extra_tall``spacious`) stated once
20+
instead of being re-derived per call site.
21+
22+
Two behavior fixes fall out of it:
23+
24+
- **Precedence is no longer inverted.** `ListView` read `densityMode` *first*, so
25+
a view carrying both keys rendered the legacy value — backwards from every
26+
other legacy/canonical pair in the schema. The canonical key now wins.
27+
- **The toolbar stops re-seeding the legacy key.** `ObjectView`'s
28+
`onDensityChange` persisted `densityMode` into stored view metadata on every
29+
density toggle, so the legacy vocabulary kept regrowing underneath the
30+
migration. It persists `rowHeight` now.
31+
32+
`densityMode` stays declared on `ListViewSchema` and in the drift guard's
33+
sanctioned set — stored views carry it and it is still valid input — but it is
34+
input-only.

packages/app-shell/src/views/ObjectView.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
import { useMemo, useState, useCallback, useEffect, useRef, lazy, Suspense, type ComponentType } from 'react';
1313
import { useParams, useSearchParams, useNavigate, useLocation } from 'react-router-dom';
14-
import { resolveFilterPlaceholders, type FilterTokenScope } from '@object-ui/core';
14+
import { resolveFilterPlaceholders, DENSITY_MODE_TO_ROW_HEIGHT, type FilterTokenScope } from '@object-ui/core';
1515
import { parseUserFilterParams, applyUserFilterParams } from './userFilterUrlState';
1616
import { buildListFilterKey, readListFilterState, writeListFilterState } from './listFilterStorage';
1717
const ObjectChart = lazy(() =>
@@ -1317,7 +1317,12 @@ function ObjectViewInner({ dataSource, objects, onEdit, externalRefreshKey }: an
13171317
hiddenFields: (viewDef as any).hiddenFields ?? listSchema.hiddenFields,
13181318
columnState: (viewDef as any).columnState ?? (listSchema as any).columnState,
13191319
onDensityChange: (mode) => {
1320-
persistViewPatch(viewDef.id, viewDef, { densityMode: mode });
1320+
// Persist the spec-canonical `rowHeight` (#2890). Writing the
1321+
// legacy `densityMode` here is what kept re-seeding it into
1322+
// stored view metadata after every toolbar toggle.
1323+
persistViewPatch(viewDef.id, viewDef, {
1324+
rowHeight: DENSITY_MODE_TO_ROW_HEIGHT[mode],
1325+
});
13211326
},
13221327
onSortChange: (sort: any) => {
13231328
persistViewPatch(viewDef.id, viewDef, { sort });

packages/core/src/utils/__tests__/normalize-list-view.test.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
*/
88

99
import { describe, it, expect } from 'vitest';
10-
import { normalizeListViewSchema } from '../normalize-list-view.js';
10+
import {
11+
normalizeListViewSchema,
12+
DENSITY_MODE_TO_ROW_HEIGHT,
13+
ROW_HEIGHT_TO_DENSITY_MODE,
14+
} from '../normalize-list-view.js';
1115

1216
describe('normalizeListViewSchema (#2890)', () => {
1317
describe('fields → columns', () => {
@@ -56,6 +60,44 @@ describe('normalizeListViewSchema (#2890)', () => {
5660
});
5761
});
5862

63+
describe('densityMode → rowHeight', () => {
64+
it('folds the legacy `densityMode` into the spec-canonical `rowHeight`', () => {
65+
const out = normalizeListViewSchema({ viewType: 'grid', densityMode: 'spacious' }) as Record<string, unknown>;
66+
expect(out.rowHeight).toBe('tall');
67+
expect('densityMode' in out).toBe(false);
68+
});
69+
70+
it('lets the canonical key win when a view carries both', () => {
71+
const out = normalizeListViewSchema({
72+
viewType: 'grid',
73+
rowHeight: 'extra_tall',
74+
densityMode: 'compact',
75+
}) as Record<string, unknown>;
76+
expect(out.rowHeight).toBe('extra_tall');
77+
expect('densityMode' in out).toBe(false);
78+
});
79+
80+
it('leaves an unrecognized density value alone rather than inventing a row height', () => {
81+
const out = normalizeListViewSchema({ viewType: 'grid', densityMode: 'cozy' }) as Record<string, unknown>;
82+
expect(out.rowHeight).toBeUndefined();
83+
expect(out.densityMode).toBe('cozy');
84+
});
85+
86+
it('round-trips every density through the widening and back', () => {
87+
// The fold widens 3 values onto 5 and the renderer narrows them back, so
88+
// a folded view must render the density the author picked.
89+
for (const mode of ['compact', 'comfortable', 'spacious'] as const) {
90+
expect(ROW_HEIGHT_TO_DENSITY_MODE[DENSITY_MODE_TO_ROW_HEIGHT[mode]]).toBe(mode);
91+
}
92+
});
93+
94+
it('narrows every spec row height onto a density the toolbar can show', () => {
95+
for (const height of ['compact', 'short', 'medium', 'tall', 'extra_tall'] as const) {
96+
expect(['compact', 'comfortable', 'spacious']).toContain(ROW_HEIGHT_TO_DENSITY_MODE[height]);
97+
}
98+
});
99+
});
100+
59101
describe('viewType defaulting', () => {
60102
it('defaults a missing view kind to the renderable `grid`', () => {
61103
expect(normalizeListViewSchema({ type: 'list-view' })).toEqual({ type: 'list-view', viewType: 'grid' });

packages/core/src/utils/normalize-list-view.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,53 @@
66
* LICENSE file in the root directory of this source tree.
77
*/
88

9+
/** ListView's toolbar density vocabulary — three steps, not the spec's five. */
10+
export type DensityMode = 'compact' | 'comfortable' | 'spacious';
11+
12+
/** The spec's `RowHeightSchema` vocabulary. */
13+
export type RowHeight = 'compact' | 'short' | 'medium' | 'tall' | 'extra_tall';
14+
15+
/**
16+
* `densityMode` → `rowHeight`. The two vocabularies are not the same size: the
17+
* spec has five row heights, ListView's toolbar offers three. Widening maps
18+
* each density onto the spec value the renderer already resolves it back to
19+
* (see {@link ROW_HEIGHT_TO_DENSITY_MODE}), so a fold followed by a read is a
20+
* round trip — `spacious` → `tall` → `spacious`.
21+
*/
22+
export const DENSITY_MODE_TO_ROW_HEIGHT: Record<DensityMode, RowHeight> = {
23+
compact: 'compact',
24+
comfortable: 'medium',
25+
spacious: 'tall',
26+
};
27+
28+
/**
29+
* `rowHeight` → `densityMode`, the read direction. Narrowing five values onto
30+
* three is lossy by construction: `short` collapses into `compact` and
31+
* `extra_tall` into `spacious`. Exported so the renderer and the persistence
32+
* layer agree on the collapse instead of each hard-coding its own table.
33+
*/
34+
export const ROW_HEIGHT_TO_DENSITY_MODE: Record<RowHeight, DensityMode> = {
35+
compact: 'compact',
36+
short: 'compact',
37+
medium: 'comfortable',
38+
tall: 'spacious',
39+
extra_tall: 'spacious',
40+
};
41+
42+
/**
43+
* Tolerant reader for {@link ROW_HEIGHT_TO_DENSITY_MODE}. View metadata is
44+
* user-authored, so `rowHeight` is not guaranteed to be one of the five spec
45+
* values — an unknown one lands on `comfortable` rather than rendering an
46+
* undefined density. Keeping the fallback here means every surface collapses
47+
* the same way.
48+
*/
49+
export function rowHeightToDensityMode(rowHeight: unknown): DensityMode {
50+
if (typeof rowHeight === 'string' && rowHeight in ROW_HEIGHT_TO_DENSITY_MODE) {
51+
return ROW_HEIGHT_TO_DENSITY_MODE[rowHeight as RowHeight];
52+
}
53+
return 'comfortable';
54+
}
55+
956
/**
1057
* ObjectUI's `list-view` node historically used a different vocabulary from
1158
* `@objectstack/spec` for the same concepts (`fields` where the spec says
@@ -39,6 +86,7 @@
3986
*
4087
* Currently folded:
4188
* - `fields` → `columns` (#2890 scope A step 1)
89+
* - `densityMode` → `rowHeight` (#2890 scope A step 2)
4290
* - `viewType`: a missing kind, or the view CATEGORY `'list'` that AI-authored
4391
* metadata stores and hosts forward verbatim, becomes the renderable `'grid'`
4492
* — otherwise it reaches the renderer's typeless default branch and shows as
@@ -50,15 +98,23 @@ export function normalizeListViewSchema<T>(schema: T): T {
5098

5199
const legacyFields = s.fields;
52100
const foldColumns = Array.isArray(legacyFields);
101+
const legacyDensity = s.densityMode;
102+
const foldRowHeight = typeof legacyDensity === 'string' && legacyDensity in DENSITY_MODE_TO_ROW_HEIGHT;
53103
const viewType = s.viewType;
54104
const defaultViewKind = !viewType || viewType === 'list';
55-
if (!foldColumns && !defaultViewKind) return schema;
105+
if (!foldColumns && !foldRowHeight && !defaultViewKind) return schema;
56106

57107
const next: Record<string, unknown> = { ...s };
58108
if (foldColumns) {
59109
if (!Array.isArray(next.columns)) next.columns = legacyFields;
60110
delete next.fields;
61111
}
112+
if (foldRowHeight) {
113+
if (typeof next.rowHeight !== 'string') {
114+
next.rowHeight = DENSITY_MODE_TO_ROW_HEIGHT[legacyDensity as DensityMode];
115+
}
116+
delete next.densityMode;
117+
}
62118
if (defaultViewKind) next.viewType = 'grid';
63119
return next as T;
64120
}

packages/plugin-list/src/ListView.tsx

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { useDensityMode } from '@object-ui/react';
1919
import type { ListViewSchema } from '@object-ui/types';
2020
import { detectStatusField } from '@object-ui/types';
2121
import { usePullToRefresh } from '@object-ui/mobile';
22-
import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveCrudAffordances, normalizeListViewSchema } from '@object-ui/core';
22+
import { resolveConditionalFormatting, buildExpandFields, buildExportFileName, resolveCrudAffordances, normalizeListViewSchema, rowHeightToDensityMode } from '@object-ui/core';
2323
import { useObjectTranslation, useObjectLabel, useSafeFieldLabel, createSafeTranslation } from '@object-ui/i18n';
2424
import { usePermissions } from '@object-ui/permissions';
2525

@@ -676,21 +676,14 @@ export const ListView = React.forwardRef<ListViewHandle, ListViewProps>(({
676676
return schema.exportOptions;
677677
}, [schema.exportOptions]);
678678

679-
// Density Mode — rowHeight maps to density if densityMode not explicitly set
679+
// Toolbar density, resolved from the spec-canonical `rowHeight` (#2890). The
680+
// legacy `densityMode` is folded into it by `normalizeListViewSchema` above —
681+
// it used to be read FIRST here, so a view carrying both rendered the legacy
682+
// value, backwards from every other pair's canonical-wins precedence.
680683
const resolvedDensity = React.useMemo(() => {
681-
if (schema.densityMode) return schema.densityMode;
682-
if (schema.rowHeight) {
683-
const map: Record<string, 'compact' | 'comfortable' | 'spacious'> = {
684-
compact: 'compact',
685-
short: 'compact',
686-
medium: 'comfortable',
687-
tall: 'spacious',
688-
extra_tall: 'spacious',
689-
};
690-
return map[schema.rowHeight] || 'comfortable';
691-
}
684+
if (schema.rowHeight) return rowHeightToDensityMode(schema.rowHeight);
692685
return 'compact';
693-
}, [schema.densityMode, schema.rowHeight]);
686+
}, [schema.rowHeight]);
694687
const density = useDensityMode(resolvedDensity, {
695688
onChange: schema.onDensityChange,
696689
});

packages/plugin-list/src/__tests__/ListView.test.tsx

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -517,20 +517,38 @@ describe('ListView', () => {
517517
expect(densityButton).toBeInTheDocument();
518518
});
519519

520-
it('should prefer densityMode over rowHeight', () => {
520+
it('should prefer the spec-canonical rowHeight over the legacy densityMode', () => {
521+
// #2890: this precedence used to be inverted — the legacy key was read
522+
// first, so a view carrying both rendered the legacy value, backwards from
523+
// every other legacy/canonical pair.
521524
const schema: ListViewSchema = {
522525
type: 'list-view',
523526
objectName: 'contacts',
524527
viewType: 'grid',
525-
fields: ['name', 'email'],
526-
rowHeight: 'compact',
528+
columns: ['name', 'email'],
529+
rowHeight: 'tall',
530+
densityMode: 'compact',
531+
showDensity: true,
532+
};
533+
534+
renderWithProvider(<ListView schema={schema} />);
535+
expect(screen.getByLabelText('Density: Spacious')).toBeInTheDocument();
536+
});
537+
538+
it('should still honor a legacy densityMode when no rowHeight is set', () => {
539+
// Stored view metadata carries `densityMode`; `normalizeListViewSchema`
540+
// folds it into `rowHeight` at the component boundary.
541+
const schema: ListViewSchema = {
542+
type: 'list-view',
543+
objectName: 'contacts',
544+
viewType: 'grid',
545+
columns: ['name', 'email'],
527546
densityMode: 'spacious',
528547
showDensity: true,
529548
};
530549

531550
renderWithProvider(<ListView schema={schema} />);
532-
const densityButton = screen.getByLabelText('Density: Spacious');
533-
expect(densityButton).toBeInTheDocument();
551+
expect(screen.getByLabelText('Density: Spacious')).toBeInTheDocument();
534552
});
535553

536554
it('should apply aria attributes to root container', () => {

packages/types/src/__tests__/list-view-spec-parity.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,9 @@ const SANCTIONED_LOCAL = new Set<string>([
8989
'showDensity',
9090
'showDescription',
9191
'allowExport',
92-
// legacy density shorthand (spec-canonical: `rowHeight`)
92+
// legacy density shorthand, INPUT-ONLY since #2890: `normalizeListViewSchema`
93+
// (@object-ui/core) folds it into the spec's `rowHeight` at the ListView
94+
// boundary and no renderer reads it.
9395
'densityMode',
9496
// legacy row/text coloring shorthand (spec-canonical: `rowColor`)
9597
'color',

packages/types/src/zod/objectql.zod.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,11 @@ export const ListViewSchema = BaseSchema
398398
showDensity: z.boolean().optional().describe('Show density/row-height button in toolbar'),
399399
showDescription: z.boolean().optional().describe('Show field descriptions'),
400400
allowExport: z.boolean().optional().describe('Allow data export'),
401+
// Legacy alias for the spec's `rowHeight`, still accepted because stored
402+
// view metadata carries it. NO renderer reads it: `normalizeListViewSchema`
403+
// (`@object-ui/core`) folds it into `rowHeight` at the ListView boundary
404+
// (#2890). Note the vocabularies differ in size — three densities widen
405+
// onto the spec's five row heights.
401406
densityMode: z.enum(['compact', 'comfortable', 'spacious']).optional().describe('Density mode'),
402407
color: z.string().optional().describe('Color field for row/card coloring'),
403408
fieldTextColor: z.string().optional().describe('Field for custom text color'),

0 commit comments

Comments
 (0)