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

fix(CustomSelect): refactor logic of empty value #7822

Merged
merged 7 commits into from
Oct 31, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ describe('CustomSelect', () => {
expect(onChange).toHaveBeenCalledTimes(0);
});

it('clear value externally with empty string', () => {
it('clear value externally with empty value', () => {
const onChange = jest.fn();

const { rerender } = render(
Expand All @@ -607,7 +607,7 @@ describe('CustomSelect', () => {
{ value: 1, label: 'Josh' },
]}
onChange={onChange}
value=""
value={null}
/>,
);

Expand Down Expand Up @@ -679,7 +679,7 @@ describe('CustomSelect', () => {
]}
allowClearButton
onChange={onChange}
value=""
value={null}
/>,
);

Expand Down Expand Up @@ -774,7 +774,7 @@ describe('CustomSelect', () => {
]}
allowClearButton
onChange={onChange}
value=""
value={null}
/>,
);

Expand Down Expand Up @@ -943,7 +943,7 @@ describe('CustomSelect', () => {
]}
allowClearButton
onChange={onChange}
value=""
value={null}
/>,
);

Expand Down Expand Up @@ -1104,6 +1104,7 @@ describe('CustomSelect', () => {
]}
placeholder="Не выбрано"
allowClearButton
value={null}
/>,
);

Expand Down
112 changes: 73 additions & 39 deletions packages/vkui/src/components/CustomSelect/CustomSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
} from '../CustomSelectOption/CustomSelectOption';
import { DropdownIcon } from '../DropdownIcon/DropdownIcon';
import type { FormFieldProps } from '../FormField/FormField';
import type { NativeSelectProps } from '../NativeSelect/NativeSelect';
import type {
NativeSelectProps,
NativeSelectValue,
SelectValue,
} from '../NativeSelect/NativeSelect';
import type { SelectType } from '../Select/Select';
import { Footnote } from '../Typography/Footnote/Footnote';
import { VisuallyHidden } from '../VisuallyHidden/VisuallyHidden';
Expand All @@ -39,6 +43,14 @@
compact: styles.sizeYCompact,
};

const NOT_SELECTED = '__vkui_internal_CustomSelect_not_selected__';

const remapFromSelectValueToNativeValue = (value: SelectValue): NativeSelectValue =>
value === null ? NOT_SELECTED : value;

const remapFromNativeValueToSelectValue = (value: NativeSelectValue): SelectValue =>
value === NOT_SELECTED ? null : value;

const findIndexAfter = (options: CustomSelectOptionInterface[] = [], startIndex = -1) => {
if (startIndex >= options.length - 1) {
return -1;
Expand Down Expand Up @@ -76,6 +88,24 @@
}
};

const checkMixControlledAndUncontrolledState = (
oldIsControlled: boolean,
newIsControlled: boolean,
) => {
if (!oldIsControlled && newIsControlled) {
warn(

Check warning on line 96 in packages/vkui/src/components/CustomSelect/CustomSelect.tsx

View check run for this annotation

Codecov / codecov/patch

packages/vkui/src/components/CustomSelect/CustomSelect.tsx#L96

Added line #L96 was not covered by tests
`Похоже, что компонент был переведен из состояния Uncontrolled в Controlled. Пожалуйста, не делайте так. Если вам нужно отобразить невыбранное состояние компонента, используйте value=null вместо undefined`,
'error',
);
}
if (oldIsControlled && !newIsControlled) {
warn(
`Похоже, что компонент был переведен из состояния Controlled в Uncontrolled. Пожалуйста, не делайте так. Если вам нужно отобразить невыбранное состояние компонента, используйте value=null вместо undefined`,
'error',
);
}
};

