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: retire deprecated api #1055

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: retire deprecated api #1055

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42725ff
feat: retire deprecated api
aojunhao123 Jan 17, 2025
e235675
docs: update demo
aojunhao123 Jan 17, 2025
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ yarn.lock
package-lock.json
.doc
.storybook
.vscode

# umi
.umi
Expand Down
62 changes: 11 additions & 51 deletions src/Slider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,12 @@ export interface SliderProps<ValueType = number | number[]> {

// Value
range?: boolean | RangeConfig;
/** @deprecated Use `range.minCount` or `range.maxCount` to handle this */
count?: number;
min?: number;
max?: number;
step?: number | null;
value?: ValueType;
defaultValue?: ValueType;
onChange?: (value: ValueType) => void;
/** @deprecated It's always better to use `onChange` instead */
onBeforeChange?: (value: ValueType) => void;
/** @deprecated Use `onChangeComplete` instead */
onAfterChange?: (value: ValueType) => void;
onChangeComplete?: (value: ValueType) => void;

// Cross
Expand All @@ -90,12 +84,6 @@ export interface SliderProps<ValueType = number | number[]> {
// Style
included?: boolean;
startPoint?: number;
/** @deprecated Please use `styles.track` instead */
trackStyle?: React.CSSProperties | React.CSSProperties[];
/** @deprecated Please use `styles.handle` instead */
handleStyle?: React.CSSProperties | React.CSSProperties[];
/** @deprecated Please use `styles.rail` instead */
railStyle?: React.CSSProperties;
dotStyle?: React.CSSProperties | ((dotValue: number) => React.CSSProperties);
Copy link
Member

Choose a reason for hiding this comment

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

看起来这两个 miss 了，是不是已经有平替了？下面那个也是

activeDotStyle?: React.CSSProperties | ((dotValue: number) => React.CSSProperties);

Expand Down Expand Up @@ -145,10 +133,7 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
value,
defaultValue,
range,
count,
onChange,
onBeforeChange,
onAfterChange,
onChangeComplete,

// Cross
Expand All @@ -162,9 +147,6 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
// Style
included = true,
startPoint,
trackStyle,
handleStyle,
railStyle,
dotStyle,
activeDotStyle,

Expand Down Expand Up @@ -259,8 +241,8 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
mergedValue === null || mergedValue === undefined
? []
: Array.isArray(mergedValue)
? mergedValue
: [mergedValue];
? mergedValue
: [mergedValue];

const [val0 = mergedMin] = valueList;
let returnValues = mergedValue === null ? [] : [val0];
Expand All @@ -269,16 +251,11 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
if (rangeEnabled) {
returnValues = [...valueList];

// When count provided or value is `undefined`, we fill values
if (count || mergedValue === undefined) {
const pointCount = count >= 0 ? count + 1 : 2;
returnValues = returnValues.slice(0, pointCount);

// Fill with count
while (returnValues.length < pointCount) {
returnValues.push(returnValues[returnValues.length - 1] ?? mergedMin);
}
// When value is `undefined`, we fill values with default 2 points
if (mergedValue === undefined) {
returnValues = [mergedMin, mergedMin];
}

returnValues.sort((a, b) => a - b);
}

Expand All @@ -288,7 +265,7 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
});

return returnValues;
}, [mergedValue, rangeEnabled, mergedMin, count, formatValue]);
}, [mergedValue, rangeEnabled, mergedMin, formatValue]);

// =========================== onChange ===========================
const getTriggerValue = (triggerValues: number[]) =>
Expand All @@ -314,11 +291,6 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
}

const finishValue = getTriggerValue(rawValues);
onAfterChange?.(finishValue);
warning(
!onAfterChange,
'[rc-slider] `onAfterChange` is deprecated. Please use `onChangeComplete` instead.',
);
onChangeComplete?.(finishValue);
});

Expand All @@ -330,7 +302,6 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
const cloneNextValues = [...rawValues];
cloneNextValues.splice(index, 1);

onBeforeChange?.(getTriggerValue(cloneNextValues));
triggerChange(cloneNextValues);

const nextFocusIndex = Math.max(0, index - 1);
Expand Down Expand Up @@ -386,26 +357,18 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
cloneNextValues[valueIndex] = newValue;
}

