From cdab55b06ab6ec4a04b09875ff7eef3aed59ab07 Mon Sep 17 00:00:00 2001 From: cccs-RyanK <102618419+cccs-RyanK@users.noreply.github.com> Date: Fri, 17 Jun 2022 16:25:00 -0400 Subject: [PATCH 01/20] Created AsyncSelect Component Changed files to reference AsyncSelect if needed --- .../src/addSlice/AddSliceContainer.tsx | 3 +- .../src/components/DatabaseSelector/index.tsx | 3 +- .../Datasource/DatasourceEditor.jsx | 3 +- .../components/Select/AsyncSelect.test.tsx | 687 ++++++++++++++++ .../src/components/Select/AsyncSelect.tsx | 736 ++++++++++++++++++ .../components/PropertiesModal/index.tsx | 7 +- .../FiltersConfigForm/DatasetSelect.tsx | 3 +- .../components/PropertiesModal/index.tsx | 3 +- .../src/views/CRUD/alert/AlertReportModal.tsx | 9 +- 9 files changed, 1442 insertions(+), 12 deletions(-) create mode 100644 superset-frontend/src/components/Select/AsyncSelect.test.tsx create mode 100644 superset-frontend/src/components/Select/AsyncSelect.tsx diff --git a/superset-frontend/src/addSlice/AddSliceContainer.tsx b/superset-frontend/src/addSlice/AddSliceContainer.tsx index 7e9f0a1a2ed5..82e4e7c77c7c 100644 --- a/superset-frontend/src/addSlice/AddSliceContainer.tsx +++ b/superset-frontend/src/addSlice/AddSliceContainer.tsx @@ -27,6 +27,7 @@ import { Tooltip } from 'src/components/Tooltip'; import VizTypeGallery, { MAX_ADVISABLE_VIZ_GALLERY_WIDTH, } from 'src/explore/components/controls/VizTypeControl/VizTypeGallery'; +import AsyncSelect from 'src/components/Select/AsyncSelect'; type Dataset = { id: number; @@ -280,7 +281,7 @@ export default class AddSliceContainer extends React.PureComponent< status={this.state.datasource?.value ? 'finish' : 'process'} description={
- option1.label.localeCompare(option2.label)); +const NULL_OPTION = { label: '', value: null } as unknown as { + label: string; + value: number; +}; + +const loadOptions = async (search: string, page: number, pageSize: number) => { + const totalCount = OPTIONS.length; + const start = page * pageSize; + const deleteCount = + start + pageSize < totalCount ? pageSize : totalCount - start; + const data = OPTIONS.filter(option => option.label.match(search)).splice( + start, + deleteCount, + ); + return { + data, + totalCount: OPTIONS.length, + }; +}; + +const defaultProps = { + allowClear: true, + ariaLabel: ARIA_LABEL, + labelInValue: true, + options: OPTIONS, + pageSize: 10, + showSearch: true, +}; + +const getElementByClassName = (className: string) => + document.querySelector(className)! as HTMLElement; + +const getElementsByClassName = (className: string) => + document.querySelectorAll(className)! as NodeListOf; + +const getSelect = () => screen.getByRole('combobox', { name: ARIA_LABEL }); + +const findSelectOption = (text: string) => + waitFor(() => + within(getElementByClassName('.rc-virtual-list')).getByText(text), + ); + +const findAllSelectOptions = () => + waitFor(() => getElementsByClassName('.ant-select-item-option-content')); + +const findSelectValue = () => + waitFor(() => getElementByClassName('.ant-select-selection-item')); + +const findAllSelectValues = () => + waitFor(() => getElementsByClassName('.ant-select-selection-item')); + +const clearAll = () => userEvent.click(screen.getByLabelText('close-circle')); + +const matchOrder = async (expectedLabels: string[]) => { + const actualLabels: string[] = []; + (await findAllSelectOptions()).forEach(option => { + actualLabels.push(option.textContent || ''); + }); + // menu is a virtual list, which means it may not render all options + expect(actualLabels.slice(0, expectedLabels.length)).toEqual( + expectedLabels.slice(0, actualLabels.length), + ); + return true; +}; + +const type = (text: string) => { + const select = getSelect(); + userEvent.clear(select); + return userEvent.type(select, text, { delay: 10 }); +}; + +const open = () => waitFor(() => userEvent.click(getSelect())); + +test('displays a header', async () => { + const headerText = 'Header'; + render(, + ); + await open(); + expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument(); + + rerender( + ); + await open(); + userEvent.click(await findSelectOption(OPTIONS[0].label)); + expect(await screen.findByLabelText('stop')).toBeInTheDocument(); +}); + +test('sort the options by label if no sort comparator is provided', async () => { + const unsortedOptions = [...OPTIONS].sort(() => Math.random()); + render(, + ); + await open(); + 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); + }); +}); + +test('should sort selected to top when in single mode', async () => { + render(); + const originalLabels = OPTIONS.map(option => option.label); + let labels = originalLabels.slice(); + + await open(); + userEvent.click(await findSelectOption(labels[1])); + expect(await matchOrder(labels)).toBe(true); + + await type('{esc}'); + await open(); + labels = labels.splice(1, 1).concat(labels); + expect(await matchOrder(labels)).toBe(true); + + await open(); + userEvent.click(await findSelectOption(labels[5])); + await type('{esc}'); + await open(); + labels = [labels.splice(0, 1)[0], labels.splice(4, 1)[0]].concat(labels); + expect(await matchOrder(labels)).toBe(true); + + // should revert to original order + clearAll(); + await type('{esc}'); + await open(); + expect(await matchOrder(originalLabels)).toBe(true); +}); + +test('searches for label or value', async () => { + const option = OPTIONS[11]; + 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('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('Liam'); + let options = await findAllSelectOptions(); + expect(options.length).toBe(1); + expect(options[0]).toHaveTextContent('Liam'); + await type('Female'); + options = await findAllSelectOptions(); + expect(options.length).toBe(6); + expect(options[0]).toHaveTextContent('Ava'); + expect(options[1]).toHaveTextContent('Charlotte'); + 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(); +}); + +test('removes duplicated values', async () => { + render(); + await open(); + expect(screen.getByRole('heading', { name: 'John' })).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Liam' })).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Olivia' })).toBeInTheDocument(); +}); + +test('searches for a word with a custom label', async () => { + const options = [ + { label: 'John', value: 1, customLabel:

John

}, + { label: 'Liam', value: 2, customLabel:

Liam

}, + { label: 'Olivia', value: 3, customLabel:

Olivia

}, + ]; + render(); + await type(NEW_OPTION); + expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument(); + await type('k'); + await waitFor(() => + expect(screen.queryByText(NEW_OPTION)).not.toBeInTheDocument(), + ); +}); + +test('clear all the values', async () => { + const onClear = jest.fn(); + render( + ); + await open(); + await type(NEW_OPTION); + expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); +}); + +test('adds the null option when selected in single mode', async () => { + render(, + ); + await open(); + userEvent.click(await findSelectOption(OPTIONS[0].label)); + userEvent.click(await findSelectOption(NULL_OPTION.label)); + const values = await findAllSelectValues(); + expect(values[0]).toHaveTextContent(OPTIONS[0].label); + expect(values[1]).toHaveTextContent(NULL_OPTION.label); +}); + +test('async - renders the select with default props', () => { + render( ({ data: [], totalCount: 0 })} + />, + ); + await open(); + expect(await screen.findByText(/no data/i)).toBeInTheDocument(); +}); + +test('async - displays the loading indicator when opening', async () => { + render(); + const optionText = 'Emma'; + await open(); + userEvent.click(await findSelectOption(optionText)); + expect(await findSelectValue()).toHaveTextContent(optionText); +}); + +test('async - multiple selections in multiple mode', async () => { + render(, + ); + await open(); + const [firstOption, secondOption] = OPTIONS; + userEvent.click(await findSelectOption(firstOption.label)); + expect(onChange).toHaveBeenCalledWith( + expect.objectContaining({ + label: firstOption.label, + value: firstOption.value, + }), + firstOption, + ); + expect(await findSelectValue()).toHaveTextContent(firstOption.label); + userEvent.click(await findSelectOption(secondOption.label)); + expect(onChange).toHaveBeenCalledWith( + expect.objectContaining({ + label: secondOption.label, + value: secondOption.value, + }), + secondOption, + ); + expect(await findSelectValue()).toHaveTextContent(secondOption.label); +}); + +test('async - deselects an item in multiple mode', async () => { + render(); + await open(); + await type(NEW_OPTION); + expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument(); +}); + +test('async - does not add a new option if the option already exists', async () => { + render(, + ); + await open(); + await type(NEW_OPTION); + expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); +}); + +test('async - does not show "No data" when allowNewOptions is true and a new option is entered', async () => { + render(); + expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label); +}); + +test('async - sets a initial value in multiple mode', async () => { + render( + ); + await open(); + 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('Alehandro'); + expect(options[1]).toHaveTextContent('Sandro'); +}); + +test('async - searches for an item in a page not loaded', async () => { + const mock = jest.fn(loadOptions); + render(); + expect(loadOptions).not.toHaveBeenCalled(); +}); + +test('async - fetches data when opening', async () => { + const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 })); + render(); + await open(); + await waitFor(() => expect(loadOptions).not.toHaveBeenCalled()); + await type('search'); + await waitFor(() => expect(loadOptions).toHaveBeenCalled()); +}); + +test('async - displays an error message when an exception is thrown while fetching', async () => { + const error = 'Fetch error'; + const loadOptions = async () => { + throw new Error(error); + }; + render(); + await type('search'); + expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); + expect(loadOptions).toHaveBeenCalledTimes(1); + clearAll(); + await type('search'); + expect(await screen.findByText(LOADING)).toBeInTheDocument(); + expect(loadOptions).toHaveBeenCalledTimes(1); +}); + +test('async - does not fire a new request if all values have been fetched', async () => { + const mock = jest.fn(loadOptions); + const search = 'George'; + const pageSize = OPTIONS.length; + render(); + await open(); + expect(mock).toHaveBeenCalledTimes(1); + 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); +}); + +/* + TODO: Add tests that require scroll interaction. Needs further investigation. + - Fetches more data when scrolling and more data is available + - Doesn't fetch more data when no more data is available + - Requests the correct page and page size + - Sets the page to zero when a new search is made + */ diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx new file mode 100644 index 000000000000..8642f80a3c61 --- /dev/null +++ b/superset-frontend/src/components/Select/AsyncSelect.tsx @@ -0,0 +1,736 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React, { + forwardRef, + ReactElement, + ReactNode, + RefObject, + UIEvent, + useEffect, + useMemo, + useState, + useRef, + useCallback, +} from 'react'; +import { ensureIsArray, styled, t } from '@superset-ui/core'; +import AntdSelect, { + SelectProps as AntdSelectProps, + SelectValue as AntdSelectValue, + LabeledValue as AntdLabeledValue, +} from 'antd/lib/select'; +import { DownOutlined, SearchOutlined } from '@ant-design/icons'; +import { Spin } from 'antd'; +import debounce from 'lodash/debounce'; +import { isEqual } from 'lodash'; +import Icons from 'src/components/Icons'; +import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import { SLOW_DEBOUNCE } from 'src/constants'; +import { rankedSearchCompare } from 'src/utils/rankedSearchCompare'; +import { getValue, hasOption, isLabeledValue } from './utils'; + +const { Option } = AntdSelect; + +type AntdSelectAllProps = AntdSelectProps; + +type PickedSelectProps = Pick< + AntdSelectAllProps, + | 'allowClear' + | 'autoFocus' + | 'disabled' + | 'filterOption' + | 'labelInValue' + | 'loading' + | 'notFoundContent' + | 'onChange' + | 'onClear' + | 'onFocus' + | 'onBlur' + | 'onDropdownVisibleChange' + | 'placeholder' + | 'showSearch' + | 'tokenSeparators' + | 'value' +>; + +export type OptionsType = Exclude; + +export type OptionsTypePage = { + data: OptionsType; + totalCount: number; +}; + +export type OptionsPagePromise = ( + search: string, + page: number, + pageSize: number, +) => Promise; + +export interface AsyncSelectProps extends PickedSelectProps { + /** + * It enables the user to create new options. + * Can be used with standard or async select types. + * Can be used with any mode, single or multiple. + * False by default. + * */ + allowNewOptions?: boolean; + /** + * It adds the aria-label tag for accessibility standards. + * Must be plain English and localized. + */ + ariaLabel: string; + /** + * It adds a header on top of the Select. + * Can be any ReactNode. + */ + header?: ReactNode; + /** + * It fires a request against the server after + * the first interaction and not on render. + * Works in async mode only (See the options property). + * True by default. + */ + lazyLoading?: boolean; + /** + * It defines whether the Select should allow for the + * selection of multiple options or single. + * Single by default. + */ + mode?: 'single' | 'multiple'; + /** + * Deprecated. + * Prefer ariaLabel instead. + */ + name?: string; // discourage usage + /** + * It allows to define which properties of the option object + * should be looked for when searching. + * By default label and value. + */ + optionFilterProps?: string[]; + /** + * 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. + */ + options: OptionsType | OptionsPagePromise; + /** + * It defines how many results should be included + * in the query response. + * Works in async mode only (See the options property). + */ + pageSize?: number; + /** + * It shows a stop-outlined icon at the far right of a selected + * option instead of the default checkmark. + * Useful to better indicate to the user that by clicking on a selected + * option it will be de-selected. + * False by default. + */ + invertSelection?: boolean; + /** + * It fires a request against the server only after + * searching. + * Works in async mode only (See the options property). + * Undefined by default. + */ + fetchOnlyOnSearch?: boolean; + /** + * It provides a callback function when an error + * is generated after a request is fired. + * Works in async mode only (See the options property). + */ + onError?: (error: string) => void; + /** + * Customize how filtered options are sorted while users search. + * Will not apply to predefined `options` array when users are not searching. + */ + sortComparator?: typeof DEFAULT_SORT_COMPARATOR; +} + +const StyledContainer = styled.div` + display: flex; + flex-direction: column; + width: 100%; +`; + +const StyledSelect = styled(AntdSelect)` + ${({ theme }) => ` + && .ant-select-selector { + border-radius: ${theme.gridUnit}px; + } + // Open the dropdown when clicking on the suffix + // This is fixed in version 4.16 + .ant-select-arrow .anticon:not(.ant-select-suffix) { + pointer-events: none; + } + `} +`; + +const StyledStopOutlined = styled(Icons.StopOutlined)` + vertical-align: 0; +`; + +const StyledCheckOutlined = styled(Icons.CheckOutlined)` + vertical-align: 0; +`; + +const StyledError = styled.div` + ${({ theme }) => ` + display: flex; + justify-content: center; + align-items: flex-start; + width: 100%; + padding: ${theme.gridUnit * 2}px; + color: ${theme.colors.error.base}; + & svg { + margin-right: ${theme.gridUnit * 2}px; + } + `} +`; + +const StyledErrorMessage = styled.div` + overflow: hidden; + text-overflow: ellipsis; +`; + +const StyledSpin = styled(Spin)` + margin-top: ${({ theme }) => -theme.gridUnit}px; +`; + +const StyledLoadingText = styled.div` + ${({ theme }) => ` + margin-left: ${theme.gridUnit * 3}px; + line-height: ${theme.gridUnit * 8}px; + color: ${theme.colors.grayscale.light1}; + `} +`; + +const MAX_TAG_COUNT = 4; +const TOKEN_SEPARATORS = [',', '\n', '\t', ';']; +const DEFAULT_PAGE_SIZE = 100; +const EMPTY_OPTIONS: OptionsType = []; + +const Error = ({ error }: { error: string }) => ( + + {error} + +); + +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') { + aText = a.label; + bText = b.label; + } else if (typeof a.value === 'string' && typeof b.value === 'string') { + aText = a.value; + bText = 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); +}; + +/** + * It creates a comparator to check for a specific property. + * Can be used with string and number property values. + * */ +export const propertyComparator = + (property: string) => (a: AntdLabeledValue, b: AntdLabeledValue) => { + if (typeof a[property] === 'string' && typeof b[property] === 'string') { + return a[property].localeCompare(b[property]); + } + 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/. + * The aim of the component was to combine all the instances of select components throughout the + * project under one and to remove the react-select component entirely. + * This Select component provides an API that is tested against all the different use cases of Superset. + * It limits and overrides the existing Antdesign API in order to keep their usage to the minimum + * and to enforce simplification and standardization. + * It is divided into two macro categories, Static and Async. + * The Static type accepts a static array of options. + * The Async type accepts a promise that will return the options. + * Each of the categories come with different abilities. For a comprehensive guide please refer to + * the storybook in src/components/Select/Select.stories.tsx. + */ +const AsyncSelect = ( + { + allowClear, + allowNewOptions = false, + ariaLabel, + fetchOnlyOnSearch, + filterOption = true, + header = null, + invertSelection = false, + labelInValue = false, + lazyLoading = true, + loading, + mode = 'single', + name, + notFoundContent, + onError, + onChange, + onClear, + onDropdownVisibleChange, + optionFilterProps = ['label', 'value'], + options, + pageSize = DEFAULT_PAGE_SIZE, + placeholder = t('Select ...'), + showSearch = true, + sortComparator = DEFAULT_SORT_COMPARATOR, + tokenSeparators, + value, + ...props + }: AsyncSelectProps, + ref: RefObject, +) => { + const isAsync = typeof options === 'function'; + const isSingleMode = mode === 'single'; + const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch; + const [selectValue, setSelectValue] = useState(value); + const [inputValue, setInputValue] = useState(''); + const [isLoading, setIsLoading] = useState(loading); + const [error, setError] = useState(''); + const [isDropdownVisible, setIsDropdownVisible] = useState(false); + const [page, setPage] = useState(0); + const [totalCount, setTotalCount] = useState(0); + const [loadingEnabled, setLoadingEnabled] = useState(!lazyLoading); + const [allValuesLoaded, setAllValuesLoaded] = useState(false); + const fetchedQueries = useRef(new Map()); + const mappedMode = isSingleMode + ? undefined + : allowNewOptions + ? 'tags' + : 'multiple'; + const allowFetch = !fetchOnlyOnSearch || inputValue; + + 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 sortComparatorForNoSearch = useCallback( + (a: AntdLabeledValue, b: AntdLabeledValue) => + sortSelectedFirst(a, b) || + // Only apply the custom sorter in async mode because we should + // preserve the options order as much as possible. + (isAsync ? sortComparator(a, b, '') : 0), + [isAsync, sortComparator, sortSelectedFirst], + ); + + const initialOptions = useMemo( + () => (options && Array.isArray(options) ? options.slice() : EMPTY_OPTIONS), + [options], + ); + const initialOptionsSorted = useMemo( + () => initialOptions.slice().sort(sortComparatorForNoSearch), + [initialOptions, sortComparatorForNoSearch], + ); + + const [selectOptions, setSelectOptions] = + useState(initialOptionsSorted); + + // 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 => + isLabeledValue(opt) ? 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 = ( + selectedItem: string | number | AntdLabeledValue | undefined, + ) => { + if (isSingleMode) { + setSelectValue(selectedItem); + } else { + setSelectValue(previousState => { + const array = ensureIsArray(previousState); + const value = getValue(selectedItem); + // Tokenized values can contain duplicated values + if (!hasOption(value, array)) { + const result = [...array, selectedItem]; + return isLabeledValue(selectedItem) + ? (result as AntdLabeledValue[]) + : (result as (string | number)[]); + } + return previousState; + }); + } + setInputValue(''); + }; + + const handleOnDeselect = ( + value: string | number | AntdLabeledValue | undefined, + ) => { + if (Array.isArray(selectValue)) { + if (isLabeledValue(value)) { + const array = selectValue as AntdLabeledValue[]; + setSelectValue(array.filter(element => element.value !== value.value)); + } else { + const array = selectValue as (string | number)[]; + setSelectValue(array.filter(element => element !== value)); + } + } + setInputValue(''); + }; + + const internalOnError = useCallback( + (response: Response) => + getClientErrorObject(response).then(e => { + const { error } = e; + setError(error); + + if (onError) { + onError(error); + } + }), + [onError], + ); + + const mergeData = useCallback( + (data: OptionsType) => { + let mergedData: OptionsType = []; + if (data && Array.isArray(data) && data.length) { + // 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(previousOption.value)) + .concat(data) + .sort(sortComparatorForNoSearch); + return mergedData; + }); + } + return mergedData; + }, + [sortComparatorForNoSearch], + ); + + const fetchPage = useMemo( + () => (search: string, page: number) => { + setPage(page); + if (allValuesLoaded) { + setIsLoading(false); + return; + } + const key = getQueryCacheKey(search, page, pageSize); + const cachedCount = fetchedQueries.current.get(key); + if (cachedCount !== undefined) { + setTotalCount(cachedCount); + setIsLoading(false); + return; + } + setIsLoading(true); + const fetchOptions = options as OptionsPagePromise; + fetchOptions(search, page, pageSize) + .then(({ data, totalCount }: OptionsTypePage) => { + const mergedData = mergeData(data); + fetchedQueries.current.set(key, totalCount); + setTotalCount(totalCount); + if ( + !fetchOnlyOnSearch && + value === '' && + mergedData.length >= totalCount + ) { + setAllValuesLoaded(true); + } + }) + .catch(internalOnError) + .finally(() => { + setIsLoading(false); + }); + }, + [ + allValuesLoaded, + fetchOnlyOnSearch, + mergeData, + internalOnError, + options, + pageSize, + value, + ], + ); + + const debouncedFetchPage = useMemo( + () => debounce(fetchPage, SLOW_DEBOUNCE), + [fetchPage], + ); + + const handleOnSearch = (search: string) => { + const searchValue = search.trim(); + if (allowNewOptions && isSingleMode) { + const newOption = searchValue && + !hasOption(searchValue, fullSelectOptions, true) && { + label: searchValue, + value: searchValue, + isNewOption: true, + }; + const cleanSelectOptions = fullSelectOptions.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)); + } + setInputValue(search); + }; + + const handlePagination = (e: UIEvent) => { + const vScroll = e.currentTarget; + const thresholdReached = + vScroll.scrollTop > (vScroll.scrollHeight - vScroll.offsetHeight) * 0.7; + const hasMoreData = page * pageSize + pageSize < totalCount; + + if (!isLoading && isAsync && hasMoreData && thresholdReached) { + const newPage = page + 1; + fetchPage(inputValue, newPage); + } + }; + + const handleFilterOption = (search: string, option: AntdLabeledValue) => { + if (typeof filterOption === 'function') { + return filterOption(search, option); + } + + if (filterOption) { + const searchValue = search.trim().toLowerCase(); + if (optionFilterProps && optionFilterProps.length) { + return optionFilterProps.some(prop => { + const optionProp = option?.[prop] + ? String(option[prop]).trim().toLowerCase() + : ''; + return optionProp.includes(searchValue); + }); + } + } + + return false; + }; + + const handleOnDropdownVisibleChange = (isDropdownVisible: boolean) => { + setIsDropdownVisible(isDropdownVisible); + + 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); + } + } + // if no search input value, force sort options because it won't be sorted by + // `filterSort`. + if (isDropdownVisible && !inputValue && selectOptions.length > 1) { + const sortedOptions = isAsync + ? selectOptions.slice().sort(sortComparatorForNoSearch) + : // if not in async mode, revert to the original select options + // (with selected options still sorted to the top) + initialOptionsSorted; + if (!isEqual(sortedOptions, selectOptions)) { + setSelectOptions(sortedOptions); + } + } + + if (onDropdownVisibleChange) { + onDropdownVisibleChange(isDropdownVisible); + } + }; + + const dropdownRender = ( + originNode: ReactElement & { ref?: RefObject }, + ) => { + if (!isDropdownVisible) { + originNode.ref?.current?.scrollTo({ top: 0 }); + } + if (isLoading && fullSelectOptions.length === 0) { + return {t('Loading...')}; + } + return error ? : originNode; + }; + + // use a function instead of component since every rerender of the + // Select component will create a new component + const getSuffixIcon = () => { + if (isLoading) { + return ; + } + if (shouldShowSearch && isDropdownVisible) { + return ; + } + return ; + }; + + const handleClear = () => { + setSelectValue(undefined); + if (onClear) { + onClear(); + } + }; + + useEffect(() => { + // when `options` list is updated from component prop, reset states + fetchedQueries.current.clear(); + setAllValuesLoaded(false); + setSelectOptions(initialOptions); + }, [initialOptions]); + + useEffect(() => { + setSelectValue(value); + }, [value]); + + // Stop the invocation of the debounced function after unmounting + useEffect( + () => () => { + debouncedFetchPage.cancel(); + }, + [debouncedFetchPage], + ); + + useEffect(() => { + if (isAsync && loadingEnabled && allowFetch) { + // trigger fetch every time inputValue changes + if (inputValue) { + debouncedFetchPage(inputValue, 0); + } else { + fetchPage('', 0); + } + } + }, [ + isAsync, + loadingEnabled, + fetchPage, + allowFetch, + inputValue, + debouncedFetchPage, + ]); + + useEffect(() => { + if (loading !== undefined && loading !== isLoading) { + setIsLoading(loading); + } + }, [isLoading, loading]); + + return ( + + {header} + triggerNode.parentNode} + labelInValue={isAsync || labelInValue} + maxTagCount={MAX_TAG_COUNT} + mode={mappedMode} + notFoundContent={isLoading ? t('Loading...') : notFoundContent} + onDeselect={handleOnDeselect} + onDropdownVisibleChange={handleOnDropdownVisibleChange} + onPopupScroll={isAsync ? handlePagination : undefined} + onSearch={shouldShowSearch ? handleOnSearch : undefined} + onSelect={handleOnSelect} + onClear={handleClear} + onChange={onChange} + options={hasCustomLabels ? undefined : fullSelectOptions} + placeholder={placeholder} + showSearch={shouldShowSearch} + showArrow + tokenSeparators={tokenSeparators || TOKEN_SEPARATORS} + value={selectValue} + suffixIcon={getSuffixIcon()} + menuItemSelectedIcon={ + invertSelection ? ( + + ) : ( + + ) + } + ref={ref} + {...props} + > + {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 ( + + ); + })} + + + ); +}; + +export default forwardRef(AsyncSelect); diff --git a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx index effd18b3d07c..ded3177bd193 100644 --- a/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/dashboard/components/PropertiesModal/index.tsx @@ -39,6 +39,7 @@ import ColorSchemeControlWrapper from 'src/dashboard/components/ColorSchemeContr import { getClientErrorObject } from 'src/utils/getClientErrorObject'; import withToasts from 'src/components/MessageToasts/withToasts'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; +import AsyncSelect from 'src/components/Select/AsyncSelect'; const StyledFormItem = styled(FormItem)` margin-bottom: 0; @@ -370,7 +371,7 @@ const PropertiesModal = ({

{t('Access')}

- -

{t('Access')}

- = ({ *
- = ({ options={loadChartOptions} onChange={onChartChange} /> - ); expect(getSelect()).toBeInTheDocument(); }); -test('async - opens the select without any data', async () => { +test('opens the select without any data', async () => { render( ); await waitFor(() => { userEvent.click(getSelect()); @@ -437,7 +437,7 @@ test('async - displays the loading indicator when opening', async () => { expect(screen.queryByText(LOADING)).not.toBeInTheDocument(); }); -test('async - makes a selection in single mode', async () => { +test('makes a selection in single mode', async () => { render(); await open(); const [firstOption, secondOption] = OPTIONS; @@ -456,7 +456,7 @@ test('async - multiple selections in multiple mode', async () => { expect(values[1]).toHaveTextContent(secondOption.label); }); -test('async - changes the selected item in single mode', async () => { +test('changes the selected item in single mode', async () => { const onChange = jest.fn(); render( ); await open(); const option3 = OPTIONS[2]; @@ -517,14 +517,14 @@ test('async - deselects an item in multiple mode', async () => { expect(values[0]).toHaveTextContent(option8.label); }); -test('async - adds a new option if none is available and allowNewOptions is true', async () => { +test('adds a new option if none is available and allowNewOptions is true', async () => { render(); const option = OPTIONS[0].label; await open(); @@ -537,7 +537,7 @@ test('async - does not add a new option if the option already exists', async () }); }); -test('async - shows "No data" when allowNewOptions is false and a new option is entered', async () => { +test('shows "No data" when allowNewOptions is false and a new option is entered', async () => { render( ); await open(); await type(NEW_OPTION); expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument(); }); -test('async - sets a initial value in single mode', async () => { +test('sets a initial value in single mode', async () => { render( { expect(values[1]).toHaveTextContent(OPTIONS[1].label); }); -test('async - searches for matches in both loaded and unloaded pages', async () => { +test('searches for matches in both loaded and unloaded pages', async () => { render(); const search = 'Sandro'; @@ -605,20 +605,20 @@ test('async - searches for an item in a page not loaded', async () => { expect(options[0]).toHaveTextContent(search); }); -test('async - does not fetches data when rendering', async () => { +test('does not fetches data when rendering', async () => { const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 })); render(); await open(); expect(loadOptions).toHaveBeenCalled(); }); -test('async - fetches data only after a search input is entered if fetchOnlyOnSearch is true', async () => { +test('fetches data only after a search input is entered if fetchOnlyOnSearch is true', async () => { const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 })); render(); await type('search'); @@ -649,7 +649,7 @@ test('async - does not fire a new request for the same search input', async () = expect(loadOptions).toHaveBeenCalledTimes(1); }); -test('async - 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 () => { const mock = jest.fn(loadOptions); const search = 'George'; const pageSize = OPTIONS.length; @@ -661,7 +661,7 @@ test('async - does not fire a new request if all values have been fetched', asyn expect(mock).toHaveBeenCalledTimes(1); }); -test('async - fires a new request if all values have not been fetched', async () => { +test('fires a new request if all values have not been fetched', async () => { const mock = jest.fn(loadOptions); const pageSize = OPTIONS.length / 2; render( { expect(values[1]).toHaveTextContent(NULL_OPTION.label); }); -test('static - renders the select with default props', () => { +test('renders the select with default props', () => { render(); await open(); expect(screen.getByText(NO_DATA)).toBeInTheDocument(); }); -test('static - makes a selection in single mode', async () => { +test('makes a selection in single mode', async () => { render(); await open(); const [firstOption, secondOption] = OPTIONS; @@ -443,7 +443,7 @@ test('static - multiple selections in multiple mode', async () => { expect(values[1]).toHaveTextContent(secondOption.label); }); -test('static - changes the selected item in single mode', async () => { +test('changes the selected item in single mode', async () => { const onChange = jest.fn(); render(); await open(); const [firstOption, secondOption] = OPTIONS; @@ -484,35 +484,35 @@ test('static - deselects an item in multiple mode', async () => { expect(values[0]).toHaveTextContent(secondOption.label); }); -test('static - adds a new option if none is available and allowNewOptions is true', async () => { +test('adds a new option if none is available and allowNewOptions is true', async () => { render(); await open(); await type(NEW_OPTION); expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); }); -test('static - does not show "No data" when allowNewOptions is true and a new option is entered', async () => { +test('does not show "No data" when allowNewOptions is true and a new option is entered', async () => { render(); await open(); await type(NEW_OPTION); expect(screen.queryByText(LOADING)).not.toBeInTheDocument(); }); -test('static - does not add a new option if the option already exists', async () => { +test('does not add a new option if the option already exists', async () => { render(); expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label); }); -test('static - sets a initial value in multiple mode', async () => { +test('sets a initial value in multiple mode', async () => { render( ); const search = 'Oli'; await type(search); @@ -547,288 +547,6 @@ test('static - searches for an item', async () => { expect(options[0]).toHaveTextContent('Oliver'); expect(options[1]).toHaveTextContent('Olivia'); }); - -test('async - renders the select with default props', () => { - render( ({ data: [], totalCount: 0 })} - />, - ); - await open(); - expect(await screen.findByText(/no data/i)).toBeInTheDocument(); -}); - -test('async - displays the loading indicator when opening', async () => { - render(); - const optionText = 'Emma'; - await open(); - userEvent.click(await findSelectOption(optionText)); - expect(await findSelectValue()).toHaveTextContent(optionText); -}); - -test('async - multiple selections in multiple mode', async () => { - render(, - ); - await open(); - const [firstOption, secondOption] = OPTIONS; - userEvent.click(await findSelectOption(firstOption.label)); - expect(onChange).toHaveBeenCalledWith( - expect.objectContaining({ - label: firstOption.label, - value: firstOption.value, - }), - firstOption, - ); - expect(await findSelectValue()).toHaveTextContent(firstOption.label); - userEvent.click(await findSelectOption(secondOption.label)); - expect(onChange).toHaveBeenCalledWith( - expect.objectContaining({ - label: secondOption.label, - value: secondOption.value, - }), - secondOption, - ); - expect(await findSelectValue()).toHaveTextContent(secondOption.label); -}); - -test('async - deselects an item in multiple mode', async () => { - render(); - await open(); - await type(NEW_OPTION); - expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument(); -}); - -test('async - does not add a new option if the option already exists', async () => { - render(, - ); - await open(); - await type(NEW_OPTION); - expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); -}); - -test('async - does not show "No data" when allowNewOptions is true and a new option is entered', async () => { - render(); - expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label); -}); - -test('async - sets a initial value in multiple mode', async () => { - render( - ); - await open(); - 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('Alehandro'); - expect(options[1]).toHaveTextContent('Sandro'); -}); - -test('async - searches for an item in a page not loaded', async () => { - const mock = jest.fn(loadOptions); - render(); - expect(loadOptions).not.toHaveBeenCalled(); -}); - -test('async - fetches data when opening', async () => { - const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 })); - render(); - await open(); - await waitFor(() => expect(loadOptions).not.toHaveBeenCalled()); - await type('search'); - await waitFor(() => expect(loadOptions).toHaveBeenCalled()); -}); - -test('async - displays an error message when an exception is thrown while fetching', async () => { - const error = 'Fetch error'; - const loadOptions = async () => { - throw new Error(error); - }; - render(); - await type('search'); - expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); - expect(loadOptions).toHaveBeenCalledTimes(1); - clearAll(); - await type('search'); - expect(await screen.findByText(LOADING)).toBeInTheDocument(); - expect(loadOptions).toHaveBeenCalledTimes(1); -}); - -test('async - does not fire a new request if all values have been fetched', async () => { - const mock = jest.fn(loadOptions); - const search = 'George'; - const pageSize = OPTIONS.length; - render(); - await open(); - expect(mock).toHaveBeenCalledTimes(1); - 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); -}); - -test('async - requests the options again after clearing the cache', async () => { - const ref: RefObject = { current: null }; - const mock = jest.fn(loadOptions); - const pageSize = OPTIONS.length; - render( - , ); @@ -383,7 +367,7 @@ test('clear all the values', async () => { }); test('does not add a new option if allowNewOptions is false', async () => { - render(); await open(); await type(NEW_OPTION); expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx index fe0c091539fc..e278d7239867 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx @@ -42,7 +42,7 @@ import Select, { propertyComparator } from 'src/components/Select/Select'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import withToasts from 'src/components/MessageToasts/withToasts'; import Owner from 'src/types/Owner'; -import { AntdCheckbox } from 'src/components'; +import { AntdCheckbox, AsyncSelect } from 'src/components'; import TextAreaControl from 'src/explore/components/controls/TextAreaControl'; import { useCommonConf } from 'src/views/CRUD/data/database/state'; import { @@ -58,7 +58,6 @@ import { import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import { AlertReportCronScheduler } from './components/AlertReportCronScheduler'; import { NotificationMethod } from './components/NotificationMethod'; -import { AsyncSelect } from 'src/components'; const TIMEOUT_MIN = 1; const TEXT_BASED_VISUALIZATION_TYPES = [ From cc549a5d8989991cc58f0574a52332395e386ee0 Mon Sep 17 00:00:00 2001 From: cccs-RyanK <102618419+cccs-RyanK@users.noreply.github.com> Date: Wed, 29 Jun 2022 15:24:09 -0400 Subject: [PATCH 04/20] fixing lint errors --- superset-frontend/src/components/Select/Select.stories.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.stories.tsx b/superset-frontend/src/components/Select/Select.stories.tsx index b8b08b7f1233..efcd91c0c38f 100644 --- a/superset-frontend/src/components/Select/Select.stories.tsx +++ b/superset-frontend/src/components/Select/Select.stories.tsx @@ -20,11 +20,7 @@ import React, { ReactNode, useState, useCallback, useRef } from 'react'; import Button from 'src/components/Button'; import ControlHeader from 'src/explore/components/ControlHeader'; import AsyncSelect, { AsyncSelectProps, AsyncSelectRef } from './AsyncSelect'; -import Select, { - SelectProps, - OptionsTypePage, - OptionsType, -} from './Select'; +import Select, { SelectProps, OptionsTypePage, OptionsType } from './Select'; export default { title: 'Select', From 7a165b49980797661724a57acb6394d4dbe8e5c5 Mon Sep 17 00:00:00 2001 From: cccs-RyanK <102618419+cccs-RyanK@users.noreply.github.com> Date: Wed, 6 Jul 2022 10:00:59 -0400 Subject: [PATCH 05/20] fixed frontend test errors --- .../src/addSlice/AddSliceContainer.test.tsx | 4 +- .../components/Select/AsyncSelect.test.tsx | 90 +++++++++---------- .../src/components/Select/Select.test.tsx | 22 +---- 3 files changed, 48 insertions(+), 68 deletions(-) diff --git a/superset-frontend/src/addSlice/AddSliceContainer.test.tsx b/superset-frontend/src/addSlice/AddSliceContainer.test.tsx index 6187f574867c..5cf855558d16 100644 --- a/superset-frontend/src/addSlice/AddSliceContainer.test.tsx +++ b/superset-frontend/src/addSlice/AddSliceContainer.test.tsx @@ -19,7 +19,7 @@ import React from 'react'; import { ReactWrapper } from 'enzyme'; import Button from 'src/components/Button'; -import { Select } from 'src/components'; +import { AsyncSelect } from 'src/components'; import AddSliceContainer, { AddSliceContainerProps, AddSliceContainerState, @@ -72,7 +72,7 @@ async function getWrapper(user = mockUser) { test('renders a select and a VizTypeControl', async () => { const wrapper = await getWrapper(); - expect(wrapper.find(Select)).toExist(); + expect(wrapper.find(AsyncSelect)).toExist(); expect(wrapper.find(VizTypeGallery)).toExist(); }); diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx index 2094252d5d27..82398b507bc6 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.test.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.test.tsx @@ -19,7 +19,7 @@ import React from 'react'; import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; -import { Select } from 'src/components'; +import { AsyncSelect } from 'src/components'; const ARIA_LABEL = 'Test'; const NEW_OPTION = 'Kyle'; @@ -125,19 +125,19 @@ const open = () => waitFor(() => userEvent.click(getSelect())); test('displays a header', async () => { const headerText = 'Header'; - render(, + , ); await open(); expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument(); rerender( - ); + render(); await open(); userEvent.click(await findSelectOption(OPTIONS[0].label)); expect(await screen.findByLabelText('stop')).toBeInTheDocument(); @@ -156,7 +156,7 @@ test('inverts the selection', async () => { test('sort the options by label if no sort comparator is provided', async () => { const unsortedOptions = [...OPTIONS].sort(() => Math.random()); - render( { }); test('should sort selected to top when in single mode', async () => { - render(); + render(); const originalLabels = OPTIONS.map(option => option.label); let labels = originalLabels.slice(); @@ -247,7 +247,7 @@ test('should sort selected to the top when in multi mode', async () => { test('searches for label or value', async () => { const option = OPTIONS[11]; - render(); + render(); await type('Her'); const options = await findAllSelectOptions(); expect(options.length).toBe(4); @@ -267,14 +267,14 @@ test('search order exact and startWith match first', async () => { }); test('ignores case when searching', async () => { - render( { }); test('ignores special keys when searching', async () => { - render(); + render(); await type('Liam'); let options = await findAllSelectOptions(); expect(options.length).toBe(1); @@ -319,7 +319,7 @@ test('searches for custom fields', async () => { }); test('removes duplicated values', async () => { - render(); + render(); await open(); expect(screen.getByRole('heading', { name: 'John' })).toBeInTheDocument(); expect(screen.getByRole('heading', { name: 'Liam' })).toBeInTheDocument(); @@ -348,7 +348,7 @@ test('searches for a word with a custom label', async () => { { label: 'Liam', value: 2, customLabel:

Liam

}, { label: 'Olivia', value: 3, customLabel:

Olivia

}, ]; - render(); + render(); await type(NEW_OPTION); expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument(); await type('k'); @@ -368,7 +368,7 @@ test('removes a new option if the user does not select it', async () => { test('clear all the values', async () => { const onClear = jest.fn(); render( - ); + render(); await open(); await type(NEW_OPTION); expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); }); test('adds the null option when selected in single mode', async () => { - render( { }); test('renders the select with default props', () => { - render( ({ data: [], totalCount: 0 })} />, @@ -429,7 +429,7 @@ test('opens the select without any data', async () => { }); test('displays the loading indicator when opening', async () => { - render(); + render(); const optionText = 'Emma'; await open(); userEvent.click(await findSelectOption(optionText)); @@ -446,7 +446,7 @@ test('makes a selection in single mode', async () => { }); test('multiple selections in multiple mode', async () => { - render(, + , ); await open(); const [firstOption, secondOption] = OPTIONS; @@ -484,7 +484,7 @@ test('changes the selected item in single mode', async () => { }); test('deselects an item in multiple mode', async () => { - render(); + render(); await open(); await type(NEW_OPTION); expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument(); }); test('does not add a new option if the option already exists', async () => { - render( { - render(); + render(); expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label); }); test('sets a initial value in multiple mode', async () => { render( - ); + render(); await open(); await type('and'); @@ -595,7 +595,7 @@ test('searches for matches in both loaded and unloaded pages', async () => { test('searches for an item in a page not loaded', async () => { const mock = jest.fn(loadOptions); - render(); + render(); expect(loadOptions).not.toHaveBeenCalled(); }); test('fetches data when opening', async () => { const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 })); - render(); + render(); await open(); await waitFor(() => expect(loadOptions).not.toHaveBeenCalled()); await type('search'); @@ -632,14 +632,14 @@ test('displays an error message when an exception is thrown while fetching', asy const loadOptions = async () => { throw new Error(error); }; - render(); + render(); await type('search'); expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); expect(loadOptions).toHaveBeenCalledTimes(1); @@ -653,7 +653,7 @@ test('does not fire a new request if all values have been fetched', async () => const mock = jest.fn(loadOptions); const search = 'George'; const pageSize = OPTIONS.length; - render(); + render(); await open(); expect(mock).toHaveBeenCalledTimes(1); await type('or'); diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx index 5a61e55d01c8..8c0d6e6f15de 100644 --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@ -20,6 +20,7 @@ import React from 'react'; import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { Select } from 'src/components'; +import { loadOptions } from '@babel/core'; const ARIA_LABEL = 'Test'; const NEW_OPTION = 'Kyle'; @@ -149,27 +150,6 @@ test('sort the options by label if no sort comparator is provided', async () => ); }); -test('sort the options using a custom sort comparator', async () => { - const sortComparator = ( - option1: typeof OPTIONS[0], - option2: typeof OPTIONS[0], - ) => option1.gender.localeCompare(option2.gender); - render( - ); const originalLabels = OPTIONS.map(option => option.label); From 684852a2848813b1c08cb2a2af174e876d6e66fd Mon Sep 17 00:00:00 2001 From: cccs-RyanK <102618419+cccs-RyanK@users.noreply.github.com> Date: Wed, 6 Jul 2022 15:12:48 -0400 Subject: [PATCH 06/20] fixed alertreportmodel tests --- .../src/components/Select/AsyncSelect.tsx | 14 +++++++++++++- .../src/views/CRUD/alert/AlertReportModal.test.jsx | 14 ++++++++++---- .../src/views/CRUD/alert/AlertReportModal.tsx | 4 ++-- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx index f9ca861fb903..2512e86e015e 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { + import React, { forwardRef, ReactElement, ReactNode, @@ -27,6 +27,7 @@ import React, { useState, useRef, useCallback, + useImperativeHandle, } from 'react'; import { ensureIsArray, styled, t } from '@superset-ui/core'; import AntdSelect, { @@ -680,6 +681,17 @@ const AsyncSelect = ( } }, [isLoading, loading]); + const clearCache = () => fetchedQueries.current.clear(); + + useImperativeHandle( + ref, + () => ({ + ...(ref.current as HTMLInputElement), + clearCache, + }), + [ref], + ); + return ( {header} diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx index 1598e5a926f3..8b3ff1d016ab 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx @@ -24,7 +24,7 @@ import fetchMock from 'fetch-mock'; import { act } from 'react-dom/test-utils'; import AlertReportModal from 'src/views/CRUD/alert/AlertReportModal'; import Modal from 'src/components/Modal'; -import { Select } from 'src/components'; +import { Select, AsyncSelect } from 'src/components'; import { Switch } from 'src/components/Switch'; import { Radio } from 'src/components/Radio'; import TextAreaControl from 'src/explore/components/controls/TextAreaControl'; @@ -182,7 +182,9 @@ describe('AlertReportModal', () => { it('renders five select elements when in report mode', () => { expect(wrapper.find(Select)).toExist(); - expect(wrapper.find(Select)).toHaveLength(5); + expect(wrapper.find(AsyncSelect)).toExist(); + expect(wrapper.find(Select)).toHaveLength(2); + expect(wrapper.find(AsyncSelect)).toHaveLength(3); }); it('renders Switch element', () => { @@ -220,7 +222,9 @@ describe('AlertReportModal', () => { it('renders five select element when in report mode', () => { expect(wrapper.find(Select)).toExist(); - expect(wrapper.find(Select)).toHaveLength(5); + expect(wrapper.find(AsyncSelect)).toExist(); + expect(wrapper.find(Select)).toHaveLength(2); + expect(wrapper.find(AsyncSelect)).toHaveLength(3); }); it('renders seven select elements when in alert mode', async () => { @@ -232,7 +236,9 @@ describe('AlertReportModal', () => { const addWrapper = await mountAndWait(props); expect(addWrapper.find(Select)).toExist(); - expect(addWrapper.find(Select)).toHaveLength(7); + expect(addWrapper.find(AsyncSelect)).toExist(); + expect(addWrapper.find(Select)).toHaveLength(3); + expect(addWrapper.find(AsyncSelect)).toHaveLength(4); }); it('renders value input element when in alert mode', async () => { diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx index e278d7239867..820a83b8c337 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx @@ -38,11 +38,11 @@ import { Switch } from 'src/components/Switch'; import Modal from 'src/components/Modal'; import TimezoneSelector from 'src/components/TimezoneSelector'; import { Radio } from 'src/components/Radio'; -import Select, { propertyComparator } from 'src/components/Select/Select'; +import { propertyComparator } from 'src/components/Select/Select'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import withToasts from 'src/components/MessageToasts/withToasts'; import Owner from 'src/types/Owner'; -import { AntdCheckbox, AsyncSelect } from 'src/components'; +import { AntdCheckbox, AsyncSelect, Select } from 'src/components'; import TextAreaControl from 'src/explore/components/controls/TextAreaControl'; import { useCommonConf } from 'src/views/CRUD/data/database/state'; import { From 405e8e4371d02642ee487a8420a14723eaec401b Mon Sep 17 00:00:00 2001 From: cccs-RyanK <102618419+cccs-RyanK@users.noreply.github.com> Date: Thu, 7 Jul 2022 09:04:44 -0400 Subject: [PATCH 07/20] removed accidental import --- superset-frontend/src/components/Select/Select.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx index 3b5bd219653f..0b353bde38ac 100644 --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@ -20,7 +20,6 @@ import React from 'react'; import { render, screen, waitFor, within } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import { Select } from 'src/components'; -import { loadOptions } from '@babel/core'; const ARIA_LABEL = 'Test'; const NEW_OPTION = 'Kyle'; From 593c521fbc04a0ae8499b16572a68a111db5ed8b Mon Sep 17 00:00:00 2001 From: cccs-RyanK <102618419+cccs-RyanK@users.noreply.github.com> Date: Thu, 7 Jul 2022 09:53:02 -0400 Subject: [PATCH 08/20] fixed lint errors --- .../components/Select/AsyncSelect.test.tsx | 36 ++++++++++++++----- .../src/components/Select/AsyncSelect.tsx | 2 +- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx index 82398b507bc6..dc6eff35d942 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.test.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.test.tsx @@ -300,7 +300,9 @@ test('ignores special keys when searching', async () => { }); test('searches for custom fields', async () => { - render(); + render( + , + ); await type('Liam'); let options = await findAllSelectOptions(); expect(options.length).toBe(1); @@ -446,7 +448,9 @@ test('makes a selection in single mode', async () => { }); test('multiple selections in multiple mode', async () => { - render(); + render( + , + ); await open(); const [firstOption, secondOption] = OPTIONS; userEvent.click(await findSelectOption(firstOption.label)); @@ -484,7 +488,9 @@ test('changes the selected item in single mode', async () => { }); test('deselects an item in multiple mode', async () => { - render(); + render( + , + ); await open(); const option3 = OPTIONS[2]; const option8 = OPTIONS[7]; @@ -518,14 +524,18 @@ test('deselects an item in multiple mode', async () => { }); test('adds a new option if none is available and allowNewOptions is true', async () => { - render(); + render( + , + ); await open(); await type(NEW_OPTION); expect(await findSelectOption(NEW_OPTION)).toBeInTheDocument(); }); test('does not add a new option if the option already exists', async () => { - render(); + render( + , + ); const option = OPTIONS[0].label; await open(); await type(option); @@ -552,14 +562,18 @@ test('shows "No data" when allowNewOptions is false and a new option is entered' }); test('does not show "No data" when allowNewOptions is true and a new option is entered', async () => { - render(); + render( + , + ); await open(); await type(NEW_OPTION); expect(screen.queryByText(NO_DATA)).not.toBeInTheDocument(); }); test('sets a initial value in single mode', async () => { - render(); + render( + , + ); expect(await findSelectValue()).toHaveTextContent(OPTIONS[0].label); }); @@ -620,7 +634,9 @@ test('fetches data when opening', async () => { test('fetches data only after a search input is entered if fetchOnlyOnSearch is true', async () => { const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 })); - render(); + render( + , + ); await open(); await waitFor(() => expect(loadOptions).not.toHaveBeenCalled()); await type('search'); @@ -639,7 +655,9 @@ test('displays an error message when an exception is thrown while fetching', asy test('does not fire a new request for the same search input', async () => { const loadOptions = jest.fn(async () => ({ data: [], totalCount: 0 })); - render(); + render( + , + ); await type('search'); expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); expect(loadOptions).toHaveBeenCalledTimes(1); diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx index 2512e86e015e..41b060d5ba83 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ - import React, { +import React, { forwardRef, ReactElement, ReactNode, From 0483fe7ecd8f96772e4a787edb979b4d51cd4f02 Mon Sep 17 00:00:00 2001 From: cccs-RyanK <102618419+cccs-RyanK@users.noreply.github.com> Date: Thu, 7 Jul 2022 13:57:07 -0400 Subject: [PATCH 09/20] updated async select --- superset-frontend/src/components/Select/AsyncSelect.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx index 41b060d5ba83..98f146f15f2a 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.tsx @@ -67,6 +67,7 @@ type PickedSelectProps = Pick< | 'showSearch' | 'tokenSeparators' | 'value' + | 'getPopupContainer' >; export type OptionsType = Exclude; @@ -316,6 +317,7 @@ const AsyncSelect = ( sortComparator = DEFAULT_SORT_COMPARATOR, tokenSeparators, value, + getPopupContainer, ...props }: AsyncSelectProps, ref: RefObject, @@ -701,7 +703,9 @@ const AsyncSelect = ( dropdownRender={dropdownRender} filterOption={handleFilterOption} filterSort={sortComparatorWithSearch} - getPopupContainer={triggerNode => triggerNode.parentNode} + getPopupContainer={ + getPopupContainer || (triggerNode => triggerNode.parentNode) + } labelInValue={isAsync || labelInValue} maxTagCount={MAX_TAG_COUNT} mode={mappedMode} From 251b0ea58863428b9c081033554f3b014df14a10 Mon Sep 17 00:00:00 2001 From: cccs-RyanK <102618419+cccs-RyanK@users.noreply.github.com> Date: Mon, 11 Jul 2022 11:25:01 -0400 Subject: [PATCH 10/20] removed code from select component --- .../components/Select/AsyncSelect.test.tsx | 104 ++++++---- .../src/components/Select/AsyncSelect.tsx | 56 +++--- .../src/components/Select/Select.test.tsx | 24 ++- .../src/components/Select/Select.tsx | 187 ++---------------- 4 files changed, 132 insertions(+), 239 deletions(-) diff --git a/superset-frontend/src/components/Select/AsyncSelect.test.tsx b/superset-frontend/src/components/Select/AsyncSelect.test.tsx index dc6eff35d942..0239fa902fa9 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.test.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.test.tsx @@ -60,7 +60,17 @@ const loadOptions = async (search: string, page: number, pageSize: number) => { const start = page * pageSize; const deleteCount = start + pageSize < totalCount ? pageSize : totalCount - start; - const data = OPTIONS.filter(option => option.label.match(search)).splice( + const searchValue = search.trim().toLowerCase(); + const optionFilterProps = ['label', 'value', 'gender']; + + const data = OPTIONS.filter(option => { + return optionFilterProps.some(prop => { + const optionProp = option?.[prop] + ? String(option[prop]).trim().toLowerCase() + : ''; + return optionProp.includes(searchValue); + }); + }).splice( start, deleteCount, ); @@ -74,7 +84,7 @@ const defaultProps = { allowClear: true, ariaLabel: ARIA_LABEL, labelInValue: true, - options: OPTIONS, + options: loadOptions, pageSize: 10, showSearch: true, }; @@ -130,17 +140,18 @@ test('displays a header', async () => { }); test('adds a new option if the value is not in the options', async () => { + let loadOptions = jest.fn(async () => ({ data: [OPTIONS[1]], totalCount: 0 })); const { rerender } = render( - , + , ); await open(); expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument(); - + let reloadOptions = jest.fn(async () => ({ data: [], totalCount: 2})); rerender( - , + , ); - await open(); const options = await findAllSelectOptions(); + expect(await findSelectOption(OPTIONS[1].label)).toBeInTheDocument(); expect(options).toHaveLength(2); options.forEach((option, i) => expect(option).toHaveTextContent(OPTIONS[i].label), @@ -155,8 +166,8 @@ test('inverts the selection', async () => { }); test('sort the options by label if no sort comparator is provided', async () => { - const unsortedOptions = [...OPTIONS].sort(() => Math.random()); - render(); + const loadUnsortedOptions = jest.fn(async () => ({ data: [...OPTIONS].sort(() => Math.random()), totalCount: 2 })); + render(); await open(); const options = await findAllSelectOptions(); options.forEach((option, key) => @@ -250,20 +261,23 @@ test('searches for label or value', async () => { render(); const search = option.value; await type(search.toString()); + expect(await findSelectOption(option.label)).toBeInTheDocument(); const options = await findAllSelectOptions(); expect(options.length).toBe(1); expect(options[0]).toHaveTextContent(option.label); }); test('search order exact and startWith match first', async () => { - render(); + render(); + await open(); await type('Her'); + expect(await findSelectOption('Guilherme')).toBeInTheDocument(); 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'); + expect(options[0]).toHaveTextContent('Her'); + expect(options[1]).toHaveTextContent('Herme'); + expect(options[2]).toHaveTextContent('Cher'); + expect(options[3]).toHaveTextContent('Guilherme'); }); test('ignores case when searching', async () => { @@ -273,15 +287,19 @@ test('ignores case when searching', async () => { }); test('same case should be ranked to the top', async () => { - render( - ( + { data: [ { value: 'Cac' }, { value: 'abac' }, { value: 'acbc' }, { value: 'CAc' }, - ]} + ], totalCount: 4 + } + )); + render( + , ); await type('Ac'); @@ -294,20 +312,28 @@ test('same case should be ranked to the top', async () => { }); test('ignores special keys when searching', async () => { - render(); + render(); await type('{shift}'); expect(screen.queryByText(LOADING)).not.toBeInTheDocument(); }); test('searches for custom fields', async () => { render( - , + , ); + await open(); await type('Liam'); + expect(await findSelectOption('Liam')).toBeInTheDocument(); let options = await findAllSelectOptions(); expect(options.length).toBe(1); expect(options[0]).toHaveTextContent('Liam'); await type('Female'); + // expect(await findSelectOption('Ava')).toBeInTheDocument(); + // expect(await findSelectOption('Charlotte')).toBeInTheDocument(); + // expect(await findSelectOption('Cher')).toBeInTheDocument(); + // expect(await findSelectOption('Emma')).toBeInTheDocument(); + // expect(await findSelectOption('Nikole')).toBeInTheDocument(); + expect(await findSelectOption('Olivia')).toBeInTheDocument(); options = await findAllSelectOptions(); expect(options.length).toBe(6); expect(options[0]).toHaveTextContent('Ava'); @@ -317,7 +343,7 @@ test('searches for custom fields', async () => { expect(options[4]).toHaveTextContent('Nikole'); expect(options[5]).toHaveTextContent('Olivia'); await type('1'); - expect(screen.getByText(NO_DATA)).toBeInTheDocument(); + expect(await screen.findByText(NO_DATA)).toBeInTheDocument(); }); test('removes duplicated values', async () => { @@ -332,12 +358,15 @@ test('removes duplicated values', async () => { }); test('renders a custom label', async () => { - const options = [ - { label: 'John', value: 1, customLabel:

John

}, - { label: 'Liam', value: 2, customLabel:

Liam

}, - { label: 'Olivia', value: 3, customLabel:

Olivia

}, - ]; - render(); + const loadOptions = jest.fn(async () => ( + { data: [ + { label: 'John', value: 1, customLabel:

John

}, + { label: 'Liam', value: 2, customLabel:

Liam

}, + { label: 'Olivia', value: 3, customLabel:

Olivia

}, + ], totalCount: 3 + } + )); + render(); await open(); expect(screen.getByRole('heading', { name: 'John' })).toBeInTheDocument(); expect(screen.getByRole('heading', { name: 'Liam' })).toBeInTheDocument(); @@ -345,12 +374,15 @@ test('renders a custom label', async () => { }); test('searches for a word with a custom label', async () => { - const options = [ - { label: 'John', value: 1, customLabel:

John

}, - { label: 'Liam', value: 2, customLabel:

Liam

}, - { label: 'Olivia', value: 3, customLabel:

Olivia

}, - ]; - render(); + const loadOptions = jest.fn(async () => ( + { data: [ + { label: 'John', value: 1, customLabel:

John

}, + { label: 'Liam', value: 2, customLabel:

Liam

}, + { label: 'Olivia', value: 3, customLabel:

Olivia

}, + ], totalCount: 3 + } + )); + render(); await type('Liam'); const selectOptions = await findAllSelectOptions(); expect(selectOptions.length).toBe(1); @@ -391,7 +423,8 @@ test('does not add a new option if allowNewOptions is false', async () => { }); test('adds the null option when selected in single mode', async () => { - render(); + const loadOptions = jest.fn(async () => ({ data: [OPTIONS[0], NULL_OPTION], totalCount: 2 })); + render(); await open(); userEvent.click(await findSelectOption(NULL_OPTION.label)); const values = await findAllSelectValues(); @@ -399,10 +432,11 @@ test('adds the null option when selected in single mode', async () => { }); test('adds the null option when selected in multiple mode', async () => { + const loadOptions = jest.fn(async () => ({ data: [OPTIONS[0], NULL_OPTION], totalCount: 2 })); render( , ); diff --git a/superset-frontend/src/components/Select/AsyncSelect.tsx b/superset-frontend/src/components/Select/AsyncSelect.tsx index 98f146f15f2a..00681341385f 100644 --- a/superset-frontend/src/components/Select/AsyncSelect.tsx +++ b/superset-frontend/src/components/Select/AsyncSelect.tsx @@ -129,11 +129,10 @@ export interface AsyncSelectProps extends PickedSelectProps { optionFilterProps?: string[]; /** * 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 + * The options are async, a promise that returns * an array of options. */ - options: OptionsType | OptionsPagePromise; + options: OptionsPagePromise; /** * It defines how many results should be included * in the query response. @@ -322,9 +321,8 @@ const AsyncSelect = ( }: AsyncSelectProps, ref: RefObject, ) => { - const isAsync = typeof options === 'function'; const isSingleMode = mode === 'single'; - const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch; + const shouldShowSearch = allowNewOptions ? true : showSearch; const [selectValue, setSelectValue] = useState(value); const [inputValue, setInputValue] = useState(''); const [isLoading, setIsLoading] = useState(loading); @@ -360,8 +358,8 @@ const AsyncSelect = ( sortSelectedFirst(a, b) || // Only apply the custom sorter in async mode because we should // preserve the options order as much as possible. - (isAsync ? sortComparator(a, b, '') : 0), - [isAsync, sortComparator, sortSelectedFirst], + sortComparator(a, b, ''), + [sortComparator, sortSelectedFirst], ); const initialOptions = useMemo( @@ -528,7 +526,6 @@ const AsyncSelect = ( setSelectOptions(newOptions); } if ( - isAsync && !allValuesLoaded && loadingEnabled && !fetchedQueries.current.has(getQueryCacheKey(searchValue, 0, pageSize)) @@ -546,7 +543,7 @@ const AsyncSelect = ( vScroll.scrollTop > (vScroll.scrollHeight - vScroll.offsetHeight) * 0.7; const hasMoreData = page * pageSize + pageSize < totalCount; - if (!isLoading && isAsync && hasMoreData && thresholdReached) { + if (!isLoading && hasMoreData && thresholdReached) { const newPage = page + 1; fetchPage(inputValue, newPage); } @@ -575,30 +572,24 @@ const AsyncSelect = ( const handleOnDropdownVisibleChange = (isDropdownVisible: boolean) => { setIsDropdownVisible(isDropdownVisible); - 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); - } + // 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); } // if no search input value, force sort options because it won't be sorted by // `filterSort`. if (isDropdownVisible && !inputValue && selectOptions.length > 1) { - const sortedOptions = isAsync - ? selectOptions.slice().sort(sortComparatorForNoSearch) - : // if not in async mode, revert to the original select options - // (with selected options still sorted to the top) - initialOptionsSorted; + const sortedOptions = selectOptions.slice().sort(sortComparatorForNoSearch); if (!isEqual(sortedOptions, selectOptions)) { setSelectOptions(sortedOptions); } @@ -660,7 +651,7 @@ const AsyncSelect = ( ); useEffect(() => { - if (isAsync && loadingEnabled && allowFetch) { + if (loadingEnabled && allowFetch) { // trigger fetch every time inputValue changes if (inputValue) { debouncedFetchPage(inputValue, 0); @@ -669,7 +660,6 @@ const AsyncSelect = ( } } }, [ - isAsync, loadingEnabled, fetchPage, allowFetch, @@ -706,13 +696,13 @@ const AsyncSelect = ( getPopupContainer={ getPopupContainer || (triggerNode => triggerNode.parentNode) } - labelInValue={isAsync || labelInValue} + labelInValue={labelInValue} maxTagCount={MAX_TAG_COUNT} mode={mappedMode} notFoundContent={isLoading ? t('Loading...') : notFoundContent} onDeselect={handleOnDeselect} onDropdownVisibleChange={handleOnDropdownVisibleChange} - onPopupScroll={isAsync ? handlePagination : undefined} + onPopupScroll={handlePagination} onSearch={shouldShowSearch ? handleOnSearch : undefined} onSelect={handleOnSelect} onClear={handleClear} diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx index 0b353bde38ac..fc6920eea67a 100644 --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@ -114,24 +114,46 @@ test('displays a header', async () => { expect(screen.getByText(headerText)).toBeInTheDocument(); }); +// test('adds a new option if the value is not in the options', async () => { +// const { rerender } = render( +// , +// ); +// await open(); +// options = await findAllSelectOptions(); +// expect(await findSelectOption(OPTIONS[1].label)).toBeInTheDocument(); +// // expect(options).toHaveLength(2); +// options.forEach((option, i) => +// expect(option).toHaveTextContent(OPTIONS[i].label), +// ); +// }); + test('adds a new option if the value is not in the options', async () => { const { rerender } = render( , ); await open(); const options = await findAllSelectOptions(); expect(options).toHaveLength(2); + expect(await findSelectOption(OPTIONS[1].label)).toBeInTheDocument(); options.forEach((option, i) => expect(option).toHaveTextContent(OPTIONS[i].label), ); }); + test('inverts the selection', async () => { render(, -// ); -// await open(); -// let options = await findAllSelectOptions(); -// expect(await findSelectOption(OPTIONS[2].label)).toBeInTheDocument(); -// expect(options).toHaveLength(1); - -// rerender( -// , ); await open(); expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument(); - rerender( + const options = await findAllSelectOptions(); + expect(options).toHaveLength(1); + options.forEach((option, i) => + expect(option).toHaveTextContent(OPTIONS[i].label), + ); +}); + +test('adds a new option if the value is not in the options, when options have values', async () => { + render( , + ); + await open(); + expect(await findSelectOption(OPTIONS[0].label)).toBeInTheDocument(); + const options = await findAllSelectOptions(); + expect(options).toHaveLength(1); +}); test('inverts the selection', async () => { render(, - ); + render({Header}} - onChange={onChange} - onClear={onClear} - options={fetchSelects ? fetchAndFormatSelects : selects} - placeholder={t('Select or type a value')} - showSearch - value={selectedOption} - /> + { + fetchSelects ? ( + {Header}} + onChange={onChange} + onClear={onClear} + options={fetchAndFormatSelects} + placeholder={t('Select or type a value')} + showSearch + value={selectedOption} + /> + ) : ( + {Header}} - labelInValue - onChange={onChange} - onClear={onClear} - options={selects} - placeholder={t('Select or type a value')} - showSearch - value={selectedOption} - /> - ) - } - + ) : ( +