From cc78fb268af3179cc5a16ec81da5ec8341750159 Mon Sep 17 00:00:00 2001 From: "e.muhamethanov" Date: Wed, 23 Oct 2024 18:06:40 +0300 Subject: [PATCH 1/5] fix(CustomSelect): refactor logic of reset value --- .../CustomSelect/CustomSelect.test.tsx | 13 +-- .../components/CustomSelect/CustomSelect.tsx | 102 ++++++++++++------ .../src/components/CustomSelect/Readme.md | 18 ++-- .../components/NativeSelect/NativeSelect.tsx | 38 ++++++- 4 files changed, 121 insertions(+), 50 deletions(-) diff --git a/packages/vkui/src/components/CustomSelect/CustomSelect.test.tsx b/packages/vkui/src/components/CustomSelect/CustomSelect.test.tsx index bbb5c39268..4c52799bd0 100644 --- a/packages/vkui/src/components/CustomSelect/CustomSelect.test.tsx +++ b/packages/vkui/src/components/CustomSelect/CustomSelect.test.tsx @@ -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( @@ -607,7 +607,7 @@ describe('CustomSelect', () => { { value: 1, label: 'Josh' }, ]} onChange={onChange} - value="" + value={null} />, ); @@ -679,7 +679,7 @@ describe('CustomSelect', () => { ]} allowClearButton onChange={onChange} - value="" + value={null} />, ); @@ -774,7 +774,7 @@ describe('CustomSelect', () => { ]} allowClearButton onChange={onChange} - value="" + value={null} />, ); @@ -943,7 +943,7 @@ describe('CustomSelect', () => { ]} allowClearButton onChange={onChange} - value="" + value={null} />, ); @@ -989,7 +989,7 @@ describe('CustomSelect', () => { expect(onChange).toHaveBeenCalledTimes(3); expect(onChange).toHaveReturnedWith('0'); }); - + // TODO it('accepts options with extended option type and Typescript does not throw', () => { const { rerender } = render( { ]} placeholder="Не выбрано" allowClearButton + value={null} />, ); diff --git a/packages/vkui/src/components/CustomSelect/CustomSelect.tsx b/packages/vkui/src/components/CustomSelect/CustomSelect.tsx index a92ce46692..3fcc2b53e0 100644 --- a/packages/vkui/src/components/CustomSelect/CustomSelect.tsx +++ b/packages/vkui/src/components/CustomSelect/CustomSelect.tsx @@ -36,6 +36,16 @@ const sizeYClassNames = { compact: styles.sizeYCompact, }; +const NOT_SELECTED = '__vkui_internal_CustomSelect_not_selected__'; + +type NativeValue = Exclude; + +const remapFromSelectValueToNativeValue = (value: NativeValue | null): NativeValue => + value === null ? NOT_SELECTED : value; + +const remapFromNativeValueToSelectValue = (value: NativeValue): SelectValue | null => + value === NOT_SELECTED ? null : value; + const findIndexAfter = (options: CustomSelectOptionInterface[] = [], startIndex = -1) => { if (startIndex >= options.length - 1) { return -1; @@ -73,6 +83,24 @@ const checkOptionsValueType = (options: T } }; +const checkMixControlledAndUncontrolledState = ( + oldIsControlled: boolean, + newIsControlled: boolean, +) => { + if (!oldIsControlled && newIsControlled) { + warn( + `Похоже, что компонент был переведен из состояния Uncontrolled в Controlled. Пожалуйста, не делайте так. Если вам нужно отобразить невыбранное состояние компонента, используйте value=null вместо undefined`, + 'error', + ); + } + if (oldIsControlled && !newIsControlled) { + warn( + `Похоже, что компонент был переведен из состояния Controlled в Uncontrolled. Пожалуйста, не делайте так. Если вам нужно отобразить невыбранное состояние компонента, используйте value=null вместо undefined`, + 'error', + ); + } +}; + function defaultRenderOptionFn({ option, ...props @@ -86,10 +114,9 @@ const handleOptionDown: MouseEventHandler = (e: React.MouseEvent) = function findSelectedIndex( options: T[] = [], - value: SelectValue, - withClear: boolean, + value: SelectValue | null, ) { - if (withClear && value === '') { + if (value === null) { return -1; } return ( @@ -280,25 +307,40 @@ export function CustomSelect(-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(() => { + if (props.value !== undefined) { + return remapFromSelectValueToNativeValue(props.value); + } + if (defaultValue !== undefined) { + return remapFromSelectValueToNativeValue(defaultValue); + } + return NOT_SELECTED; + }); const [popperPlacement, setPopperPlacement] = React.useState(popupDirection); const [options, setOptions] = React.useState(optionsProp); const [selectedOptionIndex, setSelectedOptionIndex] = React.useState( - findSelectedIndex(optionsProp, props.value ?? defaultValue, allowClearButton), + findSelectedIndex(optionsProp, props.value ?? defaultValue), ); 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 }); @@ -429,8 +471,7 @@ export function CustomSelect { const item = options[index]; - - setNativeSelectValue(item?.value); + setNativeSelectValue(item?.value ?? NOT_SELECTED); close(); const shouldTriggerOnChangeWhenControlledAndInnerValueIsOutOfSync = @@ -503,7 +544,10 @@ export function CustomSelect = (e) => { const newSelectedOptionIndex = findSelectedIndex( options, - e.currentTarget.value, - allowClearButton, + remapFromNativeValueToSelectValue(e.currentTarget.value), ); if (selectedOptionIndex !== newSelectedOptionIndex) { if (!isControlledOutside) { setSelectedOptionIndex(newSelectedOptionIndex); } - onChange?.(e); + onChange?.(e, remapFromNativeValueToSelectValue(e.currentTarget.value)); } }; @@ -546,11 +580,11 @@ export function CustomSelect { @@ -721,8 +755,8 @@ export function CustomSelect - {allowClearButton &&