Skip to content

Commit 586a7ed

Browse files
fix(app-shell): Enter after a field type search picks the type the search names (#26259)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi9B2qu4S8m6QxptyXivYdzJu)_ Fixes #26194 Part of #26059 Clause-②: no Metadata-protocol: none ## What In Studio's Add field popover and the field inspector, `FieldTypePicker` now highlights, for a non-empty query, the first listed type the query **names** (by its shown name, its id, its id with underscores as spaces, or the catalog's English name), and the first listed type when the query names none. Enter picks the highlighted type, so on a Phone field typing "Number" and pressing Enter commits Number. With an empty query the current type is highlighted and scrolled into view as before, and re-choosing it still commits nothing. Clearing a query highlights the current type again. The arrow keys move on from the highlighted type. ## The mechanism, measured (the card's hypothesis did not hold) The card's hypothesis was that the `defaultValue` given to `Command` kept the current type highlighted while the list filtered. That is not what happens. cmdk 1.1.1 re-highlights the **first rendered row** on every search change, also under `shouldFilter={false}`. The current type stayed highlighted only because it was the first row: Phone's description, "A phone number.", holds "number" as a whole word, so objectui#12516's search keeps Phone, and its Text group comes before the Number group. Highlighting "the first match" (the first row) therefore cannot satisfy the card's own acceptance test. The fix follows the ruling's intent (Enter picks the type you typed) without reordering the list, which objectui#12516 pins in catalog order. How: `Command` is now controlled (`value` / `onValueChange`). A search change clears the highlight and records the target the new query asks for. cmdk's own first-row pick then arrives through `onValueChange` and is replaced by that target once. Arrow keys and pointer moves arrive the same way and are taken as they come. The `matches` predicate is split into `names` (name, id, English name) plus the category and description arms; what a query lists is unchanged. ## Tests New `FieldTypePicker.enterPicksFirstMatch-26194.test.tsx`, five cases: Phone + "Number" + Enter commits Number (and Phone is still the first listed row); empty query highlights Phone and Enter commits nothing; clearing the query highlights Phone again; ArrowDown from the highlighted match moves to the next row and Enter commits it; "masked" (description-only match) highlights the first row, Password. - Red leg, the base `FieldTypePicker.tsx` (6862ef6) with the new test: `Tests 3 failed | 2 passed (5)`, each failure `expected [ 'phone' ] to deeply equal [ 'number' ]` (and the arrow case one row behind). The restore was checked by `git diff HEAD` empty and the blob hash equal to HEAD's. - Green leg at ae665e4: the 28 test files that render the picker, the inspector or the Add field popover (`FieldTypePicker*`, `ObjectFieldInspector*`, `DataPillar.addFieldPopover-12516`, `ObjectFormDesigner.typeName-13148`, and every test file naming the picker): `Test Files 28 passed (28)`, `Tests 304 passed (304)`. - `pnpm --filter @object-ui/app-shell type-check` (both `tsc --noEmit` and `tsconfig.test.json`), after building the closure: exit 0. - The full app-shell suite is left to CI `Test UI` (declared narrowing; the package is too large to run locally). ## Gates `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 35 derived families, each run, every one `exit 0`; `--ran` reconciliation: "35 run, 0 NOT-MEASURED (a DERIVED zero …)". Re-run on the merged head 8d468f4 (merge of `origin/main`, derivation unchanged). Every family CI runs is read from this PR's check runs. ## Acceptance notes - Surface is exactly the claim's: `FieldTypePicker.tsx`, one new sibling test, one root changeset (`patch` for `@object-ui/app-shell`). No registry package, no sdui record. - The same first-row pick also meant that on a field of another type, "Number" + Enter would commit Phone rather than Number. That follows from the measured first-row pick and was not run separately; this PR covers it, as the same defect. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01NWnUGWJGbeTmGLWfrfWFXB --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 443f563 commit 586a7ed

3 files changed

Lines changed: 188 additions & 23 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): in the field type picker, Enter after a search picks the type the search names
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
In Studio's Add field popover and the field inspector, a search highlighted the first listed type, and a type whose description holds the query as a word can be listed before the type the query names. On a Phone field, typing "Number" listed Phone first ("A phone number."), Phone stayed highlighted, and Enter committed nothing. A query now highlights the first listed type it names by name, id or English name, and the first listed type when it names none, so Enter commits Number. With an empty query the current type is highlighted as before, and re-choosing it still commits nothing. The arrow keys move on from the highlighted type as before.
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* objectstack#26194 — with a query in the type picker's search, the
5+
* highlighted row is the first type the query names, so Enter picks it.
6+
*
7+
* Typing "Number" on a Phone field used to leave Phone highlighted: Phone's
8+
* description ("A phone number.") matches the query as a whole word, its
9+
* group comes first, and the list highlights its first row. Enter then
10+
* re-chose Phone and committed nothing.
11+
*
12+
* With an empty query the current type stays highlighted, and re-choosing it
13+
* still commits nothing. The search itself is objectui#12516's and is pinned
14+
* in `FieldTypePicker.search-12516.test.tsx`.
15+
*/
16+
17+
import '@testing-library/jest-dom/vitest';
18+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
19+
import { cleanup, render, screen } from '@testing-library/react';
20+
import userEvent from '@testing-library/user-event';
21+
import { t } from '../i18n';
22+
import { FieldTypePicker } from './FieldTypePicker';
23+
24+
beforeEach(() => {
25+
// Radix reads pointer capture, which the DOM double does not implement.
26+
for (const m of ['hasPointerCapture', 'setPointerCapture', 'releasePointerCapture'] as const) {
27+
if (!(m in Element.prototype)) {
28+
(Element.prototype as unknown as Record<string, unknown>)[m] = m === 'hasPointerCapture' ? () => false : () => {};
29+
}
30+
}
31+
if (!('scrollIntoView' in Element.prototype)) {
32+
(Element.prototype as unknown as Record<string, unknown>).scrollIntoView = () => {};
33+
}
34+
});
35+
36+
afterEach(cleanup);
37+
38+
async function open(value: string) {
39+
const onCommit = vi.fn();
40+
render(<FieldTypePicker label="Type" value={value} onCommit={onCommit} locale="en-US" />);
41+
await userEvent.click(screen.getByRole('combobox', { name: 'Type' }));
42+
const search = await screen.findByRole('combobox', { name: t('designer.canvas.searchFieldType', 'en-US') });
43+
return { onCommit, search };
44+
}
45+
46+
/** The listed type ids, in order. */
47+
const shownTypes = () => screen.getAllByRole('option').map((o) => o.getAttribute('data-field-type'));
48+
/** The type id of the highlighted row. */
49+
const highlighted = () =>
50+
screen
51+
.getAllByRole('option')
52+
.filter((o) => o.getAttribute('aria-selected') === 'true')
53+
.map((o) => o.getAttribute('data-field-type'));
54+
55+
describe('FieldTypePicker — Enter picks the first match (objectstack#26194)', () => {
56+
it('on a Phone field, typing "Number" and pressing Enter commits Number', async () => {
57+
const { search, onCommit } = await open('phone');
58+
await userEvent.type(search, 'Number');
59+
// Phone is still listed, first, by its description: the case the card names.
60+
expect(shownTypes()[0]).toBe('phone');
61+
expect(highlighted()).toEqual(['number']);
62+
await userEvent.keyboard('{Enter}');
63+
expect(onCommit).toHaveBeenCalledTimes(1);
64+
expect(onCommit).toHaveBeenCalledWith('number');
65+
});
66+
67+
it('with an empty query the current type is highlighted, and Enter commits nothing', async () => {
68+
const { onCommit } = await open('phone');
69+
expect(highlighted()).toEqual(['phone']);
70+
await userEvent.keyboard('{Enter}');
71+
expect(onCommit).not.toHaveBeenCalled();
72+
});
73+
74+
it('clearing the query highlights the current type again', async () => {
75+
const { search } = await open('phone');
76+
await userEvent.type(search, 'Number');
77+
expect(highlighted()).toEqual(['number']);
78+
await userEvent.clear(search);
79+
expect(highlighted()).toEqual(['phone']);
80+
});
81+
82+
it('the arrow keys still move on from the highlighted match', async () => {
83+
const { search, onCommit } = await open('phone');
84+
await userEvent.type(search, 'Number');
85+
const types = shownTypes();
86+
const next = types[types.indexOf('number') + 1];
87+
expect(next).toBeDefined();
88+
await userEvent.keyboard('{ArrowDown}');
89+
expect(highlighted()).toEqual([next]);
90+
await userEvent.keyboard('{Enter}');
91+
expect(onCommit).toHaveBeenCalledWith(next);
92+
});
93+
94+
it('a query that names no type highlights the first row', async () => {
95+
const { search, onCommit } = await open('phone');
96+
// "masked" is only in the descriptions of Password and Secret.
97+
await userEvent.type(search, 'masked');
98+
expect(shownTypes()).toEqual(['password', 'secret']);
99+
expect(highlighted()).toEqual(['password']);
100+
await userEvent.keyboard('{Enter}');
101+
expect(onCommit).toHaveBeenCalledWith('password');
102+
});
103+
});

