Skip to content

Commit 166d7cd

Browse files
nsdeschenescodex
andcommitted
fix(compact-select): Select current search result on Enter
Recompute the current first visible search result when Enter is pressed instead of reading the search-focused key from a later effect. This prevents Enter from selecting a stale option when the query changes and the previous active key remains visible. Add a regression test for Enter immediately after a search query update. Co-Authored-By: OpenAI Codex <noreply@openai.com>
1 parent a53f9f9 commit 166d7cd

3 files changed

Lines changed: 89 additions & 27 deletions

File tree

static/app/components/core/compactSelect/compactSelect.spec.tsx

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,46 @@ describe('CompactSelect', () => {
503503
expect(searchInput).not.toHaveAttribute('aria-activedescendant');
504504
});
505505

506+
it('selects the current first search result when Enter follows a query update', async () => {
507+
const mock = jest.fn();
508+
509+
render(
510+
<CompactSelect
511+
search={{
512+
placeholder: 'Search here…',
513+
filter: (option, search) => {
514+
if (search === 'a') {
515+
return {score: option.value === 'alpha' ? 2 : 1};
516+
}
517+
if (search === 'ab') {
518+
return {score: option.value === 'beta' ? 2 : 1};
519+
}
520+
return {score: 0};
521+
},
522+
}}
523+
options={[
524+
{value: 'alpha', label: 'Alpha'},
525+
{value: 'beta', label: 'Beta'},
526+
]}
527+
value={undefined}
528+
onChange={mock}
529+
/>
530+
);
531+
532+
await userEvent.click(screen.getByRole('button'));
533+
const searchInput = screen.getByPlaceholderText('Search here…');
534+
await userEvent.type(searchInput, 'a');
535+
536+
expect(searchInput).toHaveAttribute(
537+
'aria-activedescendant',
538+
screen.getByRole('option', {name: 'Alpha'}).id
539+
);
540+
541+
await userEvent.keyboard('b{Enter}');
542+
543+
expect(mock).toHaveBeenCalledWith({value: 'beta', label: 'Beta'});
544+
});
545+
506546
it('does not select the first search result when autoFocusFirstResult is false', async () => {
507547
const mock = jest.fn();
508548

static/app/components/core/compactSelect/control.tsx

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,15 @@ interface SearchResult {
6565
interface SearchResultListController {
6666
clearFocusedKey: () => void;
6767
getFirstVisibleEnabledSearchResult: () => SearchResult | null;
68-
selectFocusedKey: () => boolean;
68+
selectFocusedKey: (key: SelectKey) => boolean;
6969
setFocusedKey: (key: SelectKey) => void;
7070
}
7171

72+
interface SearchResultTarget {
73+
controller: SearchResultListController;
74+
searchResult: SearchResult;
75+
}
76+
7277
interface ControlContextValue {
7378
overlayIsOpen: boolean;
7479
/**
@@ -292,30 +297,44 @@ export function Control({
292297
}
293298
}, []);
294299

295-
const focusFirstSearchResult = useCallback(() => {
296-
if (!searchEnabled || !autoFocusFirstResult || !searchInputValue) {
297-
clearFocusedSearchResult();
298-
return;
300+
const getFirstVisibleEnabledSearchResultTarget = useCallback(() => {
301+
for (const controller of searchResultListControllersRef.current) {
302+
const searchResult = controller.getFirstVisibleEnabledSearchResult();
303+
if (searchResult !== null) {
304+
return {controller, searchResult};
305+
}
299306
}
307+
return null;
308+
}, []);
300309

301-
const target = searchResultListControllersRef.current
302-
.map(controller => ({
303-
controller,
304-
searchResult: controller.getFirstVisibleEnabledSearchResult(),
305-
}))
306-
.find(({searchResult}) => searchResult !== null);
307-
310+
const focusSearchResultTarget = useCallback((target: SearchResultTarget | null) => {
308311
activeSearchResultListControllerRef.current = target?.controller ?? null;
309-
setActiveSearchResultId(target?.searchResult?.id);
312+
setActiveSearchResultId(target?.searchResult.id);
310313

311314
for (const controller of searchResultListControllersRef.current) {
312-
if (controller === target?.controller && target.searchResult !== null) {
315+
if (controller === target?.controller) {
313316
controller.setFocusedKey(target.searchResult.key);
314317
} else {
315318
controller.clearFocusedKey();
316319
}
317320
}
318-
}, [autoFocusFirstResult, clearFocusedSearchResult, searchEnabled, searchInputValue]);
321+
}, []);
322+
323+
const focusFirstSearchResult = useCallback(() => {
324+
if (!searchEnabled || !autoFocusFirstResult || !searchInputValue) {
325+
clearFocusedSearchResult();
326+
return;
327+
}
328+
329+
focusSearchResultTarget(getFirstVisibleEnabledSearchResultTarget());
330+
}, [
331+
autoFocusFirstResult,
332+
clearFocusedSearchResult,
333+
focusSearchResultTarget,
334+
getFirstVisibleEnabledSearchResultTarget,
335+
searchEnabled,
336+
searchInputValue,
337+
]);
319338

320339
const registerSearchResultList = useCallback(
321340
(controller: SearchResultListController) => {
@@ -341,14 +360,22 @@ export function Control({
341360
return false;
342361
}
343362

344-
if (activeSearchResultListControllerRef.current?.selectFocusedKey()) {
345-
return true;
363+
const target = getFirstVisibleEnabledSearchResultTarget();
364+
if (target === null) {
365+
clearFocusedSearchResult();
366+
return false;
346367
}
347368

348-
return searchResultListControllersRef.current.some(controller =>
349-
controller.selectFocusedKey()
350-
);
351-
}, [autoFocusFirstResult, searchEnabled, searchInputValue]);
369+
focusSearchResultTarget(target);
370+
return target.controller.selectFocusedKey(target.searchResult.key);
371+
}, [
372+
autoFocusFirstResult,
373+
clearFocusedSearchResult,
374+
focusSearchResultTarget,
375+
getFirstVisibleEnabledSearchResultTarget,
376+
searchEnabled,
377+
searchInputValue,
378+
]);
352379

353380
const updateSearch = (newValue: string) => {
354381
normalizedSearch?.onChange?.(newValue);

static/app/components/core/compactSelect/list.tsx

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -326,21 +326,16 @@ export function List<Value extends SelectKey>({
326326
firstVisibleEnabledSearchResultRef.current = firstVisibleEnabledSearchResult;
327327
const hiddenOptionsRef = useRef(hiddenOptions);
328328
hiddenOptionsRef.current = hiddenOptions;
329-
const searchFocusedKeyRef = useRef(searchFocusedKey);
330-
searchFocusedKeyRef.current = searchFocusedKey;
331-
332329
const searchResultListController = useMemo(
333330
() => ({
334331
clearFocusedKey: () => {
335332
setSearchFocusedKey(null);
336333
},
337334
getFirstVisibleEnabledSearchResult: () =>
338335
firstVisibleEnabledSearchResultRef.current,
339-
selectFocusedKey: () => {
336+
selectFocusedKey: (focusedKey: SelectKey) => {
340337
const selectionManager = listStateRef.current.selectionManager;
341-
const focusedKey = searchFocusedKeyRef.current;
342338
if (
343-
focusedKey === null ||
344339
hiddenOptionsRef.current.has(focusedKey) ||
345340
selectionManager.isDisabled(focusedKey)
346341
) {

0 commit comments

Comments
 (0)