chore: Adds lazy loading to the Select component (apache#15799)

QAlexBall · Jul 22, 2021 · 1f44df7 · 1f44df7
1 parent 0c07714
commit 1f44df7
Show file tree

Hide file tree

Showing 8 changed files with 195 additions and 98 deletions.
diff --git a/superset-frontend/src/components/Select/Select.stories.tsx b/superset-frontend/src/components/Select/Select.stories.tsx
@@ -144,6 +144,11 @@ InteractiveSelect.argTypes = {
       disable: true,
     },
   },
+  fetchOnlyOnSearch: {
+    table: {
+      disable: true,
+    },
+  },
 };
 
 InteractiveSelect.story = {
@@ -296,10 +301,12 @@ const USERS = [
 
 export const AsyncSelect = ({
   withError,
+  withInitialValue,
   responseTime,
   ...rest
 }: SelectProps & {
   withError: boolean;
+  withInitialValue: boolean;
   responseTime: number;
 }) => {
   const [requests, setRequests] = useState<ReactNode[]>([]);
@@ -375,6 +382,11 @@ export const AsyncSelect = ({
         <Select
           {...rest}
           options={withError ? fetchUserListError : fetchUserListPage}
+          value={
+            withInitialValue
+              ? { label: 'Valentina', value: 'Valentina' }
+              : undefined
+          }
         />
       </div>
       <div
@@ -398,9 +410,11 @@ export const AsyncSelect = ({
 };
 
 AsyncSelect.args = {
-  withError: false,
-  pageSize: 10,
   allowNewOptions: false,
+  fetchOnlyOnSearch: false,
+  pageSize: 10,
+  withError: false,
+  withInitialValue: false,
 };
 
 AsyncSelect.argTypes = {
@@ -431,6 +445,7 @@ AsyncSelect.argTypes = {
       type: 'range',
       min: 0.5,
       max: 5,
+      step: 0.5,
     },
   },
 };

diff --git a/superset-frontend/src/components/Select/Select.tsx b/superset-frontend/src/components/Select/Select.tsx
@@ -28,16 +28,17 @@ import React, {
   useCallback,
 } from 'react';
 import { styled, t } from '@superset-ui/core';
-import { Select as AntdSelect } from 'antd';
-import Icons from 'src/components/Icons';
-import {
+import AntdSelect, {
   SelectProps as AntdSelectProps,
   SelectValue as AntdSelectValue,
   LabeledValue as AntdLabeledValue,
 } from 'antd/lib/select';
+import { DownOutlined, SearchOutlined } from '@ant-design/icons';
 import debounce from 'lodash/debounce';
-import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 import { isEqual } from 'lodash';
+import { Spin } from 'antd';
+import Icons from 'src/components/Icons';
+import { getClientErrorObject } from 'src/utils/getClientErrorObject';
 import { hasOption } from './utils';
 
 type AntdSelectAllProps = AntdSelectProps<AntdSelectValue>;
@@ -47,7 +48,6 @@ type PickedSelectProps = Pick<
   | 'allowClear'
   | 'autoFocus'
   | 'value'
-  | 'defaultValue'
   | 'disabled'
   | 'filterOption'
   | 'notFoundContent'
@@ -79,6 +79,7 @@ export interface SelectProps extends PickedSelectProps {
   options: OptionsType | OptionsPagePromise;
   pageSize?: number;
   invertSelection?: boolean;
+  fetchOnlyOnSearch?: boolean;
 }
 
 const StyledContainer = styled.div`
@@ -122,6 +123,10 @@ const StyledError = styled.div`
   `}
 `;
 
+const StyledSpin = styled(Spin)`
+  margin-top: ${({ theme }) => -theme.gridUnit}px;
+`;
+
 const MAX_TAG_COUNT = 4;
 const TOKEN_SEPARATORS = [',', '\n', '\t', ';'];
 const DEBOUNCE_TIMEOUT = 500;
@@ -137,15 +142,16 @@ const Error = ({ error }: { error: string }) => (
 const Select = ({
   allowNewOptions = false,
   ariaLabel,
+  fetchOnlyOnSearch,
   filterOption = true,
   header = null,
+  invertSelection = false,
   mode = 'single',
   name,
+  options,
   pageSize = DEFAULT_PAGE_SIZE,
   placeholder = t('Select ...'),
-  options,
   showSearch,
-  invertSelection = false,
   value,
   ...props
 }: SelectProps) => {
@@ -164,7 +170,8 @@ const Select = ({
   const [isDropdownVisible, setIsDropdownVisible] = useState(false);
   const [page, setPage] = useState(0);
   const [totalCount, setTotalCount] = useState(0);
-  const fetchedQueries = useRef(new Set<string>());
+  const [loadingEnabled, setLoadingEnabled] = useState(false);
+  const fetchedQueries = useRef(new Map<string, number>());
   const mappedMode = isSingleMode
     ? undefined
     : allowNewOptions
@@ -177,6 +184,26 @@ const Select = ({
     );
   }, [options]);
 
+  useEffect(() => {
+    if (isAsync && value) {
+      const array: AntdLabeledValue[] = Array.isArray(value)
+        ? (value as AntdLabeledValue[])
+        : [value as AntdLabeledValue];
+      const options: AntdLabeledValue[] = [];
+      array.forEach(element => {
+        const found = selectOptions.find(
+          option => option.value === element.value,
+        );
+        if (!found) {
+          options.push(element);
+        }
+      });
+      if (options.length > 0) {
+        setSelectOptions([...selectOptions, ...options]);
+      }
+    }
+  }, [isAsync, selectOptions, value]);
+
   useEffect(() => {
     setSelectValue(value);
   }, [value]);
@@ -185,24 +212,56 @@ const Select = ({
     (selectedValue: AntdSelectValue | undefined) => {
       // bringing selected options to the top of the list
       if (selectedValue) {
-        const currentValue = selectedValue as string[] | string;
-        const topOptions = selectOptions.filter(opt =>
-          Array.isArray(currentValue)
-            ? currentValue.includes(opt.value)
-            : currentValue === opt.value,
-        );
-        const otherOptions = selectOptions.filter(
-          opt => !topOptions.find(tOpt => tOpt.value === opt.value),
-        );
+        const topOptions: OptionsType = [];
+        const otherOptions: OptionsType = [];
+
+        selectOptions.forEach(opt => {
+          let found = false;
+          if (Array.isArray(selectedValue)) {
+            if (isAsync) {
+              found =
+                (selectedValue as AntdLabeledValue[]).find(
+                  element => element.value === opt.value,
+                ) !== undefined;
+            } else {
+              found = selectedValue.includes(opt.value);
+            }
+          } else {
+            found = isAsync
+              ? (selectedValue as AntdLabeledValue).value === opt.value
+              : selectedValue === opt.value;
+          }
+
+          if (found) {
+            topOptions.push(opt);
+          } else {
+            otherOptions.push(opt);
+          }
+        });
+
         // fallback for custom options in tags mode as they
         // do not appear in the selectOptions state
-        if (!isSingleMode && Array.isArray(currentValue)) {
-          // eslint-disable-next-line no-restricted-syntax
-          for (const val of currentValue) {
-            if (!topOptions.find(tOpt => tOpt.value === val)) {
-              topOptions.push({ label: val, value: val });
+        if (!isSingleMode && Array.isArray(selectedValue)) {
+          selectedValue.forEach((val: string | number | AntdLabeledValue) => {
+            if (
+              !topOptions.find(
+                tOpt =>
+                  tOpt.value ===
+                  (isAsync ? (val as AntdLabeledValue)?.value : val),
+              )
+            ) {
+              if (isAsync) {
+                const labelValue = val as AntdLabeledValue;
+                topOptions.push({
+                  label: labelValue.label,
+                  value: labelValue.value,
+                });
+              } else {
+                const value = val as string | number;
+                topOptions.push({ label: String(value), value });
+              }
             }
-          }
+          });
         }
 
         const sortedOptions = [...topOptions, ...otherOptions];
@@ -211,7 +270,7 @@ const Select = ({
         }
       }
     },
-    [isSingleMode, selectOptions],
+    [isAsync, isSingleMode, selectOptions],
   );
 
   const handleOnSelect = (
@@ -220,7 +279,11 @@ const Select = ({
     if (isSingleMode) {
       setSelectValue(selectedValue);
     } else {
-      const currentSelected = Array.isArray(selectValue) ? selectValue : [];
+      const currentSelected = selectValue
+        ? Array.isArray(selectValue)
+          ? selectValue
+          : [selectValue]
+        : [];
       if (
         typeof selectedValue === 'number' ||
         typeof selectedValue === 'string'
@@ -271,15 +334,17 @@ const Select = ({
   const handlePaginatedFetch = useMemo(
     () => (value: string, page: number, pageSize: number) => {
       const key = `${value};${page};${pageSize}`;
-      if (fetchedQueries.current.has(key)) {
+      const cachedCount = fetchedQueries.current.get(key);
+      if (cachedCount) {
+        setTotalCount(cachedCount);
         return;
       }
       setLoading(true);
       const fetchOptions = options as OptionsPagePromise;
       fetchOptions(value, page, pageSize)
         .then(({ data, totalCount }: OptionsTypePage) => {
           handleData(data);
-          fetchedQueries.current.add(key);
+          fetchedQueries.current.set(key, totalCount);
           setTotalCount(totalCount);
         })
         .catch(onError)
@@ -351,6 +416,11 @@ const Select = ({
 
   const handleOnDropdownVisibleChange = (isDropdownVisible: boolean) => {
     setIsDropdownVisible(isDropdownVisible);
+
+    if (isAsync && !loadingEnabled) {
+      setLoadingEnabled(true);
+    }
+
     // multiple or tags mode keep the dropdown visible while selecting options
     // this waits for the dropdown to be closed before sorting the top options
     if (!isSingleMode && !isDropdownVisible) {
@@ -359,13 +429,20 @@ const Select = ({
   };
 
   useEffect(() => {
-    const foundOption = hasOption(searchedValue, selectOptions);
-    if (isAsync && !foundOption) {
+    const allowFetch = !fetchOnlyOnSearch || searchedValue;
+    if (isAsync && loadingEnabled && allowFetch) {
       const page = 0;
       handlePaginatedFetch(searchedValue, page, pageSize);
       setPage(page);
     }
-  }, [isAsync, searchedValue, selectOptions, pageSize, handlePaginatedFetch]);
+  }, [
+    isAsync,
+    searchedValue,
+    pageSize,
+    handlePaginatedFetch,
+    loadingEnabled,
+    fetchOnlyOnSearch,
+  ]);
 
   useEffect(() => {
     if (isSingleMode) {
@@ -382,6 +459,16 @@ const Select = ({
     return error ? <Error error={error} /> : originNode;
   };
 
+  const SuffixIcon = () => {
+    if (isLoading) {
+      return <StyledSpin size="small" />;
+    }
+    if (shouldShowSearch && isDropdownVisible) {
+      return <SearchOutlined />;
+    }
+    return <DownOutlined />;
+  };
+
   return (
     <StyledContainer>
       {header}
@@ -391,7 +478,7 @@ const Select = ({
         dropdownRender={dropdownRender}
         filterOption={handleFilterOption}
         getPopupContainer={triggerNode => triggerNode.parentNode}
-        loading={isLoading}
+        labelInValue={isAsync}
         maxTagCount={MAX_TAG_COUNT}
         mode={mappedMode}
         onDeselect={handleOnDeselect}
@@ -406,6 +493,7 @@ const Select = ({
         showArrow
         tokenSeparators={TOKEN_SEPARATORS}
         value={selectValue}
+        suffixIcon={<SuffixIcon />}
         menuItemSelectedIcon={
           invertSelection ? (
             <StyledStopOutlined iconSize="m" />

diff --git a/...dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DatasetSelect.tsx b/...dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/DatasetSelect.tsx
@@ -36,18 +36,11 @@ const cachedSupersetGet = cacheWrapper(
 );
 
 interface DatasetSelectProps {
-  datasetDetails: Record<string, any> | undefined;
-  datasetId: number;
-  onChange: (value: number) => void;
-  value?: { value: number | undefined };
+  onChange: (value: { label: string; value: number }) => void;
+  value?: { label: string; value: number };
 }
 
-const DatasetSelect = ({
-  datasetDetails,
-  datasetId,
-  onChange,
-  value,
-}: DatasetSelectProps) => {
+const DatasetSelect = ({ onChange, value }: DatasetSelectProps) => {
   const getErrorMessage = useCallback(
     ({ error, message }: ClientErrorObject) => {
       let errorText = message || error || t('An error has occurred');
@@ -84,15 +77,6 @@ const DatasetSelect = ({
           .sort((a: { label: string }, b: { label: string }) =>
             a.label.localeCompare(b.label),
           );
-        if (!search) {
-          const found = data.find(element => element.value === datasetId);
-          if (!found && datasetDetails?.table_name) {
-            data.push({
-              label: datasetDetails.table_name,
-              value: datasetId,
-            });
-          }
-        }
         return {
           data,
           totalCount: response.json.count,
@@ -107,7 +91,7 @@ const DatasetSelect = ({
   return (
     <Select
       ariaLabel={t('Dataset')}
-      value={value?.value}
+      value={value}
       options={loadDatasetOptions}
       onChange={onChange}
     />