Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Adds lazy loading and fetchOnlyOnSearch to the Select component #15799

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 17 additions & 2 deletions superset-frontend/src/components/Select/Select.stories.tsx
Expand Up @@ -144,6 +144,11 @@ InteractiveSelect.argTypes = {
disable: true,
},
},
fetchOnlyOnSearch: {
table: {
disable: true,
},
},
};

InteractiveSelect.story = {
Expand Down Expand Up @@ -296,10 +301,12 @@ const USERS = [

export const AsyncSelect = ({
withError,
withInitialValue,
responseTime,
...rest
}: SelectProps & {
withError: boolean;
withInitialValue: boolean;
responseTime: number;
}) => {
const [requests, setRequests] = useState<ReactNode[]>([]);
Expand Down Expand Up @@ -375,6 +382,11 @@ export const AsyncSelect = ({
<Select
{...rest}
options={withError ? fetchUserListError : fetchUserListPage}
value={
withInitialValue
? { label: 'Valentina', value: 'Valentina' }
: undefined
}
/>
</div>
<div
Expand All @@ -398,9 +410,11 @@ export const AsyncSelect = ({
};

AsyncSelect.args = {
withError: false,
pageSize: 10,
allowNewOptions: false,
fetchOnlyOnSearch: false,
pageSize: 10,
withError: false,
withInitialValue: false,
};

AsyncSelect.argTypes = {
Expand Down Expand Up @@ -431,6 +445,7 @@ AsyncSelect.argTypes = {
type: 'range',
min: 0.5,
max: 5,
step: 0.5,
},
},
};
Expand Down
150 changes: 119 additions & 31 deletions superset-frontend/src/components/Select/Select.tsx
Expand Up @@ -28,16 +28,17 @@ import React, {
useCallback,
} from 'react';
import { styled, t } from '@superset-ui/core';
import { Select as AntdSelect } from 'antd';
import Icons from 'src/components/Icons';
import {
import AntdSelect, {
SelectProps as AntdSelectProps,
SelectValue as AntdSelectValue,
LabeledValue as AntdLabeledValue,
} from 'antd/lib/select';
import { DownOutlined, SearchOutlined } from '@ant-design/icons';
import debounce from 'lodash/debounce';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import { isEqual } from 'lodash';
import { Spin } from 'antd';
import Icons from 'src/components/Icons';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import { hasOption } from './utils';

type AntdSelectAllProps = AntdSelectProps<AntdSelectValue>;
Expand All @@ -47,7 +48,6 @@ type PickedSelectProps = Pick<
| 'allowClear'
| 'autoFocus'
| 'value'
| 'defaultValue'
| 'disabled'
| 'filterOption'
| 'notFoundContent'
Expand Down Expand Up @@ -79,6 +79,7 @@ export interface SelectProps extends PickedSelectProps {
options: OptionsType | OptionsPagePromise;
pageSize?: number;
invertSelection?: boolean;
fetchOnlyOnSearch?: boolean;
}

const StyledContainer = styled.div`
Expand Down Expand Up @@ -122,6 +123,10 @@ const StyledError = styled.div`
`}
`;

const StyledSpin = styled(Spin)`
margin-top: ${({ theme }) => -theme.gridUnit}px;
`;

const MAX_TAG_COUNT = 4;
const TOKEN_SEPARATORS = [',', '\n', '\t', ';'];
const DEBOUNCE_TIMEOUT = 500;
Expand All @@ -137,15 +142,16 @@ const Error = ({ error }: { error: string }) => (
const Select = ({
allowNewOptions = false,
ariaLabel,
fetchOnlyOnSearch,
geido marked this conversation as resolved.
Show resolved Hide resolved
filterOption = true,
header = null,
invertSelection = false,
mode = 'single',
name,
options,
pageSize = DEFAULT_PAGE_SIZE,
placeholder = t('Select ...'),
options,
showSearch,
invertSelection = false,
value,
...props
}: SelectProps) => {
Expand All @@ -164,7 +170,8 @@ const Select = ({
const [isDropdownVisible, setIsDropdownVisible] = useState(false);
const [page, setPage] = useState(0);
const [totalCount, setTotalCount] = useState(0);
const fetchedQueries = useRef(new Set<string>());
const [loadingEnabled, setLoadingEnabled] = useState(false);
const fetchedQueries = useRef(new Map<string, number>());
const mappedMode = isSingleMode
? undefined
: allowNewOptions
Expand All @@ -177,6 +184,26 @@ const Select = ({
);
}, [options]);

useEffect(() => {
if (isAsync && value) {
const array: AntdLabeledValue[] = Array.isArray(value)
? (value as AntdLabeledValue[])
: [value as AntdLabeledValue];
const options: AntdLabeledValue[] = [];
array.forEach(element => {
const found = selectOptions.find(
option => option.value === element.value,
);
if (!found) {
options.push(element);
}
});
if (options.length > 0) {
setSelectOptions([...selectOptions, ...options]);
}
}
}, [isAsync, selectOptions, value]);

useEffect(() => {
setSelectValue(value);
}, [value]);
Expand All @@ -185,24 +212,56 @@ const Select = ({
(selectedValue: AntdSelectValue | undefined) => {
// bringing selected options to the top of the list
if (selectedValue) {
const currentValue = selectedValue as string[] | string;
const topOptions = selectOptions.filter(opt =>
Array.isArray(currentValue)
? currentValue.includes(opt.value)
: currentValue === opt.value,
);
const otherOptions = selectOptions.filter(
opt => !topOptions.find(tOpt => tOpt.value === opt.value),
);
const topOptions: OptionsType = [];
const otherOptions: OptionsType = [];

selectOptions.forEach(opt => {
let found = false;
if (Array.isArray(selectedValue)) {
if (isAsync) {
found =
(selectedValue as AntdLabeledValue[]).find(
element => element.value === opt.value,
) !== undefined;
} else {
found = selectedValue.includes(opt.value);
}
} else {
found = isAsync
? (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(currentValue)) {
// eslint-disable-next-line no-restricted-syntax
for (const val of currentValue) {
if (!topOptions.find(tOpt => tOpt.value === val)) {
topOptions.push({ label: val, value: val });
if (!isSingleMode && Array.isArray(selectedValue)) {
selectedValue.forEach((val: string | number | AntdLabeledValue) => {
if (
!topOptions.find(
tOpt =>
tOpt.value ===
(isAsync ? (val as AntdLabeledValue)?.value : val),
)
) {
if (isAsync) {
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, ...otherOptions];
Expand All @@ -211,7 +270,7 @@ const Select = ({
}
}
},
[isSingleMode, selectOptions],
[isAsync, isSingleMode, selectOptions],
);

const handleOnSelect = (
Expand All @@ -220,7 +279,11 @@ const Select = ({
if (isSingleMode) {
setSelectValue(selectedValue);
} else {
const currentSelected = Array.isArray(selectValue) ? selectValue : [];
const currentSelected = selectValue
? Array.isArray(selectValue)
? selectValue
: [selectValue]
: [];
if (
typeof selectedValue === 'number' ||
typeof selectedValue === 'string'
Expand Down Expand Up @@ -271,15 +334,17 @@ const Select = ({
const handlePaginatedFetch = useMemo(
() => (value: string, page: number, pageSize: number) => {
const key = `${value};${page};${pageSize}`;
if (fetchedQueries.current.has(key)) {
const cachedCount = fetchedQueries.current.get(key);
if (cachedCount) {
setTotalCount(cachedCount);
return;
}
setLoading(true);
const fetchOptions = options as OptionsPagePromise;
fetchOptions(value, page, pageSize)
.then(({ data, totalCount }: OptionsTypePage) => {
handleData(data);
fetchedQueries.current.add(key);
fetchedQueries.current.set(key, totalCount);
setTotalCount(totalCount);
})
.catch(onError)
Expand Down Expand Up @@ -351,6 +416,11 @@ const Select = ({

const handleOnDropdownVisibleChange = (isDropdownVisible: boolean) => {
setIsDropdownVisible(isDropdownVisible);

if (isAsync && !loadingEnabled) {
setLoadingEnabled(true);
}

// multiple or tags mode keep the dropdown visible while selecting options
// this waits for the dropdown to be closed before sorting the top options
if (!isSingleMode && !isDropdownVisible) {
Expand All @@ -359,13 +429,20 @@ const Select = ({
};

useEffect(() => {
const foundOption = hasOption(searchedValue, selectOptions);
geido marked this conversation as resolved.
Show resolved Hide resolved
if (isAsync && !foundOption) {
const allowFetch = !fetchOnlyOnSearch || searchedValue;
if (isAsync && loadingEnabled && allowFetch) {
geido marked this conversation as resolved.
Show resolved Hide resolved
const page = 0;
handlePaginatedFetch(searchedValue, page, pageSize);
setPage(page);
}
}, [isAsync, searchedValue, selectOptions, pageSize, handlePaginatedFetch]);
}, [
isAsync,
searchedValue,
pageSize,
handlePaginatedFetch,
loadingEnabled,
fetchOnlyOnSearch,
]);

useEffect(() => {
if (isSingleMode) {
Expand All @@ -382,6 +459,16 @@ const Select = ({
return error ? <Error error={error} /> : originNode;
};

const SuffixIcon = () => {
if (isLoading) {
return <StyledSpin size="small" />;
}
if (shouldShowSearch && isDropdownVisible) {
return <SearchOutlined />;
}
return <DownOutlined />;
};

return (
<StyledContainer>
{header}
Expand All @@ -391,7 +478,7 @@ const Select = ({
dropdownRender={dropdownRender}
filterOption={handleFilterOption}
getPopupContainer={triggerNode => triggerNode.parentNode}
loading={isLoading}
labelInValue={isAsync}
maxTagCount={MAX_TAG_COUNT}
mode={mappedMode}
onDeselect={handleOnDeselect}
Expand All @@ -406,6 +493,7 @@ const Select = ({
showArrow
tokenSeparators={TOKEN_SEPARATORS}
value={selectValue}
suffixIcon={<SuffixIcon />}
menuItemSelectedIcon={
invertSelection ? (
<StyledStopOutlined iconSize="m" />
Expand Down
Expand Up @@ -36,18 +36,11 @@ const cachedSupersetGet = cacheWrapper(
);

interface DatasetSelectProps {
datasetDetails: Record<string, any> | undefined;
datasetId: number;
onChange: (value: number) => void;
value?: { value: number | undefined };
onChange: (value: { label: string; value: number }) => void;
value?: { label: string; value: number };
}

const DatasetSelect = ({
datasetDetails,
datasetId,
onChange,
value,
}: DatasetSelectProps) => {
const DatasetSelect = ({ onChange, value }: DatasetSelectProps) => {
const getErrorMessage = useCallback(
({ error, message }: ClientErrorObject) => {
let errorText = message || error || t('An error has occurred');
Expand Down Expand Up @@ -84,15 +77,6 @@ const DatasetSelect = ({
.sort((a: { label: string }, b: { label: string }) =>
a.label.localeCompare(b.label),
);
if (!search) {
const found = data.find(element => element.value === datasetId);
if (!found && datasetDetails?.table_name) {
data.push({
label: datasetDetails.table_name,
value: datasetId,
});
}
}
return {
data,
totalCount: response.json.count,
Expand All @@ -107,7 +91,7 @@ const DatasetSelect = ({
return (
<Select
ariaLabel={t('Dataset')}
value={value?.value}
value={value}
options={loadDatasetOptions}
onChange={onChange}
/>
Expand Down