chore: Improves the Select component UI/UX - iteration 3 (#15363)

superset-frontend/src/components/Select/Select.stories.tsx

@@ -17,7 +17,8 @@
  * under the License.
  */
 import React, { ReactNode, useState, useCallback } from 'react';
-import Select, { SelectProps, OptionsPromiseResult } from './Select';
+import ControlHeader from 'src/explore/components/ControlHeader';
+import Select, { SelectProps, OptionsType, OptionsTypePage } from './Select';
 
 export default {
   title: 'Select',
@@ -66,6 +67,98 @@ const selectPositions = [
   },
 ];
 
+const ARG_TYPES = {
+  options: {
+    defaultValue: options,
+    table: {
+      disable: true,
+    },
+  },
+  ariaLabel: {
+    table: {
+      disable: true,
+    },
+  },
+  name: {
+    table: {
+      disable: true,
+    },
+  },
+  notFoundContent: {
+    table: {
+      disable: true,
+    },
+  },
+  mode: {
+    defaultValue: 'single',
+    control: {
+      type: 'inline-radio',
+      options: ['single', 'multiple'],
+    },
+  },
+};
+
+const mountHeader = (type: String) => {
+  let header;
+  if (type === 'text') {
+    header = 'Text header';
+  } else if (type === 'control') {
+    header = (
+      <ControlHeader
+        label="Control header"
+        warning="Example of warning messsage"
+      />
+    );
+  }
+  return header;
+};
+
+export const InteractiveSelect = (args: SelectProps & { header: string }) => (
+  <div
+    style={{
+      width: DEFAULT_WIDTH,
+    }}
+  >
+    <Select {...args} header={mountHeader(args.header)} />
+  </div>
+);
+
+InteractiveSelect.args = {
+  autoFocus: false,
+  allowNewOptions: false,
+  allowClear: false,
+  showSearch: false,
+  disabled: false,
+  invertSelection: false,
+  placeholder: 'Select ...',
+};
+
+InteractiveSelect.argTypes = {
+  ...ARG_TYPES,
+  header: {
+    defaultValue: 'none',
+    control: { type: 'inline-radio', options: ['none', 'text', 'control'] },
+  },
+  pageSize: {
+    table: {
+      disable: true,
+    },
+  },
+  paginatedFetch: {
+    table: {
+      disable: true,
+    },
+  },
+};
+
+InteractiveSelect.story = {
+  parameters: {
+    knobs: {
+      disable: true,
+    },
+  },
+};
+
 export const AtEveryCorner = () => (
   <>
     {selectPositions.map(position => (
@@ -73,6 +166,7 @@ export const AtEveryCorner = () => (
         key={position.id}
         style={{
           ...position.style,
+          margin: 30,
           width: DEFAULT_WIDTH,
           position: 'absolute',
         }}
@@ -102,6 +196,56 @@ AtEveryCorner.story = {
   },
 };
 
+export const PageScroll = () => (
+  <div style={{ height: 2000, overflowY: 'auto' }}>
+    <div
+      style={{
+        width: DEFAULT_WIDTH,
+        position: 'absolute',
+        top: 30,
+        right: 30,
+      }}
+    >
+      <Select ariaLabel="page-scroll-select-1" options={options} />
+    </div>
+    <div
+      style={{
+        width: DEFAULT_WIDTH,
+        position: 'absolute',
+        bottom: 30,
+        right: 30,
+      }}
+    >
+      <Select ariaLabel="page-scroll-select-2" options={options} />
+    </div>
+    <p
+      style={{
+        position: 'absolute',
+        top: '40%',
+        left: 30,
+        width: 500,
+      }}
+    >
+      The objective of this panel is to show how the Select behaves when there's
+      a scroll on the page. In particular, how the drop-down is displayed.
+    </p>
+  </div>
+);
+
+PageScroll.story = {
+  parameters: {
+    actions: {
+      disable: true,
+    },
+    controls: {
+      disable: true,
+    },
+    knobs: {
+      disable: true,
+    },
+  },
+};
+
 const USERS = [
   'John',
   'Liam',
@@ -155,71 +299,90 @@ const USERS = [
   'Ilenia',
 ];
 
-export const AsyncSelect = (
+export const AsyncSelect = ({
-  args: SelectProps & { withError: boolean; responseTime: number },
+  withError,
-) => {
+  responseTime,
+  paginatedFetch,
+  ...rest
+}: SelectProps & {
+  withError: boolean;
+  responseTime: number;
+}) => {
   const [requests, setRequests] = useState<ReactNode[]>([]);
 
+  const getResults = (username: string) => {
+    let results: { label: string; value: string }[] = [];
+
+    if (!username) {
+      results = USERS.map(u => ({
+        label: u,
+        value: u,
+      }));
+    } else {
+      const foundUsers = USERS.filter(u => u.toLowerCase().includes(username));
+      if (foundUsers) {
+        results = foundUsers.map(u => ({ label: u, value: u }));
+      } else {
+        results = [];
+      }
+    }
+    return results;
+  };
+
+  const setRequestLog = (username: string, results: number, total: number) => {
+    const request = (
+      <>
+        Emulating network request with search <b>{username || 'empty'}</b> ...{' '}
+        <b>
+          {results}/{total}
+        </b>{' '}
+        results
+      </>
+    );
+
+    setRequests(requests => [request, ...requests]);
+  };
+
   const fetchUserList = useCallback(
-    (search: string, page = 0): Promise<OptionsPromiseResult> => {
+    (search: string): Promise<OptionsType> => {
       const username = search.trim().toLowerCase();
       return new Promise(resolve => {
-        let results: { label: string; value: string }[] = [];
+        const results = getResults(username);
-
+        setRequestLog(username, results.length, results.length);
-        if (!username) {
-          results = USERS.map(u => ({
-            label: u,
-            value: u,
-          }));
-        } else {
-          const foundUsers = USERS.find(u =>
-            u.toLowerCase().includes(username),
-          );
-          if (foundUsers && Array.isArray(foundUsers)) {
-            results = foundUsers.map(u => ({ label: u, value: u }));
-          }
-          if (foundUsers && typeof foundUsers === 'string') {
-            const u = foundUsers;
-            results = [{ label: u, value: u }];
-          }
-        }
-
-        const pageSize = 10;
-        const offset = !page ? 0 : page * pageSize;
-        const resultsNum = !page ? pageSize : (page + 1) * pageSize;
-        results = results.length ? results.splice(offset, resultsNum) : [];
-
-        const request = (
-          <>
-            Emulating network request for page <b>{page}</b> and search{' '}
-            <b>{username || 'empty'}</b> ... <b>{resultsNum}</b> results
-          </>
-        );
-
-        setRequests(requests => [request, ...requests]);
-
-        const totalPages =
-          USERS.length / pageSize + (USERS.length % pageSize > 0 ? 1 : 0);
-
-        const result: OptionsPromiseResult = {
-          data: results,
-          hasMoreData: page + 1 < totalPages,
-        };
-
         setTimeout(() => {
-          resolve(result);
+          resolve(results);
-        }, args.responseTime * 1000);
+        }, responseTime * 1000);
       });
     },
-    [args.responseTime],
+    [responseTime],
   );
 
-  async function fetchUserListError(): Promise<OptionsPromiseResult> {
+  const fetchUserListPage = useCallback(
-    return new Promise((_, reject) => {
+    (
-      // eslint-disable-next-line prefer-promise-reject-errors
+      search: string,
-      reject('This is an error');
+      offset: number,
+      limit: number,
+    ): Promise<OptionsTypePage> => {
+      const username = search.trim().toLowerCase();
+      return new Promise(resolve => {
+        let results = getResults(username);
+        const totalCount = results.length;
+        if (paginatedFetch) {
+          results = results.splice(offset, limit);
+        }
+        setRequestLog(username, offset + results.length, totalCount);
+        setTimeout(() => {
+          resolve({ data: results, totalCount });
+        }, responseTime * 1000);
+      });
+    },
+    [paginatedFetch, responseTime],
+  );
+
+  const fetchUserListError = async (): Promise<OptionsTypePage> =>
+    new Promise((_, reject) => {
+      reject(new Error('Error while fetching the names from the server'));
     });
-  }
 
   return (
     <>
@@ -229,8 +392,15 @@ export const AsyncSelect = (
         }}
       >
         <Select
-          {...args}
+          {...rest}
-          options={args.withError ? fetchUserListError : fetchUserList}
+          paginatedFetch={paginatedFetch}
+          options={
+            withError
+              ? fetchUserListError
+              : paginatedFetch
+              ? fetchUserListPage
+              : fetchUserList
+          }
         />
       </div>
       <div
@@ -245,8 +415,8 @@ export const AsyncSelect = (
           padding: 20,
         }}
       >
-        {requests.map(request => (
+        {requests.map((request, index) => (
-          <p>{request}</p>
+          <p key={`request-${index}`}>{request}</p>
         ))}
       </div>
     </>
@@ -255,20 +425,38 @@ export const AsyncSelect = (
 
 AsyncSelect.args = {
   withError: false,
-  allowNewOptions: false,
   paginatedFetch: false,
+  pageSize: 10,
+  allowNewOptions: false,
 };
 
 AsyncSelect.argTypes = {
-  mode: {
+  ...ARG_TYPES,
-    control: { type: 'select', options: ['single', 'multiple', 'tags'] },
+  header: {
+    table: {
+      disable: true,
+    },
+  },
+  invertSelection: {
+    table: {
+      disable: true,
+    },
+  },
+  pageSize: {
+    defaultValue: 10,
+    control: {
+      type: 'range',
+      min: 10,
+      max: 50,
+      step: 10,
+    },
   },
   responseTime: {
-    defaultValue: 1,
+    defaultValue: 0.5,
     name: 'responseTime (seconds)',
     control: {
       type: 'range',
-      min: 1,
+      min: 0.5,
       max: 5,
     },
   },
@@ -281,33 +469,3 @@ AsyncSelect.story = {
     },
   },
 };
-
-export const InteractiveSelect = (args: SelectProps) => (
-  <div
-    style={{
-      width: DEFAULT_WIDTH,
-    }}
-  >
-    <Select {...args} />
-  </div>
-);
-
-InteractiveSelect.args = {
-  allowNewOptions: false,
-  options,
-  showSearch: false,
-};
-
-InteractiveSelect.argTypes = {
-  mode: {
-    control: { type: 'select', options: ['single', 'multiple', 'tags'] },
-  },
-};
-
-InteractiveSelect.story = {
-  parameters: {
-    knobs: {
-      disable: true,
-    },
-  },
-};

superset-frontend/src/components/Select/Select.tsx

@@ -48,9 +48,6 @@ type PickedSelectProps = Pick<
   | 'defaultValue'
   | 'disabled'
   | 'filterOption'
-  | 'loading'
-  | 'mode'
-  | 'notFoundContent'
   | 'onChange'
   | 'placeholder'
   | 'showSearch'
@@ -59,30 +56,29 @@ type PickedSelectProps = Pick<
 
 export type OptionsType = Exclude<AntdSelectAllProps['options'], undefined>;
 
-export type OptionsPromiseResult = {
+export type OptionsTypePage = {
   data: OptionsType;
-  hasMoreData: boolean;
+  totalCount: number;
 };
 
-export type OptionsPromise = (
+export type OptionsPromise = (search: string) => Promise<OptionsType>;
-  search: string,
-  page?: number,
-) => Promise<OptionsPromiseResult>;
 
-export enum ESelectTypes {
+export type OptionsPagePromise = (
-  MULTIPLE = 'multiple',
+  search: string,
-  TAGS = 'tags',
+  offset: number,
-  SINGLE = '',
+  limit: number,
-}
+) => Promise<OptionsTypePage>;
 
 export interface SelectProps extends PickedSelectProps {
   allowNewOptions?: boolean;
   ariaLabel: string;
   header?: ReactNode;
+  mode?: 'single' | 'multiple';
   name?: string; // discourage usage
-  notFoundContent?: ReactNode;
+  options: OptionsType | OptionsPromise | OptionsPagePromise;
-  options: OptionsType | OptionsPromise;
   paginatedFetch?: boolean;
+  pageSize?: number;
+  invertSelection?: boolean;
 }
 
 const StyledContainer = styled.div`
@@ -90,80 +86,86 @@ const StyledContainer = styled.div`
   flex-direction: column;
 `;
 
-// unexposed default behaviors
+const StyledSelect = styled(AntdSelect, {
+  shouldForwardProp: prop => prop !== 'hasHeader',
+})<{ hasHeader: boolean }>`
+  ${({ theme, hasHeader }) => `
+    width: 100%;
+    margin-top: ${hasHeader ? theme.gridUnit : 0}px;
+  `}
+`;
+
+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;
+    }
+  `}
+`;
+
+// default behaviors
 const MAX_TAG_COUNT = 4;
 const TOKEN_SEPARATORS = [',', '\n', '\t', ';'];
 const DEBOUNCE_TIMEOUT = 500;
+const DEFAULT_PAGE_SIZE = 50;
 
-const Error = ({ error }: { error: string }) => {
+const Error = ({ error }: { error: string }) => (
-  const StyledError = styled.div`
+  <StyledError>
-    display: flex;
+    <Icons.ErrorSolid /> {error}
-    justify-content: center;
+  </StyledError>
-    width: 100%;
+);
-    color: ${({ theme }) => theme.colors.error};
-  `;
-  return (
-    <StyledError>
-      <Icons.Error /> {error}
-    </StyledError>
-  );
-};
-
-const DropdownContent = ({
-  content,
-  error,
-}: {
-  content: ReactElement;
-  error?: string;
-  loading?: boolean;
-}) => {
-  if (error) {
-    return <Error error={error} />;
-  }
-  return content;
-};
 
 const Select = ({
   allowNewOptions = false,
   ariaLabel,
   filterOption,
   header = null,
-  loading,
+  mode = 'single',
-  mode,
   name,
-  notFoundContent,
+  paginatedFetch,
-  paginatedFetch = false,
+  pageSize = DEFAULT_PAGE_SIZE,
   placeholder = t('Select ...'),
   options,
   showSearch,
+  invertSelection = false,
   value,
   ...props
 }: SelectProps) => {
   const isAsync = typeof options === 'function';
-  const isSingleMode =
+  const isSingleMode = mode === 'single';
-    mode !== ESelectTypes.TAGS && mode !== ESelectTypes.MULTIPLE;
   const shouldShowSearch = isAsync || allowNewOptions ? true : showSearch;
   const initialOptions = options && Array.isArray(options) ? options : [];
   const [selectOptions, setOptions] = useState<OptionsType>(initialOptions);
   const [selectValue, setSelectValue] = useState(value);
   const [searchedValue, setSearchedValue] = useState('');
-  const [isLoading, setLoading] = useState(loading);
+  const [isLoading, setLoading] = useState(false);
   const [error, setError] = useState('');
   const [isDropdownVisible, setIsDropdownVisible] = useState(false);
-  const [hasMoreData, setHasMoreData] = useState(false);
+  const [offset, setOffset] = useState(0);
-  const fetchRef = useRef(0);
+  const [totalCount, setTotalCount] = useState(0);
+  const fetchedQueries = useRef(new Set<string>());
+  const mappedMode = isSingleMode
+    ? undefined
+    : allowNewOptions
+    ? 'tags'
+    : 'multiple';
 
-  const handleSelectMode = () => {
+  const handleTopOptions = (selectedValue: AntdSelectValue | undefined) => {
-    if (allowNewOptions && mode === ESelectTypes.MULTIPLE) {
-      return ESelectTypes.TAGS;
-    }
-    if (!allowNewOptions && mode === ESelectTypes.TAGS) {
-      return ESelectTypes.MULTIPLE;
-    }
-    return mode;
-  };
-
-  const handleTopOptions = (selectedValue: any) => {
     // bringing selected options to the top of the list
     if (selectedValue) {
       const currentValue = selectedValue as string[] | string;
@@ -187,61 +189,98 @@ const Select = ({
     }
   };
 
-  const handleOnSelect = (selectedValue: any) => {
+  const handleOnSelect = (
-    if (!isSingleMode) {
+    selectedValue: string | number | AntdLabeledValue,
-      const currentSelected = Array.isArray(selectValue) ? selectValue : [];
+  ) => {
-      setSelectValue([...currentSelected, selectedValue]);
+    if (isSingleMode) {
-    } else {
       setSelectValue(selectedValue);
       // in single mode the sorting must happen on selection
       handleTopOptions(selectedValue);
+    } else {
+      const currentSelected = Array.isArray(selectValue) ? selectValue : [];
+      if (
+        typeof selectedValue === 'number' ||
+        typeof selectedValue === 'string'
+      ) {
+        setSelectValue([
+          ...(currentSelected as (string | number)[]),
+          selectedValue as string | number,
+        ]);
+      } else {
+        setSelectValue([
+          ...(currentSelected as AntdLabeledValue[]),
+          selectedValue as AntdLabeledValue,
+        ]);
+      }
     }
+    setSearchedValue('');
   };
 
-  const handleOnDeselect = (value: any) => {
+  const handleOnDeselect = (value: string | number | AntdLabeledValue) => {
     if (Array.isArray(selectValue)) {
       const selectedValues = [
         ...(selectValue as []).filter(opt => opt !== value),
       ];
       setSelectValue(selectedValues);
     }
+    setSearchedValue('');
+  };
+
+  const onError = (response: Response) =>
+    getClientErrorObject(response).then(e => {
+      const { error } = e;
+      setError(error);
+    });
+
+  const handleData = (data: OptionsType) => {
+    if (data && Array.isArray(data) && data.length) {
+      // merges with existing and creates unique options
+      setOptions(prevOptions => [
+        ...prevOptions,
+        ...data.filter(
+          newOpt =>
+            !prevOptions.find(prevOpt => prevOpt.value === newOpt.value),
+        ),
+      ]);
+    }
   };
 
   const handleFetch = useMemo(
-    () => (value: string, paginate?: 'paginate') => {
+    () => (value: string) => {
-      if (paginate) {
+      if (fetchedQueries.current.has(value)) {
-        fetchRef.current += 1;
+        return;
-      } else {
-        fetchRef.current = 0;
       }
-      const fetchId = fetchRef.current;
+      setLoading(true);
-      const page = paginatedFetch ? fetchId : undefined;
       const fetchOptions = options as OptionsPromise;
-      fetchOptions(value, page)
+      fetchOptions(value)
-        .then((result: OptionsPromiseResult) => {
+        .then((data: OptionsType) => {
-          const { data, hasMoreData } = result;
+          handleData(data);
-          setHasMoreData(hasMoreData);
+          fetchedQueries.current.add(value);
-          if (fetchId !== fetchRef.current) return;
-          if (data && Array.isArray(data) && data.length) {
-            // merges with existing and creates unique options
-            setOptions(prevOptions => [
-              ...prevOptions,
-              ...data.filter(
-                newOpt =>
-                  !prevOptions.find(prevOpt => prevOpt.value === newOpt.value),
-              ),
-            ]);
-          }
         })
-        .catch(response =>
+        .catch(onError)
-          getClientErrorObject(response).then(e => {
-            const { error } = e;
-            setError(error);
-          }),
-        )
         .finally(() => setLoading(false));
     },
-    [options, paginatedFetch],
+    [options],
+  );
+
+  const handlePaginatedFetch = useMemo(
+    () => (value: string, offset: number, limit: number) => {
+      const key = `${value};${offset};${limit}`;
+      if (fetchedQueries.current.has(key)) {
+        return;
+      }
+      setLoading(true);
+      const fetchOptions = options as OptionsPagePromise;
+      fetchOptions(value, offset, limit)
+        .then(({ data, totalCount }: OptionsTypePage) => {
+          handleData(data);
+          fetchedQueries.current.add(key);
+          setTotalCount(totalCount);
+        })
+        .catch(onError)
+        .finally(() => setLoading(false));
+    },
+    [options],
   );
 
   const handleOnSearch = debounce((search: string) => {
@@ -273,13 +312,16 @@ const Select = ({
 
   const handlePagination = (e: UIEvent<HTMLElement>) => {
     const vScroll = e.currentTarget;
-    if (
+    const thresholdReached =
-      hasMoreData &&
+      vScroll.scrollTop > (vScroll.scrollHeight - vScroll.offsetHeight) * 0.7;
-      isAsync &&
+    const hasMoreData = offset + pageSize < totalCount;
-      paginatedFetch &&
+
-      vScroll.scrollTop === vScroll.scrollHeight - vScroll.offsetHeight
+    if (!isLoading && isAsync && hasMoreData && thresholdReached) {
-    ) {
+      const newOffset = offset + pageSize;
-      handleFetch(searchedValue, 'paginate');
+      const limit =
+        newOffset + pageSize > totalCount ? totalCount - newOffset : pageSize;
+      handlePaginatedFetch(searchedValue, newOffset, limit);
+      setOffset(newOffset);
     }
   };
 
@@ -315,18 +357,24 @@ const Select = ({
 
   useEffect(() => {
     const foundOption = hasOption(searchedValue, selectOptions);
-    if (isAsync && !foundOption && !allowNewOptions) {
+    if (isAsync && !foundOption) {
-      setLoading(true);
+      if (paginatedFetch) {
-      handleFetch(searchedValue);
+        const offset = 0;
+        handlePaginatedFetch(searchedValue, offset, pageSize);
+        setOffset(offset);
+      } else {
+        handleFetch(searchedValue);
+      }
     }
-  }, [allowNewOptions, isAsync, handleFetch, searchedValue, selectOptions]);
+  }, [
-
+    isAsync,
-  useEffect(() => {
+    handleFetch,
-    if (isAsync && allowNewOptions) {
+    searchedValue,
-      setLoading(true);
+    selectOptions,
-      handleFetch(searchedValue);
+    pageSize,
-    }
+    paginatedFetch,
-  }, [allowNewOptions, isAsync, handleFetch, searchedValue]);
+    handlePaginatedFetch,
+  ]);
 
   const dropdownRender = (
     originNode: ReactElement & { ref?: RefObject<HTMLElement> },
@@ -334,32 +382,40 @@ const Select = ({
     if (!isDropdownVisible) {
       originNode.ref?.current?.scrollTo({ top: 0 });
     }
-    return <DropdownContent content={originNode} error={error} />;
+    return error ? <Error error={error} /> : originNode;
   };
 
   return (
     <StyledContainer>
       {header}
-      <AntdSelect
+      <StyledSelect
+        hasHeader={!!header}
         aria-label={ariaLabel || name}
         dropdownRender={dropdownRender}
-        filterOption={handleFilterOption as any}
+        filterOption={handleFilterOption}
         getPopupContainer={triggerNode => triggerNode.parentNode}
         loading={isLoading}
         maxTagCount={MAX_TAG_COUNT}
-        mode={handleSelectMode()}
+        mode={mappedMode}
-        notFoundContent={isLoading ? null : notFoundContent}
         onDeselect={handleOnDeselect}
         onDropdownVisibleChange={handleOnDropdownVisibleChange}
-        onPopupScroll={handlePagination}
+        onPopupScroll={paginatedFetch ? handlePagination : undefined}
         onSearch={handleOnSearch}
         onSelect={handleOnSelect}
+        onClear={() => setSelectValue(undefined)}
         options={selectOptions}
-        placeholder={shouldShowSearch ? t('Search ...') : placeholder}
+        placeholder={placeholder}
         showSearch={shouldShowSearch}
+        showArrow
         tokenSeparators={TOKEN_SEPARATORS}
         value={selectValue}
-        style={{ width: '100%' }}
+        menuItemSelectedIcon={
+          invertSelection ? (
+            <StyledStopOutlined iconSize="m" />
+          ) : (
+            <StyledCheckOutlined iconSize="m" />
+          )
+        }
         {...props}
       />
     </StyledContainer>