This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new fe91974163 chore: Remove unecessary code from async and sync select
components (#20690)
fe91974163 is described below
commit fe919741632e677ff14cdd35f886c8b2a9f0d4de
Author: cccs-RyanK <[email protected]>
AuthorDate: Thu Jul 28 09:09:37 2022 -0400
chore: Remove unecessary code from async and sync select components (#20690)
* Created AsyncSelect Component
Changed files to reference AsyncSelect if needed
* modified import of AsyncSelect, removed async tests and prefixes from
select tests
* fixed various import and lint warnings
* fixing lint errors
* fixed frontend test errors
* fixed alertreportmodel tests
* removed accidental import
* fixed lint errors
* updated async select
* removed code from select component
* fixed select test
* fixed async label value and select initial values
* cleaned up async test
* fixed lint errors
* minor fixes to sync select component
* removed unecessary variables and fixed linting
* fixed npm test errors
* fixed linting issues
* fixed showSearch and storybook
* fixed linting
---
.../src/components/DatabaseSelector/index.tsx | 1 -
.../src/components/ListView/Filters/Select.tsx | 42 +++-
.../src/components/Select/AsyncSelect.test.tsx | 150 +++++++----
.../src/components/Select/AsyncSelect.tsx | 73 +++---
.../src/components/Select/Select.stories.tsx | 28 ++-
.../src/components/Select/Select.test.tsx | 27 +-
superset-frontend/src/components/Select/Select.tsx | 280 +--------------------
.../src/components/TableSelector/index.tsx | 1 -
.../src/filters/components/GroupBy/types.ts | 3 +-
.../src/filters/components/TimeColumn/types.ts | 3 +-
.../src/filters/components/TimeGrain/types.ts | 3 +-
11 files changed, 214 insertions(+), 397 deletions(-)
diff --git a/superset-frontend/src/components/DatabaseSelector/index.tsx
b/superset-frontend/src/components/DatabaseSelector/index.tsx
index e972e95b00..1df7f78a3b 100644
--- a/superset-frontend/src/components/DatabaseSelector/index.tsx
+++ b/superset-frontend/src/components/DatabaseSelector/index.tsx
@@ -302,7 +302,6 @@ export default function DatabaseSelector({
disabled={!currentDb || readOnly}
header={<FormLabel>{t('Schema')}</FormLabel>}
labelInValue
- lazyLoading={false}
loading={loadingSchemas}
name="select-schema"
placeholder={t('Select schema or type schema name')}
diff --git a/superset-frontend/src/components/ListView/Filters/Select.tsx
b/superset-frontend/src/components/ListView/Filters/Select.tsx
index 525061fd27..ecda25a81f 100644
--- a/superset-frontend/src/components/ListView/Filters/Select.tsx
+++ b/superset-frontend/src/components/ListView/Filters/Select.tsx
@@ -26,6 +26,7 @@ import { t } from '@superset-ui/core';
import { Select } from 'src/components';
import { Filter, SelectOption } from 'src/components/ListView/types';
import { FormLabel } from 'src/components/Form';
+import AsyncSelect from 'src/components/Select/AsyncSelect';
import { FilterContainer, BaseFilter, FilterHandler } from './Base';
interface SelectFilterProps extends BaseFilter {
@@ -86,19 +87,34 @@ function SelectFilter(
return (
<FilterContainer>
- <Select
- allowClear
- ariaLabel={typeof Header === 'string' ? Header : name || t('Filter')}
- labelInValue
- data-test="filters-select"
- header={<FormLabel>{Header}</FormLabel>}
- onChange={onChange}
- onClear={onClear}
- options={fetchSelects ? fetchAndFormatSelects : selects}
- placeholder={t('Select or type a value')}
- showSearch
- value={selectedOption}
- />
+ {fetchSelects ? (
+ <AsyncSelect
+ allowClear
+ ariaLabel={typeof Header === 'string' ? Header : name || t('Filter')}
+ data-test="filters-select"
+ header={<FormLabel>{Header}</FormLabel>}
+ onChange={onChange}
+ onClear={onClear}
+ options={fetchAndFormatSelects}
+ placeholder={t('Select or type a value')}
+ showSearch
+ value={selectedOption}
+ />
+ ) : (
+ <Select
+ allowClear
+ ariaLabel={typeof Header === 'string' ? Header : name || t('Filter')}
+ data-test="filters-select"
+ header={<FormLabel>{Header}</FormLabel>}
+ labelInValue
+ onChange={onChange}
+ onClear={onClear}
+ options={selects}
+ placeholder={t('Select or type a value')}
+ showSearch
+ value={selectedOption}
+ />
+ )}
</FilterContainer>
);
}
diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx
b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
index dc6eff35d9..8a50002c86 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.test.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
@@ -60,10 +60,16 @@ const loadOptions = async (search: string, page: number,
pageSize: number) => {
const start = page * pageSize;
const deleteCount =
start + pageSize < totalCount ? pageSize : totalCount - start;
- const data = OPTIONS.filter(option => option.label.match(search)).splice(
- start,
- deleteCount,
- );
+ const searchValue = search.trim().toLowerCase();
+ const optionFilterProps = ['label', 'value', 'gender'];
+ const data = OPTIONS.filter(option =>
+ optionFilterProps.some(prop => {
+ const optionProp = option?.[prop]
+ ? String(option[prop]).trim().toLowerCase()
+ : '';
+ return optionProp.includes(searchValue);
+ }),
+ ).splice(start, deleteCount);
return {
data,
totalCount: OPTIONS.length,
@@ -74,7 +80,7 @@ const defaultProps = {
allowClear: true,
ariaLabel: ARIA_LABEL,
labelInValue: true,
- options: OPTIONS,
+ options: loadOptions,
pageSize: 10,
showSearch: true,
};
@@ -129,17 +135,31 @@ test('displays a header', async () => {
expect(screen.getByText(headerText)).toBeInTheDocument();
});
-test('adds a new option if the value is not in the options', async () => {
- const { rerender } = render(
- <AsyncSelect {...defaultProps} options={[]} value={OPTIONS[0]} />,
+test('adds a new option if the value is not in the options, when options are
empty', async () => {
+ const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 }));
+ render(
+ <AsyncSelect {...defaultProps} options={loadOptions} value={OPTIONS[0]} />,
);
await open();
expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument();
+ const options = await findAllSelectOptions();
+ expect(options).toHaveLength(1);
+ options.forEach((option, i) =>
+ expect(option).toHaveTextContent(OPTIONS[i].label),
+ );
+});
- rerender(
- <AsyncSelect {...defaultProps} options={[OPTIONS[1]]} value={OPTIONS[0]}
/>,
+test('adds a new option if the value is not in the options, when options have
values', async () => {
+ const loadOptions = jest.fn(async () => ({
+ data: [OPTIONS[1]],
+ totalCount: 1,
+ }));
+ render(
+ <AsyncSelect {...defaultProps} options={loadOptions} value={OPTIONS[0]} />,
);
await open();
+ expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument();
+ expect(await findSelectOption(OPTIONS[1].label)).toBeInTheDocument();
const options = await findAllSelectOptions();
expect(options).toHaveLength(2);
options.forEach((option, i) =>
@@ -147,6 +167,20 @@ test('adds a new option if the value is not in the
options', async () => {
);
});
+test('does not add a new option if the value is already in the options', async
() => {
+ const loadOptions = jest.fn(async () => ({
+ data: [OPTIONS[0]],
+ totalCount: 1,
+ }));
+ render(
+ <AsyncSelect {...defaultProps} options={loadOptions} value={OPTIONS[0]} />,
+ );
+ await open();
+ expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument();
+ const options = await findAllSelectOptions();
+ expect(options).toHaveLength(1);
+});
+
test('inverts the selection', async () => {
render(<AsyncSelect {...defaultProps} invertSelection />);
await open();
@@ -155,8 +189,11 @@ test('inverts the selection', async () => {
});
test('sort the options by label if no sort comparator is provided', async ()
=> {
- const unsortedOptions = [...OPTIONS].sort(() => Math.random());
- render(<AsyncSelect {...defaultProps} options={unsortedOptions} />);
+ const loadUnsortedOptions = jest.fn(async () => ({
+ data: [...OPTIONS].sort(() => Math.random()),
+ totalCount: 2,
+ }));
+ render(<AsyncSelect {...defaultProps} options={loadUnsortedOptions} />);
await open();
const options = await findAllSelectOptions();
options.forEach((option, key) =>
@@ -250,20 +287,23 @@ test('searches for label or value', async () => {
render(<AsyncSelect {...defaultProps} />);
const search = option.value;
await type(search.toString());
+ expect(await findSelectOption(option.label)).toBeInTheDocument();
const options = await findAllSelectOptions();
expect(options.length).toBe(1);
expect(options[0]).toHaveTextContent(option.label);
});
test('search order exact and startWith match first', async () => {
- render(<AsyncSelect {...defaultProps} />);
+ render(<AsyncSelect {...defaultProps} options={loadOptions} />);
+ await open();
await type('Her');
+ expect(await findSelectOption('Guilherme')).toBeInTheDocument();
const options = await findAllSelectOptions();
expect(options.length).toBe(4);
- expect(options[0]?.textContent).toEqual('Her');
- expect(options[1]?.textContent).toEqual('Herme');
- expect(options[2]?.textContent).toEqual('Cher');
- expect(options[3]?.textContent).toEqual('Guilherme');
+ expect(options[0]).toHaveTextContent('Her');
+ expect(options[1]).toHaveTextContent('Herme');
+ expect(options[2]).toHaveTextContent('Cher');
+ expect(options[3]).toHaveTextContent('Guilherme');
});
test('ignores case when searching', async () => {
@@ -273,17 +313,16 @@ test('ignores case when searching', async () => {
});
test('same case should be ranked to the top', async () => {
- render(
- <AsyncSelect
- {...defaultProps}
- options={[
- { value: 'Cac' },
- { value: 'abac' },
- { value: 'acbc' },
- { value: 'CAc' },
- ]}
- />,
- );
+ const loadOptions = jest.fn(async () => ({
+ data: [
+ { value: 'Cac' },
+ { value: 'abac' },
+ { value: 'acbc' },
+ { value: 'CAc' },
+ ],
+ totalCount: 4,
+ }));
+ render(<AsyncSelect {...defaultProps} options={loadOptions} />);
await type('Ac');
const options = await findAllSelectOptions();
expect(options.length).toBe(4);
@@ -294,7 +333,7 @@ test('same case should be ranked to the top', async () => {
});
test('ignores special keys when searching', async () => {
- render(<AsyncSelect {...defaultProps} />);
+ render(<AsyncSelect {...defaultProps} options={loadOptions} />);
await type('{shift}');
expect(screen.queryByText(LOADING)).not.toBeInTheDocument();
});
@@ -303,11 +342,16 @@ test('searches for custom fields', async () => {
render(
<AsyncSelect {...defaultProps} optionFilterProps={['label', 'gender']} />,
);
+ await open();
await type('Liam');
+ // Liam is on the second page. need to wait to fetch options
+ expect(await findSelectOption('Liam')).toBeInTheDocument();
let options = await findAllSelectOptions();
expect(options.length).toBe(1);
expect(options[0]).toHaveTextContent('Liam');
await type('Female');
+ // Olivia is on the second page. need to wait to fetch options
+ expect(await findSelectOption('Olivia')).toBeInTheDocument();
options = await findAllSelectOptions();
expect(options.length).toBe(6);
expect(options[0]).toHaveTextContent('Ava');
@@ -317,7 +361,7 @@ test('searches for custom fields', async () => {
expect(options[4]).toHaveTextContent('Nikole');
expect(options[5]).toHaveTextContent('Olivia');
await type('1');
- expect(screen.getByText(NO_DATA)).toBeInTheDocument();
+ expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
});
test('removes duplicated values', async () => {
@@ -332,12 +376,15 @@ test('removes duplicated values', async () => {
});
test('renders a custom label', async () => {
- const options = [
- { label: 'John', value: 1, customLabel: <h1>John</h1> },
- { label: 'Liam', value: 2, customLabel: <h1>Liam</h1> },
- { label: 'Olivia', value: 3, customLabel: <h1>Olivia</h1> },
- ];
- render(<AsyncSelect {...defaultProps} options={options} />);
+ const loadOptions = jest.fn(async () => ({
+ data: [
+ { label: 'John', value: 1, customLabel: <h1>John</h1> },
+ { label: 'Liam', value: 2, customLabel: <h1>Liam</h1> },
+ { label: 'Olivia', value: 3, customLabel: <h1>Olivia</h1> },
+ ],
+ totalCount: 3,
+ }));
+ render(<AsyncSelect {...defaultProps} options={loadOptions} />);
await open();
expect(screen.getByRole('heading', { name: 'John' })).toBeInTheDocument();
expect(screen.getByRole('heading', { name: 'Liam' })).toBeInTheDocument();
@@ -345,12 +392,15 @@ test('renders a custom label', async () => {
});
test('searches for a word with a custom label', async () => {
- const options = [
- { label: 'John', value: 1, customLabel: <h1>John</h1> },
- { label: 'Liam', value: 2, customLabel: <h1>Liam</h1> },
- { label: 'Olivia', value: 3, customLabel: <h1>Olivia</h1> },
- ];
- render(<AsyncSelect {...defaultProps} options={options} />);
+ const loadOptions = jest.fn(async () => ({
+ data: [
+ { label: 'John', value: 1, customLabel: <h1>John</h1> },
+ { label: 'Liam', value: 2, customLabel: <h1>Liam</h1> },
+ { label: 'Olivia', value: 3, customLabel: <h1>Olivia</h1> },
+ ],
+ totalCount: 3,
+ }));
+ render(<AsyncSelect {...defaultProps} options={loadOptions} />);
await type('Liam');
const selectOptions = await findAllSelectOptions();
expect(selectOptions.length).toBe(1);
@@ -391,7 +441,11 @@ test('does not add a new option if allowNewOptions is
false', async () => {
});
test('adds the null option when selected in single mode', async () => {
- render(<AsyncSelect {...defaultProps} options={[OPTIONS[0], NULL_OPTION]}
/>);
+ const loadOptions = jest.fn(async () => ({
+ data: [OPTIONS[0], NULL_OPTION],
+ totalCount: 2,
+ }));
+ render(<AsyncSelect {...defaultProps} options={loadOptions} />);
await open();
userEvent.click(await findSelectOption(NULL_OPTION.label));
const values = await findAllSelectValues();
@@ -399,12 +453,12 @@ test('adds the null option when selected in single mode',
async () => {
});
test('adds the null option when selected in multiple mode', async () => {
+ const loadOptions = jest.fn(async () => ({
+ data: [OPTIONS[0], NULL_OPTION],
+ totalCount: 2,
+ }));
render(
- <AsyncSelect
- {...defaultProps}
- options={[OPTIONS[0], NULL_OPTION, OPTIONS[2]]}
- mode="multiple"
- />,
+ <AsyncSelect {...defaultProps} options={loadOptions} mode="multiple" />,
);
await open();
userEvent.click(await findSelectOption(OPTIONS[0].label));
diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx
b/superset-frontend/src/components/Select/AsyncSelect.tsx
index 98f146f15f..b95f2d8f0d 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.tsx
@@ -55,7 +55,6 @@ type PickedSelectProps = Pick<
| 'autoFocus'
| 'disabled'
| 'filterOption'
- | 'labelInValue'
| 'loading'
| 'notFoundContent'
| 'onChange'
@@ -129,11 +128,10 @@ export interface AsyncSelectProps extends
PickedSelectProps {
optionFilterProps?: string[];
/**
* It defines the options of the Select.
- * The options can be static, an array of options.
- * The options can also be async, a promise that returns
+ * The options are async, a promise that returns
* an array of options.
*/
- options: OptionsType | OptionsPagePromise;
+ options: OptionsPagePromise;
/**
* It defines how many results should be included
* in the query response.
@@ -299,7 +297,6 @@ const AsyncSelect = (
filterOption = true,
header = null,
invertSelection = false,
- labelInValue = false,
lazyLoading = true,
loading,
mode = 'single',
@@ -322,9 +319,7 @@ const AsyncSelect = (
}: AsyncSelectProps,
ref: RefObject<AsyncSelectRef>,
) => {
- const isAsync = typeof options === 'function';
const isSingleMode = mode === 'single';
- const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch;
const [selectValue, setSelectValue] = useState(value);
const [inputValue, setInputValue] = useState('');
const [isLoading, setIsLoading] = useState(loading);
@@ -360,8 +355,8 @@ const AsyncSelect = (
sortSelectedFirst(a, b) ||
// Only apply the custom sorter in async mode because we should
// preserve the options order as much as possible.
- (isAsync ? sortComparator(a, b, '') : 0),
- [isAsync, sortComparator, sortSelectedFirst],
+ sortComparator(a, b, ''),
+ [sortComparator, sortSelectedFirst],
);
const initialOptions = useMemo(
@@ -528,7 +523,6 @@ const AsyncSelect = (
setSelectOptions(newOptions);
}
if (
- isAsync &&
!allValuesLoaded &&
loadingEnabled &&
!fetchedQueries.current.has(getQueryCacheKey(searchValue, 0, pageSize))
@@ -546,7 +540,7 @@ const AsyncSelect = (
vScroll.scrollTop > (vScroll.scrollHeight - vScroll.offsetHeight) * 0.7;
const hasMoreData = page * pageSize + pageSize < totalCount;
- if (!isLoading && isAsync && hasMoreData && thresholdReached) {
+ if (!isLoading && hasMoreData && thresholdReached) {
const newPage = page + 1;
fetchPage(inputValue, newPage);
}
@@ -575,30 +569,26 @@ const AsyncSelect = (
const handleOnDropdownVisibleChange = (isDropdownVisible: boolean) => {
setIsDropdownVisible(isDropdownVisible);
- if (isAsync) {
- // loading is enabled when dropdown is open,
- // disabled when dropdown is closed
- if (loadingEnabled !== isDropdownVisible) {
- setLoadingEnabled(isDropdownVisible);
- }
- // when closing dropdown, always reset loading state
- if (!isDropdownVisible && isLoading) {
- // delay is for the animation of closing the dropdown
- // so the dropdown doesn't flash between "Loading..." and "No data"
- // before closing.
- setTimeout(() => {
- setIsLoading(false);
- }, 250);
- }
+ // loading is enabled when dropdown is open,
+ // disabled when dropdown is closed
+ if (loadingEnabled !== isDropdownVisible) {
+ setLoadingEnabled(isDropdownVisible);
+ }
+ // when closing dropdown, always reset loading state
+ if (!isDropdownVisible && isLoading) {
+ // delay is for the animation of closing the dropdown
+ // so the dropdown doesn't flash between "Loading..." and "No data"
+ // before closing.
+ setTimeout(() => {
+ setIsLoading(false);
+ }, 250);
}
// if no search input value, force sort options because it won't be sorted
by
// `filterSort`.
if (isDropdownVisible && !inputValue && selectOptions.length > 1) {
- const sortedOptions = isAsync
- ? selectOptions.slice().sort(sortComparatorForNoSearch)
- : // if not in async mode, revert to the original select options
- // (with selected options still sorted to the top)
- initialOptionsSorted;
+ const sortedOptions = selectOptions
+ .slice()
+ .sort(sortComparatorForNoSearch);
if (!isEqual(sortedOptions, selectOptions)) {
setSelectOptions(sortedOptions);
}
@@ -627,7 +617,7 @@ const AsyncSelect = (
if (isLoading) {
return <StyledSpin size="small" />;
}
- if (shouldShowSearch && isDropdownVisible) {
+ if (showSearch && isDropdownVisible) {
return <SearchOutlined />;
}
return <DownOutlined />;
@@ -660,7 +650,7 @@ const AsyncSelect = (
);
useEffect(() => {
- if (isAsync && loadingEnabled && allowFetch) {
+ if (loadingEnabled && allowFetch) {
// trigger fetch every time inputValue changes
if (inputValue) {
debouncedFetchPage(inputValue, 0);
@@ -668,14 +658,7 @@ const AsyncSelect = (
fetchPage('', 0);
}
}
- }, [
- isAsync,
- loadingEnabled,
- fetchPage,
- allowFetch,
- inputValue,
- debouncedFetchPage,
- ]);
+ }, [loadingEnabled, fetchPage, allowFetch, inputValue, debouncedFetchPage]);
useEffect(() => {
if (loading !== undefined && loading !== isLoading) {
@@ -706,20 +689,20 @@ const AsyncSelect = (
getPopupContainer={
getPopupContainer || (triggerNode => triggerNode.parentNode)
}
- labelInValue={isAsync || labelInValue}
+ labelInValue
maxTagCount={MAX_TAG_COUNT}
mode={mappedMode}
notFoundContent={isLoading ? t('Loading...') : notFoundContent}
onDeselect={handleOnDeselect}
onDropdownVisibleChange={handleOnDropdownVisibleChange}
- onPopupScroll={isAsync ? handlePagination : undefined}
- onSearch={shouldShowSearch ? handleOnSearch : undefined}
+ onPopupScroll={handlePagination}
+ onSearch={showSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect}
onClear={handleClear}
onChange={onChange}
options={hasCustomLabels ? undefined : fullSelectOptions}
placeholder={placeholder}
- showSearch={shouldShowSearch}
+ showSearch={showSearch}
showArrow
tokenSeparators={tokenSeparators || TOKEN_SEPARATORS}
value={selectValue}
diff --git a/superset-frontend/src/components/Select/Select.stories.tsx
b/superset-frontend/src/components/Select/Select.stories.tsx
index efcd91c0c3..b75e1ff28b 100644
--- a/superset-frontend/src/components/Select/Select.stories.tsx
+++ b/superset-frontend/src/components/Select/Select.stories.tsx
@@ -16,11 +16,22 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React, { ReactNode, useState, useCallback, useRef } from 'react';
+import React, {
+ ReactNode,
+ useState,
+ useCallback,
+ useRef,
+ useMemo,
+} from 'react';
import Button from 'src/components/Button';
import ControlHeader from 'src/explore/components/ControlHeader';
-import AsyncSelect, { AsyncSelectProps, AsyncSelectRef } from './AsyncSelect';
-import Select, { SelectProps, OptionsTypePage, OptionsType } from './Select';
+import AsyncSelect, {
+ AsyncSelectProps,
+ AsyncSelectRef,
+ OptionsTypePage,
+} from './AsyncSelect';
+
+import Select, { SelectProps, OptionsType } from './Select';
export default {
title: 'Select',
@@ -452,6 +463,11 @@ export const AsynchronousSelect = ({
reject(new Error('Error while fetching the names from the server'));
});
+ const initialValue = useMemo(
+ () => ({ label: 'Valentina', value: 'Valentina' }),
+ [],
+ );
+
return (
<>
<div
@@ -465,11 +481,7 @@ export const AsynchronousSelect = ({
fetchOnlyOnSearch={fetchOnlyOnSearch}
options={withError ? fetchUserListError : fetchUserListPage}
placeholder={fetchOnlyOnSearch ? 'Type anything' : 'AsyncSelect...'}
- value={
- withInitialValue
- ? { label: 'Valentina', value: 'Valentina' }
- : undefined
- }
+ value={withInitialValue ? initialValue : undefined}
/>
</div>
<div
diff --git a/superset-frontend/src/components/Select/Select.test.tsx
b/superset-frontend/src/components/Select/Select.test.tsx
index 0b353bde38..18111f30ca 100644
--- a/superset-frontend/src/components/Select/Select.test.tsx
+++ b/superset-frontend/src/components/Select/Select.test.tsx
@@ -114,17 +114,24 @@ test('displays a header', async () => {
expect(screen.getByText(headerText)).toBeInTheDocument();
});
-test('adds a new option if the value is not in the options', async () => {
- const { rerender } = render(
- <Select {...defaultProps} options={[]} value={OPTIONS[0]} />,
- );
+test('adds a new option if the value is not in the options, when options are
empty', async () => {
+ render(<Select {...defaultProps} options={[]} value={OPTIONS[0]} />);
await open();
expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument();
+ const options = await findAllSelectOptions();
+ expect(options).toHaveLength(1);
+ options.forEach((option, i) =>
+ expect(option).toHaveTextContent(OPTIONS[i].label),
+ );
+});
- rerender(
+test('adds a new option if the value is not in the options, when options have
values', async () => {
+ render(
<Select {...defaultProps} options={[OPTIONS[1]]} value={OPTIONS[0]} />,
);
await open();
+ expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument();
+ expect(await findSelectOption(OPTIONS[1].label)).toBeInTheDocument();
const options = await findAllSelectOptions();
expect(options).toHaveLength(2);
options.forEach((option, i) =>
@@ -132,6 +139,16 @@ test('adds a new option if the value is not in the
options', async () => {
);
});
+test('does not add a new option if the value is already in the options', async
() => {
+ render(
+ <Select {...defaultProps} options={[OPTIONS[0]]} value={OPTIONS[0]} />,
+ );
+ await open();
+ expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument();
+ const options = await findAllSelectOptions();
+ expect(options).toHaveLength(1);
+});
+
test('inverts the selection', async () => {
render(<Select {...defaultProps} invertSelection />);
await open();
diff --git a/superset-frontend/src/components/Select/Select.tsx
b/superset-frontend/src/components/Select/Select.tsx
index 9cea5a726d..04eccec83a 100644
--- a/superset-frontend/src/components/Select/Select.tsx
+++ b/superset-frontend/src/components/Select/Select.tsx
@@ -21,13 +21,10 @@ import React, {
ReactElement,
ReactNode,
RefObject,
- UIEvent,
useEffect,
useMemo,
useState,
- useRef,
useCallback,
- useImperativeHandle,
} from 'react';
import { ensureIsArray, styled, t } from '@superset-ui/core';
import AntdSelect, {
@@ -37,11 +34,8 @@ import AntdSelect, {
} from 'antd/lib/select';
import { DownOutlined, SearchOutlined } from '@ant-design/icons';
import { Spin } from 'antd';
-import debounce from 'lodash/debounce';
import { isEqual } from 'lodash';
import Icons from 'src/components/Icons';
-import { getClientErrorObject } from 'src/utils/getClientErrorObject';
-import { SLOW_DEBOUNCE } from 'src/constants';
import { rankedSearchCompare } from 'src/utils/rankedSearchCompare';
import { getValue, hasOption, isLabeledValue } from './utils';
@@ -72,19 +66,6 @@ type PickedSelectProps = Pick<
export type OptionsType = Exclude<AntdSelectAllProps['options'], undefined>;
-export type OptionsTypePage = {
- data: OptionsType;
- totalCount: number;
-};
-
-export type OptionsPagePromise = (
- search: string,
- page: number,
- pageSize: number,
-) => Promise<OptionsTypePage>;
-
-export type SelectRef = HTMLInputElement & { clearCache: () => void };
-
export interface SelectProps extends PickedSelectProps {
/**
* It enables the user to create new options.
@@ -103,13 +84,6 @@ export interface SelectProps extends PickedSelectProps {
* Can be any ReactNode.
*/
header?: ReactNode;
- /**
- * It fires a request against the server after
- * the first interaction and not on render.
- * Works in async mode only (See the options property).
- * True by default.
- */
- lazyLoading?: boolean;
/**
* It defines whether the Select should allow for the
* selection of multiple options or single.
@@ -133,13 +107,7 @@ export interface SelectProps extends PickedSelectProps {
* The options can also be async, a promise that returns
* an array of options.
*/
- options: OptionsType | OptionsPagePromise;
- /**
- * It defines how many results should be included
- * in the query response.
- * Works in async mode only (See the options property).
- */
- pageSize?: number;
+ options: OptionsType;
/**
* It shows a stop-outlined icon at the far right of a selected
* option instead of the default checkmark.
@@ -148,19 +116,6 @@ export interface SelectProps extends PickedSelectProps {
* False by default.
*/
invertSelection?: boolean;
- /**
- * It fires a request against the server only after
- * searching.
- * Works in async mode only (See the options property).
- * Undefined by default.
- */
- fetchOnlyOnSearch?: boolean;
- /**
- * It provides a callback function when an error
- * is generated after a request is fired.
- * Works in async mode only (See the options property).
- */
- onError?: (error: string) => void;
/**
* Customize how filtered options are sorted while users search.
* Will not apply to predefined `options` array when users are not searching.
@@ -195,25 +150,6 @@ const StyledCheckOutlined = styled(Icons.CheckOutlined)`
vertical-align: 0;
`;
-const StyledError = styled.div`
- ${({ theme }) => `
- display: flex;
- justify-content: center;
- align-items: flex-start;
- width: 100%;
- padding: ${theme.gridUnit * 2}px;
- color: ${theme.colors.error.base};
- & svg {
- margin-right: ${theme.gridUnit * 2}px;
- }
- `}
-`;
-
-const StyledErrorMessage = styled.div`
- overflow: hidden;
- text-overflow: ellipsis;
-`;
-
const StyledSpin = styled(Spin)`
margin-top: ${({ theme }) => -theme.gridUnit}px;
`;
@@ -228,15 +164,8 @@ const StyledLoadingText = styled.div`
const MAX_TAG_COUNT = 4;
const TOKEN_SEPARATORS = [',', '\n', '\t', ';'];
-const DEFAULT_PAGE_SIZE = 100;
const EMPTY_OPTIONS: OptionsType = [];
-const Error = ({ error }: { error: string }) => (
- <StyledError>
- <Icons.ErrorSolid /> <StyledErrorMessage>{error}</StyledErrorMessage>
- </StyledError>
-);
-
export const DEFAULT_SORT_COMPARATOR = (
a: AntdLabeledValue,
b: AntdLabeledValue,
@@ -273,9 +202,6 @@ export const propertyComparator =
return (a[property] as number) - (b[property] as number);
};
-const getQueryCacheKey = (value: string, page: number, pageSize: number) =>
- `${value};${page};${pageSize}`;
-
/**
* This component is a customized version of the Antdesign 4.X Select component
* https://ant.design/components/select/.
@@ -295,23 +221,19 @@ const Select = (
allowClear,
allowNewOptions = false,
ariaLabel,
- fetchOnlyOnSearch,
filterOption = true,
header = null,
invertSelection = false,
labelInValue = false,
- lazyLoading = true,
loading,
mode = 'single',
name,
notFoundContent,
- onError,
onChange,
onClear,
onDropdownVisibleChange,
optionFilterProps = ['label', 'value'],
options,
- pageSize = DEFAULT_PAGE_SIZE,
placeholder = t('Select ...'),
showSearch = true,
sortComparator = DEFAULT_SORT_COMPARATOR,
@@ -320,27 +242,19 @@ const Select = (
getPopupContainer,
...props
}: SelectProps,
- ref: RefObject<SelectRef>,
+ ref: RefObject<HTMLInputElement>,
) => {
- const isAsync = typeof options === 'function';
const isSingleMode = mode === 'single';
- const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch;
+ const shouldShowSearch = allowNewOptions ? true : showSearch;
const [selectValue, setSelectValue] = useState(value);
const [inputValue, setInputValue] = useState('');
const [isLoading, setIsLoading] = useState(loading);
- const [error, setError] = useState('');
const [isDropdownVisible, setIsDropdownVisible] = useState(false);
- const [page, setPage] = useState(0);
- const [totalCount, setTotalCount] = useState(0);
- const [loadingEnabled, setLoadingEnabled] = useState(!lazyLoading);
- const [allValuesLoaded, setAllValuesLoaded] = useState(false);
- const fetchedQueries = useRef(new Map<string, number>());
const mappedMode = isSingleMode
? undefined
: allowNewOptions
? 'tags'
: 'multiple';
- const allowFetch = !fetchOnlyOnSearch || inputValue;
const sortSelectedFirst = useCallback(
(a: AntdLabeledValue, b: AntdLabeledValue) =>
@@ -355,22 +269,14 @@ const Select = (
sortSelectedFirst(a, b) || sortComparator(a, b, inputValue),
[inputValue, sortComparator, sortSelectedFirst],
);
- const sortComparatorForNoSearch = useCallback(
- (a: AntdLabeledValue, b: AntdLabeledValue) =>
- sortSelectedFirst(a, b) ||
- // Only apply the custom sorter in async mode because we should
- // preserve the options order as much as possible.
- (isAsync ? sortComparator(a, b, '') : 0),
- [isAsync, sortComparator, sortSelectedFirst],
- );
const initialOptions = useMemo(
() => (options && Array.isArray(options) ? options.slice() :
EMPTY_OPTIONS),
[options],
);
const initialOptionsSorted = useMemo(
- () => initialOptions.slice().sort(sortComparatorForNoSearch),
- [initialOptions, sortComparatorForNoSearch],
+ () => initialOptions.slice().sort(sortSelectedFirst),
+ [initialOptions, sortSelectedFirst],
);
const [selectOptions, setSelectOptions] =
@@ -427,89 +333,6 @@ const Select = (
setInputValue('');
};
- const internalOnError = useCallback(
- (response: Response) =>
- getClientErrorObject(response).then(e => {
- const { error } = e;
- setError(error);
-
- if (onError) {
- onError(error);
- }
- }),
- [onError],
- );
-
- const mergeData = useCallback(
- (data: OptionsType) => {
- let mergedData: OptionsType = [];
- if (data && Array.isArray(data) && data.length) {
- // unique option values should always be case sensitive so don't
lowercase
- const dataValues = new Set(data.map(opt => opt.value));
- // merges with existing and creates unique options
- setSelectOptions(prevOptions => {
- mergedData = prevOptions
- .filter(previousOption => !dataValues.has(previousOption.value))
- .concat(data)
- .sort(sortComparatorForNoSearch);
- return mergedData;
- });
- }
- return mergedData;
- },
- [sortComparatorForNoSearch],
- );
-
- const fetchPage = useMemo(
- () => (search: string, page: number) => {
- setPage(page);
- if (allValuesLoaded) {
- setIsLoading(false);
- return;
- }
- const key = getQueryCacheKey(search, page, pageSize);
- const cachedCount = fetchedQueries.current.get(key);
- if (cachedCount !== undefined) {
- setTotalCount(cachedCount);
- setIsLoading(false);
- return;
- }
- setIsLoading(true);
- const fetchOptions = options as OptionsPagePromise;
- fetchOptions(search, page, pageSize)
- .then(({ data, totalCount }: OptionsTypePage) => {
- const mergedData = mergeData(data);
- fetchedQueries.current.set(key, totalCount);
- setTotalCount(totalCount);
- if (
- !fetchOnlyOnSearch &&
- value === '' &&
- mergedData.length >= totalCount
- ) {
- setAllValuesLoaded(true);
- }
- })
- .catch(internalOnError)
- .finally(() => {
- setIsLoading(false);
- });
- },
- [
- allValuesLoaded,
- fetchOnlyOnSearch,
- mergeData,
- internalOnError,
- options,
- pageSize,
- value,
- ],
- );
-
- const debouncedFetchPage = useMemo(
- () => debounce(fetchPage, SLOW_DEBOUNCE),
- [fetchPage],
- );
-
const handleOnSearch = (search: string) => {
const searchValue = search.trim();
if (allowNewOptions && isSingleMode) {
@@ -527,31 +350,9 @@ const Select = (
: cleanSelectOptions;
setSelectOptions(newOptions);
}
- if (
- isAsync &&
- !allValuesLoaded &&
- loadingEnabled &&
- !fetchedQueries.current.has(getQueryCacheKey(searchValue, 0, pageSize))
- ) {
- // if fetch only on search but search value is empty, then should not be
- // in loading state
- setIsLoading(!(fetchOnlyOnSearch && !searchValue));
- }
setInputValue(search);
};
- const handlePagination = (e: UIEvent<HTMLElement>) => {
- const vScroll = e.currentTarget;
- const thresholdReached =
- vScroll.scrollTop > (vScroll.scrollHeight - vScroll.offsetHeight) * 0.7;
- const hasMoreData = page * pageSize + pageSize < totalCount;
-
- if (!isLoading && isAsync && hasMoreData && thresholdReached) {
- const newPage = page + 1;
- fetchPage(inputValue, newPage);
- }
- };
-
const handleFilterOption = (search: string, option: AntdLabeledValue) => {
if (typeof filterOption === 'function') {
return filterOption(search, option);
@@ -575,35 +376,13 @@ const Select = (
const handleOnDropdownVisibleChange = (isDropdownVisible: boolean) => {
setIsDropdownVisible(isDropdownVisible);
- if (isAsync) {
- // loading is enabled when dropdown is open,
- // disabled when dropdown is closed
- if (loadingEnabled !== isDropdownVisible) {
- setLoadingEnabled(isDropdownVisible);
- }
- // when closing dropdown, always reset loading state
- if (!isDropdownVisible && isLoading) {
- // delay is for the animation of closing the dropdown
- // so the dropdown doesn't flash between "Loading..." and "No data"
- // before closing.
- setTimeout(() => {
- setIsLoading(false);
- }, 250);
- }
- }
// if no search input value, force sort options because it won't be sorted
by
// `filterSort`.
if (isDropdownVisible && !inputValue && selectOptions.length > 1) {
- const sortedOptions = isAsync
- ? selectOptions.slice().sort(sortComparatorForNoSearch)
- : // if not in async mode, revert to the original select options
- // (with selected options still sorted to the top)
- initialOptionsSorted;
- if (!isEqual(sortedOptions, selectOptions)) {
- setSelectOptions(sortedOptions);
+ if (!isEqual(initialOptionsSorted, selectOptions)) {
+ setSelectOptions(initialOptionsSorted);
}
}
-
if (onDropdownVisibleChange) {
onDropdownVisibleChange(isDropdownVisible);
}
@@ -618,7 +397,7 @@ const Select = (
if (isLoading && fullSelectOptions.length === 0) {
return <StyledLoadingText>{t('Loading...')}</StyledLoadingText>;
}
- return error ? <Error error={error} /> : originNode;
+ return originNode;
};
// use a function instead of component since every rerender of the
@@ -642,8 +421,6 @@ const Select = (
useEffect(() => {
// when `options` list is updated from component prop, reset states
- fetchedQueries.current.clear();
- setAllValuesLoaded(false);
setSelectOptions(initialOptions);
}, [initialOptions]);
@@ -651,49 +428,12 @@ const Select = (
setSelectValue(value);
}, [value]);
- // Stop the invocation of the debounced function after unmounting
- useEffect(
- () => () => {
- debouncedFetchPage.cancel();
- },
- [debouncedFetchPage],
- );
-
- useEffect(() => {
- if (isAsync && loadingEnabled && allowFetch) {
- // trigger fetch every time inputValue changes
- if (inputValue) {
- debouncedFetchPage(inputValue, 0);
- } else {
- fetchPage('', 0);
- }
- }
- }, [
- isAsync,
- loadingEnabled,
- fetchPage,
- allowFetch,
- inputValue,
- debouncedFetchPage,
- ]);
-
useEffect(() => {
if (loading !== undefined && loading !== isLoading) {
setIsLoading(loading);
}
}, [isLoading, loading]);
- const clearCache = () => fetchedQueries.current.clear();
-
- useImperativeHandle(
- ref,
- () => ({
- ...(ref.current as HTMLInputElement),
- clearCache,
- }),
- [ref],
- );
-
return (
<StyledContainer>
{header}
@@ -706,13 +446,13 @@ const Select = (
getPopupContainer={
getPopupContainer || (triggerNode => triggerNode.parentNode)
}
- labelInValue={isAsync || labelInValue}
+ labelInValue={labelInValue}
maxTagCount={MAX_TAG_COUNT}
mode={mappedMode}
notFoundContent={isLoading ? t('Loading...') : notFoundContent}
onDeselect={handleOnDeselect}
onDropdownVisibleChange={handleOnDropdownVisibleChange}
- onPopupScroll={isAsync ? handlePagination : undefined}
+ onPopupScroll={undefined}
onSearch={shouldShowSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect}
onClear={handleClear}
diff --git a/superset-frontend/src/components/TableSelector/index.tsx
b/superset-frontend/src/components/TableSelector/index.tsx
index d7b867347a..4c07e256cc 100644
--- a/superset-frontend/src/components/TableSelector/index.tsx
+++ b/superset-frontend/src/components/TableSelector/index.tsx
@@ -336,7 +336,6 @@ const TableSelector: FunctionComponent<TableSelectorProps>
= ({
filterOption={handleFilterOption}
header={header}
labelInValue
- lazyLoading={false}
loading={loadingTables}
name="select-table"
onChange={(options: TableOption | TableOption[]) =>
diff --git a/superset-frontend/src/filters/components/GroupBy/types.ts
b/superset-frontend/src/filters/components/GroupBy/types.ts
index 1e6d756497..5775edaa2f 100644
--- a/superset-frontend/src/filters/components/GroupBy/types.ts
+++ b/superset-frontend/src/filters/components/GroupBy/types.ts
@@ -23,7 +23,6 @@ import {
QueryFormData,
} from '@superset-ui/core';
import { RefObject } from 'react';
-import { SelectRef } from 'src/components/Select/Select';
import { PluginFilterHooks, PluginFilterStylesProps } from '../types';
interface PluginFilterGroupByCustomizeProps {
@@ -41,7 +40,7 @@ export type PluginFilterGroupByProps =
PluginFilterStylesProps & {
data: DataRecord[];
filterState: FilterState;
formData: PluginFilterGroupByQueryFormData;
- inputRef: RefObject<SelectRef>;
+ inputRef: RefObject<HTMLInputElement>;
} & PluginFilterHooks;
export const DEFAULT_FORM_DATA: PluginFilterGroupByCustomizeProps = {
diff --git a/superset-frontend/src/filters/components/TimeColumn/types.ts
b/superset-frontend/src/filters/components/TimeColumn/types.ts
index bc12cb8716..95b1e5edfd 100644
--- a/superset-frontend/src/filters/components/TimeColumn/types.ts
+++ b/superset-frontend/src/filters/components/TimeColumn/types.ts
@@ -23,7 +23,6 @@ import {
QueryFormData,
} from '@superset-ui/core';
import { RefObject } from 'react';
-import { SelectRef } from 'src/components/Select/Select';
import { PluginFilterHooks, PluginFilterStylesProps } from '../types';
interface PluginFilterTimeColumnCustomizeProps {
@@ -40,7 +39,7 @@ export type PluginFilterTimeColumnProps =
PluginFilterStylesProps & {
data: DataRecord[];
filterState: FilterState;
formData: PluginFilterTimeColumnQueryFormData;
- inputRef: RefObject<SelectRef>;
+ inputRef: RefObject<HTMLInputElement>;
} & PluginFilterHooks;
export const DEFAULT_FORM_DATA: PluginFilterTimeColumnCustomizeProps = {
diff --git a/superset-frontend/src/filters/components/TimeGrain/types.ts
b/superset-frontend/src/filters/components/TimeGrain/types.ts
index 64a7166573..3dfa9a99d8 100644
--- a/superset-frontend/src/filters/components/TimeGrain/types.ts
+++ b/superset-frontend/src/filters/components/TimeGrain/types.ts
@@ -18,7 +18,6 @@
*/
import { FilterState, QueryFormData, DataRecord } from '@superset-ui/core';
import { RefObject } from 'react';
-import { SelectRef } from 'src/components/Select/Select';
import { PluginFilterHooks, PluginFilterStylesProps } from '../types';
interface PluginFilterTimeGrainCustomizeProps {
@@ -34,7 +33,7 @@ export type PluginFilterTimeGrainProps =
PluginFilterStylesProps & {
data: DataRecord[];
filterState: FilterState;
formData: PluginFilterTimeGrainQueryFormData;
- inputRef: RefObject<SelectRef>;
+ inputRef: RefObject<HTMLInputElement>;
} & PluginFilterHooks;
export const DEFAULT_FORM_DATA: PluginFilterTimeGrainCustomizeProps = {
