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.

Sign up for GitHub

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

feat(Slider)!: support form #1670

Open
wants to merge 3 commits into
base: next
Choose a base branch
from
Open

feat(Slider)!: support form #1670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6017c26
feat(Slider)!: support form
ValeraS Jun 19, 2024
c23285d
fix(BaseSlider): replace ts-expect error with a type assertion
ValeraS Jun 20, 2024
ee044da
fix: remove debounceDelay from readme
ValeraS Dec 4, 2024
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
12 changes: 6 additions & 6 deletions src/components/Slider/BaseSlider/BaseSlider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import './BaseSlider.scss';

const b = block('base-slider');

type BaseSliderProps = {stateModifiers: StateModifiers} & Omit<
SliderProps,
type BaseSliderProps<T = number | number[]> = {stateModifiers: StateModifiers} & Omit<
SliderProps<T>,
'classNames' | 'prefixCls' | 'className' | 'pushable' | 'keyboard'
>;

export const BaseSlider = React.forwardRef<SliderRef, BaseSliderProps>(function BaseSlider(
{stateModifiers, ...otherProps}: BaseSliderProps,
ref: React.ForwardedRef<SliderRef>,
{stateModifiers, ...otherProps},
ref,
) {
return (
<Slider
Expand All @@ -34,6 +34,6 @@ export const BaseSlider = React.forwardRef<SliderRef, BaseSliderProps>(function
pushable={false}
dots={false}
keyboard={true}
></Slider>
/>
);
});
}) as <T>(p: BaseSliderProps<T> & {ref?: React.Ref<SliderRef>}) => React.ReactElement;
1 change: 0 additions & 1 deletion src/components/Slider/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,6 @@ You are able to change display format of tooltip value by using `tooltipFormat`
| autoFocus | The control's `autofocus` attribute | `boolean` | |
| [availableValues](#define-available-values) | (deprecated) use `marks` and `step` === null instead. Specifies the array of available values for the slider | `number[]` | |
| className | The control's wrapper class name | `string` | |
| debounceDelay | (deprecated) use external debouncing. Specifies the delay (in milliseconds) before the processing function is called | `number` | `0` |
| [defaultValue](#slider-variants) | The control's default value, used when the component is not controlled | `number` `[number, number]` | `0` |
| [disabled](#disabled) | Indicates that the user cannot interact with the control | `boolean` | `false` |
| [errorMessage](#error) | Text of an error to show | `string` | |
Expand Down
139 changes: 69 additions & 70 deletions src/components/Slider/Slider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,15 @@

import React from 'react';

import debounce from 'lodash/debounce';

import {useControlledState} from '../../hooks';
Arucard89 marked this conversation as resolved.
Show resolved Hide resolved
import {useFormResetHandler} from '../../hooks/private';
import {useDirection} from '../theme';
import {block} from '../utils/cn';
import {filterDOMProps} from '../utils/filterDOMProps';

import {BaseSlider} from './BaseSlider/BaseSlider';
import {HandleWithTooltip} from './HandleWithTooltip/HandleWithTooltip';
import type {
HandleWithTooltipProps,
RcSliderValueType,
SliderProps,
SliderValue,
StateModifiers,
} from './types';
import type {HandleWithTooltipProps, SliderProps, StateModifiers} from './types';
import {prepareSliderInnerState} from './utils';

import './Slider.scss';
Expand All @@ -40,46 +35,25 @@ export const Slider = React.forwardRef(function Slider(
errorMessage,
validationState,
disabled = false,
debounceDelay = 0,
onBlur,
onUpdate,
onUpdateComplete,
onFocus,
autoFocus = false,
tabIndex,
className,
style,
qa,
apiRef,
'aria-label': ariaLabelForHandle,
'aria-labelledby': ariaLabelledByForHandle,
name,
form,
...otherProps
}: SliderProps,
ref: React.ForwardedRef<HTMLDivElement>,
) {
const direction = useDirection();
// eslint-disable-next-line react-hooks/exhaustive-deps
const handleUpdate = React.useCallback(
debounce(
(changedValue: RcSliderValueType) => onUpdate?.(changedValue as SliderValue),
debounceDelay,
),
[onUpdate, debounceDelay],
);

// eslint-disable-next-line react-hooks/exhaustive-deps
const handleUpdateComplete = React.useCallback(
debounce(
(changedValue: RcSliderValueType) => onUpdateComplete?.(changedValue as SliderValue),
debounceDelay,
),
[onUpdateComplete, debounceDelay],
);

React.useEffect(() => {
return () => {
handleUpdate.cancel();
handleUpdateComplete.cancel();
};
}, [handleUpdate, handleUpdateComplete]);

const innerState = prepareSliderInnerState({
availableValues,
Expand All @@ -95,47 +69,71 @@ export const Slider = React.forwardRef(function Slider(
tooltipDisplay,
tooltipFormat,
});

const stateModifiers: StateModifiers = React.useMemo<StateModifiers>(
() => ({
size,
error: validationState === 'invalid' && !disabled,
disabled,
'tooltip-display': innerState.tooltipDisplay,
rtl: direction === 'rtl',
'no-marks': Array.isArray(marks) ? marks.length === 0 : marks === 0,
}),
[direction, disabled, innerState.tooltipDisplay, size, validationState, marks],
const [innerValue, setValue] = useControlledState(
innerState.value,
innerState.defaultValue ?? min,
onUpdate,
);

const handleRender = React.useMemo(
() =>
innerState.tooltipDisplay === 'off'
? undefined
: (
originHandle: HandleWithTooltipProps['originHandle'],
originHandleProps: HandleWithTooltipProps['originHandleProps'],
) => (
<HandleWithTooltip
originHandle={originHandle}
originHandleProps={originHandleProps}
stateModifiers={stateModifiers}
className={b('tooltip')}
tooltipFormat={innerState.tooltipFormat}
/>
),
[innerState.tooltipDisplay, innerState.tooltipFormat, stateModifiers],
const handleReset = React.useCallback(
(v: number | [number, number]) => {
setValue(v);
onUpdateComplete?.(v);
},
[onUpdateComplete, setValue],
);

const inputRef = useFormResetHandler({initialValue: innerValue, onReset: handleReset});

const stateModifiers: StateModifiers = {
size,
error: validationState === 'invalid' && !disabled,
disabled,
'tooltip-display': innerState.tooltipDisplay,
rtl: direction === 'rtl',
'no-marks': Array.isArray(marks) ? marks.length === 0 : marks === 0,
};
Comment on lines +88 to +95
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you remove memoization?
it can help prevent creating object on each render and could prevent rerenders inside components


const handleRender = (
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you remove memoization?
It could prevent some rerenders

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you show me where this might prevent re-renders? Maybe I'm missing something.

originHandle: HandleWithTooltipProps['originHandle'],
originHandleProps: HandleWithTooltipProps['originHandleProps'],
) => {
const handle =
innerState.tooltipDisplay === 'off' ? (
originHandle
) : (
<HandleWithTooltip
originHandle={originHandle}
originHandleProps={originHandleProps}
stateModifiers={stateModifiers}
className={b('tooltip')}
tooltipFormat={innerState.tooltipFormat}
/>
);

return (
<React.Fragment>
{handle}
<input
ref={inputRef}
type="hidden"
name={name}
form={form}
value={originHandleProps.value}
disabled={disabled}
/>
</React.Fragment>
);
};

return (
<div className={b(null, className)} ref={ref}>
<div {...filterDOMProps(otherProps)} className={b(null, className)} style={style} ref={ref}>
<div className={b('top', {size, 'tooltip-display': tooltipDisplay})}>
{/* use this block to reserve place for tooltip */}
</div>
<BaseSlider
ref={apiRef}
value={innerState.value}
defaultValue={innerState.defaultValue}
value={innerValue}
min={innerState.min}
max={innerState.max}
step={innerState.step}
Expand All @@ -144,21 +142,22 @@ export const Slider = React.forwardRef(function Slider(
marks={innerState.marks}
onBlur={onBlur}
onFocus={onFocus}
onChange={handleUpdate}
onChangeComplete={handleUpdateComplete}
onChange={setValue}
onChangeComplete={onUpdateComplete}
stateModifiers={stateModifiers}
// eslint-disable-next-line jsx-a11y/no-autofocus
autoFocus={autoFocus}
tabIndex={tabIndex}
data-qa={qa}
handleRender={handleRender}
reverse={stateModifiers.rtl}
ariaLabelForHandle={ariaLabelForHandle}
ariaLabelledByForHandle={ariaLabelledByForHandle}
></BaseSlider>
/>
{stateModifiers.error && errorMessage && (
<div className={b('error', {size})}>{errorMessage}</div>
)}
</div>
);
});
}) as <T extends number | [number, number]>(
p: SliderProps<T> & {ref?: React.Ref<HTMLDivElement>},
) => React.ReactElement;
123 changes: 123 additions & 0 deletions src/components/Slider/__tetsts__/Slider.form.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import React from 'react';

import userEvent from '@testing-library/user-event';

import {fireEvent, render, screen} from '../../../../test-utils/utils';
import {Slider} from '../Slider';

describe('Slider form', () => {
it('should submit empty option by default', async () => {
let value;
const onSubmit = jest.fn((e) => {
e.preventDefault();
const formData = new FormData(e.currentTarget);
value = [...formData.entries()];
});
render(
<form data-qa="form" onSubmit={onSubmit}>
<Slider name="slider" />
<button type="submit" data-qa="submit">
submit
</button>
</form>,
);
await userEvent.click(screen.getByTestId('submit'));
expect(onSubmit).toHaveBeenCalledTimes(1);
expect(value).toEqual([['slider', '0']]);
});

it('should submit default option', async () => {
let value;
const onSubmit = jest.fn((e) => {
e.preventDefault();
const formData = new FormData(e.currentTarget);
value = [...formData.entries()];
});
render(
<form data-qa="form" onSubmit={onSubmit}>
<Slider name="slider" defaultValue={5} />
<button type="submit" data-qa="submit">
submit
</button>
</form>,
);
await userEvent.click(screen.getByTestId('submit'));
expect(onSubmit).toHaveBeenCalledTimes(1);
expect(value).toEqual([['slider', '5']]);
});

it('should submit multiple option', async () => {
let value;
const onSubmit = jest.fn((e) => {
e.preventDefault();
const formData = new FormData(e.currentTarget);
value = [...formData.entries()];
});
render(
<form data-qa="form" onSubmit={onSubmit}>
<Slider name="slider" defaultValue={[5, 10]} />
<button type="submit" data-qa="submit">
submit
</button>
</form>,
);
await userEvent.click(screen.getByTestId('submit'));
expect(onSubmit).toHaveBeenCalledTimes(1);
expect(value).toEqual([
['slider', '5'],
['slider', '10'],
]);
});

it('supports form reset', async () => {
function Test() {
const [value, setValue] = React.useState(5);
return (
<form data-qa="form">
<Slider name="slider" value={value} onUpdate={setValue} qa="slider" />
<input type="reset" data-qa="reset" />
</form>
);
}

render(<Test />);
const form = screen.getByTestId('form');
expect(form).toHaveFormValues({slider: '5'});

const sliderHandle = screen.getAllByRole('slider')[0];
fireEvent.keyDown(sliderHandle, {key: 'ArrowRight', keyCode: 39, code: 'ArrowRight'});
fireEvent.keyDown(sliderHandle, {key: 'ArrowRight', keyCode: 39, code: 'ArrowRight'});

expect(form).toHaveFormValues({slider: '7'});

const button = screen.getByTestId('reset');
await userEvent.click(button);
expect(form).toHaveFormValues({slider: '5'});
});

it('supports form reset range value', async () => {
function Test() {
const [value, setValue] = React.useState<[number, number]>([5, 10]);
return (
<form data-qa="form">
<Slider name="slider" value={value} onUpdate={setValue} />
<input type="reset" data-qa="reset" />
</form>
);
}

render(<Test />);
const form = screen.getByTestId('form');
expect(form).toHaveFormValues({slider: ['5', '10']});

const sliderHandle = screen.getAllByRole('slider')[1];
fireEvent.keyDown(sliderHandle, {key: 'ArrowRight', keyCode: 39, code: 'ArrowRight'});
Copy link
Contributor

Choose a reason for hiding this comment

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

May be use userEvent.keyboard? Could be shorter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

userEvent.keyboard does not set keyCode for the event, but rs-slider checks it.

fireEvent.keyDown(sliderHandle, {key: 'ArrowRight', keyCode: 39, code: 'ArrowRight'});

expect(form).toHaveFormValues({slider: ['5', '12']});

const button = screen.getByTestId('reset');
await userEvent.click(button);
expect(form).toHaveFormValues({slider: ['5', '10']});
});
});
Loading
Loading