Changed Select to fix an issue with selecting options after searching…

…. Fixes issue #4829 (#4830)
grommet · Dec 15, 2020 · 9f3828d · 9f3828d
1 parent d079c10
commit 9f3828d
Show file tree

Hide file tree

Showing 3 changed files with 678 additions and 25 deletions.
diff --git a/src/js/components/Select/SelectContainer.js b/src/js/components/Select/SelectContainer.js
@@ -74,7 +74,7 @@ const SelectContainer = forwardRef(
       onMore,
       onSearch,
       optionIndexesInValue,
-      options: optionsProp,
+      options,
       allOptions,
       searchPlaceholder,
       search,
@@ -127,18 +127,18 @@ const SelectContainer = forwardRef(
     }, [keyboardNavigation]);
 
     const optionLabel = useCallback(
-      index => applyKey(optionsProp[index], labelKey),
-      [labelKey, optionsProp],
+      index => applyKey(options[index], labelKey),
+      [labelKey, options],
     );
 
     const optionValue = useCallback(
-      index => applyKey(optionsProp[index], valueKey),
-      [optionsProp, valueKey],
+      index => applyKey(options[index], valueKey),
+      [options, valueKey],
     );
 
     const isDisabled = useCallback(
       index => {
-        const option = optionsProp[index];
+        const option = options[index];
         let result;
         if (disabledKey) {
           result = applyKey(option, disabledKey);
@@ -152,7 +152,7 @@ const SelectContainer = forwardRef(
         }
         return result;
       },
-      [disabled, disabledKey, optionsProp, optionValue],
+      [disabled, disabledKey, options, optionValue],
     );
 
     const isSelected = useCallback(
@@ -199,7 +199,7 @@ const SelectContainer = forwardRef(
           let nextSelected;
           if (multiple) {
             const nextOptionIndexesInValue = optionIndexesInValue.slice(0);
-            const allOptionsIndex = allOptions.indexOf(optionsProp[index]);
+            const allOptionsIndex = allOptions.indexOf(options[index]);
             const valueIndex = optionIndexesInValue.indexOf(allOptionsIndex);
             if (valueIndex === -1) {
               nextOptionIndexesInValue.push(allOptionsIndex);
@@ -215,25 +215,18 @@ const SelectContainer = forwardRef(
           } else {
             nextValue =
               valueKey && valueKey.reduce
-                ? applyKey(allOptions[index], valueKey)
-                : allOptions[index];
+                ? applyKey(options[index], valueKey)
+                : options[index];
             nextSelected = index;
           }
           onChange(event, {
-            option: allOptions[index],
+            option: options[index],
             value: nextValue,
             selected: nextSelected,
           });
         }
       },
-      [
-        multiple,
-        onChange,
-        optionIndexesInValue,
-        optionsProp,
-        allOptions,
-        valueKey,
-      ],
+      [multiple, onChange, optionIndexesInValue, options, allOptions, valueKey],
     );
 
     const onClear = useCallback(
@@ -248,17 +241,17 @@ const SelectContainer = forwardRef(
         event.preventDefault();
         let nextActiveIndex = activeIndex + 1;
         while (
-          nextActiveIndex < optionsProp.length &&
+          nextActiveIndex < options.length &&
           isDisabled(nextActiveIndex)
         ) {
           nextActiveIndex += 1;
         }
-        if (nextActiveIndex !== optionsProp.length) {
+        if (nextActiveIndex !== options.length) {
           setActiveIndex(nextActiveIndex);
           setKeyboardNavigation(true);
         }
       },
-      [activeIndex, isDisabled, optionsProp],
+      [activeIndex, isDisabled, options],
     );
 
     const onPreviousOption = useCallback(
@@ -343,9 +336,9 @@ const SelectContainer = forwardRef(
             />
           )}
           <OptionsBox role="menubar" tabIndex="-1" ref={optionsRef}>
-            {optionsProp.length > 0 ? (
+            {options.length > 0 ? (
               <InfiniteScroll
-                items={optionsProp}
+                items={options}
                 step={theme.select.step}
                 onMore={onMore}
                 replace={replace}
@@ -359,7 +352,7 @@ const SelectContainer = forwardRef(
                   // as an option Button kind property.
                   let child;
                   if (children)
-                    child = children(option, index, optionsProp, {
+                    child = children(option, index, options, {
                       active: optionActive,
                       disabled: optionDisabled,
                       selected: optionSelected,

diff --git a/src/js/components/Select/__tests__/Select-test.js b/src/js/components/Select/__tests__/Select-test.js
@@ -125,6 +125,44 @@ describe('Select', () => {
     expect(onSearch).toBeCalledWith('o');
   });
 
+  test('search and select', () => {
+    jest.useFakeTimers();
+    const onSearch = jest.fn();
+    const onChange = jest.fn();
+    const Test = () => {
+      const [options, setOptions] = React.useState(['one', 'two']);
+      return (
+        <Select
+          id="test-select"
+          placeholder="test select"
+          options={options}
+          onChange={onChange}
+          onSearch={arg => {
+            onSearch(arg);
+            setOptions(['two']);
+          }}
+        />
+      );
+    };
+    const { getByPlaceholderText, getByText, container } = render(<Test />);
+    expect(container.firstChild).toMatchSnapshot();
+
+    fireEvent.click(getByPlaceholderText('test select'));
+
+    // advance timers so select can open
+    act(() => jest.advanceTimersByTime(200));
+    // snapshot on search box
+    expectPortal('test-select__drop').toMatchSnapshot();
+    expect(document.activeElement).toMatchSnapshot();
+    // add content to search box
+    fireEvent.change(document.activeElement, { target: { value: 't' } });
+    expect(document.activeElement).toMatchSnapshot();
+    expect(onSearch).toBeCalledWith('t');
+
+    fireEvent.click(getByText('two'));
+    expect(onChange).toBeCalledWith(expect.objectContaining({ value: 'two' }));
+  });
+
   test('select an option with complex options', () => {
     const onChange = jest.fn();
     const { getByText, container } = render(