From bf1328839211fc610d5002180565deab13f934cb Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Thu, 17 Feb 2022 14:21:22 -0800 Subject: [PATCH 01/10] feat: remove loading indicator when typing in select --- .../src/components/Select/Select.tsx | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index a36a0d569aeb..f859a857996c 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -305,7 +305,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); @@ -481,7 +480,6 @@ const Select = ({ () => (value: string, page: number, pageSize: number) => { if (allValuesLoaded) { setIsLoading(false); - setIsTyping(false); return; } const key = `${value};${page};${pageSize}`; @@ -489,7 +487,6 @@ const Select = ({ if (cachedCount) { setTotalCount(cachedCount); setIsLoading(false); - setIsTyping(false); return; } setIsLoading(true); @@ -510,7 +507,6 @@ const Select = ({ .catch(internalOnError) .finally(() => { setIsLoading(false); - setIsTyping(false); }); }, [allValuesLoaded, fetchOnlyOnSearch, handleData, internalOnError, options], @@ -536,9 +532,6 @@ const Select = ({ setSelectOptions(newOptions); } - if (!searchValue || searchValue === searchedValue) { - setIsTyping(false); - } setSearchedValue(searchValue); }, DEBOUNCE_TIMEOUT), [allowNewOptions, isSingleMode, searchedValue, selectOptions], @@ -598,18 +591,12 @@ const Select = ({ if (!isDropdownVisible) { originNode.ref?.current?.scrollTo({ top: 0 }); } - if ((isLoading && selectOptions.length === 0) || isTyping) { + if (isLoading && selectOptions.length === 0) { return {t('Loading...')}; } return error ? : originNode; }; - const onInputKeyDown = (event: KeyboardEvent) => { - if (event.key.length === 1 && isAsync && !isTyping) { - setIsTyping(true); - } - }; - const SuffixIcon = () => { if (isLoading) { return ; @@ -722,7 +709,6 @@ const Select = ({ } onDeselect={handleOnDeselect} onDropdownVisibleChange={handleOnDropdownVisibleChange} - onInputKeyDown={onInputKeyDown} onPopupScroll={isAsync ? handlePagination : undefined} onSearch={shouldShowSearch ? handleOnSearch : undefined} onSelect={handleOnSelect} From 6957447bde9918e5584126c17705bd96be8bbf87 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Thu, 17 Feb 2022 17:32:10 -0800 Subject: [PATCH 02/10] Fix tests and more use cases --- .../src/components/Select/Select.test.tsx | 48 ++++++++++--------- .../src/components/Select/Select.tsx | 24 ++++++++-- 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx index c9982e5ef982..cd4189c29a9a 100644 --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@ -86,6 +86,9 @@ const getElementsByClassName = (className: string) => const getSelect = () => screen.getByRole('combobox', { name: ARIA_LABEL }); +const findSpinner = () => + waitFor(() => getElementByClassName('.ant-spin-spinning')); + const findSelectOption = (text: string) => waitFor(() => within(getElementByClassName('.rc-virtual-list')).getByText(text), @@ -456,15 +459,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(); const optionText = 'Emma'; @@ -587,24 +581,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(); - const search = 'Ashfaq'; + const mock = jest.fn(loadOptions); + render(); 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); }); diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index f859a857996c..e42ef3f55008 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -20,13 +20,13 @@ import React, { ReactElement, ReactNode, RefObject, - KeyboardEvent, UIEvent, useEffect, useMemo, useState, useRef, useCallback, + KeyboardEvent, } from 'react'; import { styled, t } from '@superset-ui/core'; import AntdSelect, { @@ -317,6 +317,7 @@ const Select = ({ : allowNewOptions ? 'tags' : 'multiple'; + const allowFetch = !fetchOnlyOnSearch || searchedValue; // TODO: Don't assume that isAsync is always labelInValue const handleTopOptions = useCallback( @@ -512,7 +513,7 @@ const Select = ({ [allValuesLoaded, fetchOnlyOnSearch, handleData, internalOnError, options], ); - const handleOnSearch = useMemo( + const debouncedHandleSearch = useMemo( () => debounce((search: string) => { const searchValue = search.trim(); @@ -537,6 +538,16 @@ const Select = ({ [allowNewOptions, isSingleMode, searchedValue, selectOptions], ); + const handleOnSearch = useCallback( + (search: string) => { + if (isAsync && !allValuesLoaded) { + setIsLoading(true); + } + return debouncedHandleSearch(search); + }, + [debouncedHandleSearch, isAsync, allValuesLoaded], + ); + const handlePagination = (e: UIEvent) => { const vScroll = e.currentTarget; const thresholdReached = @@ -659,10 +670,12 @@ 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); @@ -675,6 +688,7 @@ const Select = ({ handlePaginatedFetch, loadingEnabled, fetchOnlyOnSearch, + allowFetch, ]); useEffect(() => { @@ -701,7 +715,7 @@ const Select = ({ maxTagCount={MAX_TAG_COUNT} mode={mappedMode} notFoundContent={ - allowNewOptions && !fetchOnlyOnSearch ? ( + isLoading ? ( {t('Loading...')} ) : ( notFoundContent From 83f3a1d67b862d07816b92bf83776dc509fb3647 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Thu, 17 Feb 2022 18:31:25 -0800 Subject: [PATCH 03/10] The select component is a neverending landmine --- .../src/components/Select/Select.test.tsx | 18 +---- .../src/components/Select/Select.tsx | 68 +++++++++---------- 2 files changed, 35 insertions(+), 51 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx index cd4189c29a9a..e43718931a13 100644 --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@ -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'; @@ -86,9 +80,6 @@ const getElementsByClassName = (className: string) => const getSelect = () => screen.getByRole('combobox', { name: ARIA_LABEL }); -const findSpinner = () => - waitFor(() => getElementByClassName('.ant-spin-spinning')); - const findSelectOption = (text: string) => waitFor(() => within(getElementByClassName('.rc-virtual-list')).getByText(text), @@ -391,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(); const option = OPTIONS[0].label; diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index e42ef3f55008..1b3e66fadc2a 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -26,7 +26,6 @@ import React, { useState, useRef, useCallback, - KeyboardEvent, } from 'react'; import { styled, t } from '@superset-ui/core'; import AntdSelect, { @@ -63,12 +62,7 @@ type PickedSelectProps = Pick< | 'value' >; -type OptionsProps = Exclude; - -export interface OptionsType extends Omit { - label?: string; - customLabel?: ReactNode; -} +type OptionsType = Exclude; export type OptionsTypePage = { data: OptionsType; @@ -485,7 +479,7 @@ const Select = ({ } const key = `${value};${page};${pageSize}`; const cachedCount = fetchedQueries.current.get(key); - if (cachedCount) { + if (cachedCount !== undefined) { setTotalCount(cachedCount); setIsLoading(false); return; @@ -516,36 +510,44 @@ const Select = ({ 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); - } - - setSearchedValue(searchValue); + setSearchedValue(search); }, DEBOUNCE_TIMEOUT), - [allowNewOptions, isSingleMode, searchedValue, selectOptions], + [], ); const handleOnSearch = useCallback( (search: string) => { - if (isAsync && !allValuesLoaded) { - setIsLoading(true); + const searchValue = search.trim(); + if (allowNewOptions && isSingleMode) { + const newOption = searchValue && + !hasOption(searchValue, selectOptions) && { + label: searchValue, + value: searchValue, + isNewOption: true, + }; + const cleanSelectOptions = selectOptions.filter( + opt => !opt.isNewOption, + ); + const newOptions = newOption + ? [newOption, ...cleanSelectOptions] + : cleanSelectOptions; + setSelectOptions(newOptions); + } + if (isAsync && !allValuesLoaded && loadingEnabled) { + setIsLoading(search !== searchedValue); } return debouncedHandleSearch(search); }, - [debouncedHandleSearch, isAsync, allValuesLoaded], + [ + allowNewOptions, + isSingleMode, + isAsync, + allValuesLoaded, + loadingEnabled, + debouncedHandleSearch, + selectOptions, + searchedValue, + ], ); const handlePagination = (e: UIEvent) => { @@ -677,9 +679,8 @@ const Select = ({ useEffect(() => { if (isAsync && loadingEnabled && allowFetch) { - const page = 0; - handlePaginatedFetch(searchedValue, page, pageSize); - setPage(page); + handlePaginatedFetch(searchedValue, 0, pageSize); + setPage(0); } }, [ isAsync, @@ -687,7 +688,6 @@ const Select = ({ pageSize, handlePaginatedFetch, loadingEnabled, - fetchOnlyOnSearch, allowFetch, ]); From e4518413cf4783e56cbbbe84066a8ac745f334e5 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Thu, 17 Feb 2022 18:41:07 -0800 Subject: [PATCH 04/10] Use shared constant for debounce --- superset-frontend/src/components/Select/Select.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 1b3e66fadc2a..f9751f218edb 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -39,6 +39,7 @@ import { isEqual } from 'lodash'; import { Spin } from 'antd'; import Icons from 'src/components/Icons'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; +import { SLOW_DEBOUNCE } from 'src/constants'; import { hasOption } from './utils'; const { Option } = AntdSelect; @@ -214,7 +215,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 = []; @@ -511,7 +511,7 @@ const Select = ({ () => debounce((search: string) => { setSearchedValue(search); - }, DEBOUNCE_TIMEOUT), + }, SLOW_DEBOUNCE), [], ); From 1d626eef648d4bcbe69a2148df7bf32e29fc61fe Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Tue, 22 Feb 2022 10:06:59 -0800 Subject: [PATCH 05/10] fix export --- superset-frontend/src/components/Select/Select.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index f9751f218edb..4d9bc2b02cb5 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -63,7 +63,7 @@ type PickedSelectProps = Pick< | 'value' >; -type OptionsType = Exclude; +export type OptionsType = Exclude; export type OptionsTypePage = { data: OptionsType; From d7cce62eb8094e3d6457047dc545534d2b4cc7bb Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Wed, 23 Feb 2022 11:25:10 -0800 Subject: [PATCH 06/10] Address bugs from code review --- superset-frontend/src/components/Select/Select.tsx | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 4d9bc2b02cb5..3656fa0aef5c 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -673,7 +673,10 @@ const Select = ({ // Stop the invocation of the debounced function after unmounting useEffect( - () => () => debouncedHandleSearch.cancel(), + () => () => { + debouncedHandleSearch.cancel(); + setIsLoading(false); + }, [debouncedHandleSearch], ); @@ -714,13 +717,7 @@ const Select = ({ labelInValue={isAsync || labelInValue} maxTagCount={MAX_TAG_COUNT} mode={mappedMode} - notFoundContent={ - isLoading ? ( - {t('Loading...')} - ) : ( - notFoundContent - ) - } + notFoundContent={isLoading ? t('Loading...') : notFoundContent} onDeselect={handleOnDeselect} onDropdownVisibleChange={handleOnDropdownVisibleChange} onPopupScroll={isAsync ? handlePagination : undefined} From 132ebdf909b4a0baeb8d60dd17f454a77cad2315 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Wed, 23 Feb 2022 13:36:29 -0800 Subject: [PATCH 07/10] Simplify and fix Cypress tests --- .../integration/dashboard/nativeFilters.test.ts | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts index 91f39f581c33..f4a6f9df4a4c 100644 --- a/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/dashboard/nativeFilters.test.ts @@ -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 }) From 3cc313feb8eeefc34f355fe0d576104337893dd9 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Tue, 1 Mar 2022 11:54:51 -0800 Subject: [PATCH 08/10] Improve loading states --- .../src/components/Select/Select.tsx | 84 ++++++++++--------- 1 file changed, 44 insertions(+), 40 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 3656fa0aef5c..5b47f416e23f 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -57,7 +57,6 @@ type PickedSelectProps = Pick< | 'notFoundContent' | 'onChange' | 'onClear' - | 'onFocus' | 'placeholder' | 'showSearch' | 'value' @@ -515,40 +514,31 @@ const Select = ({ [], ); - const handleOnSearch = useCallback( - (search: string) => { - const searchValue = search.trim(); - if (allowNewOptions && isSingleMode) { - const newOption = searchValue && - !hasOption(searchValue, selectOptions) && { - label: searchValue, - value: searchValue, - isNewOption: true, - }; - const cleanSelectOptions = selectOptions.filter( - opt => !opt.isNewOption, - ); - const newOptions = newOption - ? [newOption, ...cleanSelectOptions] - : cleanSelectOptions; - setSelectOptions(newOptions); - } - if (isAsync && !allValuesLoaded && loadingEnabled) { - setIsLoading(search !== searchedValue); - } - return debouncedHandleSearch(search); - }, - [ - allowNewOptions, - isSingleMode, - isAsync, - allValuesLoaded, - loadingEnabled, - debouncedHandleSearch, - selectOptions, - searchedValue, - ], - ); + const handleOnSearch = (search: string) => { + const searchValue = search.trim(); + if (allowNewOptions && isSingleMode) { + const newOption = searchValue && + !hasOption(searchValue, selectOptions) && { + label: searchValue, + value: searchValue, + isNewOption: true, + }; + const cleanSelectOptions = selectOptions.filter(opt => !opt.isNewOption); + const newOptions = newOption + ? [newOption, ...cleanSelectOptions] + : cleanSelectOptions; + setSelectOptions(newOptions); + } + if ( + isAsync && + !allValuesLoaded && + loadingEnabled && + (!fetchOnlyOnSearch || searchValue) + ) { + setIsLoading(true); + } + return debouncedHandleSearch(search); + }; const handlePagination = (e: UIEvent) => { const vScroll = e.currentTarget; @@ -587,8 +577,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 @@ -610,7 +613,9 @@ const Select = ({ return error ? : originNode; }; - 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 ; } @@ -675,7 +680,6 @@ const Select = ({ useEffect( () => () => { debouncedHandleSearch.cancel(); - setIsLoading(false); }, [debouncedHandleSearch], ); @@ -731,7 +735,7 @@ const Select = ({ showArrow tokenSeparators={TOKEN_SEPARATORS} value={selectValue} - suffixIcon={} + suffixIcon={getSuffixIcon()} menuItemSelectedIcon={ invertSelection ? ( From 00f60aa41f5a451c473b7ae62cfb04c268721197 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Tue, 1 Mar 2022 15:08:36 -0800 Subject: [PATCH 09/10] More fix --- .../src/components/Select/Select.test.tsx | 2 +- .../src/components/Select/Select.tsx | 28 +++++++++++++------ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.test.tsx b/superset-frontend/src/components/Select/Select.test.tsx index e43718931a13..c23f57d523d3 100644 --- a/superset-frontend/src/components/Select/Select.test.tsx +++ b/superset-frontend/src/components/Select/Select.test.tsx @@ -633,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); }); diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index 5b47f416e23f..ca0d28dd7f57 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -245,6 +245,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/. @@ -471,12 +474,12 @@ const Select = ({ ); const handlePaginatedFetch = useMemo( - () => (value: string, page: number, pageSize: number) => { + () => (value: string, page: number) => { if (allValuesLoaded) { setIsLoading(false); return; } - const key = `${value};${page};${pageSize}`; + const key = getQueryCacheKey(value, page, pageSize); const cachedCount = fetchedQueries.current.get(key); if (cachedCount !== undefined) { setTotalCount(cachedCount); @@ -503,12 +506,20 @@ const Select = ({ setIsLoading(false); }); }, - [allValuesLoaded, fetchOnlyOnSearch, handleData, internalOnError, options], + [ + allValuesLoaded, + fetchOnlyOnSearch, + handleData, + internalOnError, + options, + pageSize, + ], ); const debouncedHandleSearch = useMemo( () => debounce((search: string) => { + // async search will triggered in handlePaginatedFetch setSearchedValue(search); }, SLOW_DEBOUNCE), [], @@ -533,9 +544,11 @@ const Select = ({ isAsync && !allValuesLoaded && loadingEnabled && - (!fetchOnlyOnSearch || searchValue) + !fetchedQueries.current.has(getQueryCacheKey(searchValue, 0, pageSize)) ) { - setIsLoading(true); + // if fetch only on search but search value is empty, then should not be + // in loading state + setIsLoading(!(fetchOnlyOnSearch && !searchValue)); } return debouncedHandleSearch(search); }; @@ -548,7 +561,7 @@ const Select = ({ if (!isLoading && isAsync && hasMoreData && thresholdReached) { const newPage = page + 1; - handlePaginatedFetch(searchedValue, newPage, pageSize); + handlePaginatedFetch(searchedValue, newPage); setPage(newPage); } }; @@ -686,13 +699,12 @@ const Select = ({ useEffect(() => { if (isAsync && loadingEnabled && allowFetch) { - handlePaginatedFetch(searchedValue, 0, pageSize); + handlePaginatedFetch(searchedValue, 0); setPage(0); } }, [ isAsync, searchedValue, - pageSize, handlePaginatedFetch, loadingEnabled, allowFetch, From e4ae7420e9d39b1f0847c554c4c29d3f760f97ae Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Wed, 2 Mar 2022 08:10:44 -0800 Subject: [PATCH 10/10] Fix allowNewOption --- .../src/components/Select/Select.tsx | 12 ++++++++---- superset-frontend/src/components/Select/utils.ts | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx index ca0d28dd7f57..12b481bcc0f7 100644 --- a/superset-frontend/src/components/Select/Select.tsx +++ b/superset-frontend/src/components/Select/Select.tsx @@ -40,7 +40,7 @@ import { Spin } from 'antd'; import Icons from 'src/components/Icons'; import { getClientErrorObject } from 'src/utils/getClientErrorObject'; import { SLOW_DEBOUNCE } from 'src/constants'; -import { hasOption } from './utils'; +import { hasOption, hasOptionIgnoreCase } from './utils'; const { Option } = AntdSelect; @@ -57,6 +57,8 @@ type PickedSelectProps = Pick< | 'notFoundContent' | 'onChange' | 'onClear' + | 'onFocus' + | 'onBlur' | 'placeholder' | 'showSearch' | 'value' @@ -519,7 +521,7 @@ const Select = ({ const debouncedHandleSearch = useMemo( () => debounce((search: string) => { - // async search will triggered in handlePaginatedFetch + // async search will be triggered in handlePaginatedFetch setSearchedValue(search); }, SLOW_DEBOUNCE), [], @@ -529,12 +531,14 @@ const Select = ({ const searchValue = search.trim(); if (allowNewOptions && isSingleMode) { const newOption = searchValue && - !hasOption(searchValue, selectOptions) && { + !hasOptionIgnoreCase(searchValue, selectOptions) && { label: searchValue, value: searchValue, isNewOption: true, }; - const cleanSelectOptions = selectOptions.filter(opt => !opt.isNewOption); + const cleanSelectOptions = selectOptions.filter( + opt => !opt.isNewOption || hasOption(opt.value, selectValue), + ); const newOptions = newOption ? [newOption, ...cleanSelectOptions] : cleanSelectOptions; diff --git a/superset-frontend/src/components/Select/utils.ts b/superset-frontend/src/components/Select/utils.ts index 71a904520591..1a3e3ca3b340 100644 --- a/superset-frontend/src/components/Select/utils.ts +++ b/superset-frontend/src/components/Select/utils.ts @@ -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( return (Array.isArray(value) ? value : [value]).map(find); } -export function hasOption(search: string, options: AntdOptionsType) { +export function hasOption( + 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;