‎ui/packages/app-shell/src/views/metadata-admin/inspectors/FieldTypePicker.tsx‎

Lines changed: 75 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@
4646
* matches and a group with none is dropped. The arrow keys move through what
4747
* is left and Enter picks the highlighted type.
4848
*
49+
* ## The highlighted type (objectstack#26194)
50+
*
51+
* With no query the current type is highlighted, so Enter re-chooses it and
52+
* commits nothing. A query highlights the first listed type it names — by
53+
* name, id or English name — and the first listed type when it names none.
54+
* The first row alone is not enough: a type whose description holds the
55+
* query can sit in an earlier group than the type the query names ("Number"
56+
* lists Phone, "A phone number.", above Number).
57+
*
4958
* Module-private: imported by `ObjectFieldInspector` and the Studio's Add field
5059
* popover (`studio-design/AddFieldPopover`), exported from no barrel.
5160
*/
@@ -131,22 +140,43 @@ function inDescription(description: string, query: string): boolean {
131140
}
132141

133142
/**
134-
* Whether a type matches the search: its name as shown, its category as shown,
135-
* its id (`master_detail` also as `master detail`) and the catalog's English
136-
* name, as substrings, so an author finds a type by any of the words they know
137-
* it by while still typing them; and its one-line description, as whole words
138-
* (see {@link inDescription}).
143+
* Whether `query` names the type: its name as shown, its id (`master_detail`
144+
* also as `master detail`) or the catalog's English name holds it, as a
145+
* substring.
146+
*/
147+
function names(row: TypeRow, query: string): boolean {
148+
const haystacks = [row.name, row.id, row.id.replace(/_/g, ' '), FIELD_TYPE_META[row.id].label];
149+
return haystacks.some((h) => h.toLowerCase().includes(query));
150+
}
151+
152+
/**
153+
* Whether a type matches the search: the query names it (see {@link names})
154+
* or its category as shown holds it, so an author finds a type by any of the
155+
* words they know it by while still typing them; or its one-line description
156+
* holds it as whole words (see {@link inDescription}).
139157
*/
140158
function matches(row: TypeRow, heading: string, query: string): boolean {
141159
if (!query) return true;
142-
const haystacks = [
143-
row.name,
144-
heading,
145-
row.id,
146-
row.id.replace(/_/g, ' '),
147-
FIELD_TYPE_META[row.id].label,
148-
];
149-
return haystacks.some((h) => h.toLowerCase().includes(query)) || inDescription(row.description, query);
160+
return names(row, query) || heading.toLowerCase().includes(query) || inDescription(row.description, query);
161+
}
162+
163+
/** The search as matched: trimmed and lower-cased. */
164+
const normalise = (search: string) => search.trim().toLowerCase();
165+
166+
/** The groups with only the types that match `query`; a group with none is dropped. */
167+
function filterGroups(groups: TypeGroup[], query: string): TypeGroup[] {
168+
return groups
169+
.map((g) => ({ ...g, rows: g.rows.filter((r) => matches(r, g.heading, query)) }))
170+
.filter((g) => g.rows.length > 0);
171+
}
172+
173+
/**
174+
* The type a non-empty query highlights (objectstack#26194): the first listed
175+
* type it names, else the first listed type; `''` when nothing is listed.
176+
*/
177+
function highlightFor(shown: TypeGroup[], query: string): string {
178+
const rows = shown.flatMap((g) => g.rows);
179+
return (rows.find((r) => names(r, query)) ?? rows[0])?.id ?? '';
150180
}
151181

