fix(combobox): onInputChange calls (#18713)

* fix(combobox): onInputChange calls

* Remove useEffect calling onInputChange on initial render
* Leverage useCombobox's onStateChange to call onInputChange
* Unit tests

* fix(combobox): onInputChange calls

* Rewrite onInputChange logic to call when inputValue has changed in useEffect
* remove explicit calls to onInputChange
* Comprehensive unit tests for this prop

* fix(combobox): onInputChange calls

---------

Co-authored-by: Preeti Bansal <146315451+preetibansalui@users.noreply.github.com>

Author: neuescodes

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

@@ -33,10 +33,14 @@ const ControlledComboBox = ({ controlledItem }) => {
   const items = generateItems(5, generateGenericItem);
   const [value, setValue] = useState(controlledItem || items[0]);
   const [onChangeCallCount, setOnChangeCallCount] = useState(0);
+  const [onInputChangeCallCount, setOnInputChangeCallCount] = useState(0);
   const controlledOnChange = ({ selectedItem }) => {
     setValue(selectedItem);
     setOnChangeCallCount((prevCount) => prevCount + 1);
   };
+  const controlledOnInputChange = () => {
+    setOnInputChangeCallCount((prevCount) => prevCount + 1);
+  };
 
   return (
     <div>
@@ -45,11 +49,13 @@ const ControlledComboBox = ({ controlledItem }) => {
         items={items}
         selectedItem={value}
         onChange={controlledOnChange}
+        onInputChange={controlledOnInputChange}
         placeholder="Filter..."
         type="default"
       />
       <div>value: {value?.label || 'none'}</div>
       <div>onChangeCallCount: {onChangeCallCount}</div>
+      <div>onInputChangeCallCount: {onInputChangeCallCount}</div>
       <button onClick={() => setValue(items[3])}>Choose item 3</button>
       <button onClick={() => setValue(null)}>reset</button>
     </div>
@@ -184,13 +190,74 @@ describe('ComboBox', () => {
     });
   });
 
-  it('capture filter text event onInputChange', async () => {
-    const onInputChange = jest.fn();
-    render(<ComboBox {...mockProps} onInputChange={onInputChange} />);
+  describe('onInputChange', () => {
+    let onInputChange;
+    beforeEach(() => {
+      onInputChange = jest.fn();
+    });
+    it('should not call onChange or onInputChange on initial render', () => {
+      render(<ComboBox {...mockProps} onInputChange={onInputChange} />);
+      expect(onInputChange).not.toHaveBeenCalled();
+      expect(mockProps.onChange).not.toHaveBeenCalled();
+    });
 
-    await userEvent.type(findInputNode(), 'something');
+    it('capture filter text event onInputChange', async () => {
+      render(<ComboBox {...mockProps} onInputChange={onInputChange} />);
+      await userEvent.type(findInputNode(), 'something');
+      expect(onInputChange).toHaveBeenCalledWith('something');
+    });
+
+    it('should call onInputChange when option is selected from dropdown', async () => {
+      render(<ComboBox {...mockProps} onInputChange={onInputChange} />);
+      await openMenu();
+      expect(onInputChange).not.toHaveBeenCalled();
+      await userEvent.click(screen.getByRole('option', { name: 'Item 2' }));
+      expect(onInputChange).toHaveBeenCalledWith('Item 2');
+    });
+
+    it('should call onInputChange when option is cleared with button', async () => {
+      render(
+        <ComboBox
+          {...mockProps}
+          initialSelectedItem={mockProps.items[0]}
+          onInputChange={onInputChange}
+        />
+      );
+      expect(onInputChange).not.toHaveBeenCalled();
+      await userEvent.click(
+        screen.getByRole('button', { name: 'Clear selected item' })
+      );
+      expect(onInputChange).toHaveBeenCalledWith('');
+    });
+
+    it('should not call onInputChange when combobox is interacted with but input value does not change', async () => {
+      render(
+        <ComboBox
+          {...mockProps}
+          initialSelectedItem={mockProps.items[0]}
+          onInputChange={onInputChange}
+        />
+      );
+      expect(onInputChange).not.toHaveBeenCalled();
+      await openMenu();
+      await userEvent.click(screen.getByRole('option', { name: 'Item 0' }));
+      expect(onInputChange).not.toHaveBeenCalled();
+    });
 
-    expect(onInputChange).toHaveBeenCalledWith('something');
+    it('should call onInputChange when custom value is entered into combobox', async () => {
+      render(
+        <ComboBox
+          {...mockProps}
+          initialSelectedItem={mockProps.items[0]}
+          onInputChange={onInputChange}
+        />
+      );
+      await userEvent.clear(findInputNode());
+      expect(onInputChange).toHaveBeenCalledWith('');
+      await userEvent.type(findInputNode(), 'custom value');
+      await userEvent.keyboard('[Enter]');
+      expect(onInputChange).toHaveBeenCalledWith('custom value');
+    });
   });
 
   it('should render custom item components', async () => {
@@ -372,6 +439,35 @@ describe('ComboBox', () => {
       await waitForPosition();
       expect(findInputNode()).toHaveDisplayValue(mockProps.items[0].label);
     });
+    it('should not call onChange or onInputChange on initial render', () => {
+      render(<ControlledComboBox />);
+      expect(screen.getByText('onChangeCallCount: 0')).toBeInTheDocument();
+      expect(screen.getByText('onInputChangeCallCount: 0')).toBeInTheDocument();
+    });
+    it('should call onInputChange when input changes', async () => {
+      render(<ControlledComboBox />);
+      await userEvent.type(findInputNode(), 'Item 2');
+      expect(screen.getByText('onInputChangeCallCount: 6')).toBeInTheDocument();
+    });
+    it('should call onInputChange when external state managing selectedItem is updated', async () => {
+      render(<ControlledComboBox />);
+      await userEvent.click(
+        screen.getByRole('button', { name: 'Choose item 3' })
+      );
+      expect(screen.getByText('onInputChangeCallCount: 1')).toBeInTheDocument();
+    });
+    it('should not call onChange or onInputChange when external state managing selectedItem is updated to same value', async () => {
+      render(
+        <ControlledComboBox
+          controlledItem={{ id: 'id-3', label: 'Item 3', value: 3 }}
+        />
+      );
+      await userEvent.click(
+        screen.getByRole('button', { name: 'Choose item 3' })
+      );
+      expect(screen.getByText('onChangeCallCount: 0')).toBeInTheDocument();
+      expect(screen.getByText('onInputChangeCallCount: 0')).toBeInTheDocument();
+    });
 
     it('should display selected item using a string type for the `selectedItem` prop', async () => {
       // Replace the 'items' property in mockProps with a list of strings

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

@@ -504,7 +504,7 @@ const ComboBox = forwardRef(
     const textInput = useRef<HTMLInputElement>(null);
     const comboBoxInstanceId = useId();
     const [isFocused, setIsFocused] = useState(false);
-    const savedOnInputChange = useRef(onInputChange);
+    const prevInputValue = useRef(inputValue);
     const prevSelectedItemProp = useRef<ItemType | null | undefined>(
       selectedItemProp
     );
@@ -547,13 +547,11 @@ const ComboBox = forwardRef(
             : defaultShouldFilterItem()
       );
 
+    // call onInputChange whenever inputValue is updated
     useEffect(() => {
-      savedOnInputChange.current = onInputChange;
-    }, [onInputChange]);
-
-    useEffect(() => {
-      if (savedOnInputChange.current) {
-        savedOnInputChange.current(inputValue);
+      if (prevInputValue.current !== inputValue) {
+        prevInputValue.current = inputValue;
+        onInputChange && onInputChange(inputValue);
       }
     }, [inputValue]);