Skip to content

Commit

Permalink
feat: deprecate onAfterChange for onChangeComplete (#953)
Browse files Browse the repository at this point in the history
  • Loading branch information
MadCcc committed Nov 30, 2023
1 parent 5d70e74 commit 86a8a7a
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 18 deletions.
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -112,7 +112,7 @@ The following APIs are shared by Slider and Range.
| dots | boolean | `false` | When the `step` value is greater than 1, you can set the `dots` to `true` if you want to render the slider with dots. |
| onBeforeChange | Function | NOOP | `onBeforeChange` will be triggered when `ontouchstart` or `onmousedown` is triggered. |
| onChange | Function | NOOP | `onChange` will be triggered while the value of Slider changing. |
| onAfterChange | Function | NOOP | `onAfterChange` will be triggered when `ontouchend` or `onmouseup` is triggered. |
| onChangeComplete | Function | NOOP | `onChangeComplete` will be triggered when `ontouchend` or `onmouseup` is triggered. |
| minimumTrackStyle | Object | | please use `trackStyle` instead. (`only used for slider, just for compatibility , will be deprecate at rc-slider@9.x `) |
| maximumTrackStyle | Object | | please use `railStyle` instead (`only used for slider, just for compatibility , will be deprecate at rc-slider@9.x`) |
| handleStyle | Array[Object] \| Object | `[{}]` | The style used for handle. (`both for slider(`Object`) and range(`Array of Object`), the array will be used for multi handle following element order`) |
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/debug.tsx
Expand Up @@ -52,7 +52,7 @@ export default () => {
console.log('Change:', nextValues);
// setValue(nextValues as any);
}}
onAfterChange={(v) => {
onChangeComplete={(v) => {
console.log('AfterChange:', v);
}}
// value={value}
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/marks.tsx
Expand Up @@ -25,7 +25,7 @@ export default () => (
<div>
<div style={style}>
<p>Slider with marks, `step=null`</p>
<Slider min={-10} marks={marks} step={null} onChange={log} defaultValue={20} onAfterChange={(v) => console.log('AfterChange:', v)} />
<Slider min={-10} marks={marks} step={null} onChange={log} defaultValue={20} onChangeComplete={(v) => console.log('AfterChange:', v)} />
</div>

<div style={style}>
Expand Down
2 changes: 1 addition & 1 deletion docs/examples/range.tsx
Expand Up @@ -194,7 +194,7 @@ export default () => (
</div>
<div style={style}>
<p>Basic Range,`step=20, dots` </p>
<Slider range dots step={20} defaultValue={[20, 40]} onAfterChange={log} />
<Slider range dots step={20} defaultValue={[20, 40]} onChangeComplete={log} />
</div>
<div style={style}>
<p>Basic Range,disabled</p>
Expand Down
8 changes: 4 additions & 4 deletions docs/examples/slider.tsx
Expand Up @@ -46,7 +46,7 @@ class NullableSlider extends React.Component<any, any> {
<Slider
value={this.state.value}
onChange={this.onSliderChange}
onAfterChange={this.onAfterChange}
onChangeComplete={this.onAfterChange}
/>
<button type="button" onClick={this.reset}>
Reset
Expand Down Expand Up @@ -93,7 +93,7 @@ class CustomizedSlider extends React.Component<any, any> {
<Slider
value={this.state.value}
onChange={this.onSliderChange}
onAfterChange={this.onAfterChange}
onChangeComplete={this.onAfterChange}
/>
);
}
Expand Down Expand Up @@ -199,7 +199,7 @@ export default () => (
</div>
<div style={style}>
<p>Basic Slider,`step=20, dots`</p>
<Slider dots step={20} defaultValue={100} onAfterChange={log} />
<Slider dots step={20} defaultValue={100} onChangeComplete={log} />
</div>
<div style={style}>
<p>
Expand All @@ -210,7 +210,7 @@ export default () => (
dots
step={20}
defaultValue={100}
onAfterChange={log}
onChangeComplete={log}
dotStyle={{ borderColor: 'orange' }}
activeDotStyle={{ borderColor: 'yellow' }}
/>
Expand Down
6 changes: 5 additions & 1 deletion src/Slider.tsx
Expand Up @@ -61,8 +61,9 @@ export interface SliderProps<ValueType = number | number[]> {
onChange?: (value: ValueType) => void;
/** @deprecated It's always better to use `onChange` instead */
onBeforeChange?: (value: ValueType) => void;
/** @deprecated It's always better to use `onChange` instead */
/** @deprecated Use `onChangeComplete` instead */
onAfterChange?: (value: ValueType) => void;
onChangeComplete?: (value: ValueType) => void;

// Cross
allowCross?: boolean;
Expand Down Expand Up @@ -131,6 +132,7 @@ const Slider = React.forwardRef((props: SliderProps, ref: React.Ref<SliderRef>)
onChange,
onBeforeChange,
onAfterChange,
onChangeComplete,

// Cross
allowCross = true,
Expand Down Expand Up @@ -290,6 +292,8 @@ const Slider = React.forwardRef((props: SliderProps, ref: React.Ref<SliderRef>)

const finishChange = () => {
onAfterChange?.(getTriggerValue(rawValuesRef.current));
warning(!onAfterChange, '[rc-slider] `onAfterChange` is deprecated. Please use `onChangeComplete` instead.');
onChangeComplete?.(getTriggerValue(rawValuesRef.current));
};

const [draggingIndex, draggingValue, cacheValues, onStartDrag] = useDrag(
Expand Down
4 changes: 2 additions & 2 deletions tests/Range.test.js
Expand Up @@ -110,7 +110,7 @@ describe('Range', () => {
it('it should trigger onAfterChange when key pressed', () => {
const onAfterChange = jest.fn();
const { container } = render(
<Slider range defaultValue={[20, 50]} onAfterChange={onAfterChange} />,
<Slider range defaultValue={[20, 50]} onChangeComplete={onAfterChange} />,
);

fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[1], {
Expand All @@ -128,7 +128,7 @@ describe('Range', () => {
it('should not change value from keyboard events when disabled', () => {
const onAfterChange = jest.fn();
const { container } = render(
<Slider range keyboard={false} defaultValue={[20, 50]} onAfterChange={onAfterChange} />,
<Slider range keyboard={false} defaultValue={[20, 50]} onChangeComplete={onAfterChange} />,
);

fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[1], {
Expand Down
8 changes: 4 additions & 4 deletions tests/Slider.test.js
Expand Up @@ -138,7 +138,7 @@ describe('Slider', () => {

it('it should trigger onAfterChange when key pressed', () => {
const onAfterChange = jest.fn();
const { container } = render(<Slider defaultValue={50} onAfterChange={onAfterChange} />);
const { container } = render(<Slider defaultValue={50} onChangeComplete={onAfterChange} />);

fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[0], {
keyCode: keyCode.RIGHT,
Expand Down Expand Up @@ -189,7 +189,7 @@ describe('Slider', () => {
it('decreases the value when key "left" is pressed', () => {
const onChange = jest.fn();
const onChangeComplete = jest.fn();
const { container } = render(<Slider defaultValue={50} onChange={onChange} onAfterChange={onChangeComplete} />);
const { container } = render(<Slider defaultValue={50} onChange={onChange} onChangeComplete={onChangeComplete} />);

fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[0], {
keyCode: keyCode.LEFT,
Expand Down Expand Up @@ -560,7 +560,7 @@ describe('Slider', () => {
<Slider
onBeforeChange={onBeforeChange}
onChange={onChange}
onAfterChange={onAfterChange}
onChangeComplete={onAfterChange}
/>,
);
fireEvent.mouseDown(container.querySelector('.rc-slider'), {
Expand Down Expand Up @@ -621,7 +621,7 @@ describe('Slider', () => {

it('onAfterChange should return number', () => {
const onAfterChange = jest.fn();
const { container } = render(<Slider onAfterChange={onAfterChange} />);
const { container } = render(<Slider onChangeComplete={onAfterChange} />);
fireEvent.mouseDown(container.querySelector('.rc-slider'), {
clientX: 20,
});
Expand Down
32 changes: 29 additions & 3 deletions tests/common.test.js
Expand Up @@ -296,7 +296,7 @@ describe('Common', () => {
value={0}
marks={marks}
onChange={sliderOnChange}
onAfterChange={sliderOnAfterChange}
onChangeComplete={sliderOnAfterChange}
/>,
);
const sliderHandleWrapper = container.querySelector(`#${labelId}`);
Expand All @@ -311,7 +311,7 @@ describe('Common', () => {

const rangeOnAfterChange = jest.fn();
const { container: container2 } = render(
<Slider range value={[0, 1]} marks={marks} onAfterChange={rangeOnAfterChange} />,
<Slider range value={[0, 1]} marks={marks} onChangeComplete={rangeOnAfterChange} />,
);
const rangeHandleWrapper = container2.querySelector(`#${labelId}`);
fireEvent.click(rangeHandleWrapper);
Expand All @@ -325,7 +325,7 @@ describe('Common', () => {
const sliderOnChange = jest.fn();
const sliderOnAfterChange = jest.fn();
const { container } = render(
<Slider value={0} onChange={sliderOnChange} onAfterChange={sliderOnAfterChange} />,
<Slider value={0} onChange={sliderOnChange} onChangeComplete={sliderOnAfterChange} />,
);

fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[0], {
Expand All @@ -342,6 +342,32 @@ describe('Common', () => {
expect(sliderOnAfterChange).toHaveBeenCalledTimes(1);
});

it('deprecate onAfterChange', () => {
const errSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
const onChangeComplete = jest.fn();
const onAfterChange = jest.fn();
const { container } = render(
<Slider value={0} onChangeComplete={onChangeComplete} onAfterChange={onAfterChange} />,
);

fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[0], {
keyCode: KeyCode.UP,
});

expect(onChangeComplete).not.toHaveBeenCalled();
expect(onAfterChange).not.toHaveBeenCalled();

fireEvent.keyUp(container.getElementsByClassName('rc-slider-handle')[0], {
keyCode: KeyCode.UP,
});
expect(onChangeComplete).toHaveBeenCalledTimes(1);
expect(onAfterChange).toHaveBeenCalledTimes(1);
expect(errSpy).toHaveBeenCalledWith(
'Warning: [rc-slider] `onAfterChange` is deprecated. Please use `onChangeComplete` instead.',
);
errSpy.mockRestore();
})

// Move to antd instead
// it('the tooltip should be attach to the container with the id tooltip', () => {
// const SliderWithTooltip = createSliderWithTooltip(Slider);
Expand Down

1 comment on commit 86a8a7a

@vercel
Copy link

@vercel vercel bot commented on 86a8a7a Nov 30, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.