Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
30af04b
fix(Picker): fix bug the content of the popup window is not consiste…
lllomh Sep 18, 2023
37fc0b8
fix(Picker): Fix the issue that scroll wheel selection list is not su…
lllomh Sep 20, 2023
72c750b
fix(Picker): Rollback of new commits last one
lllomh Sep 20, 2023
f7685da
Merge branch 'main' into main
lllomh Sep 21, 2023
c5b0ba9
Merge branch 'youzan:main' into main
lllomh Sep 27, 2023
3430626
Merge branch 'youzan:main' into main
lllomh Sep 17, 2025
be2e46d
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
c7fe1a6
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
13248f7
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
42d9458
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
5d85c3b
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
8137a85
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
5a48f52
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
d24b1e7
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
e37c8fe
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
91259b9
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
a3774da
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
288cd25
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
b5ee53a
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
06b35df
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
22ceb87
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
a7e2f52
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
bc40733
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
6bbbb14
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
08064d3
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
cb5f579
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 17, 2025
736f8b4
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 18, 2025
8810099
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 18, 2025
6cfc45d
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 18, 2025
b5a0091
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 18, 2025
5ef1075
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 18, 2025
0547793
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 18, 2025
3a39f04
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 18, 2025
ee47f30
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 18, 2025
6953c40
fix(Button): When the step value of the Slider component is relativel…
lllomh Sep 18, 2025
5dd6def
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
ef5c686
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
38ce3e7
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
fd9c90d
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
b7648e0
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
a03d1b3
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
9be4cbd
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
ca1e404
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
37f1d8a
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
aeadef6
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
f9a1d0d
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
f8c6668
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
835676e
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
e996104
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
9282a87
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
728aae9
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
e8c4b50
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
aa4f540
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
46318fe
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
7f7ce3f
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
a5e8351
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
a90cace
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 21, 2025
572efca
Merge branch 'main' into main
lllomh Sep 21, 2025
d0cafc7
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 22, 2025
8de91ac
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 22, 2025
c039ba7
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 22, 2025
ac685ee
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 22, 2025
9dcc149
fix(Slider): When the step value of the Slider component is relativel…
lllomh Sep 22, 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
12 changes: 11 additions & 1 deletion packages/vant/src/slider/Slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,18 @@ export default defineComponent({
const step = +props.step;

value = clamp(value, min, max);

const diff = Math.round((value - min) / step) * step;
return addNumber(min, diff);
const steppedValue = addNumber(min, diff);

if (steppedValue > max) {
const prevSteppedValue = addNumber(min, diff - step);
const distanceToMax = Math.abs(value - max);
const distanceToPrev = Math.abs(value - prevSteppedValue);

return distanceToMax <= distanceToPrev ? max : prevSteppedValue;
}
Comment on lines +136 to +142
Copy link
Collaborator

Choose a reason for hiding this comment

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

Under what circumstances would distanceToMax be greater than distanceToPrev?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

distanceToMax > distanceToPrev only occurs if prevSteppedValue is closer to value than max.

Specific conditions:
value must be between prevSteppedValue and max, and closer to prevSteppedValue.

Almost impossible

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you provide specific examples? Because it seems like the added unit tests don't cover it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the added unit test cases are a bit excessive. In my opinion, it’s sufficient to add test cases for just two scenarios: distanceToMax <= distanceToPrev and distanceToMax > distanceToPrev.

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 provide specific examples? Because it seems like the added unit tests don't cover it.

@inottn Yes, in the description above I explained the problem of test coverage. Could you read it? #13632 (comment)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand. What I mean is that the current PR adds 20 test cases, which seems excessive for a single fix. Refer to #13425 and #13008, for minor fixes, adding one or two test cases is usually sufficient. The changes involved in this fix aren't related to whether the Slider is in range or vertical mode. I think adding just 2 test cases would suffice—one to test the scenario where distanceToMax <= distanceToPrev, and another for distanceToMax > distanceToPrev.

We only need to ensure minimal test cases cover the logic of this fix, rather than increasing the test coverage of the entire component (which should be done in a separate PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand. What I mean is that the current PR adds 20 test cases, which seems excessive for a single fix. Refer to #13425 and #13008, for minor fixes, adding one or two test cases is usually sufficient. The changes involved in this fix aren't related to whether the Slider is in range or vertical mode. I think adding just 2 test cases would suffice—one to test the scenario where distanceToMax <= distanceToPrev, and another for distanceToMax > distanceToPrev.

We only need to ensure minimal test cases cover the logic of this fix, rather than increasing the test coverage of the entire component (which should be done in a separate PR).

@inottn OK, I will modify it as you said. It will take 5 to 10 minutes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand. What I mean is that the current PR adds 20 test cases, which seems excessive for a single fix. Refer to #13425 and #13008, for minor fixes, adding one or two test cases is usually sufficient. The changes involved in this fix aren't related to whether the Slider is in range or vertical mode. I think adding just 2 test cases would suffice—one to test the scenario where distanceToMax <= distanceToPrev, and another for distanceToMax > distanceToPrev.

We only need to ensure minimal test cases cover the logic of this fix, rather than increasing the test coverage of the entire component (which should be done in a separate PR).

@inottn Hi I have modified it. Many test cases I wrote earlier were to improve patch coverage. Now they have been deleted and only 2 are kept, but the patch coverage has been reduced. If you think this is appropriate!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@chenjiahan @inottn Hi, do I need to change something about this?

return steppedValue;
};

const updateStartValue = () => {
Expand Down
28 changes: 28 additions & 0 deletions packages/vant/src/slider/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,3 +374,31 @@ test('should update modelValue correctly after clicking the reversed vertical sl
trigger(wrapper, 'click', 0, 100);
expect(wrapper.emitted('update:modelValue')!.pop()).toEqual([0]);
});

//https://github.com/youzan/vant/issues/13625
test('should return max when distanceToMax <= distanceToPrev', () => {
const wrapper = mount(Slider, {
props: { min: 0, max: 50, step: 60, modelValue: 45 },
});

const emitted = wrapper.emitted('update:modelValue');
if (emitted && emitted.length > 0) {
const result = emitted[emitted.length - 1][0] as number;
expect(result).toBe(50); // Should return max
}
});

test('should enter steppedValue > max branch', () => {
const wrapper = mount(Slider, {
props: { min: 0, max: 12, step: 20, modelValue: 0 },
});

wrapper.setProps({ modelValue: 11 });

const emitted = wrapper.emitted('update:modelValue');
if (emitted && emitted.length > 0) {
const result = emitted[emitted.length - 1][0] as number;
expect(result).toBeGreaterThanOrEqual(0);
expect(result).toBeLessThanOrEqual(12);
}
});