Skip to content

Commit

Permalink
feat(Select, CustomSelect, NativeSelect): remove event from onChange …
Browse files Browse the repository at this point in the history
…callback
  • Loading branch information
EldarMuhamethanov committed Oct 30, 2024
1 parent d34ab47 commit 9cd1b75
Show file tree
Hide file tree
Showing 26 changed files with 208 additions and 164 deletions.
10 changes: 5 additions & 5 deletions packages/vkui/src/components/Avatar/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ const AvatarPropsForm = ({
{ label: 'blue', value: 'blue' },
]}
value={gradientColor}
onChange={(e) => setGradientColor(e.target.value)}
onChange={setGradientColor}
/>
</FormItem>

Expand All @@ -196,7 +196,7 @@ const AvatarPropsForm = ({
{ label: 'preset="online-mobile"', value: 'online-mobile' },
]}
value={badge}
onChange={(e) => setBadge(e.target.value)}
onChange={setBadge}
/>
</FormItem>
<FormItem top="Avatar.Badge[background]">
Expand All @@ -208,7 +208,7 @@ const AvatarPropsForm = ({
]}
value={badgeBackground}
disabled={badge !== 'children'}
onChange={(e) => setBadgeBackground(e.target.value)}
onChange={setBadgeBackground}
/>
</FormItem>
</FormLayoutGroup>
Expand All @@ -231,7 +231,7 @@ const AvatarPropsForm = ({
]}
value={overlayTheme}
disabled={!overlay}
onChange={(e) => setOverlayTheme(e.target.value)}
onChange={setOverlayTheme}
/>
</FormItem>
<FormItem top="Avatar.Overlay[visibility]">
Expand All @@ -243,7 +243,7 @@ const AvatarPropsForm = ({
]}
value={overlayVisibility}
disabled={!overlay}
onChange={(e) => setOverlayVisibility(e.target.value)}
onChange={setOverlayVisibility}
/>
</FormItem>
</FormLayoutGroup>
Expand Down
8 changes: 4 additions & 4 deletions packages/vkui/src/components/Button/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const Example = () => {
<FormItem top="appearance">
<Select
value={appearance}
onChange={(e) => setAppearance(e.target.value)}
onChange={setAppearance}
options={[
{ label: 'accent', value: 'accent' },
{ label: 'positive', value: 'positive' },
Expand All @@ -85,7 +85,7 @@ const Example = () => {
<FormItem top="size">
<Select
value={size}
onChange={(e) => setSize(e.target.value)}
onChange={setSize}
options={[
{ label: 's', value: 's' },
{ label: 'm', value: 'm' },
Expand All @@ -96,7 +96,7 @@ const Example = () => {
<FormItem top="align">
<Select
value={align}
onChange={(e) => setAlign(e.target.value)}
onChange={setAlign}
options={[
{ label: 'left', value: 'left' },
{ label: 'center', value: 'center' },
Expand All @@ -107,7 +107,7 @@ const Example = () => {
<FormItem top="sizeY">
<Select
value={sizeY}
onChange={(e) => setSizeY(e.target.value)}
onChange={setSizeY}
options={[
{ label: 'compact', value: 'compact' },
{
Expand Down
8 changes: 4 additions & 4 deletions packages/vkui/src/components/ButtonGroup/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const ButtonGroupPropsForm = ({
<FormItem top="mode">
<Select
value={mode}
onChange={(e) => handleChange('mode', e.target.value)}
onChange={(newValue) => handleChange('mode', newValue)}
options={[
{ label: 'vertical', value: 'vertical' },
{ label: 'horizontal', value: 'horizontal' },
Expand All @@ -43,7 +43,7 @@ const ButtonGroupPropsForm = ({
<FormItem top="gap">
<Select
value={gap}
onChange={(e) => handleChange('gap', e.target.value)}
onChange={(newValue) => handleChange('gap', newValue)}
options={[
{ label: 'none', value: 'none' },
{ label: 'space', value: 'space' },
Expand All @@ -55,7 +55,7 @@ const ButtonGroupPropsForm = ({
<FormItem top="align">
<Select
value={align}
onChange={(e) => handleChange('align', e.target.value)}
onChange={(newValue) => handleChange('align', newValue)}
options={[
{ label: 'left', value: 'left' },
{ label: 'center', value: 'center' },
Expand Down Expand Up @@ -402,7 +402,7 @@ const Example = () => {
<FormItem top="sizeY">
<Select
value={sizeY}
onChange={(e) => setSizeY(e.target.value)}
onChange={setSizeY}
options={[
{ label: 'compact', value: 'compact' },
{ label: 'regular', value: 'regular' },
Expand Down
4 changes: 2 additions & 2 deletions packages/vkui/src/components/Calendar/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const Example = () => {
<Select
style={{ width: 100 }}
value={locale}
onChange={(e) => setLocale(e.target.value)}
onChange={setLocale}
options={[
{
label: 'ru',
Expand All @@ -85,7 +85,7 @@ const Example = () => {
<Select
style={{ width: 100 }}
value={size}
onChange={(e) => setSize(e.target.value)}
onChange={setSize}
options={[
{
label: 's',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { DEFAULT_MAX_YEAR, DEFAULT_MIN_YEAR, getMonths, getYears } from '../../l
import type { HTMLAttributesWithRootRef } from '../../types';
import { AdaptivityProvider } from '../AdaptivityProvider/AdaptivityProvider';
import { useConfigProvider } from '../ConfigProvider/ConfigProviderContext';
import { CustomSelect } from '../CustomSelect/CustomSelect';
import { CustomSelect, type SelectProps } from '../CustomSelect/CustomSelect';
import { RootComponent } from '../RootComponent/RootComponent';
import { Tappable } from '../Tappable/Tappable';
import { Paragraph } from '../Typography/Paragraph/Paragraph';
Expand Down Expand Up @@ -80,13 +80,11 @@ export const CalendarHeader = ({
}: CalendarHeaderProps): React.ReactNode => {
const { locale } = useConfigProvider();
const onMonthsChange = React.useCallback(
(event: React.ChangeEvent<HTMLSelectElement>) =>
onChange(setMonth(viewDate, Number(event.target.value))),
(newValue: SelectProps['value']) => onChange(setMonth(viewDate, Number(newValue))),
[onChange, viewDate],
);
const onYearChange = React.useCallback(
(event: React.ChangeEvent<HTMLSelectElement>) =>
onChange(setYear(viewDate, Number(event.target.value))),
(newValue: SelectProps['value']) => onChange(setYear(viewDate, Number(newValue))),
[onChange, viewDate],
);

Expand Down
2 changes: 1 addition & 1 deletion packages/vkui/src/components/CalendarRange/Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const Example = () => {
<Select
style={{ width: 100 }}
value={locale}
onChange={(e) => setLocale(e.target.value)}
onChange={setLocale}
options={[
{
label: 'ru',
Expand Down
8 changes: 3 additions & 5 deletions packages/vkui/src/components/CalendarTime/CalendarTime.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as React from 'react';
import { setHours, setMinutes } from 'date-fns';
import { AdaptivityProvider } from '../AdaptivityProvider/AdaptivityProvider';
import { Button } from '../Button/Button';
import { CustomSelect } from '../CustomSelect/CustomSelect';
import { CustomSelect, type SelectProps } from '../CustomSelect/CustomSelect';
import styles from './CalendarTime.module.css';

export interface CalendarTimeProps {
Expand Down Expand Up @@ -55,13 +55,11 @@ export const CalendarTime = ({
: minutes;

const onHoursChange = React.useCallback(
(event: React.ChangeEvent<HTMLSelectElement>) =>
onChange?.(setHours(value, Number(event.target.value))),
(newValue: SelectProps['value']) => onChange?.(setHours(value, Number(newValue))),
[onChange, value],
);
const onMinutesChange = React.useCallback(
(event: React.ChangeEvent<HTMLSelectElement>) =>
onChange?.(setMinutes(value, Number(event.target.value))),
(newValue: SelectProps['value']) => onChange?.(setMinutes(value, Number(newValue))),
[onChange, value],
);

Expand Down
57 changes: 27 additions & 30 deletions packages/vkui/src/components/CustomSelect/CustomSelect.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,15 @@ const getCustomSelectValue = () => screen.getByTestId('labelTextTestId').textCon
const CustomSelectControlled = ({
options,
initialValue,
onChangeStub,
onChange,
...restProps
}: Omit<SelectProps, 'value' | 'onChange'> & {
}: Omit<SelectProps, 'value'> & {
initialValue?: string;
onChangeStub?: (event: React.ChangeEvent<HTMLSelectElement>) => void;
}) => {
const [value, setValue] = React.useState(initialValue);
const handleChange: React.ChangeEventHandler<HTMLSelectElement> = (event) => {
setValue(event.target.value);
onChangeStub?.(event);
const [value, setValue] = React.useState<SelectProps['value']>(initialValue);
const handleChange: SelectProps['onChange'] = (newValue) => {
setValue(newValue);
onChange?.(newValue);
};
return <CustomSelect {...restProps} options={options} value={value} onChange={handleChange} />;
};
Expand Down Expand Up @@ -122,7 +121,7 @@ describe('CustomSelect', () => {

it('works correctly as controlled component', () => {
const SelectController = () => {
const [value, setValue] = useState(0);
const [value, setValue] = useState<SelectProps['value']>(0);
const options = [
{ value: 0, label: 'Mike' },
{ value: 1, label: 'Josh' },
Expand All @@ -132,7 +131,7 @@ describe('CustomSelect', () => {
labelTextTestId="labelTextTestId"
options={options}
value={value}
onChange={(e) => setValue(Number(e.target.value))}
onChange={setValue}
/>
);
};
Expand Down Expand Up @@ -691,7 +690,7 @@ describe('CustomSelect', () => {
});

it('(uncontrolled): calls onChange when click on unselected option and does not call when click on selected ', async () => {
const onChange = jest.fn((event: React.ChangeEvent<HTMLSelectElement>) => event.target.value);
const onChange = jest.fn();

render(
<CustomSelect
Expand All @@ -716,7 +715,7 @@ describe('CustomSelect', () => {
fireEvent.click(unselectedOption);

expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveReturnedWith('1');
expect(onChange).toHaveBeenCalledWith('1');

fireEvent.click(screen.getByTestId('labelTextTestId'));

Expand All @@ -725,12 +724,12 @@ describe('CustomSelect', () => {
fireEvent.click(selectedOption);

expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveReturnedWith('1');
expect(onChange).toHaveBeenCalledWith('1');
});

it('(controlled): calls onChange expected amount of times after clearing component and clicking on option without updating controlled prop value', async () => {
// мы намеренно проверяем кейсы где при нажатии на опцию или на кнопку очистки value проп не меняется
const onChange = jest.fn((event: React.ChangeEvent<HTMLSelectElement>) => event.target.value);
const onChange = jest.fn();

const { rerender } = render(
<CustomSelect
Expand Down Expand Up @@ -762,7 +761,7 @@ describe('CustomSelect', () => {
fireEvent.click(unselectedOptionFirstClick);

expect(onChange).toHaveBeenCalledTimes(2);
expect(onChange).toHaveReturnedWith('1');
expect(onChange).toHaveBeenCalledWith('1');

// clear input through prop
rerender(
Expand Down Expand Up @@ -800,7 +799,7 @@ describe('CustomSelect', () => {
});

it('(controlled): calls onChange when click on unselected option without value change', async () => {
const onChange = jest.fn((event: React.ChangeEvent<HTMLSelectElement>) => event.target.value);
const onChange = jest.fn();

render(
<CustomSelect
Expand Down Expand Up @@ -829,7 +828,7 @@ describe('CustomSelect', () => {
fireEvent.click(unselectedOptionFirstClick);

expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveReturnedWith('1');
expect(onChange).toHaveBeenCalledWith('1');

// второй клик по не выбранной опции без изменения value
// нужно проверить потому что при первом клике внутреннее value CustomSelect (nativeSelectValue) изменилось
Expand All @@ -845,7 +844,7 @@ describe('CustomSelect', () => {
fireEvent.click(unselectedOptionSecondClick);

expect(onChange).toHaveBeenCalledTimes(2);
expect(onChange).toHaveReturnedWith('1');
expect(onChange).toHaveBeenCalledWith('1');

// третий клик уже по выбранной опции (соответствующей value переданному в CustomSelect)
// onChange не должен вызываться.
Expand All @@ -858,13 +857,11 @@ describe('CustomSelect', () => {
fireEvent.click(selectedOptionThirdClick);

expect(onChange).toHaveBeenCalledTimes(2);
expect(onChange).toHaveReturnedWith('1');
expect(onChange).toHaveBeenCalledWith('1');
});

it('(controlled): does not call onChange on already selected', async () => {
const onChangeStub = jest.fn((event: React.ChangeEvent<HTMLSelectElement>) => {
return event.target.value;
});
const onChangeStub = jest.fn();

render(
<CustomSelectControlled
Expand All @@ -874,7 +871,7 @@ describe('CustomSelect', () => {
{ value: 0, label: 'Mike' },
{ value: 1, label: 'Josh' },
]}
onChangeStub={onChangeStub}
onChange={onChangeStub}
/>,
);

Expand All @@ -893,7 +890,7 @@ describe('CustomSelect', () => {

// onChange должен вызываться
expect(onChangeStub).toHaveBeenCalledTimes(1);
expect(onChangeStub).toHaveReturnedWith('1');
expect(onChangeStub).toHaveBeenCalledWith('1');

// второй клик по выбранной опции
fireEvent.click(screen.getByTestId('labelTextTestId'));
Expand All @@ -903,7 +900,7 @@ describe('CustomSelect', () => {

// onChange не должен вызываться
expect(onChangeStub).toHaveBeenCalledTimes(1);
expect(onChangeStub).toHaveReturnedWith('1');
expect(onChangeStub).toHaveBeenCalledWith('1');

// третий клик по не выбранной опции
fireEvent.click(screen.getByTestId('labelTextTestId'));
Expand All @@ -917,7 +914,7 @@ describe('CustomSelect', () => {

// onChange должен быть вызван
expect(onChangeStub).toHaveBeenCalledTimes(2);
expect(onChangeStub).toHaveReturnedWith('0');
expect(onChangeStub).toHaveBeenCalledWith('0');

// четвертый клик по выбранной опции
fireEvent.click(screen.getByTestId('labelTextTestId'));
Expand All @@ -927,11 +924,11 @@ describe('CustomSelect', () => {

// onChange не должен вызываться
expect(onChangeStub).toHaveBeenCalledTimes(2);
expect(onChangeStub).toHaveReturnedWith('0');
expect(onChangeStub).toHaveBeenCalledWith('0');
});

it('(controlled): does call onChange on option click when prop value is empty and value is not changing', async () => {
const onChange = jest.fn((event: React.ChangeEvent<HTMLSelectElement>) => event.target.value);
const onChange = jest.fn();

render(
<CustomSelect
Expand Down Expand Up @@ -961,7 +958,7 @@ describe('CustomSelect', () => {

// onChange должен быть вызван
expect(onChange).toHaveBeenCalledTimes(1);
expect(onChange).toHaveReturnedWith('0');
expect(onChange).toHaveBeenCalledWith('0');

// второй клик по другой опции без изменения value
fireEvent.click(screen.getByTestId('inputTestId'));
Expand All @@ -974,7 +971,7 @@ describe('CustomSelect', () => {

// onChange должен быть вызван
expect(onChange).toHaveBeenCalledTimes(2);
expect(onChange).toHaveReturnedWith('1');
expect(onChange).toHaveBeenCalledWith('1');

// третий клик по той же опции что и в предыдущий раз
fireEvent.click(screen.getByTestId('inputTestId'));
Expand All @@ -987,7 +984,7 @@ describe('CustomSelect', () => {

// onChange должен быть вызван
expect(onChange).toHaveBeenCalledTimes(3);
expect(onChange).toHaveReturnedWith('0');
expect(onChange).toHaveBeenCalledWith('0');
});

it('accepts options with extended option type and Typescript does not throw', () => {
Expand Down
Loading

0 comments on commit 9cd1b75

Please sign in to comment.