chore: Writes the tests for the new Select component (#16638)

* chore: Writes the tests for the new Select component

* Uses array destructuring

superset-frontend/src/components/Select/Select.stories.tsx

@@ -79,6 +79,12 @@ const ARG_TYPES = {
       disable: true,
     },
   },
+  labelInValue: {
+    defaultValue: true,
+    table: {
+      disable: true,
+    },
+  },
   name: {
     table: {
       disable: true,
@@ -171,7 +177,11 @@ export const AtEveryCorner = () => (
           position: 'absolute',
         }}
       >
-        <Select ariaLabel={`gallery-${position.id}`} options={options} />
+        <Select
+          ariaLabel={`gallery-${position.id}`}
+          options={options}
+          labelInValue
+        />
       </div>
     ))}
     <p style={{ position: 'absolute', top: '40%', left: '33%', width: 500 }}>
@@ -206,7 +216,7 @@ export const PageScroll = () => (
         right: 30,
       }}
     >
-      <Select ariaLabel="page-scroll-select-1" options={options} />
+      <Select ariaLabel="page-scroll-select-1" options={options} labelInValue />
     </div>
     <div
       style={{

superset-frontend/src/components/Select/Select.test.tsx

@@ -17,66 +17,461 @@
  * under the License.
  */
 import React from 'react';
-import { render, screen } from 'spec/helpers/testing-library';
+import {
+  render,
+  screen,
+  waitFor,
+  waitForElementToBeRemoved,
+  within,
+} from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
 import { Select } from 'src/components';
 
-test('renders with default props', () => {
-  const ariaLabel = 'test';
-  render(
-    <Select
-      ariaLabel={ariaLabel}
-      options={[
-        { label: 'A', value: 0 },
-        { label: 'B', value: 1 },
-      ]}
-    />,
+const ARIA_LABEL = 'Test';
+const NEW_OPTION = 'Kyle';
+const NO_DATA = 'No Data';
+const LOADING = 'Loading...';
+const OPTIONS = [
+  { label: 'John', value: 1 },
+  { label: 'Liam', value: 2 },
+  { label: 'Olivia', value: 3 },
+  { label: 'Emma', value: 4 },
+  { label: 'Noah', value: 5 },
+  { label: 'Ava', value: 6 },
+  { label: 'Oliver', value: 7 },
+  { label: 'ElijahH', value: 8 },
+  { label: 'Charlotte', value: 9 },
+  { label: 'Giovanni', value: 10 },
+  { label: 'Franco', value: 11 },
+  { label: 'Sandro', value: 12 },
+  { label: 'Alehandro', value: 13 },
+  { label: 'Johnny', value: 14 },
+  { label: 'Nikole', value: 15 },
+  { label: 'Igor', value: 16 },
+  { label: 'Guilherme', value: 17 },
+  { label: 'Irfan', value: 18 },
+  { label: 'George', value: 19 },
+  { label: 'Ashfaq', value: 20 },
+];
+
+const loadOptions = async (search: string, page: number, pageSize: number) => {
+  const totalCount = OPTIONS.length;
+  const start = page * pageSize;
+  const deleteCount =
+    start + pageSize < totalCount ? pageSize : totalCount - start;
+  const data = OPTIONS.filter(option => option.label.match(search)).splice(
+    start,
+    deleteCount,
+  );
+  return {
+    data,
+    totalCount: OPTIONS.length,
+  };
+};
+
+const defaultProps = {
+  allowClear: true,
+  ariaLabel: ARIA_LABEL,
+  labelInValue: true,
+  options: OPTIONS,
+  pageSize: 10,
+  showSearch: true,
+};
+
+const getElementByClassName = (className: string) =>
+  document.querySelector(className)! as HTMLElement;
+
+const getElementsByClassName = (className: string) =>
+  document.querySelectorAll(className)! as NodeListOf<HTMLElement>;
+
+const getSelect = () => screen.getByRole('combobox', { name: ARIA_LABEL });
+
+const findSelectOption = (text: string) =>
+  waitFor(() =>
+    within(getElementByClassName('.rc-virtual-list')).getByText(text),
   );
 
-  expect(screen.getByRole('combobox', { name: ariaLabel })).toBeInTheDocument();
+const findAllSelectOptions = () =>
+  waitFor(() => getElementsByClassName('.ant-select-item-option-content'));
+
+const findSelectValue = () =>
+  waitFor(() => getElementByClassName('.ant-select-selection-item'));
+
+const findAllSelectValues = () =>
+  waitFor(() => getElementsByClassName('.ant-select-selection-item'));
+
+const type = (text: string) => userEvent.type(getSelect(), text, { delay: 10 });
+
+const open = () => waitFor(() => userEvent.click(getSelect()));
+
+test('displays a header', async () => {
+  const headerText = 'Header';
+  render(<Select {...defaultProps} header={headerText} />);
+  expect(screen.getByText(headerText)).toBeInTheDocument();
+});
+
+test('inverts the selection', async () => {
+  render(<Select {...defaultProps} invertSelection />);
+  await open();
+  userEvent.click(await findSelectOption(OPTIONS[0].label));
+  expect(await screen.findByLabelText('stop')).toBeInTheDocument();
+});
+
+test('displays the selected values first', async () => {
+  render(<Select {...defaultProps} mode="multiple" />);
+  const option3 = OPTIONS[2].label;
+  const option8 = OPTIONS[7].label;
+  await open();
+  userEvent.click(await findSelectOption(option3));
+  userEvent.click(await findSelectOption(option8));
+  await type('{esc}');
+  await open();
+  const sortedOptions = await findAllSelectOptions();
+  expect(sortedOptions[0]).toHaveTextContent(option3);
+  expect(sortedOptions[1]).toHaveTextContent(option8);
+});
+
+test('searches for label or value', async () => {
+  const option = OPTIONS[11];
+  render(<Select {...defaultProps} />);
+  const search = option.value;
+  await type(search.toString());
+  const options = await findAllSelectOptions();
+  expect(options.length).toBe(1);
+  expect(options[0]).toHaveTextContent(option.label);
+});
+
+test('clear all the values', async () => {
+  const onClear = jest.fn();
+  render(
+    <Select
+      {...defaultProps}
+      mode="multiple"
+      value={[OPTIONS[0], OPTIONS[1]]}
+      onClear={onClear}
+    />,
+  );
+  userEvent.click(screen.getByLabelText('close-circle'));
+  expect(onClear).toHaveBeenCalled();
+  const values = await findAllSelectValues();
+  expect(values.length).toBe(0);
+});
+
+test('does not add a new option if allowNewValue is false', async () => {
+  render(<Select {...defaultProps} options={loadOptions} />);
+  await open();
+  await type(NEW_OPTION);
+  expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
+});
+
+test('static - renders the select with default props', () => {
+  render(<Select {...defaultProps} />);
+  expect(getSelect()).toBeInTheDocument();
+});
+
+test('static - opens the select without any data', async () => {
+  render(<Select {...defaultProps} options={[]} />);
+  await open();
+  expect(screen.getByText(NO_DATA)).toBeInTheDocument();
+});
+
+test('static - makes a selection in single mode', async () => {
+  render(<Select {...defaultProps} />);
+  const optionText = 'Emma';
+  await open();
+  userEvent.click(await findSelectOption(optionText));
+  expect(await findSelectValue()).toHaveTextContent(optionText);
+});
+
+test('static - multiple selections in multiple mode', async () => {
+  render(<Select {...defaultProps} mode="multiple" />);
+  await open();
+  const [firstOption, secondOption] = OPTIONS;
+  userEvent.click(await findSelectOption(firstOption.label));
+  userEvent.click(await findSelectOption(secondOption.label));
+  const values = await findAllSelectValues();
+  expect(values[0]).toHaveTextContent(firstOption.label);
+  expect(values[1]).toHaveTextContent(secondOption.label);
+});
+
+test('static - changes the selected item in single mode', async () => {
+  const onChange = jest.fn();
+  render(<Select {...defaultProps} onChange={onChange} />);
+  await open();
+  const [firstOption, secondOption] = OPTIONS;
+  userEvent.click(await findSelectOption(firstOption.label));
+  expect(await findSelectValue()).toHaveTextContent(firstOption.label);
+  expect(onChange).toHaveBeenCalledWith(
+    expect.objectContaining(firstOption),
+    firstOption,
+  );
+  userEvent.click(await findSelectOption(secondOption.label));
+  expect(onChange).toHaveBeenCalledWith(
+    expect.objectContaining(secondOption),
+    secondOption,
+  );
+  expect(await findSelectValue()).toHaveTextContent(secondOption.label);
+});
+
+test('static - deselects an item in multiple mode', async () => {
+  render(<Select {...defaultProps} mode="multiple" />);
+  await open();
+  const [firstOption, secondOption] = OPTIONS;
+  userEvent.click(await findSelectOption(firstOption.label));
+  userEvent.click(await findSelectOption(secondOption.label));
+  let values = await findAllSelectValues();
+  expect(values.length).toBe(2);
+  expect(values[0]).toHaveTextContent(firstOption.label);
+  expect(values[1]).toHaveTextContent(secondOption.label);
+  userEvent.click(await findSelectOption(firstOption.label));
+  values = await findAllSelectValues();
+  expect(values.length).toBe(1);
+  expect(values[0]).toHaveTextContent(secondOption.label);
+});
+
+test('static - adds a new option if none is available and allowNewOptions is true', async () => {
+  render(<Select {...defaultProps} allowNewOptions />);
+  await open();
+  await type(NEW_OPTION);
+  expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
+});
+
+test('static - does not add a new option if the option already exists', async () => {
+  render(<Select {...defaultProps} allowNewOptions />);
+  const option = OPTIONS[0].label;
+  await open();
+  await type(option);
+  expect(await findSelectOption(option)).toBeInTheDocument();
+});
+
+test('static - sets a initial value in single mode', async () => {
+  render(<Select {...defaultProps} value={OPTIONS[0]} />);
+  expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label);
+});
+
+test('static - sets a initial value in multiple mode', async () => {
+  render(
+    <Select
+      {...defaultProps}
+      mode="multiple"
+      value={[OPTIONS[0], OPTIONS[1]]}
+    />,
+  );
+  const values = await findAllSelectValues();
+  expect(values[0]).toHaveTextContent(OPTIONS[0].label);
+  expect(values[1]).toHaveTextContent(OPTIONS[1].label);
+});
+
+test('static - searches for an item', async () => {
+  render(<Select {...defaultProps} />);
+  const search = 'Oli';
+  await type(search);
+  const options = await findAllSelectOptions();
+  expect(options.length).toBe(2);
+  expect(options[0]).toHaveTextContent('Olivia');
+  expect(options[1]).toHaveTextContent('Oliver');
+});
+
+test('async - renders the select with default props', () => {
+  render(<Select {...defaultProps} options={loadOptions} />);
+  expect(getSelect()).toBeInTheDocument();
+});
+
+test('async - opens the select without any data', async () => {
+  render(
+    <Select
+      {...defaultProps}
+      options={async () => ({ data: [], totalCount: 0 })}
+    />,
+  );
+  await open();
+  expect(await screen.findByText(/no data/i)).toBeInTheDocument();
+});
+
+test('async - displays the loading indicator when opening', async () => {
+  render(<Select {...defaultProps} options={loadOptions} />);
+  await waitFor(() => {
+    userEvent.click(getSelect());
+    expect(screen.getByText(LOADING)).toBeInTheDocument();
+  });
+  expect(screen.queryByText(LOADING)).not.toBeInTheDocument();
+});
+
+test('async - displays the loading indicator while searching', async () => {
+  render(<Select {...defaultProps} options={loadOptions} />);
+  await type('John');
+  expect(screen.getByText(LOADING)).toBeInTheDocument();
+  await waitFor(() =>
+    expect(screen.queryByText(LOADING)).not.toBeInTheDocument(),
+  );
+});
+
+test('async - makes a selection in single mode', async () => {
+  render(<Select {...defaultProps} options={loadOptions} />);
+  const optionText = 'Emma';
+  await open();
+  userEvent.click(await findSelectOption(optionText));
+  expect(await findSelectValue()).toHaveTextContent(optionText);
+});
+
+test('async - multiple selections in multiple mode', async () => {
+  render(<Select {...defaultProps} options={loadOptions} mode="multiple" />);
+  await open();
+  const [firstOption, secondOption] = OPTIONS;
+  userEvent.click(await findSelectOption(firstOption.label));
+  userEvent.click(await findSelectOption(secondOption.label));
+  const values = await findAllSelectValues();
+  expect(values[0]).toHaveTextContent(firstOption.label);
+  expect(values[1]).toHaveTextContent(secondOption.label);
+});
+
+test('async - changes the selected item in single mode', async () => {
+  const onChange = jest.fn();
+  render(
+    <Select {...defaultProps} options={loadOptions} onChange={onChange} />,
+  );
+  await open();
+  const [firstOption, secondOption] = OPTIONS;
+  userEvent.click(await findSelectOption(firstOption.label));
+  expect(onChange).toHaveBeenCalledWith(
+    expect.objectContaining(firstOption),
+    firstOption,
+  );
+  expect(await findSelectValue()).toHaveTextContent(firstOption.label);
+  userEvent.click(await findSelectOption(secondOption.label));
+  expect(onChange).toHaveBeenCalledWith(
+    expect.objectContaining(secondOption),
+    secondOption,
+  );
+  expect(await findSelectValue()).toHaveTextContent(secondOption.label);
+});
+
+test('async - deselects an item in multiple mode', async () => {
+  render(<Select {...defaultProps} options={loadOptions} mode="multiple" />);
+  await open();
+  const [firstOption, secondOption] = OPTIONS;
+  userEvent.click(await findSelectOption(firstOption.label));
+  userEvent.click(await findSelectOption(secondOption.label));
+  let values = await findAllSelectValues();
+  expect(values.length).toBe(2);
+  expect(values[0]).toHaveTextContent(firstOption.label);
+  expect(values[1]).toHaveTextContent(secondOption.label);
+  userEvent.click(await findSelectOption(firstOption.label));
+  values = await findAllSelectValues();
+  expect(values.length).toBe(1);
+  expect(values[0]).toHaveTextContent(secondOption.label);
+});
+
+test('async - adds a new option if none is available and allowNewOptions is true', async () => {
+  render(<Select {...defaultProps} options={loadOptions} allowNewOptions />);
+  await open();
+  await type(NEW_OPTION);
+  expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
+});
+
+test('async - does not add a new option if the option already exists', async () => {
+  render(<Select {...defaultProps} options={loadOptions} allowNewOptions />);
+  const option = OPTIONS[0].label;
+  await open();
+  await type(option);
+  await waitFor(() => {
+    const array = within(
+      getElementByClassName('.rc-virtual-list'),
+    ).getAllByText(option);
+    expect(array.length).toBe(1);
+  });
+});
+
+test('async - sets a initial value in single mode', async () => {
+  render(<Select {...defaultProps} options={loadOptions} value={OPTIONS[0]} />);
+  expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label);
+});
+
+test('async - sets a initial value in multiple mode', async () => {
+  render(
+    <Select
+      {...defaultProps}
+      mode="multiple"
+      options={loadOptions}
+      value={[OPTIONS[0], OPTIONS[1]]}
+    />,
+  );
+  const values = await findAllSelectValues();
+  expect(values[0]).toHaveTextContent(OPTIONS[0].label);
+  expect(values[1]).toHaveTextContent(OPTIONS[1].label);
+});
+
+test('async - searches for an item already loaded', async () => {
+  render(<Select {...defaultProps} options={loadOptions} />);
+  const search = 'Oli';
+  await open();
+  await type(search);
+  await waitForElementToBeRemoved(screen.getByText(LOADING));
+  const options = await findAllSelectOptions();
+  expect(options.length).toBe(2);
+  expect(options[0]).toHaveTextContent('Olivia');
+  expect(options[1]).toHaveTextContent('Oliver');
+});
+
+test('async - searches for an item in a page not loaded', async () => {
+  render(<Select {...defaultProps} options={loadOptions} />);
+  const search = 'Ashfaq';
+  await open();
+  await type(search);
+  await waitForElementToBeRemoved(screen.getByText(LOADING));
+  const options = await findAllSelectOptions();
+  expect(options.length).toBe(1);
+  expect(options[0]).toHaveTextContent(search);
+});
+
+test('async - does not fetches data when rendering', async () => {
+  const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 }));
+  render(<Select {...defaultProps} options={loadOptions} />);
+  expect(loadOptions).not.toHaveBeenCalled();
+});
+
+test('async - fetches data when opening', async () => {
+  const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 }));
+  render(<Select {...defaultProps} options={loadOptions} />);
+  await open();
+  expect(loadOptions).toHaveBeenCalled();
+});
+
+test('async - fetches data only after a search input is entered if fetchOnlyOnSearch is true', async () => {
+  const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 }));
+  render(<Select {...defaultProps} options={loadOptions} fetchOnlyOnSearch />);
+  await open();
+  await waitFor(() => expect(loadOptions).not.toHaveBeenCalled());
+  await type('search');
+  await waitFor(() => expect(loadOptions).toHaveBeenCalled());
+});
+
+test('async - displays an error message when an exception is thrown while fetching', async () => {
+  const error = 'Fetch error';
+  const loadOptions = async () => {
+    throw new Error(error);
+  };
+  render(<Select {...defaultProps} options={loadOptions} />);
+  await open();
+  expect(screen.getByText(error)).toBeInTheDocument();
+});
+
+test('async - does not fire a new request for the same search input', async () => {
+  const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 }));
+  render(<Select {...defaultProps} options={loadOptions} fetchOnlyOnSearch />);
+  await type('search');
+  expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
+  expect(loadOptions).toHaveBeenCalledTimes(1);
+  userEvent.click(screen.getByLabelText('close-circle'));
+  await type('search');
+  expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
+  expect(loadOptions).toHaveBeenCalledTimes(1);
 });
 
 /*
-Tests for the sync version of the select:
-- Opens the select without any data
-- Makes a selection in single mode
-- Makes multiple selections in multiple mode
-- Changes the selected item in single mode
-- Deselects an item in multiple mode
-- Adds a header to the select
-- Adds a new option if none is available and allowNewValue is true
-- Does not add a new option if the option already exists
-- Does not add a new option if allowNewValue is false
-- Inverts the selection
-- Sets a initial value in single mode
-- Sets a initial value in multiple mode
-- Searches for an item
-- Displays the selected items first
-- Searches for label or value
-- Clear all the values
-
-Tests for the async version of the select:
-- Opens the select without any data
-- Makes a selection in single mode
-- Makes multiple selections in multiple mode
-- Changes the selected item in single mode
-- Deselects an item in multiple mode
-- Adds a new option if none is available and allowNewValue is true
-- Does not add a new option if the option already exists
-- Does not add a new option if allowNewValue is false
-- Sets a initial value in single mode
-- Sets a initial value in multiple mode
-- Searches for an item already loaded
-- Searches for an item in a page not loaded
-- Displays the loading indicator
+TODO: Add tests that require scroll interaction. Needs further investigation.
 - Fetches more data when scrolling and more data is available
 - Doesn't fetch more data when no more data is available
 - Requests the correct page and page size
-- Fetches only after a search input is entered if fetchOnlyOnSearch is true
-- Does not fetch data when rendering
-- Fetches data when opening the select
-- Displays an error message when an exception is thrown while fetching
-- Does not fire a new request for the same search input
-- Displays the selected items first
 - Sets the page to zero when a new search is made
-- Clear all the values
 */

