QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-04-18 15:44:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: remove loading indicator when typing in select (#18799)

Browse Source
This commit is contained in:
Jesse Yang
2022-03-03 09:40:49 -08:00
committed by GitHub
parent be88cb9ba0
commit 5a8eb09afb
4 changed files with 126 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts
View File

@@ -127,21 +127,10 @@ describe('Nativefilters Sanity test', () => {
.within(() =>
cy.get('input').type('wb_health_population{enter}', { force: true }),
);
// Add following step to avoid flaky enter value in line 177
cy.get(nativeFilters.filtersPanel.inputDropdown)
.should('be.visible', { timeout: 20000 })
.last()
.click();
cy.get('.loading inline-centered css-101mkpk').should('not.exist');
// hack for unclickable country_name
cy.wait(5000);
cy.get(nativeFilters.filtersPanel.filterInfoInput)
.last()
.should('be.visible', { timeout: 30000 })
.click({ force: true });
cy.get(nativeFilters.filtersPanel.filterInfoInput)
cy.get(`${nativeFilters.filtersPanel.filterInfoInput}:visible:last`)
.last()
.focus()
.type('country_name');
cy.get(nativeFilters.filtersPanel.inputDropdown)
.should('be.visible', { timeout: 20000 })

62
superset-frontend/src/components/Select/Select.test.tsx
View File

@@ -17,13 +17,7 @@
* under the License.
*/
import React from 'react';
import {
render,
screen,
waitFor,
waitForElementToBeRemoved,
within,
} from 'spec/helpers/testing-library';
import { render, screen, waitFor, within } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { Select } from 'src/components';
@@ -388,13 +382,6 @@ test('static - does not show "Loading..." when allowNewOptions is false and a ne
expect(screen.queryByText(LOADING)).not.toBeInTheDocument();
});
test('static - shows "Loading..." when allowNewOptions is true and a new option is entered', async () => {
render(<Select {...defaultProps} allowNewOptions />);
await open();
await type(NEW_OPTION);
expect(await screen.findByText(LOADING)).toBeInTheDocument();
});
test('static - does not add a new option if the option already exists', async () => {
render(<Select {...defaultProps} allowNewOptions />);
const option = OPTIONS[0].label;
@@ -456,15 +443,6 @@ test('async - displays the loading indicator when opening', async () => {
expect(screen.queryByText(LOADING)).not.toBeInTheDocument();
});
test('async - displays the loading indicator while searching', async () => {
render(<Select {...defaultProps} options={loadOptions} />);
await type('John');
expect(screen.getByText(LOADING)).toBeInTheDocument();
await waitFor(() =>
expect(screen.queryByText(LOADING)).not.toBeInTheDocument(),
);
});
test('async - makes a selection in single mode', async () => {
render(<Select {...defaultProps} options={loadOptions} />);
const optionText = 'Emma';
@@ -587,24 +565,29 @@ test('async - sets a initial value in multiple mode', async () => {
expect(values[1]).toHaveTextContent(OPTIONS[1].label);
});
test('async - searches for an item already loaded', async () => {
test('async - searches for matches in both loaded and unloaded pages', async () => {
render(<Select {...defaultProps} options={loadOptions} />);
const search = 'Oli';
await open();
await type(search);
await waitForElementToBeRemoved(screen.getByText(LOADING));
const options = await findAllSelectOptions();
await type('and');
let options = await findAllSelectOptions();
expect(options.length).toBe(1);
expect(options[0]).toHaveTextContent('Alehandro');
await screen.findByText('Sandro');
options = await findAllSelectOptions();
expect(options.length).toBe(2);
expect(options[0]).toHaveTextContent('Oliver');
expect(options[1]).toHaveTextContent('Olivia');
expect(options[0]).toHaveTextContent('Alehandro');
expect(options[1]).toHaveTextContent('Sandro');
});
test('async - searches for an item in a page not loaded', async () => {
render(<Select {...defaultProps} options={loadOptions} />);
const search = 'Ashfaq';
const mock = jest.fn(loadOptions);
render(<Select {...defaultProps} options={mock} />);
const search = 'Sandro';
await open();
await type(search);
await waitForElementToBeRemoved(screen.getByText(LOADING));
await waitFor(() => expect(mock).toHaveBeenCalledTimes(2));
const options = await findAllSelectOptions();
expect(options.length).toBe(1);
expect(options[0]).toHaveTextContent(search);
@@ -650,7 +633,7 @@ test('async - does not fire a new request for the same search input', async () =
expect(loadOptions).toHaveBeenCalledTimes(1);
clearAll();
await type('search');
expect(await screen.findByText(NO_DATA)).toBeInTheDocument();
expect(await screen.findByText(LOADING)).toBeInTheDocument();
expect(loadOptions).toHaveBeenCalledTimes(1);
});
@@ -668,13 +651,18 @@ test('async - does not fire a new request if all values have been fetched', asyn
test('async - fires a new request if all values have not been fetched', async () => {
const mock = jest.fn(loadOptions);
const search = 'George';
const pageSize = OPTIONS.length / 2;
render(<Select {...defaultProps} options={mock} pageSize={pageSize} />);
await open();
expect(mock).toHaveBeenCalledTimes(1);
await type(search);
expect(await findSelectOption(search)).toBeInTheDocument();
await type('or');
// `George` is on the first page so when it appears the API has not been called again
expect(await findSelectOption('George')).toBeInTheDocument();
expect(mock).toHaveBeenCalledTimes(1);
// `Igor` is on the second paged API request
expect(await findSelectOption('Igor')).toBeInTheDocument();
expect(mock).toHaveBeenCalledTimes(2);
});

153
superset-frontend/src/components/Select/Select.tsx
View File

@@ -20,7 +20,6 @@ import React, {
ReactElement,
ReactNode,
RefObject,
KeyboardEvent,
UIEvent,
useEffect,
useMemo,
@@ -40,7 +39,8 @@ import { isEqual } from 'lodash';
import { Spin } from 'antd';
import Icons from 'src/components/Icons';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import { hasOption } from './utils';
import { SLOW_DEBOUNCE } from 'src/constants';
import { hasOption, hasOptionIgnoreCase } from './utils';
const { Option } = AntdSelect;
@@ -58,17 +58,13 @@ type PickedSelectProps = Pick<
| 'onChange'
| 'onClear'
| 'onFocus'
| 'onBlur'
| 'placeholder'
| 'showSearch'
| 'value'
>;
type OptionsProps = Exclude<AntdSelectAllProps['options'], undefined>;
export interface OptionsType extends Omit<OptionsProps, 'label'> {
label?: string;
customLabel?: ReactNode;
}
export type OptionsType = Exclude<AntdSelectAllProps['options'], undefined>;
export type OptionsTypePage = {
data: OptionsType;
@@ -220,7 +216,6 @@ const StyledLoadingText = styled.div`
const MAX_TAG_COUNT = 4;
const TOKEN_SEPARATORS = [',', '\n', '\t', ';'];
const DEBOUNCE_TIMEOUT = 500;
const DEFAULT_PAGE_SIZE = 100;
const EMPTY_OPTIONS: OptionsType = [];
@@ -252,6 +247,9 @@ export const propertyComparator =
return (a[property] as number) - (b[property] as number);
};
const getQueryCacheKey = (value: string, page: number, pageSize: number) =>
`${value};${page};${pageSize}`;
/**
* This component is a customized version of the Antdesign 4.X Select component
* https://ant.design/components/select/.
@@ -305,7 +303,6 @@ const Select = ({
const [selectValue, setSelectValue] = useState(value);
const [searchedValue, setSearchedValue] = useState('');
const [isLoading, setIsLoading] = useState(loading);
const [isTyping, setIsTyping] = useState(false);
const [error, setError] = useState('');
const [isDropdownVisible, setIsDropdownVisible] = useState(false);
const [page, setPage] = useState(0);
@@ -318,6 +315,7 @@ const Select = ({
: allowNewOptions
? 'tags'
: 'multiple';
const allowFetch = !fetchOnlyOnSearch || searchedValue;
// TODO: Don't assume that isAsync is always labelInValue
const handleTopOptions = useCallback(
@@ -478,18 +476,16 @@ const Select = ({
);
const handlePaginatedFetch = useMemo(
() => (value: string, page: number, pageSize: number) => {
() => (value: string, page: number) => {
if (allValuesLoaded) {
setIsLoading(false);
setIsTyping(false);
return;
}
const key = `${value};${page};${pageSize}`;
const key = getQueryCacheKey(value, page, pageSize);
const cachedCount = fetchedQueries.current.get(key);
if (cachedCount) {
if (cachedCount !== undefined) {
setTotalCount(cachedCount);
setIsLoading(false);
setIsTyping(false);
return;
}
setIsLoading(true);
@@ -510,40 +506,57 @@ const Select = ({
.catch(internalOnError)
.finally(() => {
setIsLoading(false);
setIsTyping(false);
});
},
[allValuesLoaded, fetchOnlyOnSearch, handleData, internalOnError, options],
[
allValuesLoaded,
fetchOnlyOnSearch,
handleData,
internalOnError,
options,
pageSize,
],
);
const handleOnSearch = useMemo(
const debouncedHandleSearch = useMemo(
() =>
debounce((search: string) => {
const searchValue = search.trim();
if (allowNewOptions && isSingleMode) {
const newOption = searchValue &&
!hasOption(searchValue, selectOptions) && {
label: searchValue,
value: searchValue,
};
const newOptions = newOption
? [
newOption,
...selectOptions.filter(opt => opt.value !== searchedValue),
]
: [...selectOptions.filter(opt => opt.value !== searchedValue)];
setSelectOptions(newOptions);
}
if (!searchValue || searchValue === searchedValue) {
setIsTyping(false);
}
setSearchedValue(searchValue);
}, DEBOUNCE_TIMEOUT),
[allowNewOptions, isSingleMode, searchedValue, selectOptions],
// async search will be triggered in handlePaginatedFetch
setSearchedValue(search);
}, SLOW_DEBOUNCE),
[],
);
const handleOnSearch = (search: string) => {
const searchValue = search.trim();
if (allowNewOptions && isSingleMode) {
const newOption = searchValue &&
!hasOptionIgnoreCase(searchValue, selectOptions) && {
label: searchValue,
value: searchValue,
isNewOption: true,
};
const cleanSelectOptions = selectOptions.filter(
opt => !opt.isNewOption || hasOption(opt.value, selectValue),
);
const newOptions = newOption
? [newOption, ...cleanSelectOptions]
: cleanSelectOptions;
setSelectOptions(newOptions);
}
if (
isAsync &&
!allValuesLoaded &&
loadingEnabled &&
!fetchedQueries.current.has(getQueryCacheKey(searchValue, 0, pageSize))
) {
// if fetch only on search but search value is empty, then should not be
// in loading state
setIsLoading(!(fetchOnlyOnSearch && !searchValue));
}
return debouncedHandleSearch(search);
};
const handlePagination = (e: UIEvent<HTMLElement>) => {
const vScroll = e.currentTarget;
const thresholdReached =
@@ -552,7 +565,7 @@ const Select = ({
if (!isLoading && isAsync && hasMoreData && thresholdReached) {
const newPage = page + 1;
handlePaginatedFetch(searchedValue, newPage, pageSize);
handlePaginatedFetch(searchedValue, newPage);
setPage(newPage);
}
};
@@ -581,8 +594,21 @@ const Select = ({
const handleOnDropdownVisibleChange = (isDropdownVisible: boolean) => {
setIsDropdownVisible(isDropdownVisible);
if (isAsync && !loadingEnabled) {
setLoadingEnabled(true);
if (isAsync) {
// loading is enabled when dropdown is open,
// disabled when dropdown is closed
if (loadingEnabled !== isDropdownVisible) {
setLoadingEnabled(isDropdownVisible);
}
// when closing dropdown, always reset loading state
if (!isDropdownVisible && isLoading) {
// delay is for the animation of closing the dropdown
// so the dropdown doesn't flash between "Loading..." and "No data"
// before closing.
setTimeout(() => {
setIsLoading(false);
}, 250);
}
}
// multiple or tags mode keep the dropdown visible while selecting options
@@ -598,19 +624,15 @@ const Select = ({
if (!isDropdownVisible) {
originNode.ref?.current?.scrollTo({ top: 0 });
}
if ((isLoading && selectOptions.length === 0) || isTyping) {
if (isLoading && selectOptions.length === 0) {
return <StyledLoadingText>{t('Loading...')}</StyledLoadingText>;
}
return error ? <Error error={error} /> : originNode;
};
const onInputKeyDown = (event: KeyboardEvent<HTMLInputElement>) => {
if (event.key.length === 1 && isAsync && !isTyping) {
setIsTyping(true);
}
};
const SuffixIcon = () => {
// use a function instead of component since every rerender of the
// Select component will create a new component
const getSuffixIcon = () => {
if (isLoading) {
return <StyledSpin size="small" />;
}
@@ -672,22 +694,24 @@ const Select = ({
}, [labelInValue, isAsync, selectValue]);
// Stop the invocation of the debounced function after unmounting
useEffect(() => () => handleOnSearch.cancel(), [handleOnSearch]);
useEffect(
() => () => {
debouncedHandleSearch.cancel();
},
[debouncedHandleSearch],
);
useEffect(() => {
const allowFetch = !fetchOnlyOnSearch || searchedValue;
if (isAsync && loadingEnabled && allowFetch) {
const page = 0;
handlePaginatedFetch(searchedValue, page, pageSize);
setPage(page);
handlePaginatedFetch(searchedValue, 0);
setPage(0);
}
}, [
isAsync,
searchedValue,
pageSize,
handlePaginatedFetch,
loadingEnabled,
fetchOnlyOnSearch,
allowFetch,
]);
useEffect(() => {
@@ -713,16 +737,9 @@ const Select = ({
labelInValue={isAsync || labelInValue}
maxTagCount={MAX_TAG_COUNT}
mode={mappedMode}
notFoundContent={
allowNewOptions && !fetchOnlyOnSearch ? (
<StyledLoadingText>{t('Loading...')}</StyledLoadingText>
) : (
notFoundContent
)
}
notFoundContent={isLoading ? t('Loading...') : notFoundContent}
onDeselect={handleOnDeselect}
onDropdownVisibleChange={handleOnDropdownVisibleChange}
onInputKeyDown={onInputKeyDown}
onPopupScroll={isAsync ? handlePagination : undefined}
onSearch={shouldShowSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect}
@@ -734,7 +751,7 @@ const Select = ({
showArrow
tokenSeparators={TOKEN_SEPARATORS}
value={selectValue}
suffixIcon={<SuffixIcon />}
suffixIcon={getSuffixIcon()}
menuItemSelectedIcon={
invertSelection ? (
<StyledStopOutlined iconSize="m" />

15
superset-frontend/src/components/Select/utils.ts
View File

@@ -1,3 +1,4 @@
import { ensureIsArray } from '@superset-ui/core';
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
@@ -60,7 +61,19 @@ export function findValue<OptionType extends OptionTypeBase>(
return (Array.isArray(value) ? value : [value]).map(find);
}
export function hasOption(search: string, options: AntdOptionsType) {
export function hasOption<VT extends string | number>(
value: VT,
options?: VT | VT[] | { value: VT } | { value: VT }[],
) {
const optionsArray = ensureIsArray(options);
return (
optionsArray.find(x =>
typeof x === 'object' ? x.value === value : x === value,
) !== undefined
);
}
export function hasOptionIgnoreCase(search: string, options: AntdOptionsType) {
const searchOption = search.trim().toLowerCase();
return options.find(opt => {
const { label, value } = opt;