You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2023/08/14 12:27:07 UTC
[superset] 05/06: fix: Duplicated options in Select when using numerical values (#24906)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit dd53b334d6b0103a26bad75535fbbdc061161f6f
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Fri Aug 11 14:22:15 2023 -0300
fix: Duplicated options in Select when using numerical values (#24906)
(cherry picked from commit b621ee92c9124e2e2f7c988302eb0f77f00c9fc9)
---
.../cypress-base/cypress/e2e/dashboard/utils.ts | 2 +-
.../cypress/e2e/explore/advanced_analytics.test.ts | 1 +
.../src/components/Select/AsyncSelect.stories.tsx | 34 ------
.../src/components/Select/AsyncSelect.test.tsx | 63 +++++++---
.../src/components/Select/AsyncSelect.tsx | 75 +++++++-----
.../src/components/Select/Select.stories.tsx | 5 +
.../src/components/Select/Select.test.tsx | 135 +++++++++++++++------
superset-frontend/src/components/Select/Select.tsx | 96 +++++++++------
8 files changed, 260 insertions(+), 151 deletions(-)
diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts
index 00d3eda45e..ca539039cf 100644
--- a/superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/utils.ts
@@ -322,7 +322,7 @@ export function applyNativeFilterValueWithIndex(index: number, value: string) {
cy.get(nativeFilters.filterFromDashboardView.filterValueInput)
.eq(index)
.should('exist', { timeout: 10000 })
- .type(`${value}{enter}`);
+ .type(`${value}{enter}`, { force: true });
// click the title to dismiss shown options
cy.get(nativeFilters.filterFromDashboardView.filterName)
.eq(index)
diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/advanced_analytics.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/advanced_analytics.test.ts
index fd207a64e3..8e52aa6c56 100644
--- a/superset-frontend/cypress-base/cypress/e2e/explore/advanced_analytics.test.ts
+++ b/superset-frontend/cypress-base/cypress/e2e/explore/advanced_analytics.test.ts
@@ -39,6 +39,7 @@ describe('Advanced analytics', () => {
cy.get('[data-test=time_compare]')
.find('input[type=search]')
+ .clear()
.type('1 year{enter}');
cy.get('button[data-test="run-query-button"]').click();
diff --git a/superset-frontend/src/components/Select/AsyncSelect.stories.tsx b/superset-frontend/src/components/Select/AsyncSelect.stories.tsx
index 0bdaf43f2c..4235008fb2 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.stories.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.stories.tsx
@@ -26,7 +26,6 @@ import React, {
import Button from 'src/components/Button';
import AsyncSelect from './AsyncSelect';
import {
- SelectOptionsType,
AsyncSelectProps,
AsyncSelectRef,
SelectOptionsTypePage,
@@ -39,40 +38,7 @@ export default {
const DEFAULT_WIDTH = 200;
-const options: SelectOptionsType = [
- {
- label: 'Such an incredibly awesome long long label',
- value: 'Such an incredibly awesome long long label',
- custom: 'Secret custom prop',
- },
- {
- label: 'Another incredibly awesome long long label',
- value: 'Another incredibly awesome long long label',
- },
- {
- label: 'JSX Label',
- customLabel: <div style={{ color: 'red' }}>JSX Label</div>,
- value: 'JSX Label',
- },
- { label: 'A', value: 'A' },
- { label: 'B', value: 'B' },
- { label: 'C', value: 'C' },
- { label: 'D', value: 'D' },
- { label: 'E', value: 'E' },
- { label: 'F', value: 'F' },
- { label: 'G', value: 'G' },
- { label: 'H', value: 'H' },
- { label: 'I', value: 'I' },
-];
-
const ARG_TYPES = {
- options: {
- defaultValue: options,
- description: `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 an array of options.
- `,
- },
ariaLabel: {
description: `It adds the aria-label tag for accessibility standards.
Must be plain English and localized.
diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
index 0b7cafab3a..b964f48ee7 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.test.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.test.tsx
@@ -17,7 +17,14 @@
* under the License.
*/
import React from 'react';
-import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
+import {
+ createEvent,
+ fireEvent,
+ render,
+ screen,
+ waitFor,
+ within,
+} from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { AsyncSelect } from 'src/components';
@@ -93,6 +100,9 @@ const getElementsByClassName = (className: string) =>
const getSelect = () => screen.getByRole('combobox', { name: ARIA_LABEL });
+const getAllSelectOptions = () =>
+ getElementsByClassName('.ant-select-item-option-content');
+
const findSelectOption = (text: string) =>
waitFor(() =>
within(getElementByClassName('.rc-virtual-list')).getByText(text),
@@ -323,12 +333,14 @@ test('same case should be ranked to the top', async () => {
}));
render(<AsyncSelect {...defaultProps} options={loadOptions} />);
await type('Ac');
- const options = await findAllSelectOptions();
- expect(options.length).toBe(4);
- expect(options[0]?.textContent).toEqual('acbc');
- expect(options[1]?.textContent).toEqual('CAc');
- expect(options[2]?.textContent).toEqual('abac');
- expect(options[3]?.textContent).toEqual('Cac');
+ await waitFor(() => {
+ const options = getAllSelectOptions();
+ expect(options.length).toBe(4);
+ expect(options[0]?.textContent).toEqual('acbc');
+ expect(options[1]?.textContent).toEqual('CAc');
+ expect(options[2]?.textContent).toEqual('abac');
+ expect(options[3]?.textContent).toEqual('Cac');
+ });
});
test('ignores special keys when searching', async () => {
@@ -365,7 +377,13 @@ test('searches for custom fields', async () => {
test('removes duplicated values', async () => {
render(<AsyncSelect {...defaultProps} mode="multiple" allowNewOptions />);
- await type('a,b,b,b,c,d,d');
+ const input = getElementByClassName('.ant-select-selection-search-input');
+ const paste = createEvent.paste(input, {
+ clipboardData: {
+ getData: () => 'a,b,b,b,c,d,d',
+ },
+ });
+ fireEvent(input, paste);
const values = await findAllSelectValues();
expect(values.length).toBe(4);
expect(values[0]).toHaveTextContent('a');
@@ -601,7 +619,9 @@ test('does not show "No data" when allowNewOptions is true and a new option is e
render(<AsyncSelect {...defaultProps} allowNewOptions />);
await open();
await type(NEW_OPTION);
- expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument();
+ await waitFor(() =>
+ expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument(),
+ );
});
test('sets a initial value in single mode', async () => {
@@ -690,12 +710,9 @@ test('does not fire a new request for the same search input', async () => {
<AsyncSelect {...defaultProps} options={loadOptions} fetchOnlyOnSearch />,
);
await type('search');
- expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
- expect(loadOptions).toHaveBeenCalledTimes(1);
- clearAll();
+ await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(1));
await type('search');
- expect(await screen.findByText(LOADING)).toBeInTheDocument();
- expect(loadOptions).toHaveBeenCalledTimes(1);
+ await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(1));
});
test('does not fire a new request if all values have been fetched', async () => {
@@ -823,6 +840,24 @@ test('does not fire onChange when searching but no selection', async () => {
expect(onChange).toHaveBeenCalledTimes(1);
});
+test('does not duplicate options when using numeric values', async () => {
+ render(
+ <AsyncSelect
+ {...defaultProps}
+ mode="multiple"
+ options={async () => ({
+ data: [
+ { label: '1', value: 1 },
+ { label: '2', value: 2 },
+ ],
+ totalCount: 2,
+ })}
+ />,
+ );
+ await type('1');
+ await waitFor(() => expect(getAllSelectOptions().length).toBe(1));
+});
+
/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx
index 3453ce2b5f..320d6ec3bd 100644
--- a/superset-frontend/src/components/Select/AsyncSelect.tsx
+++ b/superset-frontend/src/components/Select/AsyncSelect.tsx
@@ -27,14 +27,15 @@ import React, {
useRef,
useCallback,
useImperativeHandle,
+ ClipboardEvent,
} from 'react';
import { ensureIsArray, t, usePrevious } from '@superset-ui/core';
import { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
import debounce from 'lodash/debounce';
-import { isEqual } from 'lodash';
+import { isEqual, uniq } from 'lodash';
import Icons from 'src/components/Icons';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
-import { SLOW_DEBOUNCE } from 'src/constants';
+import { FAST_DEBOUNCE, SLOW_DEBOUNCE } from 'src/constants';
import {
getValue,
hasOption,
@@ -122,6 +123,7 @@ const AsyncSelect = forwardRef(
onClear,
onDropdownVisibleChange,
onDeselect,
+ onSearch,
onSelect,
optionFilterProps = ['label', 'value'],
options,
@@ -129,7 +131,7 @@ const AsyncSelect = forwardRef(
placeholder = t('Select ...'),
showSearch = true,
sortComparator = DEFAULT_SORT_COMPARATOR,
- tokenSeparators,
+ tokenSeparators = TOKEN_SEPARATORS,
value,
getPopupContainer,
oneLine,
@@ -150,11 +152,7 @@ const AsyncSelect = forwardRef(
const [allValuesLoaded, setAllValuesLoaded] = useState(false);
const selectValueRef = useRef(selectValue);
const fetchedQueries = useRef(new Map<string, number>());
- const mappedMode = isSingleMode
- ? undefined
- : allowNewOptions
- ? 'tags'
- : 'multiple';
+ const mappedMode = isSingleMode ? undefined : 'multiple';
const allowFetch = !fetchOnlyOnSearch || inputValue;
const [maxTagCount, setMaxTagCount] = useState(
propsMaxTagCount ?? MAX_TAG_COUNT,
@@ -253,6 +251,14 @@ const AsyncSelect = forwardRef(
const array = selectValue as (string | number)[];
setSelectValue(array.filter(element => element !== value));
}
+ // removes new option
+ if (option.isNewOption) {
+ setSelectOptions(
+ fullSelectOptions.filter(
+ option => getValue(option.value) !== getValue(value),
+ ),
+ );
+ }
}
fireOnChange();
onDeselect?.(value, option);
@@ -341,9 +347,9 @@ const AsyncSelect = forwardRef(
[fetchPage],
);
- const handleOnSearch = (search: string) => {
+ const handleOnSearch = debounce((search: string) => {
const searchValue = search.trim();
- if (allowNewOptions && isSingleMode) {
+ if (allowNewOptions) {
const newOption = searchValue &&
!hasOption(searchValue, fullSelectOptions, true) && {
label: searchValue,
@@ -368,7 +374,10 @@ const AsyncSelect = forwardRef(
setIsLoading(!(fetchOnlyOnSearch && !searchValue));
}
setInputValue(search);
- };
+ onSearch?.(searchValue);
+ }, FAST_DEBOUNCE);
+
+ useEffect(() => () => handleOnSearch.cancel(), [handleOnSearch]);
const handlePagination = (e: UIEvent<HTMLElement>) => {
const vScroll = e.currentTarget;
@@ -439,19 +448,7 @@ const AsyncSelect = forwardRef(
};
const handleOnBlur = (event: React.FocusEvent<HTMLElement>) => {
- const tagsMode = !isSingleMode && allowNewOptions;
- const searchValue = inputValue.trim();
- // Searched values will be autoselected during onBlur events when in tags mode.
- // We want to make sure a value is only selected if the user has actually selected it
- // by pressing Enter or clicking on it.
- if (
- tagsMode &&
- searchValue &&
- !hasOption(searchValue, selectValue, true)
- ) {
- // The search value will be added so we revert to the previous value
- setSelectValue(selectValue || []);
- }
+ setInputValue('');
onBlur?.(event);
};
@@ -526,6 +523,28 @@ const AsyncSelect = forwardRef(
[ref],
);
+ const onPaste = (e: ClipboardEvent<HTMLInputElement>) => {
+ const pastedText = e.clipboardData.getData('text');
+ if (isSingleMode) {
+ setSelectValue({ label: pastedText, value: pastedText });
+ } else {
+ const token = tokenSeparators.find(token => pastedText.includes(token));
+ const array = token ? uniq(pastedText.split(token)) : [pastedText];
+ setSelectValue(previous => [
+ ...((previous || []) as AntdLabeledValue[]),
+ ...array.map<AntdLabeledValue>(value => ({
+ label: value,
+ value,
+ })),
+ ]);
+ }
+ };
+
+ const shouldRenderChildrenOptions = useMemo(
+ () => hasCustomLabels(fullSelectOptions),
+ [fullSelectOptions],
+ );
+
return (
<StyledContainer headerPosition={headerPosition}>
{header && (
@@ -549,17 +568,17 @@ const AsyncSelect = forwardRef(
onBlur={handleOnBlur}
onDeselect={handleOnDeselect}
onDropdownVisibleChange={handleOnDropdownVisibleChange}
+ // @ts-ignore
+ onPaste={onPaste}
onPopupScroll={handlePagination}
onSearch={showSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect}
onClear={handleClear}
- options={
- hasCustomLabels(fullSelectOptions) ? undefined : fullSelectOptions
- }
+ options={shouldRenderChildrenOptions ? undefined : fullSelectOptions}
placeholder={placeholder}
showSearch={showSearch}
showArrow
- tokenSeparators={tokenSeparators || TOKEN_SEPARATORS}
+ tokenSeparators={tokenSeparators}
value={selectValue}
suffixIcon={getSuffixIcon(isLoading, showSearch, isDropdownVisible)}
menuItemSelectedIcon={
diff --git a/superset-frontend/src/components/Select/Select.stories.tsx b/superset-frontend/src/components/Select/Select.stories.tsx
index 58fa424700..d7a9b6e893 100644
--- a/superset-frontend/src/components/Select/Select.stories.tsx
+++ b/superset-frontend/src/components/Select/Select.stories.tsx
@@ -103,6 +103,11 @@ const ARG_TYPES = {
disable: true,
},
},
+ mappedMode: {
+ table: {
+ disable: true,
+ },
+ },
mode: {
description: `It defines whether the Select should allow for
the selection of multiple options or single. Single by default.
diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx
index bce753604f..2b204cec1c 100644
--- a/superset-frontend/src/components/Select/Select.test.tsx
+++ b/superset-frontend/src/components/Select/Select.test.tsx
@@ -17,7 +17,14 @@
* under the License.
*/
import React from 'react';
-import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
+import {
+ createEvent,
+ fireEvent,
+ render,
+ screen,
+ waitFor,
+ within,
+} from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import Select from 'src/components/Select/Select';
import { SELECT_ALL_VALUE } from './utils';
@@ -68,7 +75,6 @@ const defaultProps = {
ariaLabel: ARIA_LABEL,
labelInValue: true,
options: OPTIONS,
- pageSize: 10,
showSearch: true,
};
@@ -93,6 +99,9 @@ const querySelectOption = (text: string) =>
within(getElementByClassName('.rc-virtual-list')).queryByText(text),
);
+const getAllSelectOptions = () =>
+ getElementsByClassName('.ant-select-item-option-content');
+
const findAllSelectOptions = () =>
waitFor(() => getElementsByClassName('.ant-select-item-option-content'));
@@ -134,6 +143,11 @@ const clearTypedText = () => {
const open = () => waitFor(() => userEvent.click(getSelect()));
+const reopen = async () => {
+ await type('{esc}');
+ await open();
+};
+
test('displays a header', async () => {
const headerText = 'Header';
render(<Select {...defaultProps} header={headerText} />);
@@ -201,8 +215,7 @@ test('should sort selected to top when in single mode', async () => {
expect(await matchOrder(originalLabels)).toBe(true);
// order selected to top when reopen
- await type('{esc}');
- await open();
+ await reopen();
let labels = originalLabels.slice();
labels = labels.splice(1, 1).concat(labels);
expect(await matchOrder(labels)).toBe(true);
@@ -211,16 +224,14 @@ test('should sort selected to top when in single mode', async () => {
// original order
userEvent.click(await findSelectOption(originalLabels[5]));
await matchOrder(labels);
- await type('{esc}');
- await open();
+ await reopen();
labels = originalLabels.slice();
labels = labels.splice(5, 1).concat(labels);
expect(await matchOrder(labels)).toBe(true);
// should revert to original order
clearAll();
- await type('{esc}');
- await open();
+ await reopen();
expect(await matchOrder(originalLabels)).toBe(true);
});
@@ -235,8 +246,7 @@ test('should sort selected to the top when in multi mode', async () => {
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
).toBe(true);
- await type('{esc}');
- await open();
+ await reopen();
labels = labels.splice(2, 1).concat(labels);
expect(
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
@@ -244,8 +254,7 @@ test('should sort selected to the top when in multi mode', async () => {
await open();
userEvent.click(await findSelectOption(labels[5]));
- await type('{esc}');
- await open();
+ await reopen();
labels = [labels.splice(0, 1)[0], labels.splice(4, 1)[0]].concat(labels);
expect(
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
@@ -253,8 +262,7 @@ test('should sort selected to the top when in multi mode', async () => {
// should revert to original order
clearAll();
- await type('{esc}');
- await open();
+ await reopen();
expect(
await matchOrder([
selectAllOptionLabel(originalLabels.length),
@@ -276,12 +284,14 @@ test('searches for label or value', async () => {
test('search order exact and startWith match first', async () => {
render(<Select {...defaultProps} />);
await type('Her');
- 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');
+ await waitFor(() => {
+ const options = getAllSelectOptions();
+ 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');
+ });
});
test('ignores case when searching', async () => {
@@ -303,12 +313,14 @@ test('same case should be ranked to the top', async () => {
/>,
);
await type('Ac');
- const options = await findAllSelectOptions();
- expect(options.length).toBe(4);
- expect(options[0]?.textContent).toEqual('acbc');
- expect(options[1]?.textContent).toEqual('CAc');
- expect(options[2]?.textContent).toEqual('abac');
- expect(options[3]?.textContent).toEqual('Cac');
+ await waitFor(() => {
+ const options = getAllSelectOptions();
+ expect(options.length).toBe(4);
+ expect(options[0]?.textContent).toEqual('acbc');
+ expect(options[1]?.textContent).toEqual('CAc');
+ expect(options[2]?.textContent).toEqual('abac');
+ expect(options[3]?.textContent).toEqual('Cac');
+ });
});
test('ignores special keys when searching', async () => {
@@ -338,7 +350,13 @@ test('searches for custom fields', async () => {
test('removes duplicated values', async () => {
render(<Select {...defaultProps} mode="multiple" allowNewOptions />);
- await type('a,b,b,b,c,d,d');
+ const input = getElementByClassName('.ant-select-selection-search-input');
+ const paste = createEvent.paste(input, {
+ clipboardData: {
+ getData: () => 'a,b,b,b,c,d,d',
+ },
+ });
+ fireEvent(input, paste);
const values = await findAllSelectValues();
expect(values.length).toBe(4);
expect(values[0]).toHaveTextContent('a');
@@ -519,7 +537,9 @@ test('does not show "No data" when allowNewOptions is true and a new option is e
render(<Select {...defaultProps} allowNewOptions />);
await open();
await type(NEW_OPTION);
- expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument();
+ await waitFor(() =>
+ expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument(),
+ );
});
test('does not show "Loading..." when allowNewOptions is false and a new option is entered', async () => {
@@ -625,9 +645,11 @@ test('does not render "Select all" when searching', async () => {
render(<Select {...defaultProps} options={OPTIONS} mode="multiple" />);
await open();
await type('Select');
- expect(
- screen.queryByText(selectAllOptionLabel(OPTIONS.length)),
- ).not.toBeInTheDocument();
+ await waitFor(() =>
+ expect(
+ screen.queryByText(selectAllOptionLabel(OPTIONS.length)),
+ ).not.toBeInTheDocument(),
+ );
});
test('does not render "Select all" as one of the tags after selection', async () => {
@@ -707,6 +729,24 @@ test('deselecting a value also deselects "Select all"', async () => {
expect(values[0]).not.toHaveTextContent(selectAllOptionLabel(10));
});
+test('deselecting a new value also removes it from the options', async () => {
+ render(
+ <Select
+ {...defaultProps}
+ options={OPTIONS.slice(0, 10)}
+ mode="multiple"
+ allowNewOptions
+ />,
+ );
+ await open();
+ await type(NEW_OPTION);
+ expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
+ await type('{enter}');
+ clearTypedText();
+ userEvent.click(await findSelectOption(NEW_OPTION));
+ expect(await querySelectOption(NEW_OPTION)).not.toBeInTheDocument();
+});
+
test('selecting all values also selects "Select all"', async () => {
render(
<Select
@@ -805,10 +845,10 @@ test('"Select All" is checked when unchecking a newly added option and all the o
userEvent.click(await findSelectOption(selectAllOptionLabel(10)));
expect(await findSelectOption(selectAllOptionLabel(10))).toBeInTheDocument();
// add a new option
- await type(`${NEW_OPTION}{enter}`);
+ await type(NEW_OPTION);
+ expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
clearTypedText();
expect(await findSelectOption(selectAllOptionLabel(11))).toBeInTheDocument();
- expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
// select all should be selected
let values = await findAllCheckedValues();
expect(values[0]).toHaveTextContent(selectAllOptionLabel(11));
@@ -834,10 +874,18 @@ test('does not render "Select All" when there are 0 or 1 options', async () => {
allowNewOptions
/>,
);
+ await open();
expect(screen.queryByText(selectAllOptionLabel(1))).not.toBeInTheDocument();
- await type(`${NEW_OPTION}{enter}`);
- clearTypedText();
- expect(screen.queryByText(selectAllOptionLabel(2))).toBeInTheDocument();
+ rerender(
+ <Select
+ {...defaultProps}
+ options={OPTIONS.slice(0, 2)}
+ mode="multiple"
+ allowNewOptions
+ />,
+ );
+ await open();
+ expect(screen.getByText(selectAllOptionLabel(2))).toBeInTheDocument();
});
test('do not count unselected disabled options in "Select All"', async () => {
@@ -909,6 +957,21 @@ test('does not fire onChange when searching but no selection', async () => {
expect(onChange).toHaveBeenCalledTimes(1);
});
+test('does not duplicate options when using numeric values', async () => {
+ render(
+ <Select
+ {...defaultProps}
+ mode="multiple"
+ options={[
+ { label: '1', value: 1 },
+ { label: '2', value: 2 },
+ ]}
+ />,
+ );
+ await type('1');
+ await waitFor(() => expect(getAllSelectOptions().length).toBe(1));
+});
+
/*
TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available
diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx
index d9b022224c..770ef1df4a 100644
--- a/superset-frontend/src/components/Select/Select.tsx
+++ b/superset-frontend/src/components/Select/Select.tsx
@@ -24,6 +24,7 @@ import React, {
useMemo,
useState,
useCallback,
+ ClipboardEvent,
} from 'react';
import {
ensureIsArray,
@@ -33,7 +34,8 @@ import {
usePrevious,
} from '@superset-ui/core';
import AntdSelect, { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
-import { isEqual } from 'lodash';
+import { debounce, isEqual, uniq } from 'lodash';
+import { FAST_DEBOUNCE } from 'src/constants';
import {
getValue,
hasOption,
@@ -50,7 +52,7 @@ import {
mapOptions,
hasCustomLabels,
} from './utils';
-import { SelectOptionsType, SelectProps } from './types';
+import { RawValue, SelectOptionsType, SelectProps } from './types';
import {
StyledCheckOutlined,
StyledContainer,
@@ -103,13 +105,14 @@ const Select = forwardRef(
onClear,
onDropdownVisibleChange,
onDeselect,
+ onSearch,
onSelect,
optionFilterProps = ['label', 'value'],
options,
placeholder = t('Select ...'),
showSearch = true,
sortComparator = DEFAULT_SORT_COMPARATOR,
- tokenSeparators,
+ tokenSeparators = TOKEN_SEPARATORS,
value,
getPopupContainer,
oneLine,
@@ -141,11 +144,7 @@ const Select = forwardRef(
}
}, [isDropdownVisible, oneLine]);
- const mappedMode = isSingleMode
- ? undefined
- : allowNewOptions
- ? 'tags'
- : 'multiple';
+ const mappedMode = isSingleMode ? undefined : 'multiple';
const { Option } = AntdSelect;
@@ -167,8 +166,7 @@ const Select = forwardRef(
);
const initialOptions = useMemo(
- () =>
- options && Array.isArray(options) ? options.slice() : EMPTY_OPTIONS,
+ () => (Array.isArray(options) ? options.slice() : EMPTY_OPTIONS),
[options],
);
const initialOptionsSorted = useMemo(
@@ -210,13 +208,13 @@ const Select = forwardRef(
() =>
!isSingleMode &&
allowSelectAll &&
- selectOptions.length > 0 &&
+ fullSelectOptions.length > 0 &&
enabledOptions.length > 1 &&
!inputValue,
[
isSingleMode,
allowSelectAll,
- selectOptions.length,
+ fullSelectOptions.length,
enabledOptions.length,
inputValue,
],
@@ -295,24 +293,30 @@ const Select = forwardRef(
element => getValue(element) !== getValue(value),
);
// if this was not a new item, deselect select all option
- if (
- selectAllMode &&
- selectOptions.some(opt => opt.value === getValue(value))
- ) {
+ if (selectAllMode && !option.isNewOption) {
array = array.filter(
element => getValue(element) !== SELECT_ALL_VALUE,
);
}
setSelectValue(array);
+
+ // removes new option
+ if (option.isNewOption) {
+ setSelectOptions(
+ fullSelectOptions.filter(
+ option => getValue(option.value) !== getValue(value),
+ ),
+ );
+ }
}
}
fireOnChange();
onDeselect?.(value, option);
};
- const handleOnSearch = (search: string) => {
+ const handleOnSearch = debounce((search: string) => {
const searchValue = search.trim();
- if (allowNewOptions && isSingleMode) {
+ if (allowNewOptions) {
const newOption = searchValue &&
!hasOption(searchValue, fullSelectOptions, true) && {
label: searchValue,
@@ -328,7 +332,10 @@ const Select = forwardRef(
setSelectOptions(newOptions);
}
setInputValue(searchValue);
- };
+ onSearch?.(searchValue);
+ }, FAST_DEBOUNCE);
+
+ useEffect(() => () => handleOnSearch.cancel(), [handleOnSearch]);
const handleFilterOption = (search: string, option: AntdLabeledValue) =>
handleFilterOptionHelper(search, option, optionFilterProps, filterOption);
@@ -390,10 +397,7 @@ const Select = forwardRef(
setSelectValue(
labelInValue
? ([...ensureIsArray(value), selectAllOption] as AntdLabeledValue[])
- : ([
- ...ensureIsArray(value),
- SELECT_ALL_VALUE,
- ] as AntdLabeledValue[]),
+ : ([...ensureIsArray(value), SELECT_ALL_VALUE] as RawValue[]),
);
}
}, [labelInValue, selectAllEligible.length, selectAllEnabled, value]);
@@ -429,19 +433,7 @@ const Select = forwardRef(
);
const handleOnBlur = (event: React.FocusEvent<HTMLElement>) => {
- const tagsMode = !isSingleMode && allowNewOptions;
- const searchValue = inputValue.trim();
- // Searched values will be autoselected during onBlur events when in tags mode.
- // We want to make sure a value is only selected if the user has actually selected it
- // by pressing Enter or clicking on it.
- if (
- tagsMode &&
- searchValue &&
- !hasOption(searchValue, selectValue, true)
- ) {
- // The search value will be added so we revert to the previous value
- setSelectValue(selectValue || []);
- }
+ setInputValue('');
onBlur?.(event);
};
@@ -538,6 +530,32 @@ const Select = forwardRef(
actualMaxTagCount -= 1;
}
+ const onPaste = (e: ClipboardEvent<HTMLInputElement>) => {
+ const pastedText = e.clipboardData.getData('text');
+ if (isSingleMode) {
+ setSelectValue(
+ labelInValue ? { label: pastedText, value: pastedText } : pastedText,
+ );
+ } else {
+ const token = tokenSeparators.find(token => pastedText.includes(token));
+ const array = token ? uniq(pastedText.split(token)) : [pastedText];
+ if (labelInValue) {
+ setSelectValue(previous => [
+ ...((previous || []) as AntdLabeledValue[]),
+ ...array.map<AntdLabeledValue>(value => ({
+ label: value,
+ value,
+ })),
+ ]);
+ } else {
+ setSelectValue(previous => [
+ ...((previous || []) as string[]),
+ ...array,
+ ]);
+ }
+ }
+ };
+
return (
<StyledContainer headerPosition={headerPosition}>
{header && (
@@ -562,6 +580,8 @@ const Select = forwardRef(
onBlur={handleOnBlur}
onDeselect={handleOnDeselect}
onDropdownVisibleChange={handleOnDropdownVisibleChange}
+ // @ts-ignore
+ onPaste={onPaste}
onPopupScroll={undefined}
onSearch={shouldShowSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect}
@@ -569,7 +589,7 @@ const Select = forwardRef(
placeholder={placeholder}
showSearch={shouldShowSearch}
showArrow
- tokenSeparators={tokenSeparators || TOKEN_SEPARATORS}
+ tokenSeparators={tokenSeparators}
value={selectValue}
suffixIcon={getSuffixIcon(
isLoading,
@@ -583,7 +603,7 @@ const Select = forwardRef(
<StyledCheckOutlined iconSize="m" aria-label="check" />
)
}
- {...(!shouldRenderChildrenOptions && { options: fullSelectOptions })}
+ options={shouldRenderChildrenOptions ? undefined : fullSelectOptions}
oneLine={oneLine}
tagRender={customTagRender}
{...props}