From da17a4234943b424853a0aa20822f0e02d60c660 Mon Sep 17 00:00:00 2001 From: zkt Date: Fri, 9 Jan 2026 09:55:04 +0800 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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(); - }); }); From a808a3cda9352d7a733af52b1767db8b1e375594 Mon Sep 17 00:00:00 2001 From: zkt Date: Mon, 2 Feb 2026 15:55:15 +0800 Subject: [PATCH 5/5] =?UTF-8?q?fix(Dialog):=20=E7=A7=BB=E9=99=A4=20maskClo?= =?UTF-8?q?sable=20=E6=9D=A1=E4=BB=B6=E4=B8=8B=E4=B8=8D=E5=BF=85=E8=A6=81?= =?UTF-8?q?=E7=9A=84=20onClose=20=E5=AD=98=E5=9C=A8=E6=80=A7=E6=A3=80?= =?UTF-8?q?=E6=9F=A5=EF=BC=8C=E7=A1=AE=E4=BF=9D=E6=8B=96=E6=8B=BD=E8=A1=8C?= =?UTF-8?q?=E4=B8=BA=E4=B8=8D=E4=BC=9A=E6=84=8F=E5=A4=96=E8=A7=A6=E5=8F=91?= =?UTF-8?q?=E5=AF=B9=E8=AF=9D=E6=A1=86=E5=85=B3=E9=97=AD=E3=80=82=E5=90=8C?= =?UTF-8?q?=E6=97=B6=E5=B0=86=E7=9B=B8=E5=85=B3=E6=B5=8B=E8=AF=95=E4=BB=8E?= =?UTF-8?q?=E7=8B=AC=E7=AB=8B=E6=96=87=E4=BB=B6=E5=90=88=E5=B9=B6=E5=88=B0?= =?UTF-8?q?=E4=B8=BB=E6=B5=8B=E8=AF=95=E6=96=87=E4=BB=B6=E4=B8=AD=E4=BB=A5?= =?UTF-8?q?=E7=AE=80=E5=8C=96=E7=BB=93=E6=9E=84=E3=80=82?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Dialog/index.tsx | 6 +--- tests/index.spec.tsx | 24 +++++++++++++++ tests/mask-closable.spec.tsx | 60 ------------------------------------ 3 files changed, 25 insertions(+), 65 deletions(-) delete mode 100644 tests/mask-closable.spec.tsx diff --git a/src/Dialog/index.tsx b/src/Dialog/index.tsx index f2237557..d714eaeb 100644 --- a/src/Dialog/index.tsx +++ b/src/Dialog/index.tsx @@ -117,11 +117,7 @@ const Dialog: React.FC = (props) => { let onWrapperClick: (e: React.SyntheticEvent) => void = null; if (maskClosable) { onWrapperClick = (e) => { - if ( - onClose && - wrapperRef.current === e.target && - mouseDownOnMaskRef.current - ) { + if (wrapperRef.current === e.target && mouseDownOnMaskRef.current) { onInternalClose(e); } }; diff --git a/tests/index.spec.tsx b/tests/index.spec.tsx index 337a94a7..0b13cbd0 100644 --- a/tests/index.spec.tsx +++ b/tests/index.spec.tsx @@ -188,6 +188,30 @@ describe('dialog', () => { expect(onClose).not.toHaveBeenCalled(); }); + it('should not close when dragging from content to mask', () => { + const onClose = jest.fn(); + const { getByText } = render( + + Content + + ); + + 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('renderToBody', () => { const container = document.createElement('div'); document.body.appendChild(container); diff --git a/tests/mask-closable.spec.tsx b/tests/mask-closable.spec.tsx deleted file mode 100644 index cb2b720e..00000000 --- a/tests/mask-closable.spec.tsx +++ /dev/null @@ -1,60 +0,0 @@ -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(); - }); -});