Skip to content

Commit 4cbf8f4

Browse files
authored
fix(ComboBox): update Enter key handling (#18737)
* fix(ComboBox): update Enter key handling * refactor: delete test story
1 parent 7d2f677 commit 4cbf8f4

File tree

2 files changed

+80
-30
lines changed

2 files changed

+80
-30
lines changed

packages/react/src/components/ComboBox/ComboBox-test.js

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ const prefix = 'cds';
3131

3232
const ControlledComboBox = ({ controlledItem }) => {
3333
const items = generateItems(5, generateGenericItem);
34-
const [value, setValue] = useState(controlledItem || items[0]);
34+
const [value, setValue] = useState(
35+
typeof controlledItem !== 'undefined' ? controlledItem : items[0]
36+
);
3537
const [onChangeCallCount, setOnChangeCallCount] = useState(0);
3638
const [onInputChangeCallCount, setOnInputChangeCallCount] = useState(0);
3739
const controlledOnChange = ({ selectedItem }) => {
@@ -53,7 +55,7 @@ const ControlledComboBox = ({ controlledItem }) => {
5355
placeholder="Filter..."
5456
type="default"
5557
/>
56-
<div>value: {value?.label || 'none'}</div>
58+
<div data-testid="selected-item">{value?.label || 'none'}</div>
5759
<div>onChangeCallCount: {onChangeCallCount}</div>
5860
<div>onInputChangeCallCount: {onInputChangeCallCount}</div>
5961
<button onClick={() => setValue(items[3])}>Choose item 3</button>
@@ -375,6 +377,44 @@ describe('ComboBox', () => {
375377
);
376378
});
377379

380+
it('should yield highlighted item as `selectedItem` when pressing Enter with an unmodified input value', async () => {
381+
render(<ControlledComboBox controlledItem={null} />);
382+
383+
const input = findInputNode();
384+
385+
expect(screen.getByTestId('selected-item').textContent).toBe('none');
386+
387+
await userEvent.click(input);
388+
await userEvent.type(input, 'Item 1');
389+
await userEvent.keyboard('{Enter}');
390+
391+
expect(screen.getByTestId('selected-item').textContent).toBe('Item 1');
392+
393+
await userEvent.type(input, '{backspace}');
394+
await userEvent.type(input, '1');
395+
await userEvent.keyboard('{Enter}');
396+
397+
expect(screen.getByTestId('selected-item').textContent).toBe('Item 1');
398+
});
399+
400+
it('should yield highlighted item as `selectedItem` when pressing Enter with a modified input value', async () => {
401+
render(<ControlledComboBox controlledItem={null} />);
402+
const input = findInputNode();
403+
404+
expect(screen.getByTestId('selected-item').textContent).toBe('none');
405+
406+
await userEvent.click(input);
407+
await userEvent.type(input, 'Item 2');
408+
await userEvent.keyboard('{Enter}');
409+
410+
expect(screen.getByTestId('selected-item').textContent).toBe('Item 2');
411+
412+
await userEvent.type(input, '{backspace}');
413+
await userEvent.keyboard('{Enter}');
414+
415+
expect(screen.getByTestId('selected-item').textContent).toBe('Item 0');
416+
});
417+
378418
describe('should display initially selected item found in `initialSelectedItem`', () => {
379419
it('using an object type for the `initialSelectedItem` prop', async () => {
380420
render(
@@ -506,7 +546,7 @@ describe('ComboBox', () => {
506546
await openMenu();
507547
await userEvent.click(screen.getByRole('option', { name: 'Item 2' }));
508548
expect(screen.getByText('onChangeCallCount: 1')).toBeInTheDocument();
509-
expect(screen.getByText('value: Item 2')).toBeInTheDocument();
549+
expect(screen.getByTestId('selected-item').textContent).toBe('Item 2');
510550
expect(
511551
screen.getByRole('combobox', { value: 'Item 2' })
512552
).toBeInTheDocument();
@@ -521,7 +561,7 @@ describe('ComboBox', () => {
521561
screen.getByRole('button', { name: 'Clear selected item' })
522562
);
523563
expect(screen.getByText('onChangeCallCount: 2')).toBeInTheDocument();
524-
expect(screen.getByText('value: none')).toBeInTheDocument();
564+
expect(screen.getByTestId('selected-item').textContent).toBe('none');
525565
expect(findInputNode()).toHaveDisplayValue('');
526566
});
527567
it('should update and call `onChange` once when selection is cleared from the combobox after an external update is made, and the external state managing selectedItem is updated', async () => {
@@ -536,7 +576,7 @@ describe('ComboBox', () => {
536576
screen.getByRole('button', { name: 'Clear selected item' })
537577
);
538578
expect(screen.getByText('onChangeCallCount: 2')).toBeInTheDocument();
539-
expect(screen.getByText('value: none')).toBeInTheDocument();
579+
expect(screen.getByTestId('selected-item').textContent).toBe('none');
540580
expect(findInputNode()).toHaveDisplayValue('');
541581
});
542582
it('should update and call `onChange` when a combination of external and combobox selections are made', async () => {
@@ -547,17 +587,17 @@ describe('ComboBox', () => {
547587
);
548588
expect(screen.getByText('onChangeCallCount: 1')).toBeInTheDocument();
549589
expect(findInputNode()).toHaveDisplayValue('Item 3');
550-
expect(screen.getByText('value: Item 3')).toBeInTheDocument();
590+
expect(screen.getByTestId('selected-item').textContent).toBe('Item 3');
551591
await openMenu();
552592
await userEvent.click(screen.getByRole('option', { name: 'Item 2' }));
553593
expect(screen.getByText('onChangeCallCount: 2')).toBeInTheDocument();
554594
expect(findInputNode()).toHaveDisplayValue('Item 2');
555-
expect(screen.getByText('value: Item 2')).toBeInTheDocument();
595+
expect(screen.getByTestId('selected-item').textContent).toBe('Item 2');
556596
await userEvent.click(
557597
screen.getByRole('button', { name: 'Clear selected item' })
558598
);
559599
expect(screen.getByText('onChangeCallCount: 3')).toBeInTheDocument();
560-
expect(screen.getByText('value: none')).toBeInTheDocument();
600+
expect(screen.getByTestId('selected-item').textContent).toBe('none');
561601
expect(findInputNode()).toHaveDisplayValue('');
562602
});
563603
it('should update and call `onChange` once when selection is updated externally', async () => {
@@ -576,7 +616,7 @@ describe('ComboBox', () => {
576616
await userEvent.click(screen.getByRole('option', { name: 'Item 2' }));
577617
await userEvent.click(screen.getByRole('button', { name: 'reset' }));
578618
expect(screen.getByText('onChangeCallCount: 2')).toBeInTheDocument();
579-
expect(screen.getByText('value: none')).toBeInTheDocument();
619+
expect(screen.getByTestId('selected-item').textContent).toBe('none');
580620
expect(findInputNode()).toHaveDisplayValue('');
581621
});
582622
it('should clear selected item when `selectedItem` is updated to `null` externally', async () => {

packages/react/src/components/ComboBox/ComboBox.tsx

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -610,29 +610,39 @@ const ComboBox = forwardRef(
610610
}
611611

612612
case InputKeyDownEnter:
613-
if (
614-
highlightedIndex === -1 &&
615-
!allowCustomValue &&
616-
state.selectedItem
617-
) {
618-
return {
619-
...changes,
620-
selectedItem: null,
621-
inputValue: state.inputValue,
622-
};
623-
}
624-
if (allowCustomValue) {
625-
setInputValue(inputValue);
626-
setHighlightedIndex(changes.selectedItem);
627-
if (onChange) {
628-
onChange({ selectedItem: changes.selectedItem, inputValue });
613+
if (!allowCustomValue) {
614+
const highlightedIndex = indexToHighlight(inputValue);
615+
const matchingItem = items[highlightedIndex];
616+
617+
if (matchingItem) {
618+
// Prevent matching items that are marked as `disabled` from
619+
// being selected.
620+
if ((matchingItem as any).disabled) {
621+
return state;
622+
}
623+
624+
// Select the matching item.
625+
return {
626+
...changes,
627+
selectedItem: matchingItem,
628+
inputValue: itemToString(matchingItem),
629+
};
630+
}
631+
632+
// If no matching item is found and there is an existing
633+
// selection, clear the selection.
634+
if (state.selectedItem !== null) {
635+
return {
636+
...changes,
637+
selectedItem: null,
638+
inputValue,
639+
};
629640
}
630-
return changes;
631-
} else if (changes.selectedItem && !allowCustomValue) {
632-
return changes;
633-
} else {
634-
return { ...changes, isOpen: true };
635641
}
642+
643+
// For `allowCustomValue` or if no matching item is found, keep the
644+
// menu open.
645+
return { ...changes, isOpen: true };
636646
case FunctionToggleMenu:
637647
case ToggleButtonClick:
638648
if (

0 commit comments

Comments
 (0)