// Fill value to match default 2 (only when `rawValues` is empty)
if (rangeEnabled && !rawValues.length && count === undefined) {
if (rangeEnabled && !rawValues.length) {
cloneNextValues.push(newValue);
}

const nextValue = getTriggerValue(cloneNextValues);
onBeforeChange?.(nextValue);
triggerChange(cloneNextValues);

if (e) {
(document.activeElement as HTMLElement)?.blur?.();
handlesRef.current.focus(focusIndex);
onStartDrag(e, focusIndex, cloneNextValues);
} else {
// https://github.com/ant-design/ant-design/issues/49997
onAfterChange?.(nextValue);
warning(
!onAfterChange,
'[rc-slider] `onAfterChange` is deprecated. Please use `onChangeComplete` instead.',
);
onChangeComplete?.(nextValue);
}
}
Expand Down Expand Up @@ -448,7 +411,6 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
if (!disabled) {
const next = offsetValues(rawValues, offset, valueIndex);

onBeforeChange?.(getTriggerValue(rawValues));
triggerChange(next.values);

setKeyboardValue(next.value);
Expand Down Expand Up @@ -479,8 +441,6 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop

const onStartMove: OnStartMove = useEvent((e, valueIndex) => {
onStartDrag(e, valueIndex);

onBeforeChange?.(getTriggerValue(rawValues));
});

// Auto focus for updated handle
Expand Down Expand Up @@ -587,13 +547,13 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
>
<div
className={cls(`${prefixCls}-rail`, classNames?.rail)}
style={{ ...railStyle, ...styles?.rail }}
style={styles?.rail}
/>

{track !== false && (
<Tracks
prefixCls={prefixCls}
style={trackStyle}
style={styles?.track}
values={rawValues}
startPoint={startPoint}
onStartMove={mergedDraggableTrack ? onStartMove : undefined}
Expand All @@ -611,7 +571,7 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
<Handles
ref={handlesRef}
prefixCls={prefixCls}
style={handleStyle}
style={styles?.handle}
values={cacheValues}
draggingIndex={draggingIndex}
draggingDelete={draggingDelete}
Expand Down
11 changes: 3 additions & 8 deletions tests/Range.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ describe('Range', () => {
expect(asFragment().firstChild).toMatchSnapshot();
});

it('should render Multi-Range with correct DOM structure', () => {
const { asFragment } = render(<Slider range count={3} />);
expect(asFragment().firstChild).toMatchSnapshot();
});

it('should render Range with value correctly', async () => {
const { container } = render(<Slider range value={[0, 50]} />);

Expand Down Expand Up @@ -173,11 +168,11 @@ describe('Range', () => {
keyCode: keyCode.RIGHT,
});

expect(onAfterChange).not.toBeCalled();
expect(onAfterChange).not.toHaveBeenCalled();
});

it('should render Multi-Range with value correctly', () => {
const { container } = render(<Slider range count={3} value={[0, 25, 50, 75]} />);
const { container } = render(<Slider range value={[0, 25, 50, 75]} />);

expect(container.getElementsByClassName('rc-slider-handle')[0]).toHaveStyle('left: 0%');
expect(container.getElementsByClassName('rc-slider-handle')[1]).toHaveStyle('left: 25%');
Expand Down Expand Up @@ -558,7 +553,7 @@ describe('Range', () => {
const handleFocus = jest.fn();
const { container } = render(<Slider range min={0} max={20} onFocus={handleFocus} />);
container.querySelector<HTMLDivElement>('.rc-slider-handle').focus();
expect(handleFocus).toBeCalled();
expect(handleFocus).toHaveBeenCalled();
});

it('blur()', () => {
Expand Down
12 changes: 3 additions & 9 deletions tests/Slider.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -562,28 +562,22 @@ describe('Slider', () => {
expect(onChange).toHaveBeenCalledWith([20, 20]);
});

it('should call onBeforeChange, onChange, and onAfterChange', () => {
const onBeforeChange = jest.fn();
it('should call onChange', () => {
const onChange = jest.fn();
const onAfterChange = jest.fn();
const { container } = render(
<Slider
onBeforeChange={onBeforeChange}
onChange={onChange}
onChangeComplete={onAfterChange}
/>,
);
fireEvent.mouseDown(container.querySelector('.rc-slider'), {
clientX: 20,
});

expect(onBeforeChange).toHaveBeenCalledWith(20);
expect(onChange).toHaveBeenCalledWith(20);
expect(onAfterChange).not.toHaveBeenCalled();
fireEvent.mouseUp(container.querySelector('.rc-slider'), {
clientX: 20,
});
expect(onAfterChange).toHaveBeenCalledWith(20);
expect(onChange).toHaveBeenCalledWith(20);
});
});

Expand Down Expand Up @@ -665,7 +659,7 @@ describe('Slider', () => {

it('tipFormatter should not crash with undefined value', () => {
[undefined, null].forEach((value) => {
render(<Slider value={value} tooltip={{ open: true }} styles={{ tracks: {} }}/>);
render(<Slider value={value} tooltip={{ open: true }} styles={{ tracks: {} }} />);
});
});
});
43 changes: 0 additions & 43 deletions tests/common.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,47 +341,4 @@ describe('Common', () => {
expect(sliderOnAfterChange).toHaveBeenCalled();
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);
// const tooltipPrefixer = {
// prefixCls: 'slider-tooltip',
// };
// const tooltipParent = document.createElement('div');
// tooltipParent.setAttribute('id', 'tooltip');
// const { container } = render(
// <SliderWithTooltip
// tipProps={tooltipPrefixer}
// getTooltipContainer={() => document.getElementById('tooltip')}
// />,
// );
// expect(wrapper.instance().props.getTooltipContainer).toBeTruthy();
// });
});