From da17a4234943b424853a0aa20822f0e02d60c660 Mon Sep 17 00:00:00 2001 From: zkt Date: Fri, 9 Jan 2026 09:55:04 +0800 Subject: [PATCH 1/4] 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 https://github.com/ant-design/ant-design/issues/56348 --- src/Dialog/index.tsx | 51 ++++++++++---------- tests/index.spec.tsx | 5 +- tests/mask-closable.spec.tsx | 91 ++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 25 deletions(-) create mode 100644 tests/mask-closable.spec.tsx diff --git a/src/Dialog/index.tsx b/src/Dialog/index.tsx index 5e509097..93b966c4 100644 --- a/src/Dialog/index.tsx +++ b/src/Dialog/index.tsx @@ -110,34 +110,43 @@ const Dialog: React.FC = (props) => { } // >>> Content - const contentClickRef = useRef(false); - const contentTimeoutRef = useRef>(null); + const mouseDownOnMaskRef = useRef(false); + const mouseUpOnMaskRef = useRef(false); - // We need record content click in case content popup out of dialog - const onContentMouseDown: React.MouseEventHandler = () => { - clearTimeout(contentTimeoutRef.current); - contentClickRef.current = true; - }; - const onContentMouseUp: React.MouseEventHandler = () => { - contentTimeoutRef.current = setTimeout(() => { - contentClickRef.current = false; - }); - }; // >>> Wrapper // Close only when element not on dialog let onWrapperClick: (e: React.SyntheticEvent) => void = null; if (maskClosable) { onWrapperClick = (e) => { - if (contentClickRef.current) { - contentClickRef.current = false; - } else if (wrapperRef.current === e.target) { + if ( + onClose && + wrapperRef.current === e.target && + mouseDownOnMaskRef.current && + mouseUpOnMaskRef.current + ) { onInternalClose(e); } }; } + function onModulesMouseDownCapture(e: React.MouseEvent) { + if (e.target === wrapperRef.current) { + mouseDownOnMaskRef.current = true; + } else { + mouseDownOnMaskRef.current = false; + } + } + + function onModulesMouseUpCapture(e: React.MouseEvent) { + if (e.target === wrapperRef.current) { + mouseUpOnMaskRef.current = true; + } else { + mouseUpOnMaskRef.current = false; + } + } + // ========================= Effect ========================= useEffect(() => { if (visible) { @@ -158,13 +167,7 @@ const Dialog: React.FC = (props) => { } }, [visible]); - // Remove direct should also check the scroll bar update - useEffect( - () => () => { - clearTimeout(contentTimeoutRef.current); - }, - [], - ); + const mergedStyle: React.CSSProperties = { zIndex, @@ -192,14 +195,14 @@ const Dialog: React.FC = (props) => { className={clsx(`${prefixCls}-wrap`, wrapClassName, modalClassNames?.wrapper)} ref={wrapperRef} onClick={onWrapperClick} + onMouseDownCapture={onModulesMouseDownCapture} + onMouseUpCapture={onModulesMouseUpCapture} style={mergedStyle} {...wrapProps} > { const { rerender } = render(); // Mask close - fireEvent.click(document.querySelector('.rc-dialog-wrap')); + const mask = document.querySelector('.rc-dialog-wrap'); + fireEvent.mouseDown(mask); + fireEvent.mouseUp(mask); + fireEvent.click(mask); jest.runAllTimers(); expect(onClose).toHaveBeenCalled(); onClose.mockReset(); diff --git a/tests/mask-closable.spec.tsx b/tests/mask-closable.spec.tsx new file mode 100644 index 00000000..4fe31b6c --- /dev/null +++ b/tests/mask-closable.spec.tsx @@ -0,0 +1,91 @@ + +import React from 'react'; +import { render, fireEvent, act } from '@testing-library/react'; +import Dialog from '../src'; + +describe('Dialog.MaskClosable', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('should close when click on mask', () => { + const onClose = jest.fn(); + render( + + Content + + ); + + act(() => { + jest.runAllTimers(); + }); + + const mask = document.querySelector('.rc-dialog-wrap'); + if (!mask) throw new Error('Mask not found'); + + fireEvent.mouseDown(mask); + fireEvent.mouseUp(mask); + fireEvent.click(mask); + expect(onClose).toHaveBeenCalled(); + }); + + it('should not close when dragging from content to mask', () => { + const onClose = jest.fn(); + const { getByText } = render( + + Content + + ); + + act(() => { + jest.runAllTimers(); + }); + + const content = getByText('Content'); + const mask = document.querySelector('.rc-dialog-wrap'); + if (!mask) throw new Error('Mask not found'); + + // Simulate mouse down on content + fireEvent.mouseDown(content); + // Simulate mouse up on mask + fireEvent.mouseUp(mask); + // Simulate click on mask (since click follows mouseup) + fireEvent.click(mask); + + expect(onClose).not.toHaveBeenCalled(); + }); + + it('should not close when dragging from mask to content', () => { + const onClose = jest.fn(); + const { getByText } = render( + + Content + + ); + + act(() => { + jest.runAllTimers(); + }); + + const content = getByText('Content'); + const mask = document.querySelector('.rc-dialog-wrap'); + if (!mask) throw new Error('Mask not found'); + + // Simulate mouse down on mask + fireEvent.mouseDown(mask); + // Simulate mouse up on content + fireEvent.mouseUp(content); + // Simulate click on mask (since click follows mouseup) + // Note: In real browser, click event might trigger on the common ancestor or user logic might vary, + // but here we simulate what happens if a click event reaches the wrapper. + // If we drag from mask to content, the click likely happens on content or common parent. + // But if propagation reaches wrapper, we want to ensure it doesn't close. + fireEvent.click(mask); + + expect(onClose).not.toHaveBeenCalled(); + }); +}); From 8cbf1c540441cd81152412056a26162b0234db17 Mon Sep 17 00:00:00 2001 From: zkt Date: Fri, 9 Jan 2026 10:10:28 +0800 Subject: [PATCH 2/4] refactor: optimize logic and reset refs based on review --- src/Dialog/index.tsx | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/Dialog/index.tsx b/src/Dialog/index.tsx index 93b966c4..692cfb8b 100644 --- a/src/Dialog/index.tsx +++ b/src/Dialog/index.tsx @@ -132,24 +132,18 @@ const Dialog: React.FC = (props) => { } function onModulesMouseDownCapture(e: React.MouseEvent) { - if (e.target === wrapperRef.current) { - mouseDownOnMaskRef.current = true; - } else { - mouseDownOnMaskRef.current = false; - } + mouseDownOnMaskRef.current = e.target === wrapperRef.current; } function onModulesMouseUpCapture(e: React.MouseEvent) { - if (e.target === wrapperRef.current) { - mouseUpOnMaskRef.current = true; - } else { - mouseUpOnMaskRef.current = false; - } + mouseUpOnMaskRef.current = e.target === wrapperRef.current; } // ========================= Effect ========================= useEffect(() => { if (visible) { + mouseDownOnMaskRef.current = false; + mouseUpOnMaskRef.current = false; setAnimatedVisible(true); saveLastOutSideActiveElementRef(); From fcb258d9061bd44452591a14b59cab55197edf2d Mon Sep 17 00:00:00 2001 From: zkt Date: Fri, 9 Jan 2026 11:22:11 +0800 Subject: [PATCH 3/4] style: optimize code formatting and naming convention --- src/Dialog/index.tsx | 12 ++++-------- tests/mask-closable.spec.tsx | 1 - 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Dialog/index.tsx b/src/Dialog/index.tsx index 692cfb8b..d33f78db 100644 --- a/src/Dialog/index.tsx +++ b/src/Dialog/index.tsx @@ -113,8 +113,6 @@ const Dialog: React.FC = (props) => { const mouseDownOnMaskRef = useRef(false); const mouseUpOnMaskRef = useRef(false); - - // >>> Wrapper // Close only when element not on dialog let onWrapperClick: (e: React.SyntheticEvent) => void = null; @@ -131,11 +129,11 @@ const Dialog: React.FC = (props) => { }; } - function onModulesMouseDownCapture(e: React.MouseEvent) { + function onWrapperMouseDownCapture(e: React.MouseEvent) { mouseDownOnMaskRef.current = e.target === wrapperRef.current; } - function onModulesMouseUpCapture(e: React.MouseEvent) { + function onWrapperMouseUpCapture(e: React.MouseEvent) { mouseUpOnMaskRef.current = e.target === wrapperRef.current; } @@ -161,8 +159,6 @@ const Dialog: React.FC = (props) => { } }, [visible]); - - const mergedStyle: React.CSSProperties = { zIndex, ...wrapStyle, @@ -189,8 +185,8 @@ const Dialog: React.FC = (props) => { className={clsx(`${prefixCls}-wrap`, wrapClassName, modalClassNames?.wrapper)} ref={wrapperRef} onClick={onWrapperClick} - onMouseDownCapture={onModulesMouseDownCapture} - onMouseUpCapture={onModulesMouseUpCapture} + onMouseDownCapture={onWrapperMouseDownCapture} + onMouseUpCapture={onWrapperMouseUpCapture} style={mergedStyle} {...wrapProps} > diff --git a/tests/mask-closable.spec.tsx b/tests/mask-closable.spec.tsx index 4fe31b6c..4a789dfd 100644 --- a/tests/mask-closable.spec.tsx +++ b/tests/mask-closable.spec.tsx @@ -1,4 +1,3 @@ - import React from 'react'; import { render, fireEvent, act } from '@testing-library/react'; import Dialog from '../src'; From f4c2f1d70a601fbfa31f063eff99eb24b8460ffe Mon Sep 17 00:00:00 2001 From: zkt Date: Wed, 14 Jan 2026 10:58:39 +0800 Subject: [PATCH 4/4] fix: prevent modal close when dragging from content to mask - Use bubbling `onMouseDown` to track if the click originated from the mask. - Only trigger close in `onClick` if both mousedown and click targets are the wrapper. - Reset tracking refs when dialog becomes visible to prevent state pollution. - Remove capture listeners to avoid potential event conflicts. --- src/Dialog/index.tsx | 14 +++----------- tests/mask-closable.spec.tsx | 30 ------------------------------ 2 files changed, 3 insertions(+), 41 deletions(-) diff --git a/src/Dialog/index.tsx b/src/Dialog/index.tsx index d33f78db..f2237557 100644 --- a/src/Dialog/index.tsx +++ b/src/Dialog/index.tsx @@ -111,7 +111,6 @@ const Dialog: React.FC = (props) => { // >>> Content const mouseDownOnMaskRef = useRef(false); - const mouseUpOnMaskRef = useRef(false); // >>> Wrapper // Close only when element not on dialog @@ -121,27 +120,21 @@ const Dialog: React.FC = (props) => { if ( onClose && wrapperRef.current === e.target && - mouseDownOnMaskRef.current && - mouseUpOnMaskRef.current + mouseDownOnMaskRef.current ) { onInternalClose(e); } }; } - function onWrapperMouseDownCapture(e: React.MouseEvent) { + function onWrapperMouseDown(e: React.MouseEvent) { mouseDownOnMaskRef.current = e.target === wrapperRef.current; } - function onWrapperMouseUpCapture(e: React.MouseEvent) { - mouseUpOnMaskRef.current = e.target === wrapperRef.current; - } - // ========================= Effect ========================= useEffect(() => { if (visible) { mouseDownOnMaskRef.current = false; - mouseUpOnMaskRef.current = false; setAnimatedVisible(true); saveLastOutSideActiveElementRef(); @@ -185,8 +178,7 @@ const Dialog: React.FC = (props) => { className={clsx(`${prefixCls}-wrap`, wrapClassName, modalClassNames?.wrapper)} ref={wrapperRef} onClick={onWrapperClick} - onMouseDownCapture={onWrapperMouseDownCapture} - onMouseUpCapture={onWrapperMouseUpCapture} + onMouseDown={onWrapperMouseDown} style={mergedStyle} {...wrapProps} > diff --git a/tests/mask-closable.spec.tsx b/tests/mask-closable.spec.tsx index 4a789dfd..cb2b720e 100644 --- a/tests/mask-closable.spec.tsx +++ b/tests/mask-closable.spec.tsx @@ -57,34 +57,4 @@ describe('Dialog.MaskClosable', () => { expect(onClose).not.toHaveBeenCalled(); }); - - it('should not close when dragging from mask to content', () => { - const onClose = jest.fn(); - const { getByText } = render( - - Content - - ); - - act(() => { - jest.runAllTimers(); - }); - - const content = getByText('Content'); - const mask = document.querySelector('.rc-dialog-wrap'); - if (!mask) throw new Error('Mask not found'); - - // Simulate mouse down on mask - fireEvent.mouseDown(mask); - // Simulate mouse up on content - fireEvent.mouseUp(content); - // Simulate click on mask (since click follows mouseup) - // Note: In real browser, click event might trigger on the common ancestor or user logic might vary, - // but here we simulate what happens if a click event reaches the wrapper. - // If we drag from mask to content, the click likely happens on content or common parent. - // But if propagation reaches wrapper, we want to ensure it doesn't close. - fireEvent.click(mask); - - expect(onClose).not.toHaveBeenCalled(); - }); });