diff --git a/change/@fluentui-react-utilities-62a5aa48-dba4-4ff0-974d-d5ea0a87b2fc.json b/change/@fluentui-react-utilities-62a5aa48-dba4-4ff0-974d-d5ea0a87b2fc.json new file mode 100644 index 0000000000000..b9f36d0543d33 --- /dev/null +++ b/change/@fluentui-react-utilities-62a5aa48-dba4-4ff0-974d-d5ea0a87b2fc.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: `useOnClickOutside` should consider text selection starting inside and finishing outside as an inside click", + "packageName": "@fluentui/react-utilities", + "email": "yuanboxue@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-utilities/src/hooks/useOnClickOutside.cy.tsx b/packages/react-components/react-utilities/src/hooks/useOnClickOutside.cy.tsx index 9316da8866640..88a3c5e9973c7 100644 --- a/packages/react-components/react-utilities/src/hooks/useOnClickOutside.cy.tsx +++ b/packages/react-components/react-utilities/src/hooks/useOnClickOutside.cy.tsx @@ -90,4 +90,21 @@ describe('useOnClickOutside', () => { expect(onOutsideClick).to.be.calledTwice; }); }); + + it('should not call callback with inside text selection finishing outside', () => { + const onOutsideClick = cy.spy(); + + mount(); + + cy.get('#inside-button') + .trigger('mousedown', { which: 1 }) + .trigger('mousemove') + .get('#outside-buttonA') + .trigger('mousemove') + .trigger('mouseup') + .trigger('click') + .then(() => { + expect(onOutsideClick).to.not.be.called; + }); + }); }); diff --git a/packages/react-components/react-utilities/src/hooks/useOnClickOutside.test.ts b/packages/react-components/react-utilities/src/hooks/useOnClickOutside.test.ts index 9c74a17f2a8d4..31ae3cafc5e01 100644 --- a/packages/react-components/react-utilities/src/hooks/useOnClickOutside.test.ts +++ b/packages/react-components/react-utilities/src/hooks/useOnClickOutside.test.ts @@ -6,7 +6,7 @@ describe('useOnClickOutside', () => { jest.useRealTimers(); }); - const supportedEvents = ['click', 'touchstart', 'contextmenu', 'fuiframefocus']; + const supportedEvents = ['click', 'touchstart', 'contextmenu', 'fuiframefocus', 'mousedown']; it.each(supportedEvents)('should add %s listener', event => { // Arrange @@ -16,7 +16,7 @@ describe('useOnClickOutside', () => { renderHook(() => useOnClickOutside({ element, callback: jest.fn(), refs: [] })); // Assert - expect(element.addEventListener).toHaveBeenCalledTimes(4); + expect(element.addEventListener).toHaveBeenCalledTimes(supportedEvents.length); expect(element.addEventListener).toHaveBeenCalledWith(event, expect.anything(), true); }); @@ -29,7 +29,7 @@ describe('useOnClickOutside', () => { unmount(); // Assert - expect(element.removeEventListener).toHaveBeenCalledTimes(4); + expect(element.removeEventListener).toHaveBeenCalledTimes(supportedEvents.length); expect(element.removeEventListener).toHaveBeenCalledWith(event, expect.anything(), true); }); diff --git a/packages/react-components/react-utilities/src/hooks/useOnClickOutside.ts b/packages/react-components/react-utilities/src/hooks/useOnClickOutside.ts index 33c8d3c1ce3fa..e1032f0616230 100644 --- a/packages/react-components/react-utilities/src/hooks/useOnClickOutside.ts +++ b/packages/react-components/react-utilities/src/hooks/useOnClickOutside.ts @@ -42,9 +42,16 @@ export const useOnClickOutside = (options: UseOnClickOrScrollOutsideOptions) => const timeoutId = React.useRef(undefined); useIFrameFocus(options); + const isMouseDownInsideRef = React.useRef(false); + + const contains: UseOnClickOrScrollOutsideOptions['contains'] = + containsProp || ((parent, child) => !!parent?.contains(child)); + const listener = useEventCallback((ev: MouseEvent | TouchEvent) => { - const contains: UseOnClickOrScrollOutsideOptions['contains'] = - containsProp || ((parent, child) => !!parent?.contains(child)); + if (isMouseDownInsideRef.current) { + isMouseDownInsideRef.current = false; + return; + } const target = ev.composedPath()[0] as HTMLElement; const isOutside = refs.every(ref => !contains(ref.current || null, target)); @@ -54,6 +61,13 @@ export const useOnClickOutside = (options: UseOnClickOrScrollOutsideOptions) => } }); + const handleMouseDown = useEventCallback((ev: MouseEvent) => { + // Selecting text from inside to outside will rigger click event. + // In this case click event target is outside but mouse down event target is inside. + // And this click event should be considered as inside click. + isMouseDownInsideRef.current = refs.some(ref => contains(ref.current || null, ev.target as HTMLElement)); + }); + React.useEffect(() => { if (disabled) { return; @@ -78,6 +92,7 @@ export const useOnClickOutside = (options: UseOnClickOrScrollOutsideOptions) => element?.addEventListener('click', conditionalHandler, true); element?.addEventListener('touchstart', conditionalHandler, true); element?.addEventListener('contextmenu', conditionalHandler, true); + element?.addEventListener('mousedown', handleMouseDown, true); // Garbage collect this event after it's no longer useful to avoid memory leaks timeoutId.current = window.setTimeout(() => { @@ -88,11 +103,12 @@ export const useOnClickOutside = (options: UseOnClickOrScrollOutsideOptions) => element?.removeEventListener('click', conditionalHandler, true); element?.removeEventListener('touchstart', conditionalHandler, true); element?.removeEventListener('contextmenu', conditionalHandler, true); + element?.removeEventListener('mousedown', handleMouseDown, true); clearTimeout(timeoutId.current); currentEvent = undefined; }; - }, [listener, element, disabled]); + }, [listener, element, disabled, handleMouseDown]); }; const getWindowEvent = (target: Node | Window): Event | undefined => {