Skip to content

feat: add handle index on change events #1062

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.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions src/Slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ export interface SliderProps<ValueType = number | number[]> {
step?: number | null;
value?: ValueType;
defaultValue?: ValueType;
onChange?: (value: ValueType) => void;
onChange?: (value: ValueType, index: number) => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

更新了 onChange 回调函数签名,添加了 index 参数

SliderProps 接口中,onChange 回调函数签名现在包含了一个额外的 index 参数,用于表示正在操作的滑块句柄的索引。这个改进使开发者能够根据特定句柄的索引执行计算或逻辑。

请注意,这是一个破坏性变更,可能需要更新现有的使用此组件的代码。

建议在文档中明确说明这个变更,并更新示例代码以展示如何利用这个新参数。例如,在 docs/examples/multiple.tsxdocs/examples/editable.tsx 中的示例可以更新为:

-onChange={(nextValue) => {
+onChange={(nextValue, handleIndex) => {
+  console.log('Changed handle index:', handleIndex);
   // 其他逻辑...
}}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents (early access)
In src/Slider.tsx at line 75, the onChange callback signature in the SliderProps
interface has been updated to include an additional index parameter representing
the handle index. To fix this, update all existing usages of the onChange prop
to accept and handle the new index argument accordingly. Additionally, update
the component documentation and example files such as docs/examples/multiple.tsx
and docs/examples/editable.tsx to reflect this signature change and demonstrate
how to use the index parameter in the callback.

/** @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;
onChangeComplete?: (value: ValueType, index: number) => void;
Comment on lines +75 to +80
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

建议:考虑更新现有示例代码

从相关代码片段看,现有的示例代码(如 docs/examples/multiple.tsxdocs/examples/editable.tsx)还没有利用新增的 index 参数。为了帮助用户理解这个新功能,建议更新这些示例,展示如何使用句柄索引。


🏁 Script executed:

#!/bin/bash
# 检查当前是否有使用新 index 参数的示例代码

# 在 docs 目录下搜索 onChange 或 onChangeComplete 的使用
echo "检查示例代码中的回调函数使用情况:"
rg -A 2 "onChange(.*\).* =>\s*{|onChangeComplete(.*\).* =>\s*{" docs/

Length of output: 462


🏁 Script executed:

#!/bin/bash
# 重新检查 docs/examples 目录中是否有使用新 index 参数的回调
echo "查找 docs/examples 目录中 onChange 和 onChangeComplete 的调用:"
rg "onChange\s*=" -n docs/examples
rg "onChangeComplete\s*=" -n docs/examples

Length of output: 5236


更新示例以演示 index 参数

现有示例(如 docs/examples/multiple.tsxdocs/examples/editable.tsx 等)在调用 onChangeonChangeComplete 时仅使用了单个参数。为了帮助用户理解新加入的第二个 index 回调参数,建议:

  • 在相关示例中将回调签名由
    (value) => { … }
    改为
    (value, index) => { … }
  • 并在示例逻辑中展示如何根据 index 区分不同滑块句柄或进行不同处理。

请按需更新以下文件:

  • docs/examples/multiple.tsx
  • docs/examples/editable.tsx
  • (可选)其他使用 onChange/onChangeComplete 的示例文件
🤖 Prompt for AI Agents (early access)
In src/Slider.tsx around lines 75 to 80, the onChange and onChangeComplete
callbacks now include a second index parameter that is not demonstrated in the
example files. Update the example files docs/examples/multiple.tsx and
docs/examples/editable.tsx to change the callback signatures from using a single
value parameter to using both value and index parameters. Modify the example
logic to show how to handle different slider handles based on the index, helping
users understand and utilize the new index argument effectively.


// Cross
allowCross?: boolean;
Expand Down Expand Up @@ -300,7 +300,7 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop

// Trigger event if needed
if (onChange && !isEqual(cloneNextValues, rawValues, true)) {
onChange(getTriggerValue(cloneNextValues));
onChange(getTriggerValue(cloneNextValues), draggingIndex);
}

// We set this later since it will re-render component immediately
Expand All @@ -319,7 +319,7 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
!onAfterChange,
'[rc-slider] `onAfterChange` is deprecated. Please use `onChangeComplete` instead.',
);
onChangeComplete?.(finishValue);
onChangeComplete?.(finishValue, draggingIndex);
});

const onDelete = (index: number) => {
Expand Down Expand Up @@ -406,7 +406,7 @@ const Slider = React.forwardRef<SliderRef, SliderProps<number | number[]>>((prop
!onAfterChange,
'[rc-slider] `onAfterChange` is deprecated. Please use `onChangeComplete` instead.',
);
onChangeComplete?.(nextValue);
onChangeComplete?.(nextValue, draggingIndex);
}
}
};
Expand Down
46 changes: 23 additions & 23 deletions tests/Range.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ describe('Range', () => {
fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[0], {
keyCode: keyCode.UP,
});
expect(onChange).toHaveBeenCalledWith([30, 40]);
expect(onChange).toHaveBeenCalledWith([30, 40], -1);

onChange.mockReset();
fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[0], {
Expand All @@ -270,15 +270,15 @@ describe('Range', () => {
fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[1], {
keyCode: keyCode.UP,
});
expect(onChange).toHaveBeenCalledWith([30, 41]);
expect(onChange).toHaveBeenCalledWith([30, 41], -1);

// Push to the edge
for (let i = 0; i < 99; i += 1) {
fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[1], {
keyCode: keyCode.DOWN,
});
}
expect(onChange).toHaveBeenCalledWith([30, 40]);
expect(onChange).toHaveBeenCalledWith([30, 40], -1);

onChange.mockReset();
fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[1], {
Expand All @@ -299,31 +299,31 @@ describe('Range', () => {
keyCode: keyCode.UP,
});
}
expect(onChange).toHaveBeenCalledWith([80, 90, 100]);
expect(onChange).toHaveBeenCalledWith([80, 90, 100], -1);

// Center to Left
for (let i = 0; i < 99; i += 1) {
fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[1], {
keyCode: keyCode.DOWN,
});
}
expect(onChange).toHaveBeenCalledWith([0, 10, 100]);
expect(onChange).toHaveBeenCalledWith([0, 10, 100], -1);

// Right to Right
for (let i = 0; i < 99; i += 1) {
fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[2], {
keyCode: keyCode.DOWN,
});
}
expect(onChange).toHaveBeenCalledWith([0, 10, 20]);
expect(onChange).toHaveBeenCalledWith([0, 10, 20], -1);

// Center to Right
for (let i = 0; i < 99; i += 1) {
fireEvent.keyDown(container.getElementsByClassName('rc-slider-handle')[1], {
keyCode: keyCode.UP,
});
}
expect(onChange).toHaveBeenCalledWith([0, 90, 100]);
expect(onChange).toHaveBeenCalledWith([0, 90, 100], -1);
});

describe('should render correctly when allowCross', () => {
Expand All @@ -337,7 +337,7 @@ describe('Range', () => {
// Do move
func(container);

expect(onChange).toHaveBeenCalledWith([40, 100]);
expect(onChange).toHaveBeenCalledWith([40, 100], 0);

unmount();
});
Expand All @@ -355,7 +355,7 @@ describe('Range', () => {
// Do move
doMouseMove(container, 0, -10);

expect(onChange).toHaveBeenCalledWith([30, 40]);
expect(onChange).toHaveBeenCalledWith([30, 40], 0);
});

it('vertical', () => {
Expand All @@ -367,7 +367,7 @@ describe('Range', () => {
// Do move
doMouseMove(container, 0, -10);

expect(onChange).toHaveBeenCalledWith([30, 40]);
expect(onChange).toHaveBeenCalledWith([30, 40], 0);
});

it('vertical & reverse', () => {
Expand All @@ -379,7 +379,7 @@ describe('Range', () => {
// Do move
doMouseMove(container, 0, -10);

expect(onChange).toHaveBeenCalledWith([10, 40]);
expect(onChange).toHaveBeenCalledWith([10, 40], 0);
});
});

Expand Down Expand Up @@ -433,7 +433,7 @@ describe('Range', () => {
// Do move
func(container);

expect(onChange).toHaveBeenCalledWith([20, 50]);
expect(onChange).toHaveBeenCalledWith([20, 50], -1);

unmount();
});
Expand Down Expand Up @@ -663,7 +663,7 @@ describe('Range', () => {

doMouseDown(container, 50, 'rc-slider', true);

expect(onChange).toHaveBeenCalledWith([0, 50, 100]);
expect(onChange).toHaveBeenCalledWith([0, 50, 100], -1);
});

it('can not editable with draggableTrack at same time', () => {
Expand Down Expand Up @@ -692,13 +692,13 @@ describe('Range', () => {
);

doMouseMove(container, 0, 1000);
expect(onChange).toHaveBeenCalledWith([50, 100]);
expect(onChange).toHaveBeenCalledWith([50, 100], 0);

expect(container.querySelectorAll('.rc-slider-track')).toHaveLength(1);

// Fire mouse up
fireEvent.mouseUp(container.querySelector('.rc-slider-handle'));
expect(onChangeComplete).toHaveBeenCalledWith([50, 100]);
expect(onChangeComplete).toHaveBeenCalledWith([50, 100], 0);
});

it('out and back', () => {
Expand All @@ -716,14 +716,14 @@ describe('Range', () => {
);

doMouseMove(container, 0, 1000);
expect(onChange).toHaveBeenCalledWith([50]);
expect(onChange).toHaveBeenCalledWith([50], 0);

doMouseDrag(0);
expect(onChange).toHaveBeenCalledWith([0, 50]);
expect(onChange).toHaveBeenCalledWith([0, 50], 0);

// Fire mouse up
fireEvent.mouseUp(container.querySelector('.rc-slider-handle'));
expect(onChangeComplete).toHaveBeenCalledWith([0, 50]);
expect(onChangeComplete).toHaveBeenCalledWith([0, 50], 0);
});

it('controlled', () => {
Expand Down Expand Up @@ -754,7 +754,7 @@ describe('Range', () => {

// Fire mouse up
fireEvent.mouseUp(container.querySelector('.rc-slider-handle'));
expect(onChangeComplete).toHaveBeenCalledWith([50, 100]);
expect(onChangeComplete).toHaveBeenCalledWith([50, 100], 0);
});
});

Expand All @@ -780,7 +780,7 @@ describe('Range', () => {
keyCode: keyCode.DELETE,
});

expect(onChange).toHaveBeenCalledWith([0, 100]);
expect(onChange).toHaveBeenCalledWith([0, 100], -1);

// Clear all
fireEvent.keyDown(container.querySelector('.rc-slider-handle'), {
Expand All @@ -789,7 +789,7 @@ describe('Range', () => {
fireEvent.keyDown(container.querySelector('.rc-slider-handle'), {
keyCode: keyCode.DELETE,
});
expect(onChange).toHaveBeenCalledWith([]);
expect(onChange).toHaveBeenCalledWith([], -1);

// 2 handle
expect(container.querySelectorAll('.rc-slider-handle')).toHaveLength(0);
Expand Down Expand Up @@ -820,7 +820,7 @@ describe('Range', () => {

// Mouse
doMouseMove(container, 0, 1000);
expect(onChange).toHaveBeenCalledWith([100]);
expect(onChange).toHaveBeenCalledWith([100], 0);
});

it('maxCount not add', () => {
Expand All @@ -836,7 +836,7 @@ describe('Range', () => {
);

doMouseDown(container, 50, 'rc-slider', true);
expect(onChange).toHaveBeenCalledWith([0, 50]);
expect(onChange).toHaveBeenCalledWith([0, 50], -1);
});
});
});
Loading
Loading