Skip to content

Commit da17a42

Browse files
zktzkt
authored andcommitted
fix(Dialog): prevent mask close when dragging from content to mask
- Use event capture to track `mousedown` and `mouseup` on the wrapper - Ensure close is only triggered when the click action fully occurs on the mask - Fix issue where dragging from content to mask (e.g. slider or text selection) triggers unintended close Closes ant-design/ant-design#56348
1 parent 10b7c44 commit da17a42

File tree

3 files changed

+122
-25
lines changed

3 files changed

+122
-25
lines changed

src/Dialog/index.tsx

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -110,34 +110,43 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
110110
}
111111

112112
// >>> Content
113-
const contentClickRef = useRef(false);
114-
const contentTimeoutRef = useRef<ReturnType<typeof setTimeout>>(null);
113+
const mouseDownOnMaskRef = useRef(false);
114+
const mouseUpOnMaskRef = useRef(false);
115115

116-
// We need record content click in case content popup out of dialog
117-
const onContentMouseDown: React.MouseEventHandler = () => {
118-
clearTimeout(contentTimeoutRef.current);
119-
contentClickRef.current = true;
120-
};
121116

122-
const onContentMouseUp: React.MouseEventHandler = () => {
123-
contentTimeoutRef.current = setTimeout(() => {
124-
contentClickRef.current = false;
125-
});
126-
};
127117

128118
// >>> Wrapper
129119
// Close only when element not on dialog
130120
let onWrapperClick: (e: React.SyntheticEvent) => void = null;
131121
if (maskClosable) {
132122
onWrapperClick = (e) => {
133-
if (contentClickRef.current) {
134-
contentClickRef.current = false;
135-
} else if (wrapperRef.current === e.target) {
123+
if (
124+
onClose &&
125+
wrapperRef.current === e.target &&
126+
mouseDownOnMaskRef.current &&
127+
mouseUpOnMaskRef.current
128+
) {
136129
onInternalClose(e);
137130
}
138131
};
139132
}
140133

134+
function onModulesMouseDownCapture(e: React.MouseEvent) {
135+
if (e.target === wrapperRef.current) {
136+
mouseDownOnMaskRef.current = true;
137+
} else {
138+
mouseDownOnMaskRef.current = false;
139+
}
140+
}
141+
142+
function onModulesMouseUpCapture(e: React.MouseEvent) {
143+
if (e.target === wrapperRef.current) {
144+
mouseUpOnMaskRef.current = true;
145+
} else {
146+
mouseUpOnMaskRef.current = false;
147+
}
148+
}
149+
141150
// ========================= Effect =========================
142151
useEffect(() => {
143152
if (visible) {
@@ -158,13 +167,7 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
158167
}
159168
}, [visible]);
160169

161-
// Remove direct should also check the scroll bar update
162-
useEffect(
163-
() => () => {
164-
clearTimeout(contentTimeoutRef.current);
165-
},
166-
[],
167-
);
170+
168171

169172
const mergedStyle: React.CSSProperties = {
170173
zIndex,
@@ -192,14 +195,14 @@ const Dialog: React.FC<IDialogPropTypes> = (props) => {
192195
className={clsx(`${prefixCls}-wrap`, wrapClassName, modalClassNames?.wrapper)}
193196
ref={wrapperRef}
194197
onClick={onWrapperClick}
198+
onMouseDownCapture={onModulesMouseDownCapture}
199+
onMouseUpCapture={onModulesMouseUpCapture}
195200
style={mergedStyle}
196201
{...wrapProps}
197202
>
198203
<Content
199204
{...props}
200205
isFixedPos={isFixedPos}
201-
onMouseDown={onContentMouseDown}
202-
onMouseUp={onContentMouseUp}
203206
ref={contentRef}
204207
closable={closable}
205208
ariaId={ariaId}

tests/index.spec.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,10 @@ describe('dialog', () => {
173173
const { rerender } = render(<Dialog onClose={onClose} visible />);
174174

175175
// Mask close
176-
fireEvent.click(document.querySelector('.rc-dialog-wrap'));
176+
const mask = document.querySelector('.rc-dialog-wrap');
177+
fireEvent.mouseDown(mask);
178+
fireEvent.mouseUp(mask);
179+
fireEvent.click(mask);
177180
jest.runAllTimers();
178181
expect(onClose).toHaveBeenCalled();
179182
onClose.mockReset();

tests/mask-closable.spec.tsx

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
2+
import React from 'react';
3+
import { render, fireEvent, act } from '@testing-library/react';
4+
import Dialog from '../src';
5+
6+
describe('Dialog.MaskClosable', () => {
7+
beforeEach(() => {
8+
jest.useFakeTimers();
9+
});
10+
11+
afterEach(() => {
12+
jest.useRealTimers();
13+
});
14+
15+
it('should close when click on mask', () => {
16+
const onClose = jest.fn();
17+
render(
18+
<Dialog visible maskClosable onClose={onClose}>
19+
Content
20+
</Dialog>
21+
);
22+
23+
act(() => {
24+
jest.runAllTimers();
25+
});
26+
27+
const mask = document.querySelector('.rc-dialog-wrap');
28+
if (!mask) throw new Error('Mask not found');
29+
30+
fireEvent.mouseDown(mask);
31+
fireEvent.mouseUp(mask);
32+
fireEvent.click(mask);
33+
expect(onClose).toHaveBeenCalled();
34+
});
35+
36+
it('should not close when dragging from content to mask', () => {
37+
const onClose = jest.fn();
38+
const { getByText } = render(
39+
<Dialog visible maskClosable onClose={onClose}>
40+
Content
41+
</Dialog>
42+
);
43+
44+
act(() => {
45+
jest.runAllTimers();
46+
});
47+
48+
const content = getByText('Content');
49+
const mask = document.querySelector('.rc-dialog-wrap');
50+
if (!mask) throw new Error('Mask not found');
51+
52+
// Simulate mouse down on content
53+
fireEvent.mouseDown(content);
54+
// Simulate mouse up on mask
55+
fireEvent.mouseUp(mask);
56+
// Simulate click on mask (since click follows mouseup)
57+
fireEvent.click(mask);
58+
59+
expect(onClose).not.toHaveBeenCalled();
60+
});
61+
62+
it('should not close when dragging from mask to content', () => {
63+
const onClose = jest.fn();
64+
const { getByText } = render(
65+
<Dialog visible maskClosable onClose={onClose}>
66+
Content
67+
</Dialog>
68+
);
69+
70+
act(() => {
71+
jest.runAllTimers();
72+
});
73+
74+
const content = getByText('Content');
75+
const mask = document.querySelector('.rc-dialog-wrap');
76+
if (!mask) throw new Error('Mask not found');
77+
78+
// Simulate mouse down on mask
79+
fireEvent.mouseDown(mask);
80+
// Simulate mouse up on content
81+
fireEvent.mouseUp(content);
82+
// Simulate click on mask (since click follows mouseup)
83+
// Note: In real browser, click event might trigger on the common ancestor or user logic might vary,
84+
// but here we simulate what happens if a click event reaches the wrapper.
85+
// If we drag from mask to content, the click likely happens on content or common parent.
86+
// But if propagation reaches wrapper, we want to ensure it doesn't close.
87+
fireEvent.click(mask);
88+
89+
expect(onClose).not.toHaveBeenCalled();
90+
});
91+
});

0 commit comments

Comments
 (0)