Skip to content

Commit dfaad4d

Browse files
fix(fields): lookup subtitle no longer repeats the referenced object's name field (#26062)
Fixes #25860 Clause-②: no Metadata-protocol: none ## What The lookup quick-picker shows each candidate as a title over a subtitle (the preview line) of the referenced object's other columns. A lookup that declares no `displayField` titles the option through the referenced object's own name field (`recordToOption` calls `getRecordDisplayName`: declared `nameField`, else a `titleFormat` template, else the derived title field). But `previewColumns` in `ui/packages/fields/src/widgets/LookupField.tsx` seeded its `seen` set with `displayField` only, and without a declared one that is the widget's `'name'` guess. So on an object whose name field is anything other than `name` (`title`, `customer_name`, every AI-built object), the subtitle's first and widest slot repeated the title. The phone number, the field that tells two records apart, was pushed into truncation. `previewColumns` now also adds the referenced object's resolved name field to `seen` (`resolveNameField(refObjectSchema)` from `@object-ui/core`) when the lookup declares no `displayField`. It follows the same ladder the title follows: - A declared `displayField` titles the option, so it alone is skipped and the object's name field stays in the subtitle (unchanged). - An object with no declared name field but a `titleFormat` is titled by the template, not by a field, so nothing more is skipped (`declaredNameField` decides this). - An explicitly authored `descriptionField` still leads the subtitle (unchanged). The memo's deps gain `declaredDisplayField` and `refObjectSchema`. The import line gains `declaredNameField` and `resolveNameField`. Nothing else in the component changed. `scripts/sdui-manifest.record.json` is re-recorded (`node scripts/gen-sdui-manifest-node.mjs`, under the verify lock), because the registry source digest covers `ui/packages/fields/src`. `sdui.manifest.json` itself is byte-unchanged (sha256 `1f6e1ccb925a…`, 106 components). The branch merged `origin/main` before this PR to pick up main's own re-record of that file. The merge took main's copy and the next commit (`dcee0130`) re-recorded it on the merged tree, so the digest describes the tree this PR lands. ## Tests New `ui/packages/fields/src/widgets/LookupField.subtitleNameField-25860.test.tsx` (5 cases). It opens the real dropdown against a stub data source and reads each option's title and its previewed fields (`data-lookup-preview`) in order: - The object declares `nameField: 'title'`: the subtitle is `phone, source`, and `title` is no longer first. - No `nameField` is declared (the title field is derived as `title`): the same result. - An authored `descriptionField: 'source'` still leads: `source, phone`. - Control: a declared `displayField: 'code'` titles the option, and `title` stays in the subtitle. - Control: a `titleFormat`-only object keeps the derived `title` in the subtitle. Ablation, run once and not kept. The fix was committed first. `node scripts/ablation-replace.mjs` replaced `if (nameField) seen.add(nameField);` with a comment (anchor count 1 to 0, blob `04c7cbb215e3` to `b6be553ded33`). The result was `Tests 3 failed | 2 passed (5)`: the three behaviour cases went red and both controls stayed green. The restore was verified (blob equal to HEAD `04c7cbb215e3`, `git diff HEAD` empty). No build or `dist` is involved: the test imports `./LookupField` from source, and the ui vitest config points `@object-ui/core` at `packages/core/src`. All runs below went through `scripts/pm/os-verify-lock.sh`. The `@object-ui/fields^...` closure was built first. - `cd ui && pnpm exec vitest run --maxWorkers=2 packages/fields/`: `Test Files 278 passed | 1 skipped (279)`, `Tests 4295 passed | 7 skipped (4302)`, VERDICT command-exit 0. This ran on the pre-merge head `a200f71d`. The merge brought no change under `ui/packages/fields`, `ui/packages/core` or `ui/packages/types`. - After the merge, at `dcee0130`: the new test plus `labelOrder-10343`, `optionDescription` and `previewBadges-13123` gave `Test Files 4 passed (4)`, `Tests 21 passed (21)`, VERDICT command-exit 0. - `pnpm --filter @object-ui/fields type-check` (`tsc --noEmit && tsc -p tsconfig.test.json`): exit 0. `tsconfig.test.json` includes `src/**/*.test.tsx`, so it checks the new test too. Gates: at `dcee0130`, `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 46 commands. All 46 exit 0, including `node scripts/check-sdui-manifest.mjs`, `pnpm check:nul-bytes` and `pnpm check:type-check-coverage`. The `--ran` reconciliation reports 46 derived, 46 run, 0 NOT-MEASURED (a derived zero, with every exit code recorded). CI runs the rest of the families. ## Acceptance notes - `effectiveDescriptionField` still falls back to the first picker column that is not `displayField`, so for an undeclared lookup it can be the name field. It is only used for the option's `description`, which feeds the popover's text match and is not rendered as the subtitle. It is outside this card's file surface, so it is left as is. No user-visible effect was measured. - `getRecordDisplayName` decides per record. If the declared name field is blank on one record, that record's title falls through to the template or the derived field. The subtitle skips the name field for every row, and a blank value would not render anyway (`previewOf` drops empty cells). --- _Generated by [Claude Code](https://claude.ai/code/session_01SMps7wq7rBTk53e8KTSEup)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 21075ce commit dfaad4d

