Skip to content

Commit 88cfc19

Browse files
atomiksclaude
authored and
atomiks
committed
[combobox] Fix primitive value-shape edge cases in label/index resolution
- Resolve the rendered value before calling itemToStringValue/itemToStringLabel during browser autofill so a stringifier written for the primitive value is never handed the outer `{ value, label }` item. - Gate label inference out of virtualized mode (input and Combobox.Value) so the displayed label matches what selection sync can actually highlight. - Fall through to inferred-value matching in findItemIndexByValue when no whole item matches, so an inner `{ value, label }`-shaped value resolves its index. - Qualify the `items` JSDoc for single vs multiple selection. - Add regression tests (autofill stringifier shape, virtualized gating, inner-object highlight, labeled-items filtering, findItemIndexByValue units). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent c05db74 commit 88cfc19

6 files changed

Lines changed: 296 additions & 60 deletions

File tree

‎docs/src/app/(docs)/react/components/combobox/types.md‎

Lines changed: 40 additions & 40 deletions
Large diffs are not rendered by default.

‎packages/react/src/combobox/root/AriaCombobox.tsx‎

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,12 @@ export function AriaCombobox<Value = any, Mode extends SelectionMode = 'none'>(
176176
const single = selectionMode === 'single';
177177
const hasInputValue = inputValueProp !== undefined || defaultInputValueProp !== undefined;
178178
const hasItems = items !== undefined;
179+
180+
// Resolving a primitive selected value to its `{ value, label }` item's label is only
181+
// supported for non-virtualized lists (virtualized selection sync matches whole items),
182+
// so virtualized lists resolve labels from the value's own shape to keep the displayed
183+
// label consistent with what can actually be highlighted.
184+
const labelItems = virtualized ? undefined : items;
179185
const hasFilteredItemsProp = filteredItemsProp !== undefined;
180186

181187
let autoHighlightMode: false | 'input-change' | 'always';
@@ -213,7 +219,7 @@ export function AriaCombobox<Value = any, Mode extends SelectionMode = 'none'>(
213219
return defaultInputValueProp ?? '';
214220
}
215221
if (single) {
216-
return resolveLabelString(selectedValue, items, itemToStringLabel, isItemEqualToValue);
222+
return resolveLabelString(selectedValue, labelItems, itemToStringLabel, isItemEqualToValue);
217223
}
218224
return '';
219225
},
@@ -240,8 +246,10 @@ export function AriaCombobox<Value = any, Mode extends SelectionMode = 'none'>(
240246
// render (e.g. each keystroke) when the selection and items are unchanged.
241247
const selectedLabelString = React.useMemo(
242248
() =>
243-
single ? resolveLabelString(selectedValue, items, itemToStringLabel, isItemEqualToValue) : '',
244-
[single, selectedValue, items, itemToStringLabel, isItemEqualToValue],
249+
single
250+
? resolveLabelString(selectedValue, labelItems, itemToStringLabel, isItemEqualToValue)
251+
: '',
252+
[single, selectedValue, labelItems, itemToStringLabel, isItemEqualToValue],
245253
);
246254

247255
const shouldBypassFiltering =
@@ -651,7 +659,7 @@ export function AriaCombobox<Value = any, Mode extends SelectionMode = 'none'>(
651659

652660
if (shouldFillInput) {
653661
setInputValue(
654-
resolveLabelString(nextValue, items, itemToStringLabel, isItemEqualToValue),
662+
resolveLabelString(nextValue, labelItems, itemToStringLabel, isItemEqualToValue),
655663
createChangeEventDetails(eventDetails.reason, eventDetails.event),
656664
);
657665
}
@@ -1335,27 +1343,38 @@ export function AriaCombobox<Value = any, Mode extends SelectionMode = 'none'>(
13351343
const candidates = items ? flatFilteredItems : valuesRef.current;
13361344
const matchingIndex = candidates.findIndex((candidate, index) => {
13371345
const renderedValue = valuesRef.current[index];
1346+
// The exact value passed to `<Combobox.Item value>`, falling back to the
1347+
// item itself (virtualized) or its inferred value (non-virtualized) for
1348+
// slots not registered by a mounted item. User stringifiers expect this
1349+
// shape, so resolve it before calling them — never the outer
1350+
// `{ value, label }` item.
1351+
let resolvedValue = renderedValue;
1352+
if (resolvedValue === undefined) {
1353+
resolvedValue = virtualized ? candidate : inferItemValue(candidate);
1354+
}
13381355

13391356
// Try matching by value first (e.g., "US" for country code)
1340-
const candidateValue = stringifyAsValue(candidate, itemToStringValue);
1357+
const candidateValue = stringifyAsValue(resolvedValue, itemToStringValue);
13411358
if (candidateValue.toLowerCase() === nextValue.toLowerCase()) {
13421359
return true;
13431360
}
13441361
// Also try matching by label for browser autofill compatibility
13451362
// (browsers autofill with displayed text like "United States", not the underlying value)
1346-
const candidateLabel = stringifyAsLabel(candidate, itemToStringLabel);
1363+
const candidateLabel = stringifyAsLabel(resolvedValue, itemToStringLabel);
13471364
if (candidateLabel.toLowerCase() === nextValue.toLowerCase()) {
13481365
return true;
13491366
}
13501367

1351-
if (renderedValue !== undefined && renderedValue !== candidate) {
1352-
const renderedValueString = stringifyAsValue(renderedValue, itemToStringValue);
1353-
if (renderedValueString.toLowerCase() === nextValue.toLowerCase()) {
1368+
// When the rendered value differs from the `{ value, label }` item
1369+
// (primitive inference), also match against the item's own value/label so
1370+
// autofill by displayed label still works — using the callback-free
1371+
// stringifiers, which read `.value`/`.label` without handing the outer item
1372+
// to a user stringifier written for the rendered value.
1373+
if (resolvedValue !== candidate) {
1374+
if (stringifyAsValue(candidate).toLowerCase() === nextValue.toLowerCase()) {
13541375
return true;
13551376
}
1356-
1357-
const renderedLabel = stringifyAsLabel(renderedValue, itemToStringLabel);
1358-
if (renderedLabel.toLowerCase() === nextValue.toLowerCase()) {
1377+
if (stringifyAsLabel(candidate).toLowerCase() === nextValue.toLowerCase()) {
13591378
return true;
13601379
}
13611380
}
@@ -1568,8 +1587,10 @@ interface ComboboxRootProps<ItemValue> {
15681587
* The items to be displayed in the list.
15691588
* Can be either a flat array of items or an array of groups with items.
15701589
* In non-virtualized lists, when items use the `{ value, label }` shape, a primitive
1571-
* `<Combobox.Item value>` resolves its text label from the matching item, and
1572-
* `<Combobox.Value>` and the input render the label instead of the raw value.
1590+
* `<Combobox.Item value>` resolves its text label from the matching item. In single
1591+
* selection mode `<Combobox.Value>` and the input render that label instead of the raw
1592+
* value; in multiple selection mode the input stays the query field, so only
1593+
* `<Combobox.Value>` renders the selected labels.
15731594
* Virtualized lists do not support this primitive value inference.
15741595
*/
15751596
items?: readonly any[] | readonly Group<any>[] | undefined;

‎packages/react/src/combobox/root/ComboboxRoot.test.tsx‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,52 @@ describe('<Combobox.Root />', () => {
893893
);
894894
});
895895

896+
it('calls itemToStringValue with the rendered value during browser autofill', async () => {
897+
const items = [
898+
{ value: 'US', label: 'United States' },
899+
{ value: 'CA', label: 'Canada' },
900+
];
901+
const onValueChange = vi.fn();
902+
// Written for the rendered primitive value; throws if handed the outer
903+
// `{ value, label }` item.
904+
const itemToStringValue = (value: string) => value.toLowerCase();
905+
906+
await render(
907+
<Combobox.Root
908+
items={items}
909+
name="country"
910+
onValueChange={onValueChange}
911+
itemToStringValue={itemToStringValue}
912+
>
913+
<Combobox.Input data-testid="input" />
914+
<Combobox.Portal>
915+
<Combobox.Positioner>
916+
<Combobox.Popup>
917+
<Combobox.List>
918+
{(item: (typeof items)[number]) => (
919+
<Combobox.Item key={item.value} value={item.value}>
920+
{item.label}
921+
</Combobox.Item>
922+
)}
923+
</Combobox.List>
924+
</Combobox.Popup>
925+
</Combobox.Positioner>
926+
</Combobox.Portal>
927+
</Combobox.Root>,
928+
);
929+
930+
fireEvent.change(
931+
screen.getAllByDisplayValue('').find((el) => el.getAttribute('name') === 'country')!,
932+
{ target: { value: 'us' } },
933+
);
934+
await flushMicrotasks();
935+
936+
expect(onValueChange).toHaveBeenCalledWith(
937+
'US',
938+
expect.objectContaining({ reason: 'none' }),
939+
);
940+
});
941+
896942
it('ignores browser autofill matches for null item values', async () => {
897943
const items = [
898944
{ value: null, label: 'None' },
@@ -1041,6 +1087,23 @@ describe('<Combobox.Root />', () => {
10411087
expect(screen.getByRole('option', { name: 'Banana' }).id).not.toContain('--1');
10421088
});
10431089

1090+
it('does not infer a label from items for primitive values when virtualized', async () => {
1091+
await render(
1092+
<Combobox.Root items={labeledItems} virtualized defaultValue={2}>
1093+
<div data-testid="value">
1094+
<Combobox.Value />
1095+
</div>
1096+
<Combobox.Input data-testid="input" />
1097+
</Combobox.Root>,
1098+
);
1099+
1100+
// Virtualized lists don't support primitive value inference, so the input and
1101+
// `Combobox.Value` show the raw value rather than a label that cannot be
1102+
// highlighted in the list.
1103+
expect(screen.getByTestId('input')).toHaveValue('2');
1104+
expect(screen.getByTestId('value')).toHaveTextContent('2');
1105+
});
1106+
10441107
it('does not mutate the items array when virtualized rendered items unmount', async () => {
10451108
const items = [
10461109
{ value: 'apple', label: 'Apple' },
@@ -1237,6 +1300,43 @@ describe('<Combobox.Root />', () => {
12371300
expect(input).toHaveAttribute('aria-activedescendant', canada.id);
12381301
});
12391302
});
1303+
1304+
it('highlights items whose inner object value is itself a labeled item shape', async () => {
1305+
const items = [
1306+
{ value: { value: 'ca', label: 'Canada' }, label: 'CA' },
1307+
{ value: { value: 'us', label: 'United States' }, label: 'US' },
1308+
];
1309+
1310+
const { user } = await render(
1311+
<Combobox.Root items={items} defaultValue={items[0].value}>
1312+
<Combobox.Input data-testid="input" />
1313+
<Combobox.Portal>
1314+
<Combobox.Positioner>
1315+
<Combobox.Popup>
1316+
<Combobox.List>
1317+
{(item: (typeof items)[number]) => (
1318+
<Combobox.Item key={item.label} value={item.value}>
1319+
{item.label}
1320+
</Combobox.Item>
1321+
)}
1322+
</Combobox.List>
1323+
</Combobox.Popup>
1324+
</Combobox.Positioner>
1325+
</Combobox.Portal>
1326+
</Combobox.Root>,
1327+
);
1328+
1329+
const input = screen.getByRole('combobox');
1330+
await user.click(screen.getByTestId('input'));
1331+
1332+
// The selected value (`{ value, label }`-shaped) must not be misclassified as a
1333+
// whole item; the inner-value match still resolves the selected index.
1334+
const ca = await screen.findByRole('option', { name: 'CA' });
1335+
await waitFor(() => {
1336+
expect(ca).toHaveAttribute('data-highlighted');
1337+
expect(input).toHaveAttribute('aria-activedescendant', ca.id);
1338+
});
1339+
});
12401340
});
12411341

12421342
describe('object values with labeled items', () => {
@@ -1676,6 +1776,61 @@ describe('<Combobox.Root />', () => {
16761776
});
16771777
});
16781778

1779+
it('starts keyboard navigation from the filtered labeled items and emits primitive values', async () => {
1780+
const items = [
1781+
{ value: 'apple', label: 'Apple' },
1782+
{ value: 'banana', label: 'Banana' },
1783+
{ value: 'cherry', label: 'Cherry' },
1784+
];
1785+
const onValueChange = vi.fn();
1786+
1787+
const { user } = await render(
1788+
<Combobox.Root
1789+
items={items}
1790+
multiple
1791+
defaultValue={['cherry']}
1792+
onValueChange={onValueChange}
1793+
>
1794+
<Combobox.Input data-testid="input" />
1795+
<Combobox.Portal>
1796+
<Combobox.Positioner>
1797+
<Combobox.Popup>
1798+
<Combobox.List>
1799+
{(item: (typeof items)[number]) => (
1800+
<Combobox.Item key={item.value} value={item.value}>
1801+
{item.label}
1802+
</Combobox.Item>
1803+
)}
1804+
</Combobox.List>
1805+
</Combobox.Popup>
1806+
</Combobox.Positioner>
1807+
</Combobox.Portal>
1808+
</Combobox.Root>,
1809+
);
1810+
1811+
const input = screen.getByTestId('input');
1812+
1813+
await user.click(input);
1814+
await user.type(input, 'ba');
1815+
const banana = await screen.findByRole('option', { name: 'Banana' });
1816+
1817+
await user.keyboard('{ArrowDown}');
1818+
1819+
await waitFor(() => {
1820+
expect(banana).toHaveAttribute('data-highlighted');
1821+
expect(input).toHaveAttribute('aria-activedescendant', banana.id);
1822+
});
1823+
1824+
await user.keyboard('{Enter}');
1825+
1826+
// The emitted value keeps the primitive shape rendered by `value={item.value}`
1827+
// rather than the outer `{ value, label }` item.
1828+
expect(onValueChange).toHaveBeenLastCalledWith(
1829+
['cherry', 'banana'],
1830+
expect.objectContaining({ reason: REASONS.itemPress }),
1831+
);
1832+
});
1833+
16791834
it('should create multiple hidden inputs for form submission', async () => {
16801835
const items = ['a', 'b', 'c'];
16811836
await render(

‎packages/react/src/combobox/value/ComboboxValue.tsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,16 @@ export function ComboboxValue(props: ComboboxValue.Props): React.ReactElement {
1919
const itemToStringLabel = useStore(store, selectors.itemToStringLabel);
2020
const isItemEqualToValue = useStore(store, selectors.isItemEqualToValue);
2121
const selectedValue = useStore(store, selectors.selectedValue);
22-
const items = useStore(store, selectors.items);
22+
const itemsProp = useStore(store, selectors.items);
23+
const virtualized = useStore(store, selectors.virtualized);
2324
const multiple = useStore(store, selectors.selectionMode) === 'multiple';
2425
const hasSelectedValue = useStore(store, selectors.hasSelectedValue);
2526

27+
// Resolving a primitive selected value to its `{ value, label }` item's label is only
28+
// supported for non-virtualized lists, so virtualized lists resolve the label from the
29+
// value's own shape (matching the input and selection-sync behavior).
30+
const items = virtualized ? undefined : itemsProp;
31+
2632
const shouldCheckNullItemLabel = !hasSelectedValue && placeholder != null && childrenProp == null;
2733
const hasNullLabel = useStore(store, selectors.hasNullItemLabel, shouldCheckNullItemLabel);
2834

‎packages/react/src/internals/resolveValueLabel.test.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { expect } from 'vitest';
2-
import { hasNullItemLabel } from './resolveValueLabel';
2+
import { findItemIndexByValue, hasNullItemLabel } from './resolveValueLabel';
3+
import { defaultItemEquality } from './itemEquality';
34

45
describe('resolveValueLabel', () => {
56
describe('hasNullItemLabel', () => {
@@ -98,4 +99,52 @@ describe('resolveValueLabel', () => {
9899
expect(hasNullItemLabel(undefined)).toBe(false);
99100
});
100101
});
102+
103+
describe('findItemIndexByValue', () => {
104+
it('matches a primitive value against a labeled item by its inferred value', () => {
105+
const items = [
106+
{ value: 'a', label: 'A' },
107+
{ value: 'b', label: 'B' },
108+
];
109+
110+
expect(findItemIndexByValue(items, 'b', defaultItemEquality)).toBe(1);
111+
});
112+
113+
it('matches a whole `{ value, label }` value against the full item', () => {
114+
const items = [
115+
{ value: 'a', label: 'A' },
116+
{ value: 'b', label: 'B' },
117+
];
118+
119+
expect(findItemIndexByValue(items, items[1], defaultItemEquality)).toBe(1);
120+
});
121+
122+
it('matches an inner object value shaped like a labeled item by the item value', () => {
123+
const inner = { value: 'ca', label: 'Canada' };
124+
const items = [
125+
{ value: { value: 'us', label: 'United States' }, label: 'US' },
126+
{ value: inner, label: 'CA' },
127+
];
128+
129+
// `inner` looks like a `{ value, label }` item but is an item *value*, so it must
130+
// match by the inferred item value rather than being treated as a whole item.
131+
expect(findItemIndexByValue(items, inner, defaultItemEquality)).toBe(1);
132+
});
133+
134+
it('only hands whole items to a whole-item comparator', () => {
135+
const items = [
136+
{ value: { id: 1 }, label: 'One' },
137+
{ value: { id: 2 }, label: 'Two' },
138+
];
139+
const seen: any[] = [];
140+
const comparator = (a: any, b: any) => {
141+
seen.push(a);
142+
// Reads `a.value.id`; would throw if handed an inferred inner value.
143+
return a.value.id === b.value.id;
144+
};
145+
146+
expect(findItemIndexByValue(items, items[1], comparator)).toBe(1);
147+
expect(seen.every((item) => item && 'label' in item)).toBe(true);
148+
});
149+
});
101150
});

‎packages/react/src/internals/resolveValueLabel.tsx‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,18 @@ export function findItemIndexByValue(
106106
value: any,
107107
isItemEqualToValue: ItemEqualityComparer,
108108
): number {
109-
// When the selected value is itself a `{ value, label }` item (whole-item mode), only
110-
// compare against the full item so a comparator written for that shape is never handed
111-
// the inferred inner value.
109+
// When the selected value is itself a `{ value, label }` item (whole-item mode passed via
110+
// `value={item}`), first compare against the full item so a comparator written for that
111+
// shape is handed matching shapes. The selected value's shape alone is ambiguous though:
112+
// an item's inner value (`value={item.value}`) can itself be `{ value, label }`-shaped, so
113+
// fall through to inferred-value matching when no whole item matches.
112114
if (isLabeledItem(value)) {
113-
return items.findIndex(
115+
const wholeItemIndex = items.findIndex(
114116
(item) => item !== undefined && compareItemEquality(item, value, isItemEqualToValue),
115117
);
118+
if (wholeItemIndex !== -1) {
119+
return wholeItemIndex;
120+
}
116121
}
117122

118123
return items.findIndex((item) => {

0 commit comments

Comments
 (0)