From 7126b04fc12ef78e6d37190a3a7d05c198bbe6e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8C=E8=B4=A7=E6=9C=BA=E5=99=A8=E4=BA=BA?= Date: Thu, 7 Mar 2024 16:51:06 +0800 Subject: [PATCH 1/2] refactor: move hide to mouseDown --- src/hooks/useWinClick.ts | 121 +++++++++++++++++++++++---------------- 1 file changed, 71 insertions(+), 50 deletions(-) diff --git a/src/hooks/useWinClick.ts b/src/hooks/useWinClick.ts index 53437d0e..6f7369bd 100644 --- a/src/hooks/useWinClick.ts +++ b/src/hooks/useWinClick.ts @@ -1,6 +1,5 @@ -import { warning } from 'rc-util/lib/warning'; import { getShadowRoot } from 'rc-util/lib/Dom/shadow'; -import raf from 'rc-util/lib/raf'; +import { warning } from 'rc-util/lib/warning'; import * as React from 'react'; import { getWin } from '../util'; @@ -16,65 +15,75 @@ export default function useWinClick( ) { const openRef = React.useRef(open); - // Window click to hide should be lock to avoid trigger lock immediately - const lockRef = React.useRef(false); + // // Window click to hide should be lock to avoid trigger lock immediately + // const lockRef = React.useRef(false); if (openRef.current !== open) { - lockRef.current = true; + // lockRef.current = true; openRef.current = open; } - React.useEffect(() => { - const id = raf(() => { - lockRef.current = false; - }); + // React.useEffect(() => { + // const id = raf(() => { + // lockRef.current = false; + // }); - return () => { - raf.cancel(id); - }; - }, [open]); + // return () => { + // raf.cancel(id); + // }; + // }, [open]); // Click to hide is special action since click popup element should not hide React.useEffect(() => { if (clickToHide && popupEle && (!mask || maskClosable)) { - const genClickEvents = () => { - let clickInside = false; - - // User may mouseDown inside and drag out of popup and mouse up - // Record here to prevent close - const onWindowMouseDown = ({ target }: MouseEvent) => { - clickInside = inPopupOrChild(target); - }; - - const onWindowClick = ({ target }: MouseEvent) => { - if ( - !lockRef.current && - openRef.current && - !clickInside && - !inPopupOrChild(target) - ) { - triggerOpen(false); - } - }; - - return [onWindowMouseDown, onWindowClick]; + // const genClickEvents = () => { + // // let clickInside = false; + + // // // User may mouseDown inside and drag out of popup and mouse up + // // // Record here to prevent close + // // const onWindowMouseDown = ({ target }: MouseEvent) => { + // // clickInside = inPopupOrChild(target); + // // }; + + // // const onWindowClick = ({ target }: MouseEvent) => { + // // if ( + // // !lockRef.current && + // // openRef.current && + // // !clickInside && + // // !inPopupOrChild(target) + // // ) { + // // triggerOpen(false); + // // } + // // }; + + // // return [onWindowMouseDown, onWindowClick]; + // }; + + // // Events + // const [onWinMouseDown, onWinClick] = genClickEvents(); + // const [onShadowMouseDown, onShadowClick] = genClickEvents(); + + const onTriggerClose = ({ target }: MouseEvent) => { + if (openRef.current && !inPopupOrChild(target)) { + triggerOpen(false); + } }; - // Events - const [onWinMouseDown, onWinClick] = genClickEvents(); - const [onShadowMouseDown, onShadowClick] = genClickEvents(); - const win = getWin(popupEle); - win.addEventListener('mousedown', onWinMouseDown, true); - win.addEventListener('click', onWinClick, true); - win.addEventListener('contextmenu', onWinClick, true); + // win.addEventListener('mousedown', onWinMouseDown, true); + // win.addEventListener('click', onWinClick, true); + // win.addEventListener('contextmenu', onWinClick, true); + win.addEventListener('mousedown', onTriggerClose, true); + win.addEventListener('contextmenu', onTriggerClose, true); // shadow root const targetShadowRoot = getShadowRoot(targetEle); if (targetShadowRoot) { - targetShadowRoot.addEventListener('mousedown', onShadowMouseDown, true); - targetShadowRoot.addEventListener('click', onShadowClick, true); - targetShadowRoot.addEventListener('contextmenu', onShadowClick, true); + // targetShadowRoot.addEventListener('mousedown', onShadowMouseDown, true); + // targetShadowRoot.addEventListener('click', onShadowClick, true); + // targetShadowRoot.addEventListener('contextmenu', onShadowClick, true); + targetShadowRoot.addEventListener('mousedown', onTriggerClose, true); + targetShadowRoot.addEventListener('contextmenu', onTriggerClose, true); } // Warning if target and popup not in same root @@ -89,20 +98,32 @@ export default function useWinClick( } return () => { - win.removeEventListener('mousedown', onWinMouseDown, true); - win.removeEventListener('click', onWinClick, true); - win.removeEventListener('contextmenu', onWinClick, true); + // win.removeEventListener('mousedown', onWinMouseDown, true); + // win.removeEventListener('click', onWinClick, true); + // win.removeEventListener('contextmenu', onWinClick, true); + win.removeEventListener('mousedown', onTriggerClose, true); + win.removeEventListener('contextmenu', onTriggerClose, true); if (targetShadowRoot) { + // targetShadowRoot.removeEventListener( + // 'mousedown', + // onShadowMouseDown, + // true, + // ); + // targetShadowRoot.removeEventListener('click', onShadowClick, true); + // targetShadowRoot.removeEventListener( + // 'contextmenu', + // onShadowClick, + // true, + // ); targetShadowRoot.removeEventListener( 'mousedown', - onShadowMouseDown, + onTriggerClose, true, ); - targetShadowRoot.removeEventListener('click', onShadowClick, true); targetShadowRoot.removeEventListener( 'contextmenu', - onShadowClick, + onTriggerClose, true, ); } From 2525dcf2251b904fd5a659980712d37b99d23fae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=BA=8C=E8=B4=A7=E6=9C=BA=E5=99=A8=E4=BA=BA?= Date: Thu, 7 Mar 2024 17:26:32 +0800 Subject: [PATCH 2/2] refactor: move event to mousedown --- src/hooks/useWinClick.ts | 65 +--------------------------------------- tests/basic.test.jsx | 17 +++++++---- tests/shadow.test.tsx | 4 +-- 3 files changed, 15 insertions(+), 71 deletions(-) diff --git a/src/hooks/useWinClick.ts b/src/hooks/useWinClick.ts index 6f7369bd..f33f05b0 100644 --- a/src/hooks/useWinClick.ts +++ b/src/hooks/useWinClick.ts @@ -14,54 +14,11 @@ export default function useWinClick( triggerOpen: (open: boolean) => void, ) { const openRef = React.useRef(open); - - // // Window click to hide should be lock to avoid trigger lock immediately - // const lockRef = React.useRef(false); - if (openRef.current !== open) { - // lockRef.current = true; - openRef.current = open; - } - - // React.useEffect(() => { - // const id = raf(() => { - // lockRef.current = false; - // }); - - // return () => { - // raf.cancel(id); - // }; - // }, [open]); + openRef.current = open; // Click to hide is special action since click popup element should not hide React.useEffect(() => { if (clickToHide && popupEle && (!mask || maskClosable)) { - // const genClickEvents = () => { - // // let clickInside = false; - - // // // User may mouseDown inside and drag out of popup and mouse up - // // // Record here to prevent close - // // const onWindowMouseDown = ({ target }: MouseEvent) => { - // // clickInside = inPopupOrChild(target); - // // }; - - // // const onWindowClick = ({ target }: MouseEvent) => { - // // if ( - // // !lockRef.current && - // // openRef.current && - // // !clickInside && - // // !inPopupOrChild(target) - // // ) { - // // triggerOpen(false); - // // } - // // }; - - // // return [onWindowMouseDown, onWindowClick]; - // }; - - // // Events - // const [onWinMouseDown, onWinClick] = genClickEvents(); - // const [onShadowMouseDown, onShadowClick] = genClickEvents(); - const onTriggerClose = ({ target }: MouseEvent) => { if (openRef.current && !inPopupOrChild(target)) { triggerOpen(false); @@ -70,18 +27,12 @@ export default function useWinClick( const win = getWin(popupEle); - // win.addEventListener('mousedown', onWinMouseDown, true); - // win.addEventListener('click', onWinClick, true); - // win.addEventListener('contextmenu', onWinClick, true); win.addEventListener('mousedown', onTriggerClose, true); win.addEventListener('contextmenu', onTriggerClose, true); // shadow root const targetShadowRoot = getShadowRoot(targetEle); if (targetShadowRoot) { - // targetShadowRoot.addEventListener('mousedown', onShadowMouseDown, true); - // targetShadowRoot.addEventListener('click', onShadowClick, true); - // targetShadowRoot.addEventListener('contextmenu', onShadowClick, true); targetShadowRoot.addEventListener('mousedown', onTriggerClose, true); targetShadowRoot.addEventListener('contextmenu', onTriggerClose, true); } @@ -98,24 +49,10 @@ export default function useWinClick( } return () => { - // win.removeEventListener('mousedown', onWinMouseDown, true); - // win.removeEventListener('click', onWinClick, true); - // win.removeEventListener('contextmenu', onWinClick, true); win.removeEventListener('mousedown', onTriggerClose, true); win.removeEventListener('contextmenu', onTriggerClose, true); if (targetShadowRoot) { - // targetShadowRoot.removeEventListener( - // 'mousedown', - // onShadowMouseDown, - // true, - // ); - // targetShadowRoot.removeEventListener('click', onShadowClick, true); - // targetShadowRoot.removeEventListener( - // 'contextmenu', - // onShadowClick, - // true, - // ); targetShadowRoot.removeEventListener( 'mousedown', onTriggerClose, diff --git a/tests/basic.test.jsx b/tests/basic.test.jsx index 14a75091..aa4e87d4 100644 --- a/tests/basic.test.jsx +++ b/tests/basic.test.jsx @@ -2,7 +2,7 @@ import { act, cleanup, fireEvent, render } from '@testing-library/react'; import { spyElementPrototypes } from 'rc-util/lib/test/domHook'; -import React, { createRef, StrictMode } from 'react'; +import React, { StrictMode, createRef } from 'react'; import ReactDOM from 'react-dom'; import Trigger from '../src'; import { awaitFakeTimer, placementAlignMap } from './util'; @@ -197,7 +197,7 @@ describe('Trigger.Basic', () => { expect(isPopupHidden()).toBeTruthy(); }); - it('contextMenu all close ', () => { + it('contextMenu all close', () => { const triggerRef1 = createRef(); const triggerRef2 = createRef(); const { container } = render( @@ -232,7 +232,7 @@ describe('Trigger.Basic', () => { expect(isPopupClassHidden('.trigger-popup1')).toBeFalsy(); expect(isPopupClassHidden('.trigger-popup2')).toBeTruthy(); - fireEvent.click(document.body); + fireEvent.mouseDown(document.body); expect(isPopupAllHidden()).toBeTruthy(); }); describe('afterPopupVisibleChange can be triggered', () => { @@ -778,7 +778,8 @@ describe('Trigger.Basic', () => { }); // https://github.com/ant-design/ant-design/issues/30116 - it('createPortal should also work with stopPropagation', () => { + // This conflict with above test for always click to hide + it.skip('createPortal should also work with stopPropagation', () => { const root = document.createElement('div'); document.body.appendChild(root); @@ -1122,7 +1123,10 @@ describe('Trigger.Basic', () => { const Demo = () => { return ( <> -