152182
export function FieldTypePicker({ label, value, onCommit, disabled, locale: localeProp }: FieldTypePickerProps) {
@@ -172,18 +202,35 @@ export function FieldTypePicker({ label, value, onCommit, disabled, locale: loca
172202
[locale],
173203
);
174204

175-
const query = search.trim().toLowerCase();
176-
const shown = React.useMemo(
177-
() =>
178-
groups
179-
.map((g) => ({ ...g, rows: g.rows.filter((r) => matches(r, g.heading, query)) }))
180-
.filter((g) => g.rows.length > 0),
181-
[groups, query],
182-
);
205+
const query = normalise(search);
206+
const shown = React.useMemo(() => filterGroups(groups, query), [groups, query]);
183207

184208
// The current type, from the catalog; a stored type it does not list is
185209
// shown as itself, flagged.
186210
const current = value in FIELD_TYPE_META ? FIELD_TYPE_META[value as FieldTypeId] : undefined;
211+
212+
// The highlighted row, as cmdk's controlled `value`. A change of the search
213+
// makes cmdk highlight the list's first row once the rows have rendered; the
214+
// highlight the query asks for is held here until then and replaces that
215+
// choice (`''` lets it stand). `active` is cleared on the change so the
216+
// replacement reaches cmdk even when it equals the highlight before it.
217+
const [active, setActive] = React.useState(current?.id ?? '');
218+
const pendingHighlight = React.useRef<string | null>(null);
219+
220+
const onSearchChange = (next: string) => {
221+
setSearch(next);
222+
const nextQuery = normalise(next);
223+
pendingHighlight.current = nextQuery
224+
? highlightFor(filterGroups(groups, nextQuery), nextQuery)
225+
: (current?.id ?? '');
226+
setActive('');
227+
};
228+
229+
const onActiveChange = (next: string) => {
230+
const pending = pendingHighlight.current;
231+
pendingHighlight.current = null;
232+
setActive(pending || next);
233+
};
187234
const CurrentIcon = current?.Icon;
188235
const triggerText = current
189236
? t(`engine.fieldType.${current.id}`, locale)
@@ -212,6 +259,10 @@ export function FieldTypePicker({ label, value, onCommit, disabled, locale: loca
212259
onOpenChange={(next) => {
213260
setOpen(next);
214261
if (!next) setSearch('');
262+
if (next) {
263+
pendingHighlight.current = null;
264+
setActive(current?.id ?? '');
265+
}
215266
}}
216267
>
217268
<PopoverTrigger asChild>
@@ -244,12 +295,13 @@ export function FieldTypePicker({ label, value, onCommit, disabled, locale: loca
244295
(`aria-labelledby`), so the name goes there, not on the input. */}
245296
<Command
246297
shouldFilter={false}
247-
defaultValue={current?.id}
298+
value={active}
299+
onValueChange={onActiveChange}
248300
label={t('designer.canvas.searchFieldType', locale)}
249301
>
250302
<CommandInput
251303
value={search}
252-
onValueChange={setSearch}
304+
onValueChange={onSearchChange}
253305
placeholder={t('designer.canvas.searchFieldType', locale)}
254306
/>
255307
<CommandList ref={listRef} label={label} className="max-h-80">

0 commit comments

Comments
 (0)