4 files changed

Lines changed: 160 additions & 3 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@object-ui/fields': patch
3+
---
4+
5+
fix(fields): a lookup's option subtitle no longer repeats the record name when the referenced object's name field is not `name`
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The quick-picker shows each candidate as its title over a subtitle of the referenced object's other columns. A lookup that declares no `displayField` titles the option through the referenced object's name field (`nameField`, else the derived title field), but the subtitle skipped only a field literally called `name`. On an object whose name field is `title`, `customer_name` or any other name (every AI-built object), the subtitle's first and widest slot repeated the title and pushed the column that tells two records apart, such as a phone number, into truncation.
11+
12+
The subtitle now also skips the referenced object's resolved name field when the lookup declares no `displayField`. Unchanged: a declared `displayField` is the field skipped, so the object's name field stays in the subtitle; an object titled by a `titleFormat` template keeps every field in the subtitle; an explicitly authored `descriptionField` still leads the subtitle.
13+
14+
What to do: nothing.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
{"package":"ui/packages/fields","sources":96,"sha256":"89537e9f86037ddb8d0c6ce1a9fbcfbc8d756fd38b1a990c2c5ccc277a359d1a","manifestSha256":"1f6e1ccb925a678eaa393f9091dfc7d0285e4e3626e0a5c61a5474687debabb7"}
1+
{"package":"ui/packages/fields","sources":96,"sha256":"a80484e00e8bc69907a6efbec0af09bd7eb0d1a78b057a4f5092b90432d921cc","manifestSha256":"1f6e1ccb925a678eaa393f9091dfc7d0285e4e3626e0a5c61a5474687debabb7"}
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* The quick-picker's subtitle never repeats the field the option's title reads.
11+
*
12+
* A lookup that declares no `displayField` titles each option through the
13+
* referenced object's own name field (`getRecordDisplayName`), while the
14+
* subtitle skipped only the widget's `'name'` guess. On an object whose name
15+
* field is anything else — `title`, `customer_name`, every AI-built object —
16+
* the subtitle's first, widest slot repeated the title and pushed the field
17+
* that tells two records apart (the phone number) into truncation.
18+
*
19+
* The subtitle now skips the field the title resolved from: the declared
20+
* `displayField`, else the object's resolved name field — unless a
21+
* `titleFormat` template, not a field, titles the record.
22+
*/
23+
24+
import * as React from 'react';
25+
import { describe, it, expect, vi, afterEach } from 'vitest';
26+
import { render, screen, cleanup, waitFor, act, fireEvent } from '@testing-library/react';
27+
import '@testing-library/jest-dom';
28+
import { LookupField } from './LookupField';
29+
30+
const FIELDS: Record<string, { type: string; label: string }> = {
31+
title: { type: 'text', label: 'Customer Name' },
32+
phone: { type: 'phone', label: 'Phone' },
33+
source: { type: 'text', label: 'Source' },
34+
code: { type: 'text', label: 'Code' },
35+
};
36+
37+
const ROWS: Record<string, unknown>[] = [
38+
{ id: 'c1', title: 'Wang Jing', phone: '18624570001', source: 'Store', code: 'C-001' },
39+
{ id: 'c2', title: 'Zhou Mingxuan', phone: '18624570002', source: 'Web', code: 'C-002' },
40+
];
41+
42+
function makeBackend(decl: Record<string, unknown>) {
43+
return {
44+
find: vi.fn(async () => ({ data: ROWS.map((r) => ({ ...r })), total: ROWS.length })),
45+
findOne: vi.fn(async (_o: string, id: string) => ROWS.find((r) => r.id === id) ?? null),
46+
getObjectSchema: vi.fn(async (objectName: string) => ({ name: objectName, fields: FIELDS, ...decl })),
47+
} as any;
48+
}
49+
50+
async function settle(): Promise<void> {
51+
for (let i = 0; i < 6; i++) {
52+
await act(async () => {
53+
await new Promise((r) => setTimeout(r, 0));
54+
});
55+
}
56+
}
57+
58+
/**
59+
* Open the dropdown and read, per option, its title and the fields its
60+
* subtitle previews, in order.
61+
*/
62+
async function openPicker(
63+
decl: Record<string, unknown>,
64+
field: Record<string, unknown> = {},
65+
): Promise<Array<{ title: string; preview: string[] }>> {
66+
const ds = makeBackend(decl);
67+
render(
68+
<LookupField
69+
value={undefined}
70+
onChange={() => {}}
71+
dataSource={ds}
72+
field={{ reference: 'customer', ...field } as never}
73+
/>,
74+
);
75+
await waitFor(() => expect(ds.getObjectSchema).toHaveBeenCalledWith('customer'));
76+
await settle();
77+
await act(async () => {
78+
fireEvent.click(screen.getByTestId('lookup-trigger'));
79+
});
80+
await waitFor(() => expect(screen.getAllByRole('option')).toHaveLength(2));
81+
await settle();
82+
return screen.getAllByRole('option').map((el) => ({
83+
title: el.getAttribute('title') ?? '',
84+
preview: Array.from(el.querySelectorAll<HTMLElement>('[data-lookup-preview]')).map(
85+
(v) => v.dataset.lookupPreview ?? '',
86+
),
87+
}));
88+
}
89+
90+
afterEach(() => {
91+
cleanup();
92+
try {
93+
localStorage.clear();
94+
} catch {
95+
/* no storage in this environment */
96+
}
97+
});
98+
99+
describe('LookupField subtitle skips the field the title reads', () => {
100+
it("the object declares nameField 'title': the subtitle starts with the phone, not the title again", async () => {
101+
const options = await openPicker({ nameField: 'title' });
102+
expect(options.map((o) => o.title)).toEqual(['Wang Jing', 'Zhou Mingxuan']);
103+
for (const o of options) expect(o.preview).toEqual(['phone', 'source']);
104+
});
105+
106+
it("no nameField declared: the derived name field 'title' is skipped the same way", async () => {
107+
const options = await openPicker({});
108+
expect(options.map((o) => o.title)).toEqual(['Wang Jing', 'Zhou Mingxuan']);
109+
for (const o of options) expect(o.preview).toEqual(['phone', 'source']);
110+
});
111+
112+
it('an explicitly authored descriptionField still leads the subtitle', async () => {
113+
const options = await openPicker({ nameField: 'title' }, { descriptionField: 'source' });
114+
for (const o of options) expect(o.preview).toEqual(['source', 'phone']);
115+
});
116+
117+
it("control: a declared displayField titles the option, so the object's name field stays in the subtitle", async () => {
118+
const options = await openPicker({ nameField: 'title' }, { displayField: 'code' });
119+
expect(options.map((o) => o.title)).toEqual(['C-001', 'C-002']);
120+
for (const o of options) expect(o.preview).toEqual(['title', 'phone', 'source']);
121+
});
122+
123+
it('control: a titleFormat template titles the record, so the derived field stays in the subtitle', async () => {
124+
const options = await openPicker({ titleFormat: '{code} / {source}' });
125+
expect(options.map((o) => o.title)).toEqual(['C-001 / Store', 'C-002 / Web']);
126+
for (const o of options) expect(o.preview).toEqual(['title', 'phone', 'source']);
127+
});
128+
});

