diff --git a/superset-frontend/src/components/Select/Select.stories.tsx b/superset-frontend/src/components/Select/Select.stories.tsx
index d64eb78a4..204ebff80 100644
--- a/superset-frontend/src/components/Select/Select.stories.tsx
+++ b/superset-frontend/src/components/Select/Select.stories.tsx
@@ -18,7 +18,7 @@
*/
import React, { ReactNode, useState, useCallback } from 'react';
import ControlHeader from 'src/explore/components/ControlHeader';
-import Select, { SelectProps, OptionsTypePage } from './Select';
+import Select, { SelectProps, OptionsTypePage, OptionsType } from './Select';
export default {
title: 'Select',
@@ -27,7 +27,7 @@ export default {
const DEFAULT_WIDTH = 200;
-const options = [
+const options: OptionsType = [
{
label: 'Such an incredibly awesome long long label',
value: 'Such an incredibly awesome long long label',
@@ -147,13 +147,42 @@ const mountHeader = (type: String) => {
return header;
};
-export const InteractiveSelect = (args: SelectProps & { header: string }) => (
+const generateOptions = (opts: OptionsType, count: number) => {
+ let generated = opts.slice();
+ let iteration = 0;
+ while (generated.length < count) {
+ iteration += 1;
+ generated = generated.concat(
+ // eslint-disable-next-line no-loop-func
+ generated.map(({ label, value }) => ({
+ label: `${label} ${iteration}`,
+ value: `${value} ${iteration}`,
+ })),
+ );
+ }
+ return generated.slice(0, count);
+};
+
+export const InteractiveSelect = ({
+ header,
+ options,
+ optionsCount,
+ ...args
+}: SelectProps & { header: string; optionsCount: number }) => (
-
+
);
@@ -170,6 +199,12 @@ InteractiveSelect.args = {
InteractiveSelect.argTypes = {
...ARG_TYPES,
+ optionsCount: {
+ defaultValue: options.length,
+ control: {
+ type: 'number',
+ },
+ },
header: {
defaultValue: 'none',
description: `It adds a header on top of the Select. Can be any ReactNode.`,
diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx
index c23f57d52..15489c14e 100644
--- a/superset-frontend/src/components/Select/Select.test.tsx
+++ b/superset-frontend/src/components/Select/Select.test.tsx
@@ -46,6 +46,9 @@ const OPTIONS = [
{ label: 'Irfan', value: 18, gender: 'Male' },
{ label: 'George', value: 19, gender: 'Male' },
{ label: 'Ashfaq', value: 20, gender: 'Male' },
+ { label: 'Herme', value: 21, gender: 'Male' },
+ { label: 'Cher', value: 22, gender: 'Female' },
+ { label: 'Her', value: 23, gender: 'Male' },
].sort((option1, option2) => option1.label.localeCompare(option2.label));
const loadOptions = async (search: string, page: number, pageSize: number) => {
@@ -111,9 +114,21 @@ test('displays a header', async () => {
});
test('adds a new option if the value is not in the options', async () => {
- render();
+ const { rerender } = render(
+ ,
+ );
await open();
expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument();
+
+ rerender(
+ ,
+ );
+ await open();
+ const options = await findAllSelectOptions();
+ expect(options).toHaveLength(2);
+ options.forEach((option, i) =>
+ expect(option).toHaveTextContent(OPTIONS[i].label),
+ );
});
test('inverts the selection', async () => {
@@ -149,9 +164,9 @@ test('sort the options using a custom sort comparator', async () => {
const options = await findAllSelectOptions();
const optionsPage = OPTIONS.slice(0, defaultProps.pageSize);
const sortedOptions = optionsPage.sort(sortComparator);
- options.forEach((option, key) =>
- expect(option).toHaveTextContent(sortedOptions[key].label),
- );
+ options.forEach((option, key) => {
+ expect(option).toHaveTextContent(sortedOptions[key].label);
+ });
});
test('displays the selected values first', async () => {
@@ -194,12 +209,44 @@ test('searches for label or value', async () => {
expect(options[0]).toHaveTextContent(option.label);
});
+test('search order exact and startWith match first', async () => {
+ render();
+ 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');
+});
+
test('ignores case when searching', async () => {
render();
await type('george');
expect(await findSelectOption('George')).toBeInTheDocument();
});
+test('same case should be ranked to the top', async () => {
+ render(
+ ,
+ );
+ 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');
+});
+
test('ignores special keys when searching', async () => {
render();
await type('{shift}');
@@ -214,12 +261,13 @@ test('searches for custom fields', async () => {
expect(options[0]).toHaveTextContent('Liam');
await type('Female');
options = await findAllSelectOptions();
- expect(options.length).toBe(5);
+ expect(options.length).toBe(6);
expect(options[0]).toHaveTextContent('Ava');
expect(options[1]).toHaveTextContent('Charlotte');
- expect(options[2]).toHaveTextContent('Emma');
- expect(options[3]).toHaveTextContent('Nikole');
- expect(options[4]).toHaveTextContent('Olivia');
+ expect(options[2]).toHaveTextContent('Cher');
+ expect(options[3]).toHaveTextContent('Emma');
+ expect(options[4]).toHaveTextContent('Nikole');
+ expect(options[5]).toHaveTextContent('Olivia');
await type('1');
expect(screen.getByText(NO_DATA)).toBeInTheDocument();
});
diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx
index ec13791cd..a68b6135c 100644
--- a/superset-frontend/src/components/Select/Select.tsx
+++ b/superset-frontend/src/components/Select/Select.tsx
@@ -28,7 +28,7 @@ import React, {
useRef,
useCallback,
} from 'react';
-import { styled, t } from '@superset-ui/core';
+import { ensureIsArray, styled, t } from '@superset-ui/core';
import AntdSelect, {
SelectProps as AntdSelectProps,
SelectValue as AntdSelectValue,
@@ -41,7 +41,8 @@ import { Spin } from 'antd';
import Icons from 'src/components/Icons';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import { SLOW_DEBOUNCE } from 'src/constants';
-import { hasOption, hasOptionIgnoreCase } from './utils';
+import { rankedSearchCompare } from 'src/utils/rankedSearchCompare';
+import { getValue, hasOption } from './utils';
const { Option } = AntdSelect;
@@ -155,7 +156,7 @@ export interface SelectProps extends PickedSelectProps {
* Works in async mode only (See the options property).
*/
onError?: (error: string) => void;
- sortComparator?: (a: AntdLabeledValue, b: AntdLabeledValue) => number;
+ sortComparator?: typeof DEFAULT_SORT_COMPARATOR;
}
const StyledContainer = styled.div`
@@ -227,12 +228,26 @@ const Error = ({ error }: { error: string }) => (
);
-const defaultSortComparator = (a: AntdLabeledValue, b: AntdLabeledValue) => {
+export const DEFAULT_SORT_COMPARATOR = (
+ a: AntdLabeledValue,
+ b: AntdLabeledValue,
+ search?: string,
+) => {
+ let aText: string | undefined;
+ let bText: string | undefined;
if (typeof a.label === 'string' && typeof b.label === 'string') {
- return a.label.localeCompare(b.label);
+ aText = a.label;
+ bText = b.label;
+ } else if (typeof a.value === 'string' && typeof b.value === 'string') {
+ aText = a.value;
+ bText = b.value;
}
- if (typeof a.value === 'string' && typeof b.value === 'string') {
- return a.value.localeCompare(b.value);
+ // sort selected options first
+ if (typeof aText === 'string' && typeof bText === 'string') {
+ if (search) {
+ return rankedSearchCompare(aText, bText, search);
+ }
+ return aText.localeCompare(bText);
}
return (a.value as number) - (b.value as number);
};
@@ -289,7 +304,7 @@ const Select = (
pageSize = DEFAULT_PAGE_SIZE,
placeholder = t('Select ...'),
showSearch = true,
- sortComparator = defaultSortComparator,
+ sortComparator = DEFAULT_SORT_COMPARATOR,
value,
...props
}: SelectProps,
@@ -299,15 +314,9 @@ const Select = (
const isSingleMode = mode === 'single';
const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch;
const initialOptions =
- options && Array.isArray(options) ? options : EMPTY_OPTIONS;
- const [selectOptions, setSelectOptions] = useState(
- initialOptions.sort(sortComparator),
- );
- const shouldUseChildrenOptions = !!selectOptions.find(
- opt => opt?.customLabel,
- );
+ options && Array.isArray(options) ? options.slice() : EMPTY_OPTIONS;
const [selectValue, setSelectValue] = useState(value);
- const [searchedValue, setSearchedValue] = useState('');
+ const [inputValue, setInputValue] = useState('');
const [isLoading, setIsLoading] = useState(loading);
const [error, setError] = useState('');
const [isDropdownVisible, setIsDropdownVisible] = useState(false);
@@ -321,81 +330,41 @@ const Select = (
: allowNewOptions
? 'tags'
: 'multiple';
- const allowFetch = !fetchOnlyOnSearch || searchedValue;
+ const allowFetch = !fetchOnlyOnSearch || inputValue;
- // TODO: Don't assume that isAsync is always labelInValue
- const handleTopOptions = useCallback(
- (selectedValue: AntdSelectValue | undefined) => {
- // bringing selected options to the top of the list
- if (selectedValue !== undefined && selectedValue !== null) {
- const isLabeledValue = isAsync || labelInValue;
- const topOptions: OptionsType = [];
- const otherOptions: OptionsType = [];
-
- selectOptions.forEach(opt => {
- let found = false;
- if (Array.isArray(selectedValue)) {
- if (isLabeledValue) {
- found =
- (selectedValue as AntdLabeledValue[]).find(
- element => element.value === opt.value,
- ) !== undefined;
- } else {
- found = selectedValue.includes(opt.value);
- }
- } else {
- found = isLabeledValue
- ? (selectedValue as AntdLabeledValue).value === opt.value
- : selectedValue === opt.value;
- }
-
- if (found) {
- topOptions.push(opt);
- } else {
- otherOptions.push(opt);
- }
- });
-
- // fallback for custom options in tags mode as they
- // do not appear in the selectOptions state
- if (!isSingleMode && Array.isArray(selectedValue)) {
- selectedValue.forEach((val: string | number | AntdLabeledValue) => {
- if (
- !topOptions.find(
- tOpt =>
- tOpt.value ===
- (isLabeledValue ? (val as AntdLabeledValue)?.value : val),
- )
- ) {
- if (isLabeledValue) {
- const labelValue = val as AntdLabeledValue;
- topOptions.push({
- label: labelValue.label,
- value: labelValue.value,
- });
- } else {
- const value = val as string | number;
- topOptions.push({ label: String(value), value });
- }
- }
- });
- }
- const sortedOptions = [
- ...topOptions.sort(sortComparator),
- ...otherOptions.sort(sortComparator),
- ];
- if (!isEqual(sortedOptions, selectOptions)) {
- setSelectOptions(sortedOptions);
- }
- } else {
- const sortedOptions = [...selectOptions].sort(sortComparator);
- if (!isEqual(sortedOptions, selectOptions)) {
- setSelectOptions(sortedOptions);
- }
- }
- },
- [isAsync, isSingleMode, labelInValue, selectOptions, sortComparator],
+ const sortSelectedFirst = useCallback(
+ (a: AntdLabeledValue, b: AntdLabeledValue) =>
+ selectValue && a.value !== undefined && b.value !== undefined
+ ? Number(hasOption(b.value, selectValue)) -
+ Number(hasOption(a.value, selectValue))
+ : 0,
+ [selectValue],
);
+ const sortComparatorWithSearch = useCallback(
+ (a: AntdLabeledValue, b: AntdLabeledValue) =>
+ sortSelectedFirst(a, b) || sortComparator(a, b, inputValue),
+ [inputValue, sortComparator, sortSelectedFirst],
+ );
+ const sortComparatorWithoutSearch = useCallback(
+ (a: AntdLabeledValue, b: AntdLabeledValue) =>
+ sortSelectedFirst(a, b) || sortComparator(a, b, ''),
+ [sortComparator, sortSelectedFirst],
+ );
+ const [selectOptions, setSelectOptions] =
+ useState(initialOptions);
+ // add selected values to options list if they are not in it
+ const fullSelectOptions = useMemo(() => {
+ const missingValues: OptionsType = ensureIsArray(selectValue)
+ .filter(opt => !hasOption(getValue(opt), selectOptions))
+ .map(opt =>
+ typeof opt === 'object' ? opt : { value: opt, label: String(opt) },
+ );
+ return missingValues.length > 0
+ ? missingValues.concat(selectOptions)
+ : selectOptions;
+ }, [selectOptions, selectValue]);
+
+ const hasCustomLabels = fullSelectOptions.some(opt => !!opt?.customLabel);
const handleOnSelect = (
selectedValue: string | number | AntdLabeledValue,
@@ -423,7 +392,7 @@ const Select = (
]);
}
}
- setSearchedValue('');
+ setInputValue('');
};
const handleOnDeselect = (value: string | number | AntdLabeledValue) => {
@@ -436,7 +405,7 @@ const Select = (
setSelectValue(array.filter(element => element.value !== value.value));
}
}
- setSearchedValue('');
+ setInputValue('');
};
const internalOnError = useCallback(
@@ -452,42 +421,34 @@ const Select = (
[onError],
);
- const handleData = useCallback(
+ const mergeData = useCallback(
(data: OptionsType) => {
let mergedData: OptionsType = [];
if (data && Array.isArray(data) && data.length) {
- const dataValues = new Set();
- data.forEach(option =>
- dataValues.add(String(option.value).toLocaleLowerCase()),
- );
-
+ // 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(
- String(previousOption.value).toLocaleLowerCase(),
- ),
- ),
- ...data,
- ];
- mergedData.sort(sortComparator);
+ mergedData = prevOptions
+ .filter(previousOption => !dataValues.has(previousOption.value))
+ .concat(data)
+ .sort(sortComparatorWithoutSearch);
return mergedData;
});
}
return mergedData;
},
- [sortComparator],
+ [sortComparatorWithoutSearch],
);
- const handlePaginatedFetch = useMemo(
- () => (value: string, page: number) => {
+ const fetchPage = useMemo(
+ () => (search: string, page: number) => {
+ setPage(page);
if (allValuesLoaded) {
setIsLoading(false);
return;
}
- const key = getQueryCacheKey(value, page, pageSize);
+ const key = getQueryCacheKey(search, page, pageSize);
const cachedCount = fetchedQueries.current.get(key);
if (cachedCount !== undefined) {
setTotalCount(cachedCount);
@@ -496,9 +457,9 @@ const Select = (
}
setIsLoading(true);
const fetchOptions = options as OptionsPagePromise;
- fetchOptions(value, page, pageSize)
+ fetchOptions(search, page, pageSize)
.then(({ data, totalCount }: OptionsTypePage) => {
- const mergedData = handleData(data);
+ const mergedData = mergeData(data);
fetchedQueries.current.set(key, totalCount);
setTotalCount(totalCount);
if (
@@ -517,32 +478,29 @@ const Select = (
[
allValuesLoaded,
fetchOnlyOnSearch,
- handleData,
+ mergeData,
internalOnError,
options,
pageSize,
+ value,
],
);
- const debouncedHandleSearch = useMemo(
- () =>
- debounce((search: string) => {
- // async search will be triggered in handlePaginatedFetch
- setSearchedValue(search);
- }, SLOW_DEBOUNCE),
- [],
+ const debouncedFetchPage = useMemo(
+ () => debounce(fetchPage, SLOW_DEBOUNCE),
+ [fetchPage],
);
const handleOnSearch = (search: string) => {
const searchValue = search.trim();
if (allowNewOptions && isSingleMode) {
const newOption = searchValue &&
- !hasOptionIgnoreCase(searchValue, selectOptions) && {
+ !hasOption(searchValue, fullSelectOptions, true) && {
label: searchValue,
value: searchValue,
isNewOption: true,
};
- const cleanSelectOptions = selectOptions.filter(
+ const cleanSelectOptions = fullSelectOptions.filter(
opt => !opt.isNewOption || hasOption(opt.value, selectValue),
);
const newOptions = newOption
@@ -560,7 +518,7 @@ const Select = (
// in loading state
setIsLoading(!(fetchOnlyOnSearch && !searchValue));
}
- return debouncedHandleSearch(search);
+ setInputValue(search);
};
const handlePagination = (e: UIEvent) => {
@@ -571,8 +529,7 @@ const Select = (
if (!isLoading && isAsync && hasMoreData && thresholdReached) {
const newPage = page + 1;
- handlePaginatedFetch(searchedValue, newPage);
- setPage(newPage);
+ fetchPage(inputValue, newPage);
}
};
@@ -583,7 +540,6 @@ const Select = (
if (filterOption) {
const searchValue = search.trim().toLowerCase();
-
if (optionFilterProps && optionFilterProps.length) {
return optionFilterProps.some(prop => {
const optionProp = option?.[prop]
@@ -616,11 +572,15 @@ const Select = (
}, 250);
}
}
-
- // multiple or tags mode keep the dropdown visible while selecting options
- // this waits for the dropdown to be opened before sorting the top options
- if (!isSingleMode && isDropdownVisible) {
- handleTopOptions(selectValue);
+ // if no search input value, force sort options because it won't be sorted by
+ // `filterSort`.
+ if (isDropdownVisible && !inputValue && fullSelectOptions.length > 0) {
+ const sortedOptions = [...fullSelectOptions].sort(
+ sortComparatorWithSearch,
+ );
+ if (!isEqual(sortedOptions, fullSelectOptions)) {
+ setSelectOptions(sortedOptions);
+ }
}
if (onDropdownVisibleChange) {
@@ -634,7 +594,7 @@ const Select = (
if (!isDropdownVisible) {
originNode.ref?.current?.scrollTo({ top: 0 });
}
- if (isLoading && selectOptions.length === 0) {
+ if (isLoading && fullSelectOptions.length === 0) {
return {t('Loading...')};
}
return error ? : originNode;
@@ -660,76 +620,44 @@ const Select = (
};
useEffect(() => {
+ // when `options` list is updated from component prop, reset states
fetchedQueries.current.clear();
+ setAllValuesLoaded(false);
setSelectOptions(
options && Array.isArray(options) ? options : EMPTY_OPTIONS,
);
- setAllValuesLoaded(false);
}, [options]);
useEffect(() => {
setSelectValue(value);
}, [value]);
- useEffect(() => {
- if (selectValue) {
- setSelectOptions(selectOptions => {
- const array = Array.isArray(selectValue)
- ? (selectValue as AntdLabeledValue[])
- : [selectValue as AntdLabeledValue | string | number];
- const options: AntdLabeledValue[] = [];
- const isLabeledValue = isAsync || labelInValue;
- array.forEach(element => {
- const found = selectOptions.find(
- (option: { value: string | number }) =>
- isLabeledValue
- ? option.value === (element as AntdLabeledValue).value
- : option.value === element,
- );
- if (!found) {
- options.push(
- isLabeledValue
- ? (element as AntdLabeledValue)
- : ({ value: element, label: element } as AntdLabeledValue),
- );
- }
- });
- if (options.length > 0) {
- return [...options, ...selectOptions];
- }
- // return same options won't trigger a re-render
- return selectOptions;
- });
- }
- }, [labelInValue, isAsync, selectValue]);
-
// Stop the invocation of the debounced function after unmounting
useEffect(
() => () => {
- debouncedHandleSearch.cancel();
+ debouncedFetchPage.cancel();
},
- [debouncedHandleSearch],
+ [debouncedFetchPage],
);
useEffect(() => {
if (isAsync && loadingEnabled && allowFetch) {
- handlePaginatedFetch(searchedValue, 0);
- setPage(0);
+ // trigger fetch every time inputValue changes
+ if (inputValue) {
+ debouncedFetchPage(inputValue, 0);
+ } else {
+ fetchPage('', 0);
+ }
}
}, [
isAsync,
- searchedValue,
- handlePaginatedFetch,
loadingEnabled,
+ fetchPage,
allowFetch,
+ inputValue,
+ debouncedFetchPage,
]);
- useEffect(() => {
- if (isSingleMode) {
- handleTopOptions(selectValue);
- }
- }, [handleTopOptions, isSingleMode, selectValue]);
-
useEffect(() => {
if (loading !== undefined && loading !== isLoading) {
setIsLoading(loading);
@@ -743,6 +671,7 @@ const Select = (
aria-label={ariaLabel || name}
dropdownRender={dropdownRender}
filterOption={handleFilterOption}
+ filterSort={sortComparatorWithSearch}
getPopupContainer={triggerNode => triggerNode.parentNode}
labelInValue={isAsync || labelInValue}
maxTagCount={MAX_TAG_COUNT}
@@ -755,7 +684,7 @@ const Select = (
onSelect={handleOnSelect}
onClear={handleClear}
onChange={onChange}
- options={shouldUseChildrenOptions ? undefined : selectOptions}
+ options={hasCustomLabels ? undefined : fullSelectOptions}
placeholder={placeholder}
showSearch={shouldShowSearch}
showArrow
@@ -772,13 +701,12 @@ const Select = (
ref={ref}
{...props}
>
- {shouldUseChildrenOptions &&
- selectOptions.map(opt => {
+ {hasCustomLabels &&
+ fullSelectOptions.map(opt => {
const isOptObject = typeof opt === 'object';
const label = isOptObject ? opt?.label || opt.value : opt;
const value = isOptObject ? opt.value : opt;
const { customLabel, ...optProps } = opt;
-
return (