Skip to content

Commit

Permalink
fix: use initialSelectedItem only on initial render in ComboBox (#18602)
Browse files Browse the repository at this point in the history
* fix: use initialSelectedItem only on initial render

* feat: add test story
  • Loading branch information
adamalston authored Feb 24, 2025
1 parent b67877b commit 84fd364
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 13 deletions.
36 changes: 35 additions & 1 deletion packages/react/src/components/ComboBox/ComboBox-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright IBM Corp. 2016, 2023
* Copyright IBM Corp. 2016, 2025
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
Expand Down Expand Up @@ -329,6 +329,40 @@ describe('ComboBox', () => {
await waitForPosition();
expect(findInputNode()).toHaveDisplayValue(mockProps.items[1]);
});

it('should not revert to initialSelectedItem after clearing selection in uncontrolled mode', async () => {
// Render a non-fully controlled `ComboBox` using `initialSelectedItem`.
render(
<ComboBox {...mockProps} initialSelectedItem={mockProps.items[0]} />
);
await waitForPosition();
// Verify that the input initially displays `initialSelectedItem`.
expect(findInputNode()).toHaveDisplayValue(mockProps.items[0].label);

// Simulate clearing the selection by clicking the clear button.
await userEvent.click(
screen.getByRole('button', { name: 'Clear selected item' })
);
// After clearing, the input should be empty rather than reverting to
// `initialSelectedItem`.
expect(findInputNode()).toHaveDisplayValue('');
});

it('should ignore updates to initialSelectedItem after initial render in uncontrolled mode', async () => {
// Render a non-fully controlled `ComboBox` using `initialSelectedItem`.
const { rerender } = render(
<ComboBox {...mockProps} initialSelectedItem={mockProps.items[0]} />
);
await waitForPosition();
expect(findInputNode()).toHaveDisplayValue(mockProps.items[0].label);

// Rerender the component with a different `initialSelectedItem`.
rerender(
<ComboBox {...mockProps} initialSelectedItem={mockProps.items[2]} />
);
// The displayed value should still be the one from the first render.
expect(findInputNode()).toHaveDisplayValue(mockProps.items[0].label);
});
});

describe('provided `selectedItem`', () => {
Expand Down
26 changes: 26 additions & 0 deletions packages/react/src/components/ComboBox/ComboBox.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,32 @@ export const _fullyControlled = (args) => {

_fullyControlled.argTypes = { ...sharedArgTypes };

export const _fullyControlled2 = () => {
const [selectedItem, setSelectedItem] = useState(null);

return (
<div
style={{
display: 'flex',
flexDirection: 'column',
gap: '1rem',
width: '256px',
}}>
<ComboBox
id="carbon-combobox"
items={['1', '2', '3']}
onChange={({ selectedItem }) => setSelectedItem(selectedItem)}
selectedItem={selectedItem}
titleText="Fully Controlled ComboBox title"
/>
<Button kind="danger" onClick={() => setSelectedItem(null)} size="md">
Reset
</Button>
<p>Selected value: {`${selectedItem}`}</p>
</div>
);
};

AutocompleteWithTypeahead.argTypes = {
onChange: { action: 'onChange' },
};
25 changes: 13 additions & 12 deletions packages/react/src/components/ComboBox/ComboBox.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright IBM Corp. 2016, 2023
* Copyright IBM Corp. 2016, 2025
*
* This source code is licensed under the Apache-2.0 license found in the
* LICENSE file in the root directory of this source tree.
Expand Down Expand Up @@ -100,30 +100,33 @@ const autocompleteCustomFilter = ({

const getInputValue = <ItemType,>({
initialSelectedItem,
inputValue,
itemToString,
selectedItem,
prevSelectedItem,
}: {
initialSelectedItem?: ItemType | null;
inputValue: string;
itemToString: ItemToStringHandler<ItemType>;
selectedItem?: ItemType | null;
prevSelectedItem?: ItemType | null;
}) => {
if (selectedItem) {
// If there's a current selection (even if it's an object or string), use it.
if (selectedItem !== null && typeof selectedItem !== 'undefined') {
return itemToString(selectedItem);
}

if (initialSelectedItem) {
// On the very first render (when no previous value exists), use
// `initialSelectedItem`.
if (
typeof prevSelectedItem === 'undefined' &&
initialSelectedItem !== null &&
typeof initialSelectedItem !== 'undefined'
) {
return itemToString(initialSelectedItem);
}

if (!selectedItem && prevSelectedItem) {
return '';
}

return inputValue || '';
// Otherwise (i.e., after the user has cleared the selection), return an empty
// string.
return '';
};

const findHighlightedIndex = <ItemType,>(
Expand Down Expand Up @@ -463,7 +466,6 @@ const ComboBox = forwardRef(
const [inputValue, setInputValue] = useState(
getInputValue({
initialSelectedItem,
inputValue: '',
itemToString,
selectedItem: selectedItemProp,
})
Expand Down Expand Up @@ -512,7 +514,6 @@ const ComboBox = forwardRef(
if (prevSelectedItemProp.current !== selectedItemProp) {
const currentInputValue = getInputValue({
initialSelectedItem,
inputValue,
itemToString,
selectedItem: selectedItemProp,
prevSelectedItem: prevSelectedItemProp.current,
Expand Down

0 comments on commit 84fd364

Please sign in to comment.