‎ui/packages/fields/src/widgets/LookupField.tsx‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { PeoplePicker } from './PeoplePicker.js';
1919
import { useRecordQuery } from './useRecordQuery.js';
2020
import { objectResolvesSearch } from './searchOffer.js';
2121
import { deriveLookupColumns } from './deriveLookupColumns.js';
22-
import { buildExpandFields, getRecordDisplayName, mergeFilterNodes, resolveAffordance, toPredicateRecord, withoutDeniedFields } from '@object-ui/core';
22+
import { buildExpandFields, declaredNameField, getRecordDisplayName, mergeFilterNodes, resolveAffordance, resolveNameField, toPredicateRecord, withoutDeniedFields } from '@object-ui/core';
2323
import { getRecentLookupIds, pushRecentLookupId } from './recentLookups.js';
2424
import { getPersonInitials } from './personDisplay.js';
2525
import { getCellRendererResolver } from './_cell-renderer-bridge.js';
@@ -581,10 +581,25 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro
581581
* still leads (the author picked that column to be the subtitle), followed by
582582
* every non-display picker column. Both halves render through
583583
* `renderLookupColumnValue`, the picker's own renderer.
584+
*
585+
* "Non-display" means not the field the option's TITLE reads. A declared
586+
* `displayField` is that field. Without one, `displayField` is only the
587+
* `'name'` guess, and the title comes from `getRecordDisplayName`'s object
588+
* ladder (`recordToOption`): the declared name field, else a `titleFormat`
589+
* template, else the derived title field. So the referenced object's
590+
* resolved name field is skipped too, unless a template rather than a field
591+
* titles the record. Without this an object whose name field is not
592+
* literally `name` (`title`, `customer_name`, every AI-built object) spent
593+
* the subtitle's first slot repeating the title.
584594
*/
585595
const previewColumns = useMemo<LookupColumnDef[]>(() => {
586596
const cols: LookupColumnDef[] = [];
587597
const seen = new Set<string>([displayField]);
598+
if (!declaredDisplayField && refObjectSchema) {
599+
const titledByTemplate = !!refObjectSchema.titleFormat && !declaredNameField(refObjectSchema);
600+
const nameField = titledByTemplate ? undefined : resolveNameField(refObjectSchema);
601+
if (nameField) seen.add(nameField);
602+
}
588603
if (descriptionField) {
589604
cols.push({ field: descriptionField });
590605
seen.add(descriptionField);
@@ -596,7 +611,7 @@ export function LookupField({ value, onChange, field, readonly, error: fieldErro
596611
cols.push(col);
597612
}
598613
return cols;
599-
}, [descriptionField, pickerColumns, displayField]);
614+
}, [descriptionField, pickerColumns, displayField, declaredDisplayField, refObjectSchema]);
600615

601616
/**
602617
* Field descriptors for the previewed columns — the SAME builder the picker

0 commit comments

Comments
 (0)