fix: Duplicated options in Select when using numerical values (#24906)

This commit is contained in:
Michael S. Molina 2023-08-11 14:22:15 -03:00 committed by GitHub
parent a1e32dbfa6
commit b621ee92c9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 260 additions and 151 deletions

View File

@ -322,7 +322,7 @@ export function applyNativeFilterValueWithIndex(index: number, value: string) {
cy.get(nativeFilters.filterFromDashboardView.filterValueInput) cy.get(nativeFilters.filterFromDashboardView.filterValueInput)
.eq(index) .eq(index)
.should('exist', { timeout: 10000 }) .should('exist', { timeout: 10000 })
.type(`${value}{enter}`); .type(`${value}{enter}`, { force: true });
// click the title to dismiss shown options // click the title to dismiss shown options
cy.get(nativeFilters.filterFromDashboardView.filterName) cy.get(nativeFilters.filterFromDashboardView.filterName)
.eq(index) .eq(index)

View File

@ -39,6 +39,7 @@ describe('Advanced analytics', () => {
cy.get('[data-test=time_compare]') cy.get('[data-test=time_compare]')
.find('input[type=search]') .find('input[type=search]')
.clear()
.type('1 year{enter}'); .type('1 year{enter}');
cy.get('button[data-test="run-query-button"]').click(); cy.get('button[data-test="run-query-button"]').click();

View File

@ -26,7 +26,6 @@ import React, {
import Button from 'src/components/Button'; import Button from 'src/components/Button';
import AsyncSelect from './AsyncSelect'; import AsyncSelect from './AsyncSelect';
import { import {
SelectOptionsType,
AsyncSelectProps, AsyncSelectProps,
AsyncSelectRef, AsyncSelectRef,
SelectOptionsTypePage, SelectOptionsTypePage,
@ -39,40 +38,7 @@ export default {
const DEFAULT_WIDTH = 200; 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 = { 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: { ariaLabel: {
description: `It adds the aria-label tag for accessibility standards. description: `It adds the aria-label tag for accessibility standards.
Must be plain English and localized. Must be plain English and localized.

View File

@ -17,7 +17,14 @@
* under the License. * under the License.
*/ */
import React from 'react'; 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 userEvent from '@testing-library/user-event';
import { AsyncSelect } from 'src/components'; import { AsyncSelect } from 'src/components';
@ -93,6 +100,9 @@ const getElementsByClassName = (className: string) =>
const getSelect = () => screen.getByRole('combobox', { name: ARIA_LABEL }); const getSelect = () => screen.getByRole('combobox', { name: ARIA_LABEL });
const getAllSelectOptions = () =>
getElementsByClassName('.ant-select-item-option-content');
const findSelectOption = (text: string) => const findSelectOption = (text: string) =>
waitFor(() => waitFor(() =>
within(getElementByClassName('.rc-virtual-list')).getByText(text), 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} />); render(<AsyncSelect {...defaultProps} options={loadOptions} />);
await type('Ac'); await type('Ac');
const options = await findAllSelectOptions(); await waitFor(() => {
expect(options.length).toBe(4); const options = getAllSelectOptions();
expect(options[0]?.textContent).toEqual('acbc'); expect(options.length).toBe(4);
expect(options[1]?.textContent).toEqual('CAc'); expect(options[0]?.textContent).toEqual('acbc');
expect(options[2]?.textContent).toEqual('abac'); expect(options[1]?.textContent).toEqual('CAc');
expect(options[3]?.textContent).toEqual('Cac'); expect(options[2]?.textContent).toEqual('abac');
expect(options[3]?.textContent).toEqual('Cac');
});
}); });
test('ignores special keys when searching', async () => { test('ignores special keys when searching', async () => {
@ -365,7 +377,13 @@ test('searches for custom fields', async () => {
test('removes duplicated values', async () => { test('removes duplicated values', async () => {
render(<AsyncSelect {...defaultProps} mode="multiple" allowNewOptions />); 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(); const values = await findAllSelectValues();
expect(values.length).toBe(4); expect(values.length).toBe(4);
expect(values[0]).toHaveTextContent('a'); 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 />); render(<AsyncSelect {...defaultProps} allowNewOptions />);
await open(); await open();
await type(NEW_OPTION); 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 () => { 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 />, <AsyncSelect {...defaultProps} options={loadOptions} fetchOnlyOnSearch />,
); );
await type('search'); await type('search');
expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(1));
expect(loadOptions).toHaveBeenCalledTimes(1);
clearAll();
await type('search'); await type('search');
expect(await screen.findByText(LOADING)).toBeInTheDocument(); await waitFor(() => expect(loadOptions).toHaveBeenCalledTimes(1));
expect(loadOptions).toHaveBeenCalledTimes(1);
}); });
test('does not fire a new request if all values have been fetched', async () => { 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); 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. TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available - Fetches more data when scrolling and more data is available

View File

@ -27,14 +27,15 @@ import React, {
useRef, useRef,
useCallback, useCallback,
useImperativeHandle, useImperativeHandle,
ClipboardEvent,
} from 'react'; } from 'react';
import { ensureIsArray, t, usePrevious } from '@superset-ui/core'; import { ensureIsArray, t, usePrevious } from '@superset-ui/core';
import { LabeledValue as AntdLabeledValue } from 'antd/lib/select'; import { LabeledValue as AntdLabeledValue } from 'antd/lib/select';
import debounce from 'lodash/debounce'; import debounce from 'lodash/debounce';
import { isEqual } from 'lodash'; import { isEqual, uniq } from 'lodash';
import Icons from 'src/components/Icons'; import Icons from 'src/components/Icons';
import { getClientErrorObject } from 'src/utils/getClientErrorObject'; import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import { SLOW_DEBOUNCE } from 'src/constants'; import { FAST_DEBOUNCE, SLOW_DEBOUNCE } from 'src/constants';
import { import {
getValue, getValue,
hasOption, hasOption,
@ -122,6 +123,7 @@ const AsyncSelect = forwardRef(
onClear, onClear,
onDropdownVisibleChange, onDropdownVisibleChange,
onDeselect, onDeselect,
onSearch,
onSelect, onSelect,
optionFilterProps = ['label', 'value'], optionFilterProps = ['label', 'value'],
options, options,
@ -129,7 +131,7 @@ const AsyncSelect = forwardRef(
placeholder = t('Select ...'), placeholder = t('Select ...'),
showSearch = true, showSearch = true,
sortComparator = DEFAULT_SORT_COMPARATOR, sortComparator = DEFAULT_SORT_COMPARATOR,
tokenSeparators, tokenSeparators = TOKEN_SEPARATORS,
value, value,
getPopupContainer, getPopupContainer,
oneLine, oneLine,
@ -150,11 +152,7 @@ const AsyncSelect = forwardRef(
const [allValuesLoaded, setAllValuesLoaded] = useState(false); const [allValuesLoaded, setAllValuesLoaded] = useState(false);
const selectValueRef = useRef(selectValue); const selectValueRef = useRef(selectValue);
const fetchedQueries = useRef(new Map<string, number>()); const fetchedQueries = useRef(new Map<string, number>());
const mappedMode = isSingleMode const mappedMode = isSingleMode ? undefined : 'multiple';
? undefined
: allowNewOptions
? 'tags'
: 'multiple';
const allowFetch = !fetchOnlyOnSearch || inputValue; const allowFetch = !fetchOnlyOnSearch || inputValue;
const [maxTagCount, setMaxTagCount] = useState( const [maxTagCount, setMaxTagCount] = useState(
propsMaxTagCount ?? MAX_TAG_COUNT, propsMaxTagCount ?? MAX_TAG_COUNT,
@ -253,6 +251,14 @@ const AsyncSelect = forwardRef(
const array = selectValue as (string | number)[]; const array = selectValue as (string | number)[];
setSelectValue(array.filter(element => element !== value)); setSelectValue(array.filter(element => element !== value));
} }
// removes new option
if (option.isNewOption) {
setSelectOptions(
fullSelectOptions.filter(
option => getValue(option.value) !== getValue(value),
),
);
}
} }
fireOnChange(); fireOnChange();
onDeselect?.(value, option); onDeselect?.(value, option);
@ -341,9 +347,9 @@ const AsyncSelect = forwardRef(
[fetchPage], [fetchPage],
); );
const handleOnSearch = (search: string) => { const handleOnSearch = debounce((search: string) => {
const searchValue = search.trim(); const searchValue = search.trim();
if (allowNewOptions && isSingleMode) { if (allowNewOptions) {
const newOption = searchValue && const newOption = searchValue &&
!hasOption(searchValue, fullSelectOptions, true) && { !hasOption(searchValue, fullSelectOptions, true) && {
label: searchValue, label: searchValue,
@ -368,7 +374,10 @@ const AsyncSelect = forwardRef(
setIsLoading(!(fetchOnlyOnSearch && !searchValue)); setIsLoading(!(fetchOnlyOnSearch && !searchValue));
} }
setInputValue(search); setInputValue(search);
}; onSearch?.(searchValue);
}, FAST_DEBOUNCE);
useEffect(() => () => handleOnSearch.cancel(), [handleOnSearch]);
const handlePagination = (e: UIEvent<HTMLElement>) => { const handlePagination = (e: UIEvent<HTMLElement>) => {
const vScroll = e.currentTarget; const vScroll = e.currentTarget;
@ -439,19 +448,7 @@ const AsyncSelect = forwardRef(
}; };
const handleOnBlur = (event: React.FocusEvent<HTMLElement>) => { const handleOnBlur = (event: React.FocusEvent<HTMLElement>) => {
const tagsMode = !isSingleMode && allowNewOptions; setInputValue('');
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 || []);
}
onBlur?.(event); onBlur?.(event);
}; };
@ -526,6 +523,28 @@ const AsyncSelect = forwardRef(
[ref], [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 ( return (
<StyledContainer headerPosition={headerPosition}> <StyledContainer headerPosition={headerPosition}>
{header && ( {header && (
@ -549,17 +568,17 @@ const AsyncSelect = forwardRef(
onBlur={handleOnBlur} onBlur={handleOnBlur}
onDeselect={handleOnDeselect} onDeselect={handleOnDeselect}
onDropdownVisibleChange={handleOnDropdownVisibleChange} onDropdownVisibleChange={handleOnDropdownVisibleChange}
// @ts-ignore
onPaste={onPaste}
onPopupScroll={handlePagination} onPopupScroll={handlePagination}
onSearch={showSearch ? handleOnSearch : undefined} onSearch={showSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect} onSelect={handleOnSelect}
onClear={handleClear} onClear={handleClear}
options={ options={shouldRenderChildrenOptions ? undefined : fullSelectOptions}
hasCustomLabels(fullSelectOptions) ? undefined : fullSelectOptions
}
placeholder={placeholder} placeholder={placeholder}
showSearch={showSearch} showSearch={showSearch}
showArrow showArrow
tokenSeparators={tokenSeparators || TOKEN_SEPARATORS} tokenSeparators={tokenSeparators}
value={selectValue} value={selectValue}
suffixIcon={getSuffixIcon(isLoading, showSearch, isDropdownVisible)} suffixIcon={getSuffixIcon(isLoading, showSearch, isDropdownVisible)}
menuItemSelectedIcon={ menuItemSelectedIcon={

View File

@ -103,6 +103,11 @@ const ARG_TYPES = {
disable: true, disable: true,
}, },
}, },
mappedMode: {
table: {
disable: true,
},
},
mode: { mode: {
description: `It defines whether the Select should allow for description: `It defines whether the Select should allow for
the selection of multiple options or single. Single by default. the selection of multiple options or single. Single by default.

View File

@ -17,7 +17,14 @@
* under the License. * under the License.
*/ */
import React from 'react'; 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 userEvent from '@testing-library/user-event';
import Select from 'src/components/Select/Select'; import Select from 'src/components/Select/Select';
import { SELECT_ALL_VALUE } from './utils'; import { SELECT_ALL_VALUE } from './utils';
@ -68,7 +75,6 @@ const defaultProps = {
ariaLabel: ARIA_LABEL, ariaLabel: ARIA_LABEL,
labelInValue: true, labelInValue: true,
options: OPTIONS, options: OPTIONS,
pageSize: 10,
showSearch: true, showSearch: true,
}; };
@ -93,6 +99,9 @@ const querySelectOption = (text: string) =>
within(getElementByClassName('.rc-virtual-list')).queryByText(text), within(getElementByClassName('.rc-virtual-list')).queryByText(text),
); );
const getAllSelectOptions = () =>
getElementsByClassName('.ant-select-item-option-content');
const findAllSelectOptions = () => const findAllSelectOptions = () =>
waitFor(() => getElementsByClassName('.ant-select-item-option-content')); waitFor(() => getElementsByClassName('.ant-select-item-option-content'));
@ -134,6 +143,11 @@ const clearTypedText = () => {
const open = () => waitFor(() => userEvent.click(getSelect())); const open = () => waitFor(() => userEvent.click(getSelect()));
const reopen = async () => {
await type('{esc}');
await open();
};
test('displays a header', async () => { test('displays a header', async () => {
const headerText = 'Header'; const headerText = 'Header';
render(<Select {...defaultProps} header={headerText} />); 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); expect(await matchOrder(originalLabels)).toBe(true);
// order selected to top when reopen // order selected to top when reopen
await type('{esc}'); await reopen();
await open();
let labels = originalLabels.slice(); let labels = originalLabels.slice();
labels = labels.splice(1, 1).concat(labels); labels = labels.splice(1, 1).concat(labels);
expect(await matchOrder(labels)).toBe(true); expect(await matchOrder(labels)).toBe(true);
@ -211,16 +224,14 @@ test('should sort selected to top when in single mode', async () => {
// original order // original order
userEvent.click(await findSelectOption(originalLabels[5])); userEvent.click(await findSelectOption(originalLabels[5]));
await matchOrder(labels); await matchOrder(labels);
await type('{esc}'); await reopen();
await open();
labels = originalLabels.slice(); labels = originalLabels.slice();
labels = labels.splice(5, 1).concat(labels); labels = labels.splice(5, 1).concat(labels);
expect(await matchOrder(labels)).toBe(true); expect(await matchOrder(labels)).toBe(true);
// should revert to original order // should revert to original order
clearAll(); clearAll();
await type('{esc}'); await reopen();
await open();
expect(await matchOrder(originalLabels)).toBe(true); 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]), await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
).toBe(true); ).toBe(true);
await type('{esc}'); await reopen();
await open();
labels = labels.splice(2, 1).concat(labels); labels = labels.splice(2, 1).concat(labels);
expect( expect(
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]), await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]),
@ -244,8 +254,7 @@ test('should sort selected to the top when in multi mode', async () => {
await open(); await open();
userEvent.click(await findSelectOption(labels[5])); userEvent.click(await findSelectOption(labels[5]));
await type('{esc}'); await reopen();
await open();
labels = [labels.splice(0, 1)[0], labels.splice(4, 1)[0]].concat(labels); labels = [labels.splice(0, 1)[0], labels.splice(4, 1)[0]].concat(labels);
expect( expect(
await matchOrder([selectAllOptionLabel(originalLabels.length), ...labels]), 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 // should revert to original order
clearAll(); clearAll();
await type('{esc}'); await reopen();
await open();
expect( expect(
await matchOrder([ await matchOrder([
selectAllOptionLabel(originalLabels.length), selectAllOptionLabel(originalLabels.length),
@ -276,12 +284,14 @@ test('searches for label or value', async () => {
test('search order exact and startWith match first', async () => { test('search order exact and startWith match first', async () => {
render(<Select {...defaultProps} />); render(<Select {...defaultProps} />);
await type('Her'); await type('Her');
const options = await findAllSelectOptions(); await waitFor(() => {
expect(options.length).toBe(4); const options = getAllSelectOptions();
expect(options[0]?.textContent).toEqual('Her'); expect(options.length).toBe(4);
expect(options[1]?.textContent).toEqual('Herme'); expect(options[0]?.textContent).toEqual('Her');
expect(options[2]?.textContent).toEqual('Cher'); expect(options[1]?.textContent).toEqual('Herme');
expect(options[3]?.textContent).toEqual('Guilherme'); expect(options[2]?.textContent).toEqual('Cher');
expect(options[3]?.textContent).toEqual('Guilherme');
});
}); });
test('ignores case when searching', async () => { test('ignores case when searching', async () => {
@ -303,12 +313,14 @@ test('same case should be ranked to the top', async () => {
/>, />,
); );
await type('Ac'); await type('Ac');
const options = await findAllSelectOptions(); await waitFor(() => {
expect(options.length).toBe(4); const options = getAllSelectOptions();
expect(options[0]?.textContent).toEqual('acbc'); expect(options.length).toBe(4);
expect(options[1]?.textContent).toEqual('CAc'); expect(options[0]?.textContent).toEqual('acbc');
expect(options[2]?.textContent).toEqual('abac'); expect(options[1]?.textContent).toEqual('CAc');
expect(options[3]?.textContent).toEqual('Cac'); expect(options[2]?.textContent).toEqual('abac');
expect(options[3]?.textContent).toEqual('Cac');
});
}); });
test('ignores special keys when searching', async () => { test('ignores special keys when searching', async () => {
@ -338,7 +350,13 @@ test('searches for custom fields', async () => {
test('removes duplicated values', async () => { test('removes duplicated values', async () => {
render(<Select {...defaultProps} mode="multiple" allowNewOptions />); 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(); const values = await findAllSelectValues();
expect(values.length).toBe(4); expect(values.length).toBe(4);
expect(values[0]).toHaveTextContent('a'); 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 />); render(<Select {...defaultProps} allowNewOptions />);
await open(); await open();
await type(NEW_OPTION); 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 () => { 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" />); render(<Select {...defaultProps} options={OPTIONS} mode="multiple" />);
await open(); await open();
await type('Select'); await type('Select');
expect( await waitFor(() =>
screen.queryByText(selectAllOptionLabel(OPTIONS.length)), expect(
).not.toBeInTheDocument(); screen.queryByText(selectAllOptionLabel(OPTIONS.length)),
).not.toBeInTheDocument(),
);
}); });
test('does not render "Select all" as one of the tags after selection', async () => { 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)); 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 () => { test('selecting all values also selects "Select all"', async () => {
render( render(
<Select <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))); userEvent.click(await findSelectOption(selectAllOptionLabel(10)));
expect(await findSelectOption(selectAllOptionLabel(10))).toBeInTheDocument(); expect(await findSelectOption(selectAllOptionLabel(10))).toBeInTheDocument();
// add a new option // add a new option
await type(`${NEW_OPTION}{enter}`); await type(NEW_OPTION);
expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
clearTypedText(); clearTypedText();
expect(await findSelectOption(selectAllOptionLabel(11))).toBeInTheDocument(); expect(await findSelectOption(selectAllOptionLabel(11))).toBeInTheDocument();
expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument();
// select all should be selected // select all should be selected
let values = await findAllCheckedValues(); let values = await findAllCheckedValues();
expect(values[0]).toHaveTextContent(selectAllOptionLabel(11)); 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 allowNewOptions
/>, />,
); );
await open();
expect(screen.queryByText(selectAllOptionLabel(1))).not.toBeInTheDocument(); expect(screen.queryByText(selectAllOptionLabel(1))).not.toBeInTheDocument();
await type(`${NEW_OPTION}{enter}`); rerender(
clearTypedText(); <Select
expect(screen.queryByText(selectAllOptionLabel(2))).toBeInTheDocument(); {...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 () => { 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); 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. TODO: Add tests that require scroll interaction. Needs further investigation.
- Fetches more data when scrolling and more data is available - Fetches more data when scrolling and more data is available

View File

@ -24,6 +24,7 @@ import React, {
useMemo, useMemo,
useState, useState,
useCallback, useCallback,
ClipboardEvent,
} from 'react'; } from 'react';
import { import {
ensureIsArray, ensureIsArray,
@ -33,7 +34,8 @@ import {
usePrevious, usePrevious,
} from '@superset-ui/core'; } from '@superset-ui/core';
import AntdSelect, { LabeledValue as AntdLabeledValue } from 'antd/lib/select'; 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 { import {
getValue, getValue,
hasOption, hasOption,
@ -50,7 +52,7 @@ import {
mapOptions, mapOptions,
hasCustomLabels, hasCustomLabels,
} from './utils'; } from './utils';
import { SelectOptionsType, SelectProps } from './types'; import { RawValue, SelectOptionsType, SelectProps } from './types';
import { import {
StyledCheckOutlined, StyledCheckOutlined,
StyledContainer, StyledContainer,
@ -103,13 +105,14 @@ const Select = forwardRef(
onClear, onClear,
onDropdownVisibleChange, onDropdownVisibleChange,
onDeselect, onDeselect,
onSearch,
onSelect, onSelect,
optionFilterProps = ['label', 'value'], optionFilterProps = ['label', 'value'],
options, options,
placeholder = t('Select ...'), placeholder = t('Select ...'),
showSearch = true, showSearch = true,
sortComparator = DEFAULT_SORT_COMPARATOR, sortComparator = DEFAULT_SORT_COMPARATOR,
tokenSeparators, tokenSeparators = TOKEN_SEPARATORS,
value, value,
getPopupContainer, getPopupContainer,
oneLine, oneLine,
@ -141,11 +144,7 @@ const Select = forwardRef(
} }
}, [isDropdownVisible, oneLine]); }, [isDropdownVisible, oneLine]);
const mappedMode = isSingleMode const mappedMode = isSingleMode ? undefined : 'multiple';
? undefined
: allowNewOptions
? 'tags'
: 'multiple';
const { Option } = AntdSelect; const { Option } = AntdSelect;
@ -167,8 +166,7 @@ const Select = forwardRef(
); );
const initialOptions = useMemo( const initialOptions = useMemo(
() => () => (Array.isArray(options) ? options.slice() : EMPTY_OPTIONS),
options && Array.isArray(options) ? options.slice() : EMPTY_OPTIONS,
[options], [options],
); );
const initialOptionsSorted = useMemo( const initialOptionsSorted = useMemo(
@ -210,13 +208,13 @@ const Select = forwardRef(
() => () =>
!isSingleMode && !isSingleMode &&
allowSelectAll && allowSelectAll &&
selectOptions.length > 0 && fullSelectOptions.length > 0 &&
enabledOptions.length > 1 && enabledOptions.length > 1 &&
!inputValue, !inputValue,
[ [
isSingleMode, isSingleMode,
allowSelectAll, allowSelectAll,
selectOptions.length, fullSelectOptions.length,
enabledOptions.length, enabledOptions.length,
inputValue, inputValue,
], ],
@ -295,24 +293,30 @@ const Select = forwardRef(
element => getValue(element) !== getValue(value), element => getValue(element) !== getValue(value),
); );
// if this was not a new item, deselect select all option // if this was not a new item, deselect select all option
if ( if (selectAllMode && !option.isNewOption) {
selectAllMode &&
selectOptions.some(opt => opt.value === getValue(value))
) {
array = array.filter( array = array.filter(
element => getValue(element) !== SELECT_ALL_VALUE, element => getValue(element) !== SELECT_ALL_VALUE,
); );
} }
setSelectValue(array); setSelectValue(array);
// removes new option
if (option.isNewOption) {
setSelectOptions(
fullSelectOptions.filter(
option => getValue(option.value) !== getValue(value),
),
);
}
} }
} }
fireOnChange(); fireOnChange();
onDeselect?.(value, option); onDeselect?.(value, option);
}; };
const handleOnSearch = (search: string) => { const handleOnSearch = debounce((search: string) => {
const searchValue = search.trim(); const searchValue = search.trim();
if (allowNewOptions && isSingleMode) { if (allowNewOptions) {
const newOption = searchValue && const newOption = searchValue &&
!hasOption(searchValue, fullSelectOptions, true) && { !hasOption(searchValue, fullSelectOptions, true) && {
label: searchValue, label: searchValue,
@ -328,7 +332,10 @@ const Select = forwardRef(
setSelectOptions(newOptions); setSelectOptions(newOptions);
} }
setInputValue(searchValue); setInputValue(searchValue);
}; onSearch?.(searchValue);
}, FAST_DEBOUNCE);
useEffect(() => () => handleOnSearch.cancel(), [handleOnSearch]);
const handleFilterOption = (search: string, option: AntdLabeledValue) => const handleFilterOption = (search: string, option: AntdLabeledValue) =>
handleFilterOptionHelper(search, option, optionFilterProps, filterOption); handleFilterOptionHelper(search, option, optionFilterProps, filterOption);
@ -390,10 +397,7 @@ const Select = forwardRef(
setSelectValue( setSelectValue(
labelInValue labelInValue
? ([...ensureIsArray(value), selectAllOption] as AntdLabeledValue[]) ? ([...ensureIsArray(value), selectAllOption] as AntdLabeledValue[])
: ([ : ([...ensureIsArray(value), SELECT_ALL_VALUE] as RawValue[]),
...ensureIsArray(value),
SELECT_ALL_VALUE,
] as AntdLabeledValue[]),
); );
} }
}, [labelInValue, selectAllEligible.length, selectAllEnabled, value]); }, [labelInValue, selectAllEligible.length, selectAllEnabled, value]);
@ -429,19 +433,7 @@ const Select = forwardRef(
); );
const handleOnBlur = (event: React.FocusEvent<HTMLElement>) => { const handleOnBlur = (event: React.FocusEvent<HTMLElement>) => {
const tagsMode = !isSingleMode && allowNewOptions; setInputValue('');
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 || []);
}
onBlur?.(event); onBlur?.(event);
}; };
@ -538,6 +530,32 @@ const Select = forwardRef(
actualMaxTagCount -= 1; 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 ( return (
<StyledContainer headerPosition={headerPosition}> <StyledContainer headerPosition={headerPosition}>
{header && ( {header && (
@ -562,6 +580,8 @@ const Select = forwardRef(
onBlur={handleOnBlur} onBlur={handleOnBlur}
onDeselect={handleOnDeselect} onDeselect={handleOnDeselect}
onDropdownVisibleChange={handleOnDropdownVisibleChange} onDropdownVisibleChange={handleOnDropdownVisibleChange}
// @ts-ignore
onPaste={onPaste}
onPopupScroll={undefined} onPopupScroll={undefined}
onSearch={shouldShowSearch ? handleOnSearch : undefined} onSearch={shouldShowSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect} onSelect={handleOnSelect}
@ -569,7 +589,7 @@ const Select = forwardRef(
placeholder={placeholder} placeholder={placeholder}
showSearch={shouldShowSearch} showSearch={shouldShowSearch}
showArrow showArrow
tokenSeparators={tokenSeparators || TOKEN_SEPARATORS} tokenSeparators={tokenSeparators}
value={selectValue} value={selectValue}
suffixIcon={getSuffixIcon( suffixIcon={getSuffixIcon(
isLoading, isLoading,
@ -583,7 +603,7 @@ const Select = forwardRef(
<StyledCheckOutlined iconSize="m" aria-label="check" /> <StyledCheckOutlined iconSize="m" aria-label="check" />
) )
} }
{...(!shouldRenderChildrenOptions && { options: fullSelectOptions })} options={shouldRenderChildrenOptions ? undefined : fullSelectOptions}
oneLine={oneLine} oneLine={oneLine}
tagRender={customTagRender} tagRender={customTagRender}
{...props} {...props}