superset-frontend/src/components/Select/Select.tsx

@@ -166,10 +166,11 @@ const Select = ({
   invertSelection = false,
   labelInValue = false,
   lazyLoading = true,
-  loading = false,
+  loading,
   mode = 'single',
   name,
   onChange,
+  onClear,
   options,
   pageSize = DEFAULT_PAGE_SIZE,
   placeholder = t('Select ...'),
@@ -232,6 +233,8 @@ const Select = ({
     }
   }, [isAsync, selectOptions, selectValue]);
 
+  // TODO: Simplify the code. We're only accepting label, value options.
+  // TODO: Remove labelInValue prop.
   const handleTopOptions = useCallback(
     (selectedValue: AntdSelectValue | undefined) => {
       // bringing selected options to the top of the list
@@ -365,6 +368,7 @@ const Select = ({
       const cachedCount = fetchedQueries.current.get(key);
       if (cachedCount) {
         setTotalCount(cachedCount);
+        setIsLoading(false);
         setIsTyping(false);
         return;
       }
@@ -411,17 +415,17 @@ const Select = ({
             // adds a custom option
             const newOptions = [...selectOptions, newOption];
             setSelectOptions(newOptions);
-            setSelectValue(searchValue);
+            setSelectValue(newOption);
 
             if (onChange) {
               onChange(searchValue, newOptions);
             }
           }
         }
-        setSearchedValue(searchValue);
-        if (!searchValue) {
+        if (!searchValue || searchValue === searchedValue) {
           setIsTyping(false);
         }
+        setSearchedValue(searchValue);
       }, DEBOUNCE_TIMEOUT),
     [
       allowNewOptions,
@@ -433,6 +437,9 @@ const Select = ({
     ],
   );
 
+  // Stop the invocation of the debounced function after unmounting
+  useEffect(() => () => handleOnSearch.cancel(), [handleOnSearch]);
+
   const handlePagination = (e: UIEvent<HTMLElement>) => {
     const vScroll = e.currentTarget;
     const thresholdReached =
@@ -535,6 +542,13 @@ const Select = ({
     return <DownOutlined />;
   };
 
+  const handleClear = () => {
+    setSelectValue(undefined);
+    if (onClear) {
+      onClear();
+    }
+  };
+
   return (
     <StyledContainer>
       {header}
@@ -552,7 +566,7 @@ const Select = ({
         onPopupScroll={isAsync ? handlePagination : undefined}
         onSearch={shouldShowSearch ? handleOnSearch : undefined}
         onSelect={handleOnSelect}
-        onClear={() => setSelectValue(undefined)}
+        onClear={handleClear}
         onChange={onChange}
         options={selectOptions}
         placeholder={placeholder}