Skip to content

fix: finish drag when mouseup propagation stops #1089

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

Open
nrps9909 wants to merge 4 commits into
base: master
Choose a base branch
from
+114 −17
Open

fix: finish drag when mouseup propagation stops #1089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5ceeade
fix: finish drag when mouseup propagation stops
nrps9909 Aug 26, 2026
ba0d927
Merge branch 'master' into codex/capture-drag-end
yoyo837 Aug 27, 2026
88b4752
fix: preserve mouseup default behavior
nrps9909 Aug 27, 2026
a536a73
test: cover touch drag completion
nrps9909 Aug 27, 2026
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: 6 additions & 4 deletions src/hooks/useDrag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function useDrag(
document.removeEventListener('mousemove', mouseMoveEventRef.current);
}
if (mouseUpEventRef.current) {
document.removeEventListener('mouseup', mouseUpEventRef.current);
document.removeEventListener('mouseup', mouseUpEventRef.current, true);
}
if (touchEventTargetRef.current) {
if (mouseMoveEventRef.current) {
Expand Down Expand Up @@ -211,9 +211,11 @@ function useDrag(

// End
const onMouseUp: EventListener = (event) => {
event.preventDefault();
if (event.type === 'touchend') {
event.preventDefault();
}

document.removeEventListener('mouseup', onMouseUp);
document.removeEventListener('mouseup', onMouseUp, true);
document.removeEventListener('mousemove', onMouseMove);
if (touchEventTargetRef.current) {
if (mouseMoveEventRef.current) {
Expand All @@ -233,7 +235,7 @@ function useDrag(
setDraggingDelete(false);
};

document.addEventListener('mouseup', onMouseUp);
document.addEventListener('mouseup', onMouseUp, true);
document.addEventListener('mousemove', onMouseMove);
e.currentTarget.addEventListener('touchend', onMouseUp);
e.currentTarget.addEventListener('touchmove', onMouseMove);
Expand Down
92 changes: 79 additions & 13 deletions tests/Range.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,28 @@ describe('Range', () => {
fireEvent(container.getElementsByClassName(element)[0], touchMove);
}

it('prevents the native default action when a touch drag ends', () => {
const onChangeComplete = jest.fn();
const { container } = render(
<Slider range defaultValue={[20, 40]} onChangeComplete={onChangeComplete} />,
);
const handle = container.getElementsByClassName('rc-slider-handle')[0];
const touchStart = createEvent.touchStart(handle, {
touches: [{}],
targetTouches: [{}],
});
(touchStart as any).targetTouches[0].pageX = 20;
fireEvent(handle, touchStart);

const touchEnd = createEvent.touchEnd(handle);
const preventDefault = jest.fn();
Object.defineProperty(touchEnd, 'preventDefault', { value: preventDefault });
fireEvent(handle, touchEnd);

expect(preventDefault).toHaveBeenCalledTimes(1);
expect(onChangeComplete).toHaveBeenCalledWith([20, 40]);
});

it('should render Range with correct DOM structure', () => {
const { asFragment } = render(<Slider range />);
expect(asFragment().firstChild).toMatchSnapshot();
Expand Down Expand Up @@ -140,9 +162,7 @@ describe('Range', () => {
});

it('should render Range without tabIndex (equal null) correctly', () => {
const { container } = render(
<Slider range tabIndex={[null, null] as any} />,
);
const { container } = render(<Slider range tabIndex={[null, null] as any} />);
expect(container.getElementsByClassName('rc-slider-handle')[0]).not.toHaveAttribute('tabIndex');
expect(container.getElementsByClassName('rc-slider-handle')[1]).not.toHaveAttribute('tabIndex');
});
Expand Down Expand Up @@ -872,7 +892,12 @@ describe('Range', () => {
it('respects handle disabled state and boolean disabled fallback', () => {
const onChange = jest.fn();
const { container, rerender } = render(
<Slider range defaultValue={[0, 50, 100]} disabled={[true, false, true]} onChange={onChange} />,
<Slider
range
defaultValue={[0, 50, 100]}
disabled={[true, false, true]}
onChange={onChange}
/>,
);

const disabledHandle = getHandle(container, 0);
Expand Down Expand Up @@ -909,17 +934,23 @@ describe('Range', () => {
expect(onChange).toHaveBeenCalledWith([0, 10, 100]);

onChange.mockClear();
rerender(<Slider range value={[20, 50, 80]} disabled={[true, false, false]} onChange={onChange} />);
rerender(
<Slider range value={[20, 50, 80]} disabled={[true, false, false]} onChange={onChange} />,
);
doMouseDown(container, 10, 'rc-slider', true);
fireEvent.mouseUp(document);
expect(onChange).not.toHaveBeenCalled();

rerender(<Slider range value={[20, 50, 80]} disabled={[false, false, true]} onChange={onChange} />);
rerender(
<Slider range value={[20, 50, 80]} disabled={[false, false, true]} onChange={onChange} />,
);
doMouseDown(container, 90, 'rc-slider', true);
fireEvent.mouseUp(document);
expect(onChange).not.toHaveBeenCalled();

rerender(<Slider range value={[0, 50, 100]} disabled={[true, true, true]} onChange={onChange} />);
rerender(
<Slider range value={[0, 50, 100]} disabled={[true, true, true]} onChange={onChange} />,
);
doMouseDown(container, 10, 'rc-slider', true);
fireEvent.mouseUp(document);
expect(onChange).not.toHaveBeenCalled();
Expand Down Expand Up @@ -963,7 +994,12 @@ describe('Range', () => {
it('disables draggableTrack only when rendered handles are disabled', () => {
const onChange = jest.fn();
const { container, unmount } = render(
<Slider range={{ draggableTrack: true }} defaultValue={[0, 50]} disabled={[false, true]} onChange={onChange} />,
<Slider
range={{ draggableTrack: true }}
defaultValue={[0, 50]}
disabled={[false, true]}
onChange={onChange}
/>,
);

const track = container.getElementsByClassName('rc-slider-track')[0];
Expand Down Expand Up @@ -1002,7 +1038,12 @@ describe('Range', () => {
it('keeps keyboard movement inside disabled handle boundaries', () => {
const onChange = jest.fn();
const { container, unmount } = render(
<Slider range defaultValue={[20, 50, 80]} disabled={[false, true, false]} onChange={onChange} />,
<Slider
range
defaultValue={[20, 50, 80]}
disabled={[false, true, false]}
onChange={onChange}
/>,
);

repeatKeyDown(getHandle(container, 0), keyCode.RIGHT, 50);
Expand All @@ -1012,7 +1053,12 @@ describe('Range', () => {
onChange.mockClear();

const { container: boundaryContainer } = render(
<Slider range defaultValue={[20, 50, 80]} disabled={[true, false, true]} onChange={onChange} />,
<Slider
range
defaultValue={[20, 50, 80]}
disabled={[true, false, true]}
onChange={onChange}
/>,
);
const middleHandle = getHandle(boundaryContainer, 1);

Expand All @@ -1029,7 +1075,13 @@ describe('Range', () => {
it('respects pushable boundaries around disabled handles', () => {
const onChange = jest.fn();
const { container } = render(
<Slider range defaultValue={[20, 40, 60, 80]} disabled={[false, true, false, false]} pushable={10} onChange={onChange} />,
<Slider
range
defaultValue={[20, 40, 60, 80]}
disabled={[false, true, false, false]}
pushable={10}
onChange={onChange}
/>,
);

repeatKeyDown(getHandle(container, 0), keyCode.UP, 30);
Expand All @@ -1046,7 +1098,13 @@ describe('Range', () => {
it('respects disabled boundaries with allowCross=false and step=null', () => {
const onChange = jest.fn();
const { container, unmount } = render(
<Slider range defaultValue={[20, 50, 80]} disabled={[false, true, false]} allowCross={false} onChange={onChange} />,
<Slider
range
defaultValue={[20, 50, 80]}
disabled={[false, true, false]}
allowCross={false}
onChange={onChange}
/>,
);

repeatKeyDown(getHandle(container, 0), keyCode.RIGHT, 50);
Expand All @@ -1056,7 +1114,15 @@ describe('Range', () => {
onChange.mockClear();

const { container: stepContainer } = render(
<Slider range defaultValue={[20, 50, 80]} disabled={[false, true, false]} step={null} marks={{ 0: '0', 50: '50', 100: '100' }} pushable={10} onChange={onChange} />,
<Slider
range
defaultValue={[20, 50, 80]}
disabled={[false, true, false]}
step={null}
marks={{ 0: '0', 50: '50', 100: '100' }}
pushable={10}
onChange={onChange}
/>,
);

fireEvent.keyDown(getHandle(stepContainer, 0), { keyCode: keyCode.RIGHT });
Expand Down
29 changes: 29 additions & 0 deletions tests/Slider.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,35 @@ describe('Slider', () => {
expect(onAfterChange).toHaveBeenCalledWith(20);
});

it('should finish dragging when a parent stops mouseup propagation', () => {
const onChange = jest.fn();
const onChangeComplete = jest.fn();
const onParentMouseUp = jest.fn((event) => event.stopPropagation());
const { container, getByTestId } = render(
<div data-testid="parent" onMouseUp={onParentMouseUp}>
<Slider defaultValue={50} onChange={onChange} onChangeComplete={onChangeComplete} />
</div>,
);
const handle = container.querySelector('.rc-slider-handle');

fireEvent.mouseDown(handle, { pageX: 50 });
const firstMove = createEvent.mouseMove(document);
firstMove.pageX = 60;
fireEvent(document, firstMove);
expect(onChange).toHaveBeenCalled();
const lastDraggedValue = onChange.mock.calls[onChange.mock.calls.length - 1][0];

onChange.mockClear();
fireEvent.mouseUp(getByTestId('parent'));
expect(onParentMouseUp.mock.calls[0][0].defaultPrevented).toBe(false);
expect(onChangeComplete).toHaveBeenCalledWith(lastDraggedValue);

const moveAfterRelease = createEvent.mouseMove(document);
moveAfterRelease.pageX = 70;
fireEvent(document, moveAfterRelease);
expect(onChange).not.toHaveBeenCalled();
});

// https://github.com/react-component/slider/pull/948
it('could drag handler after click tracker', () => {
const onChange = jest.fn();
Expand Down
Loading