fix(combobox): correct behavior of selection onChange with filtered i…

…tems (#16835) * fix(combobox): onChange returns the proper item with shouldFilterItem * WIP * fix(combobox): use filterItems for items * fix(combobox): correct selection when filtered items
carbon-design-system · Jun 21, 2024 · 22dea80 · 22dea80
1 parent 3dc88a8
commit 22dea80
Show file tree

Hide file tree

Showing 4 changed files with 127 additions and 6 deletions.
diff --git a/packages/react/src/components/ComboBox/ComboBox-test.js b/packages/react/src/components/ComboBox/ComboBox-test.js
@@ -14,6 +14,7 @@ import {
   assertMenuClosed,
   generateItems,
   generateGenericItem,
+  cognateItems,
   waitForPosition,
 } from '../ListBox/test-helpers';
 import ComboBox from '../ComboBox';
@@ -63,7 +64,71 @@ describe('ComboBox', () => {
     }
   });
 
-  it('capture filter text events', async () => {
+  it('should call `onChange` with the proper item when `shouldFilterItem` is provided', async () => {
+    const filterItems = (menu) => {
+      return menu?.item?.label
+        ?.toLowerCase()
+        .includes(menu?.inputValue?.toLowerCase());
+    };
+    const onInputChange = jest.fn();
+
+    render(
+      <ComboBox
+        {...mockProps}
+        shouldFilterItem={filterItems}
+        onInputChange={onInputChange}
+      />
+    );
+
+    await userEvent.type(findInputNode(), 'Item 2');
+    expect(onInputChange).toHaveBeenCalledWith('Item 2');
+
+    await userEvent.click(screen.getAllByRole('option')[0]);
+    expect(mockProps.onChange).toHaveBeenCalledTimes(1);
+    expect(mockProps.onChange).toHaveBeenCalledWith({
+      selectedItem: mockProps.items[2],
+    });
+  });
+
+  it('should select the correct item from the filtered list after text input on click', async () => {
+    const user = userEvent.setup();
+
+    render(<ComboBox {...mockProps} items={cognateItems} />);
+
+    await user.type(findInputNode(), 'struct');
+
+    await user.click(screen.getAllByRole('option')[1]);
+
+    expect(mockProps.onChange).toHaveBeenCalledTimes(1);
+    expect(mockProps.onChange).toHaveBeenCalledWith({
+      selectedItem: {
+        id: 'construct',
+        text: 'Construct',
+      },
+    });
+  });
+
+  it('should select the correct item from the filtered list after text input on [Enter]', async () => {
+    const user = userEvent.setup();
+
+    render(<ComboBox {...mockProps} items={cognateItems} />);
+
+    await user.type(findInputNode(), 'struct');
+
+    await userEvent.keyboard('{ArrowDown}');
+    await userEvent.keyboard('{ArrowDown}');
+    await userEvent.keyboard('[Enter]');
+
+    expect(mockProps.onChange).toHaveBeenCalledTimes(1);
+    expect(mockProps.onChange).toHaveBeenCalledWith({
+      selectedItem: {
+        id: 'construct',
+        text: 'Construct',
+      },
+    });
+  });
+
+  it('capture filter text event onInputChange', async () => {
     const onInputChange = jest.fn();
     render(<ComboBox {...mockProps} onInputChange={onInputChange} />);
 
@@ -103,6 +168,7 @@ describe('ComboBox', () => {
     expect(mockProps.onChange).toHaveBeenCalledWith({
       selectedItem: mockProps.items[1],
     });
+    expect(screen.getByRole('combobox')).toHaveDisplayValue('Item 1');
   });
 
   it('should not let the user select an option by clicking on the disabled option node', async () => {

diff --git a/packages/react/src/components/ComboBox/ComboBox.stories.js b/packages/react/src/components/ComboBox/ComboBox.stories.js
@@ -74,7 +74,7 @@ export const Default = () => (
   </div>
 );
 
-export const AllowCustomValue = () => {
+export const AllowCustomValue = (args) => {
   const filterItems = (menu) => {
     return menu?.item?.toLowerCase().includes(menu?.inputValue?.toLowerCase());
   };
@@ -83,7 +83,7 @@ export const AllowCustomValue = () => {
       <ComboBox
         allowCustomValue
         shouldFilterItem={filterItems}
-        onChange={() => {}}
+        onChange={args.onChange}
         id="carbon-combobox"
         items={['Apple', 'Orange', 'Banana', 'Pineapple', 'Raspberry', 'Lime']}
         titleText="ComboBox title"
@@ -108,6 +108,10 @@ export const ExperimentalAutoAlign = () => (
   </div>
 );
 
+AllowCustomValue.argTypes = {
+  onChange: { action: 'onChange' },
+};
+
 export const _WithLayer = () => (
   <WithLayer>
     {(layer) => (

diff --git a/packages/react/src/components/ComboBox/ComboBox.tsx b/packages/react/src/components/ComboBox/ComboBox.tsx
@@ -590,7 +590,7 @@ const ComboBox = forwardRef(
       setHighlightedIndex,
     } = useCombobox({
       ...downshiftProps,
-      items,
+      items: filterItems(items, itemToString, inputValue),
       inputValue: inputValue,
       itemToString: (item) => {
         return itemToString(item);
@@ -708,7 +708,11 @@ const ComboBox = forwardRef(
                     toggleMenu();
 
                     if (highlightedIndex !== -1) {
-                      selectItem(items[highlightedIndex]);
+                      selectItem(
+                        filterItems(items, itemToString, inputValue)[
+                          highlightedIndex
+                        ]
+                      );
                     }
 
                     event.preventDownshiftDefault = true;

diff --git a/packages/react/src/components/ListBox/test-helpers.js b/packages/react/src/components/ListBox/test-helpers.js
@@ -107,4 +107,51 @@ export const generateItems = (amount, generator) =>
 
 export const customItemToString = ({ field }) => field;
 
-export const waitForPosition = () => act(async () => {}); // Flush microtasks. Position state is ready by this line.
+/**
+ * This object contains two sets of three items that share the same root
+ * word in different portions of the string (beginning, middle, end):
+ *
+ *  - 'struct'
+ *  - 'port'
+ *
+ * Separated by a disabled item, these derivative words are helpful when
+ * testing fuzzy search functions and components that do substring filtering.
+ */
+export const cognateItems = [
+  {
+    id: 'structure',
+    text: 'Structure',
+  },
+  {
+    id: 'construct',
+    text: 'Construct',
+  },
+  {
+    id: 'instruction',
+    text: 'Instruction',
+  },
+  {
+    id: 'disabled-item',
+    text: 'A disabled item',
+    disabled: true,
+  },
+  {
+    id: 'transport',
+    text: 'Transport',
+  },
+  {
+    id: 'portable',
+    text: 'Portable',
+  },
+  {
+    id: 'import',
+    text: 'Import',
+  },
+];
+
+/**
+ * Flushes microtasks to ensure element position state is settled
+ * From https://floating-ui.com/docs/react#testing
+ * More context here: https://github.com/floating-ui/react-popper/issues/368#issuecomment-1340413010
+ */
+export const waitForPosition = () => act(async () => {});