diff --git a/change/@fluentui-react-popover-ea9e0418-25e8-45e1-8dda-ffbe8169164a.json b/change/@fluentui-react-popover-ea9e0418-25e8-45e1-8dda-ffbe8169164a.json new file mode 100644 index 0000000000000..2901078e0ee16 --- /dev/null +++ b/change/@fluentui-react-popover-ea9e0418-25e8-45e1-8dda-ffbe8169164a.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: close Popover when focus escapes outside while trapFocus is enabled", + "packageName": "@fluentui/react-popover", + "email": "petrduda@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-popover/library/etc/react-popover.api.md b/packages/react-components/react-popover/library/etc/react-popover.api.md index 239f1482b6e14..3a82e29fd5a73 100644 --- a/packages/react-components/react-popover/library/etc/react-popover.api.md +++ b/packages/react-components/react-popover/library/etc/react-popover.api.md @@ -33,7 +33,7 @@ export type OnOpenChangeData = { }; // @public -export type OpenPopoverEvents = MouseEvent | TouchEvent | React_2.FocusEvent | React_2.KeyboardEvent | React_2.MouseEvent; +export type OpenPopoverEvents = MouseEvent | TouchEvent | FocusEvent | React_2.FocusEvent | React_2.KeyboardEvent | React_2.MouseEvent; // @public export const Popover: React_2.FC; diff --git a/packages/react-components/react-popover/library/src/components/Popover/Popover.cy.tsx b/packages/react-components/react-popover/library/src/components/Popover/Popover.cy.tsx index 39ee679ee7990..6a0d888e9c597 100644 --- a/packages/react-components/react-popover/library/src/components/Popover/Popover.cy.tsx +++ b/packages/react-components/react-popover/library/src/components/Popover/Popover.cy.tsx @@ -453,6 +453,108 @@ describe('Popover', () => { cy.contains('Two').should('have.focus'); }); }); + + describe('close on focus escape', () => { + it('should close when focus is programmatically moved outside', () => { + mount( + <> + + + + + + + + + + , + ); + + cy.get(popoverTriggerSelector).click(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + cy.get('#outside').focus(); + cy.get(popoverInteractiveContentSelector).should('not.exist'); + }); + + it('should close with inertTrapFocus when focus is programmatically moved outside', () => { + mount( + <> + + + + + + + + + + , + ); + + cy.get(popoverTriggerSelector).click(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + cy.get('#outside').focus(); + cy.get(popoverInteractiveContentSelector).should('not.exist'); + }); + + it('should not close without trapFocus when focus moves outside', () => { + mount( + <> + + + + + + This is a popover + + , + ); + + cy.get(popoverTriggerSelector).click(); + cy.get(popoverContentSelector).should('be.visible'); + cy.get('#outside').focus(); + cy.get(popoverContentSelector).should('be.visible'); + }); + + it('should not close when closeOnFocusOutside is false', () => { + mount( + <> + + + + + + + + + + , + ); + + cy.get(popoverTriggerSelector).click(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + cy.get('#outside').focus(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + }); + + it('should not close when focus moves to the trigger', () => { + mount( + + + + + + + + , + ); + + cy.get(popoverTriggerSelector).click(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + cy.get(popoverTriggerSelector).focus(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + }); + }); }); describe('with Iframe', () => { diff --git a/packages/react-components/react-popover/library/src/components/Popover/Popover.test.tsx b/packages/react-components/react-popover/library/src/components/Popover/Popover.test.tsx index 7a5c20e00b508..b6339053a9f9c 100644 --- a/packages/react-components/react-popover/library/src/components/Popover/Popover.test.tsx +++ b/packages/react-components/react-popover/library/src/components/Popover/Popover.test.tsx @@ -1,8 +1,9 @@ import * as React from 'react'; import { Popover } from './Popover'; -import { renderHook } from '@testing-library/react-hooks'; +import { renderHook, act } from '@testing-library/react-hooks'; import { usePopover_unstable } from './usePopover'; import { isConformant } from '../../testing/isConformant'; +import type { PopoverProps } from './Popover.types'; describe('Popover', () => { isConformant({ @@ -37,4 +38,182 @@ describe('Popover', () => { // Assert expect(result.current.withArrow).toBe(false); }); + + describe('close on focus outside', () => { + it('should close when trapFocus is enabled and focus moves outside', () => { + const onOpenChange = jest.fn(); + const outsideButton = document.createElement('button'); + const popoverContent = document.createElement('div'); + document.body.appendChild(outsideButton); + document.body.appendChild(popoverContent); + + const { result } = renderHook( + ({ open }) => + usePopover_unstable({ + open, + trapFocus: true, + onOpenChange, + children:
, + }), + { initialProps: { open: true } }, + ); + + // Set the contentRef to simulate mounted popover content + act(() => { + (result.current.contentRef as React.RefObject).current = popoverContent; + }); + + act(() => { + outsideButton.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); + }); + + expect(onOpenChange).toHaveBeenCalledWith(expect.anything(), { open: false }); + + document.body.removeChild(outsideButton); + document.body.removeChild(popoverContent); + }); + + it('should not close when trapFocus is not enabled and focus moves outside', () => { + const onOpenChange = jest.fn(); + const outsideButton = document.createElement('button'); + document.body.appendChild(outsideButton); + + renderHook(() => + usePopover_unstable({ + open: true, + trapFocus: false, + onOpenChange, + children:
, + }), + ); + + act(() => { + outsideButton.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); + }); + + expect(onOpenChange).not.toHaveBeenCalled(); + + document.body.removeChild(outsideButton); + }); + + it('should not close when popover is not open', () => { + const onOpenChange = jest.fn(); + const outsideButton = document.createElement('button'); + document.body.appendChild(outsideButton); + + renderHook(() => + usePopover_unstable({ + open: false, + trapFocus: true, + onOpenChange, + children:
, + }), + ); + + act(() => { + outsideButton.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); + }); + + expect(onOpenChange).not.toHaveBeenCalled(); + + document.body.removeChild(outsideButton); + }); + + it('should also close when inertTrapFocus is enabled and focus moves to a page element outside', () => { + const onOpenChange = jest.fn(); + const outsideButton = document.createElement('button'); + const popoverContent = document.createElement('div'); + document.body.appendChild(outsideButton); + document.body.appendChild(popoverContent); + + const { result } = renderHook(() => + usePopover_unstable({ + open: true, + trapFocus: true, + inertTrapFocus: true, + onOpenChange, + children:
, + }), + ); + + act(() => { + (result.current.contentRef as React.RefObject).current = popoverContent; + }); + + act(() => { + outsideButton.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); + }); + + expect(onOpenChange).toHaveBeenCalledWith(expect.anything(), { open: false }); + + document.body.removeChild(outsideButton); + document.body.removeChild(popoverContent); + }); + + it('should not close when closeOnFocusOutside is false', () => { + const onOpenChange = jest.fn(); + const outsideButton = document.createElement('button'); + const popoverContent = document.createElement('div'); + document.body.appendChild(outsideButton); + document.body.appendChild(popoverContent); + + const { result } = renderHook( + ({ open }) => + usePopover_unstable({ + open, + trapFocus: true, + closeOnFocusOutside: false, + onOpenChange, + children:
, + } as unknown as PopoverProps), + { initialProps: { open: true } }, + ); + + act(() => { + (result.current.contentRef as React.RefObject).current = popoverContent; + }); + + act(() => { + outsideButton.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); + }); + + expect(onOpenChange).not.toHaveBeenCalled(); + + document.body.removeChild(outsideButton); + document.body.removeChild(popoverContent); + }); + + it('should not close when focus moves to the trigger element', () => { + const onOpenChange = jest.fn(); + const triggerButton = document.createElement('button'); + const popoverContent = document.createElement('div'); + document.body.appendChild(triggerButton); + document.body.appendChild(popoverContent); + + const { result } = renderHook( + ({ open }) => + usePopover_unstable({ + open, + trapFocus: true, + onOpenChange, + children:
, + }), + { initialProps: { open: true } }, + ); + + act(() => { + (result.current.contentRef as React.RefObject).current = popoverContent; + (result.current.triggerRef as React.RefObject).current = triggerButton; + }); + + act(() => { + triggerButton.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); + }); + + expect(onOpenChange).not.toHaveBeenCalled(); + + document.body.removeChild(triggerButton); + document.body.removeChild(popoverContent); + }); + }); }); diff --git a/packages/react-components/react-popover/library/src/components/Popover/Popover.types.ts b/packages/react-components/react-popover/library/src/components/Popover/Popover.types.ts index 251eff842d04d..cac9e90d6d36e 100644 --- a/packages/react-components/react-popover/library/src/components/Popover/Popover.types.ts +++ b/packages/react-components/react-popover/library/src/components/Popover/Popover.types.ts @@ -236,6 +236,7 @@ export type OnOpenChangeData = { open: boolean }; export type OpenPopoverEvents = | MouseEvent | TouchEvent + | FocusEvent | React.FocusEvent | React.KeyboardEvent | React.MouseEvent; diff --git a/packages/react-components/react-popover/library/src/components/Popover/usePopover.ts b/packages/react-components/react-popover/library/src/components/Popover/usePopover.ts index 265425dbad100..a19e11fbc1848 100644 --- a/packages/react-components/react-popover/library/src/components/Popover/usePopover.ts +++ b/packages/react-components/react-popover/library/src/components/Popover/usePopover.ts @@ -168,6 +168,39 @@ export const usePopoverBase_unstable = (props: PopoverBaseProps): PopoverBaseSta disabled: !open || !closeOnScroll, }); + // When trapFocus is enabled, close the popover if focus is programmatically moved outside + // (e.g. via element.focus()), which doesn't trigger click or scroll dismiss handlers. + // Internal `closeOnFocusOutside` prop allows consumers to opt out during gradual rollout. + const closeOnFocusOutside = + (props as PopoverBaseProps & { closeOnFocusOutside?: boolean }).closeOnFocusOutside ?? true; + + const closeOnFocusOutCallback = useEventCallback((ev: FocusEvent) => { + const target = (ev.composedPath()[0] ?? ev.target) as HTMLElement; + const contentElement = positioningRefs.contentRef.current; + const triggerElement = positioningRefs.triggerRef.current ?? null; + + if (!contentElement) { + return; + } + + const isOutside = !elementContains(contentElement, target) && !elementContains(triggerElement, target); + + if (isOutside) { + setOpen(ev, false); + } + }); + + React.useEffect(() => { + if (!open || !props.trapFocus || !closeOnFocusOutside) { + return; + } + + targetDocument?.addEventListener('focusin', closeOnFocusOutCallback, true); + return () => { + targetDocument?.removeEventListener('focusin', closeOnFocusOutCallback, true); + }; + }, [open, props.trapFocus, closeOnFocusOutside, targetDocument, closeOnFocusOutCallback]); + const { findFirstFocusable } = useFocusFinders(); const activateModal = useActivateModal();