function defaultRenderOptionFn<T extends CustomSelectOptionInterface>({
option,
...props
Expand All @@ -90,9 +120,8 @@
function findSelectedIndex<T extends CustomSelectOptionInterface>(
options: T[] = [],
value: SelectValue,
withClear: boolean,
) {
if (withClear && value === '') {
if (value === null) {
return -1;
}
return (
Expand All @@ -113,10 +142,8 @@
: options;
};

type SelectValue = React.SelectHTMLAttributes<HTMLSelectElement>['value'];

export interface CustomSelectOptionInterface {
value: SelectValue;
value: Exclude<SelectValue, null>;
label: React.ReactElement | string;
disabled?: boolean;
[index: string]: any;
Expand Down Expand Up @@ -283,25 +310,40 @@
const [focusedOptionIndex, setFocusedOptionIndex] = React.useState<number | undefined>(-1);
const [isControlledOutside, setIsControlledOutside] = React.useState(props.value !== undefined);
const [inputValue, setInputValue] = React.useState('');
const [nativeSelectValue, setNativeSelectValue] = React.useState(
() => props.value ?? defaultValue ?? (allowClearButton ? '' : undefined),
);
const [nativeSelectValue, setNativeSelectValue] = React.useState<NativeSelectValue>(() => {
if (props.value !== undefined) {
return remapFromSelectValueToNativeValue(props.value);
}
if (defaultValue !== undefined) {
return remapFromSelectValueToNativeValue(defaultValue);
}
return NOT_SELECTED;
});

const [popperPlacement, setPopperPlacement] = React.useState<Placement>(popupDirection);
const [options, setOptions] = React.useState(optionsProp);
const [selectedOptionIndex, setSelectedOptionIndex] = React.useState<number | undefined>(
findSelectedIndex(optionsProp, props.value ?? defaultValue, allowClearButton),
findSelectedIndex(optionsProp, props.value ?? defaultValue ?? null),
);

React.useEffect(() => {
setIsControlledOutside(props.value !== undefined);
setNativeSelectValue((nativeSelectValue) => props.value ?? nativeSelectValue);
setIsControlledOutside((oldIsControlled) => {
const newIsControlled = props.value !== undefined;
checkMixControlledAndUncontrolledState(oldIsControlled, newIsControlled);
return newIsControlled;
});
setNativeSelectValue((nativeSelectValue) => {
if (props.value !== undefined) {
return remapFromSelectValueToNativeValue(props.value);
}
return nativeSelectValue;
});
}, [props.value]);

useIsomorphicLayoutEffect(() => {
if (
options.some(({ value }) => nativeSelectValue === value) ||
(allowClearButton && nativeSelectValue === '')
(allowClearButton && nativeSelectValue === NOT_SELECTED)
) {
const event = new Event('change', { bubbles: true });

Expand Down Expand Up @@ -432,8 +474,7 @@
const selectOption = React.useCallback(
(index: number) => {
const item = options[index];

setNativeSelectValue(item?.value);
setNativeSelectValue(item?.value ?? NOT_SELECTED);
EldarMuhamethanov marked this conversation as resolved.
Show resolved Hide resolved
close();

const shouldTriggerOnChangeWhenControlledAndInnerValueIsOutOfSync =
Expand Down Expand Up @@ -506,40 +547,31 @@

React.useEffect(
function updateOptionsAndSelectedOptionIndex() {
const value = props.value ?? nativeSelectValue ?? defaultValue;
const value =
props.value !== undefined
? props.value
: remapFromNativeValueToSelectValue(nativeSelectValue);

const options =
searchable && inputValue !== undefined
? filter(optionsProp, inputValue, filterFn)
: optionsProp;

setOptions(options);
setSelectedOptionIndex(findSelectedIndex(options, value, allowClearButton));
setSelectedOptionIndex(findSelectedIndex(options, value));
},
[
filterFn,
inputValue,
nativeSelectValue,
optionsProp,
defaultValue,
props.value,
searchable,
allowClearButton,
],
[filterFn, inputValue, nativeSelectValue, optionsProp, defaultValue, props.value, searchable],
);

const onNativeSelectChange: React.ChangeEventHandler<HTMLSelectElement> = (e) => {
const newSelectedOptionIndex = findSelectedIndex(
options,
e.currentTarget.value,
allowClearButton,
);
const remappedNativeValue = remapFromNativeValueToSelectValue(e.currentTarget.value);
const newSelectedOptionIndex = findSelectedIndex(options, remappedNativeValue);

if (selectedOptionIndex !== newSelectedOptionIndex) {
if (!isControlledOutside) {
setSelectedOptionIndex(newSelectedOptionIndex);
}
onChange?.(e);
onChange?.(e, remappedNativeValue);
}
};

Expand All @@ -549,11 +581,11 @@

const options = filter(optionsProp, e.target.value, filterFn);
setOptions(options);
setSelectedOptionIndex(findSelectedIndex(options, nativeSelectValue, allowClearButton));
setSelectedOptionIndex(findSelectedIndex(options, nativeSelectValue));

setInputValue(e.target.value);
},
[filterFn, nativeSelectValue, onInputChangeProp, optionsProp, allowClearButton],
[filterFn, nativeSelectValue, onInputChangeProp, optionsProp],
);

const areOptionsShown = React.useCallback(() => {
Expand Down Expand Up @@ -711,8 +743,8 @@

const selectInputRef = useExternRef(getSelectInputRef);

const controlledValueSet = isControlledOutside && props.value !== '';
const uncontrolledValueSet = !isControlledOutside && nativeSelectValue !== '';
const controlledValueSet = isControlledOutside && props.value !== null;
EldarMuhamethanov marked this conversation as resolved.
Show resolved Hide resolved
const uncontrolledValueSet = !isControlledOutside && nativeSelectValue !== NOT_SELECTED;
const clearButtonShown =
allowClearButton && !opened && (controlledValueSet || uncontrolledValueSet);

Expand All @@ -725,7 +757,7 @@
<ClearButton
className={iconProp === undefined ? styles.clearIcon : undefined}
onClick={function clearSelectState() {
setNativeSelectValue('');
setNativeSelectValue(NOT_SELECTED);
setInputValue('');
selectInputRef.current && selectInputRef.current.focus();
}}
Expand Down Expand Up @@ -872,7 +904,9 @@
data-testid={nativeSelectTestId}
required={required}
>
{allowClearButton && <option key="" value="" />}
{(allowClearButton || nativeSelectValue === NOT_SELECTED) && (
<option key={NOT_SELECTED} value={NOT_SELECTED} />
)}
{optionsProp.map((item) => (
<option key={`${item.value}`} value={item.value} />
))}
Expand Down
18 changes: 9 additions & 9 deletions packages/vkui/src/components/CustomSelect/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ const getUsers = (usersArray) =>
description: user.screen_name,
}));

const users = [...getUsers(getRandomUsers(10))];

const Example = () => {
const selectTypes = [
{
Expand All @@ -46,9 +48,7 @@ const Example = () => {
},
];

const [selectType, setSelectType] = React.useState(undefined);

const users = [...getUsers(getRandomUsers(10))];
const [selectType, setSelectType] = React.useState(null);

return (
<Div>
Expand All @@ -64,7 +64,7 @@ const Example = () => {
id="administrator-select-id"
placeholder="Не выбран"
options={users}
selectType={selectType}
selectType={selectType || 'default'}
allowClearButton
/>
</FormItem>
Expand All @@ -79,7 +79,7 @@ const Example = () => {
value={selectType}
placeholder="Не задан"
options={selectTypes}
onChange={(e) => setSelectType(e.target.value)}
onChange={(_, newType) => setSelectType(newType)}
EldarMuhamethanov marked this conversation as resolved.
Show resolved Hide resolved
renderOption={({ option, ...restProps }) => (
<CustomSelectOption {...restProps} description={`"${option.value}"`} />
)}
Expand Down Expand Up @@ -115,7 +115,7 @@ const Example = () => {
id="administrator-select-id-3"
placeholder="Не выбран"
options={users}
selectType={selectType}
selectType={selectType || 'default'}
/>
</FormItem>

Expand Down Expand Up @@ -176,12 +176,12 @@ const CustomSearchLogicSelect = ({ id }) => {
return options;
};

const onCustomSearchChange = (e) => {
if (e.target.value === '0') {
const onCustomSearchChange = (_, newValue) => {
if (newValue === '0') {
setNewUsers([...newUsers, { label: query, value: query }]);
setValue(query);
} else {
setValue(e.target.value);
setValue(newValue);
}
setQuery('');
};
Expand Down
Loading