Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Merges latest Select changes #16587

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 43 additions & 25 deletions superset-frontend/src/components/Select/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@ type PickedSelectProps = Pick<
| 'autoFocus'
| 'disabled'
| 'filterOption'
| 'labelInValue'
| 'loading'
| 'notFoundContent'
| 'onChange'
| 'onClear'
| 'onFocus'
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geido I didn't add defaultValue assuming that we can use value.

| 'placeholder'
| 'showSearch'
| 'value'
Expand All @@ -73,6 +77,7 @@ export interface SelectProps extends PickedSelectProps {
allowNewOptions?: boolean;
ariaLabel: string;
header?: ReactNode;
lazyLoading?: boolean;
mode?: 'single' | 'multiple';
name?: string; // discourage usage
options: OptionsType | OptionsPagePromise;
Expand All @@ -84,15 +89,11 @@ export interface SelectProps extends PickedSelectProps {
const StyledContainer = styled.div`
display: flex;
flex-direction: column;
width: 100%;
`;

const StyledSelect = styled(AntdSelect, {
shouldForwardProp: prop => prop !== 'hasHeader',
})<{ hasHeader: boolean }>`
${({ theme, hasHeader }) => `
width: 100%;
margin-top: ${hasHeader ? theme.gridUnit : 0}px;

const StyledSelect = styled(AntdSelect)`
${({ theme }) => `
&& .ant-select-selector {
border-radius: ${theme.gridUnit}px;
}
Expand Down Expand Up @@ -163,8 +164,12 @@ const Select = ({
filterOption = true,
header = null,
invertSelection = false,
labelInValue = false,
lazyLoading = true,
loading = false,
mode = 'single',
name,
onChange,
options,
pageSize = DEFAULT_PAGE_SIZE,
placeholder = t('Select ...'),
Expand All @@ -182,13 +187,13 @@ const Select = ({
);
const [selectValue, setSelectValue] = useState(value);
const [searchedValue, setSearchedValue] = useState('');
const [isLoading, setIsLoading] = useState(false);
const [isLoading, setIsLoading] = useState(loading);
const [isTyping, setIsTyping] = useState(false);
const [error, setError] = useState('');
const [isDropdownVisible, setIsDropdownVisible] = useState(false);
const [page, setPage] = useState(0);
const [totalCount, setTotalCount] = useState(0);
const [loadingEnabled, setLoadingEnabled] = useState(false);
const [loadingEnabled, setLoadingEnabled] = useState(!lazyLoading);
const fetchedQueries = useRef(new Map<string, number>());
const mappedMode = isSingleMode
? undefined
Expand All @@ -197,16 +202,21 @@ const Select = ({
: 'multiple';

useEffect(() => {
fetchedQueries.current.clear();
setSelectOptions(
options && Array.isArray(options) ? options : EMPTY_OPTIONS,
);
}, [options]);

useEffect(() => {
if (isAsync && value) {
const array: AntdLabeledValue[] = Array.isArray(value)
? (value as AntdLabeledValue[])
: [value as AntdLabeledValue];
setSelectValue(value);
}, [value]);

useEffect(() => {
if (isAsync && selectValue) {
const array: AntdLabeledValue[] = Array.isArray(selectValue)
? (selectValue as AntdLabeledValue[])
: [selectValue as AntdLabeledValue];
const options: AntdLabeledValue[] = [];
array.forEach(element => {
const found = selectOptions.find(
Expand All @@ -220,23 +230,20 @@ const Select = ({
setSelectOptions([...selectOptions, ...options]);
}
}
}, [isAsync, selectOptions, value]);

useEffect(() => {
setSelectValue(value);
}, [value]);
}, [isAsync, selectOptions, selectValue]);

const handleTopOptions = useCallback(
(selectedValue: AntdSelectValue | undefined) => {
// bringing selected options to the top of the list
if (selectedValue !== undefined && selectedValue !== null) {
const isLabeledValue = isAsync || labelInValue;
const topOptions: OptionsType = [];
const otherOptions: OptionsType = [];

selectOptions.forEach(opt => {
let found = false;
if (Array.isArray(selectedValue)) {
if (isAsync) {
if (isLabeledValue) {
found =
(selectedValue as AntdLabeledValue[]).find(
element => element.value === opt.value,
Expand All @@ -245,7 +252,7 @@ const Select = ({
found = selectedValue.includes(opt.value);
}
} else {
found = isAsync
found = isLabeledValue
? (selectedValue as AntdLabeledValue).value === opt.value
: selectedValue === opt.value;
}
Expand All @@ -265,10 +272,10 @@ const Select = ({
!topOptions.find(
tOpt =>
tOpt.value ===
(isAsync ? (val as AntdLabeledValue)?.value : val),
(isLabeledValue ? (val as AntdLabeledValue)?.value : val),
)
) {
if (isAsync) {
if (isLabeledValue) {
const labelValue = val as AntdLabeledValue;
topOptions.push({
label: labelValue.label,
Expand All @@ -288,7 +295,7 @@ const Select = ({
}
}
},
[isAsync, isSingleMode, selectOptions],
[isAsync, isSingleMode, labelInValue, selectOptions],
);

const handleOnSelect = (
Expand Down Expand Up @@ -405,6 +412,10 @@ const Select = ({
const newOptions = [...selectOptions, newOption];
setSelectOptions(newOptions);
setSelectValue(searchValue);

if (onChange) {
onChange(searchValue, newOptions);
}
}
}
setSearchedValue(searchValue);
Expand All @@ -416,6 +427,7 @@ const Select = ({
allowNewOptions,
initialOptions,
isSingleMode,
onChange,
Copy link
Member Author

@michael-s-molina michael-s-molina Sep 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@geido onChange was not in the list of dependencies in your original PR. It's a good idea to check that you're passing a memoized function 😉

searchedValue,
selectOptions,
],
Expand Down Expand Up @@ -489,6 +501,12 @@ const Select = ({
}
}, [handleTopOptions, isSingleMode, selectValue]);

useEffect(() => {
if (loading !== undefined && loading !== isLoading) {
setIsLoading(loading);
}
}, [isLoading, loading]);

const dropdownRender = (
originNode: ReactElement & { ref?: RefObject<HTMLElement> },
) => {
Expand Down Expand Up @@ -521,12 +539,11 @@ const Select = ({
<StyledContainer>
{header}
<StyledSelect
hasHeader={!!header}
aria-label={ariaLabel || name}
dropdownRender={dropdownRender}
filterOption={handleFilterOption}
getPopupContainer={triggerNode => triggerNode.parentNode}
labelInValue={isAsync}
labelInValue={isAsync || labelInValue}
maxTagCount={MAX_TAG_COUNT}
mode={mappedMode}
onDeselect={handleOnDeselect}
Expand All @@ -536,6 +553,7 @@ const Select = ({
onSearch={shouldShowSearch ? handleOnSearch : undefined}
onSelect={handleOnSelect}
onClear={() => setSelectValue(undefined)}
onChange={onChange}
options={selectOptions}
placeholder={placeholder}
showSearch={shouldShowSearch}
Expand Down