This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new a7e1037 chore: Improves the Select component UI/UX - iteration 3
(#15363)
a7e1037 is described below
commit a7e103765a8a6a4974a2d4a532553c52f797deb7
Author: Michael S. Molina <[email protected]>
AuthorDate: Fri Jun 25 14:01:39 2021 -0300
chore: Improves the Select component UI/UX - iteration 3 (#15363)
---
.../src/components/Select/Select.stories.tsx | 342 +++++++++++++++------
superset-frontend/src/components/Select/Select.tsx | 304 ++++++++++--------
2 files changed, 430 insertions(+), 216 deletions(-)
diff --git a/superset-frontend/src/components/Select/Select.stories.tsx
b/superset-frontend/src/components/Select/Select.stories.tsx
index 8454712..9925148 100644
--- a/superset-frontend/src/components/Select/Select.stories.tsx
+++ b/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 = (
- args: SelectProps & { withError: boolean; responseTime: number },
-) => {
+export const AsyncSelect = ({
+ withError,
+ responseTime,
+ paginatedFetch,
+ ...rest
+}: SelectProps & {
+ withError: boolean;
+ responseTime: number;
+}) => {
const [requests, setRequests] = useState<ReactNode[]>([]);
- const fetchUserList = useCallback(
- (search: string, page = 0): Promise<OptionsPromiseResult> => {
- const username = search.trim().toLowerCase();
- return new Promise(resolve => {
- let results: { label: string; value: string }[] = [];
-
- 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 getResults = (username: string) => {
+ let results: { label: string; value: string }[] = [];
- const request = (
- <>
- Emulating network request for page <b>{page}</b> and search{' '}
- <b>{username || 'empty'}</b> ... <b>{resultsNum}</b> results
- </>
- );
+ 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;
+ };
- setRequests(requests => [request, ...requests]);
+ const setRequestLog = (username: string, results: number, total: number) => {
+ const request = (
+ <>
+ Emulating network request with search <b>{username || 'empty'}</b>
...{' '}
+ <b>
+ {results}/{total}
+ </b>{' '}
+ results
+ </>
+ );
- const totalPages =
- USERS.length / pageSize + (USERS.length % pageSize > 0 ? 1 : 0);
+ setRequests(requests => [request, ...requests]);
+ };
- const result: OptionsPromiseResult = {
- data: results,
- hasMoreData: page + 1 < totalPages,
- };
+ const fetchUserList = useCallback(
+ (search: string): Promise<OptionsType> => {
+ const username = search.trim().toLowerCase();
+ return new Promise(resolve => {
+ const results = getResults(username);
+ setRequestLog(username, results.length, results.length);
+ setTimeout(() => {
+ resolve(results);
+ }, responseTime * 1000);
+ });
+ },
+ [responseTime],
+ );
+ const fetchUserListPage = useCallback(
+ (
+ search: string,
+ 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(result);
- }, args.responseTime * 1000);
+ resolve({ data: results, totalCount });
+ }, responseTime * 1000);
});
},
- [args.responseTime],
+ [paginatedFetch, responseTime],
);
- async function fetchUserListError(): Promise<OptionsPromiseResult> {
- return new Promise((_, reject) => {
- // eslint-disable-next-line prefer-promise-reject-errors
- reject('This is an error');
+ 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}
- options={args.withError ? fetchUserListError : fetchUserList}
+ {...rest}
+ paginatedFetch={paginatedFetch}
+ options={
+ withError
+ ? fetchUserListError
+ : paginatedFetch
+ ? fetchUserListPage
+ : fetchUserList
+ }
/>
</div>
<div
@@ -245,8 +415,8 @@ export const AsyncSelect = (
padding: 20,
}}
>
- {requests.map(request => (
- <p>{request}</p>
+ {requests.map((request, index) => (
+ <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: {
- control: { type: 'select', options: ['single', 'multiple', 'tags'] },
+ ...ARG_TYPES,
+ 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,
- },
- },
-};
diff --git a/superset-frontend/src/components/Select/Select.tsx
b/superset-frontend/src/components/Select/Select.tsx
index f7df21e..cc02ae8 100644
--- a/superset-frontend/src/components/Select/Select.tsx
+++ b/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 = (
- search: string,
- page?: number,
-) => Promise<OptionsPromiseResult>;
+export type OptionsPromise = (search: string) => Promise<OptionsType>;
-export enum ESelectTypes {
- MULTIPLE = 'multiple',
- TAGS = 'tags',
- SINGLE = '',
-}
+export type OptionsPagePromise = (
+ search: string,
+ offset: number,
+ 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;
+ options: OptionsType | OptionsPromise | OptionsPagePromise;
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 MAX_TAG_COUNT = 4;
-const TOKEN_SEPARATORS = [',', '\n', '\t', ';'];
-const DEBOUNCE_TIMEOUT = 500;
+const StyledSelect = styled(AntdSelect, {
+ shouldForwardProp: prop => prop !== 'hasHeader',
+})<{ hasHeader: boolean }>`
+ ${({ theme, hasHeader }) => `
+ width: 100%;
+ margin-top: ${hasHeader ? theme.gridUnit : 0}px;
+ `}
+`;
-const Error = ({ error }: { error: string }) => {
- const StyledError = styled.div`
+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%;
- color: ${({ theme }) => theme.colors.error};
- `;
- return (
- <StyledError>
- <Icons.Error /> {error}
- </StyledError>
- );
-};
+ padding: ${theme.gridUnit * 2}px;
+ color: ${theme.colors.error.base};
-const DropdownContent = ({
- content,
- error,
-}: {
- content: ReactElement;
- error?: string;
- loading?: boolean;
-}) => {
- if (error) {
- return <Error error={error} />;
- }
- return content;
-};
+ & 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 }) => (
+ <StyledError>
+ <Icons.ErrorSolid /> {error}
+ </StyledError>
+);
const Select = ({
allowNewOptions = false,
ariaLabel,
filterOption,
header = null,
- loading,
- mode,
+ mode = 'single',
name,
- notFoundContent,
- paginatedFetch = false,
+ paginatedFetch,
+ pageSize = DEFAULT_PAGE_SIZE,
placeholder = t('Select ...'),
options,
showSearch,
+ invertSelection = false,
value,
...props
}: SelectProps) => {
const isAsync = typeof options === 'function';
- const isSingleMode =
- mode !== ESelectTypes.TAGS && mode !== ESelectTypes.MULTIPLE;
+ const isSingleMode = mode === 'single';
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 fetchRef = useRef(0);
+ const [offset, setOffset] = useState(0);
+ const [totalCount, setTotalCount] = useState(0);
+ const fetchedQueries = useRef(new Set<string>());
+ const mappedMode = isSingleMode
+ ? undefined
+ : allowNewOptions
+ ? 'tags'
+ : 'multiple';
- const handleSelectMode = () => {
- if (allowNewOptions && mode === ESelectTypes.MULTIPLE) {
- return ESelectTypes.TAGS;
- }
- if (!allowNewOptions && mode === ESelectTypes.TAGS) {
- return ESelectTypes.MULTIPLE;
- }
- return mode;
- };
-
- const handleTopOptions = (selectedValue: any) => {
+ const handleTopOptions = (selectedValue: AntdSelectValue | undefined) => {
// 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) => {
- if (!isSingleMode) {
- const currentSelected = Array.isArray(selectValue) ? selectValue : [];
- setSelectValue([...currentSelected, selectedValue]);
- } else {
+ const handleOnSelect = (
+ selectedValue: string | number | AntdLabeledValue,
+ ) => {
+ if (isSingleMode) {
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') => {
- if (paginate) {
- fetchRef.current += 1;
- } else {
- fetchRef.current = 0;
+ () => (value: string) => {
+ if (fetchedQueries.current.has(value)) {
+ return;
}
- const fetchId = fetchRef.current;
- const page = paginatedFetch ? fetchId : undefined;
+ setLoading(true);
const fetchOptions = options as OptionsPromise;
- fetchOptions(value, page)
- .then((result: OptionsPromiseResult) => {
- const { data, hasMoreData } = result;
- setHasMoreData(hasMoreData);
- 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),
- ),
- ]);
- }
+ fetchOptions(value)
+ .then((data: OptionsType) => {
+ handleData(data);
+ fetchedQueries.current.add(value);
+ })
+ .catch(onError)
+ .finally(() => setLoading(false));
+ },
+ [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(response =>
- getClientErrorObject(response).then(e => {
- const { error } = e;
- setError(error);
- }),
- )
+ .catch(onError)
.finally(() => setLoading(false));
},
- [options, paginatedFetch],
+ [options],
);
const handleOnSearch = debounce((search: string) => {
@@ -273,13 +312,16 @@ const Select = ({
const handlePagination = (e: UIEvent<HTMLElement>) => {
const vScroll = e.currentTarget;
- if (
- hasMoreData &&
- isAsync &&
- paginatedFetch &&
- vScroll.scrollTop === vScroll.scrollHeight - vScroll.offsetHeight
- ) {
- handleFetch(searchedValue, 'paginate');
+ const thresholdReached =
+ vScroll.scrollTop > (vScroll.scrollHeight - vScroll.offsetHeight) * 0.7;
+ const hasMoreData = offset + pageSize < totalCount;
+
+ if (!isLoading && isAsync && hasMoreData && thresholdReached) {
+ const newOffset = offset + pageSize;
+ 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) {
- setLoading(true);
- handleFetch(searchedValue);
- }
- }, [allowNewOptions, isAsync, handleFetch, searchedValue, selectOptions]);
-
- useEffect(() => {
- if (isAsync && allowNewOptions) {
- setLoading(true);
- handleFetch(searchedValue);
+ if (isAsync && !foundOption) {
+ if (paginatedFetch) {
+ const offset = 0;
+ handlePaginatedFetch(searchedValue, offset, pageSize);
+ setOffset(offset);
+ } else {
+ handleFetch(searchedValue);
+ }
}
- }, [allowNewOptions, isAsync, handleFetch, searchedValue]);
+ }, [
+ isAsync,
+ handleFetch,
+ searchedValue,
+ selectOptions,
+ pageSize,
+ paginatedFetch,
+ 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()}
- notFoundContent={isLoading ? null : notFoundContent}
+ mode={mappedMode}
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>
