diff --git a/change/@fluentui-react-popover-bb8e7207-ae4d-4f2f-b158-35fd8b91a648.json b/change/@fluentui-react-popover-bb8e7207-ae4d-4f2f-b158-35fd8b91a648.json new file mode 100644 index 0000000000000..97e4028dfd33f --- /dev/null +++ b/change/@fluentui-react-popover-bb8e7207-ae4d-4f2f-b158-35fd8b91a648.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: close Popover on focus escape only when focus was inside the surface, so a popover whose focus never entered (e.g. an external input stays focused) is not dismissed when focus moves programmatically", + "packageName": "@fluentui/react-popover", + "email": "petrduda@microsoft.com", + "dependentChangeType": "patch" +} 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 6a0d888e9c597..f3a1a301dfe1a 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 @@ -537,6 +537,78 @@ describe('Popover', () => { cy.get(popoverInteractiveContentSelector).should('be.visible'); }); + it('should not close when focus moves outside but was never inside the popover', () => { + const ControlledPopover = () => { + const [open, setOpen] = React.useState(false); + + return ( + <> + + + setOpen(data.open)} + trapFocus + unstable_disableAutoFocus + > + + + + + + + + + ); + }; + + mount(); + + cy.get('#outside').focus(); + cy.get('#open-programmatically').click(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + cy.get('#outside').focus(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + }); + + it('should close when autofocus is disabled but focus moves from inside to outside', () => { + const ControlledPopover = () => { + const [open, setOpen] = React.useState(false); + + return ( + <> + + + setOpen(data.open)} + trapFocus + unstable_disableAutoFocus + > + + + + + + + + + ); + }; + + mount(); + + cy.get('#open-programmatically').click(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + cy.contains('Inside').focus(); + cy.get('#outside').focus(); + cy.get(popoverInteractiveContentSelector).should('not.exist'); + }); + it('should not close when focus moves to the trigger', () => { mount( 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 b6339053a9f9c..c08868056fe1d 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 @@ -40,7 +40,7 @@ describe('Popover', () => { }); describe('close on focus outside', () => { - it('should close when trapFocus is enabled and focus moves outside', () => { + it('should close when trapFocus is enabled and focus moves from inside to outside', () => { const onOpenChange = jest.fn(); const outsideButton = document.createElement('button'); const popoverContent = document.createElement('div'); @@ -63,6 +63,12 @@ describe('Popover', () => { (result.current.contentRef as React.RefObject).current = popoverContent; }); + // Focus first enters the popover content... + act(() => { + popoverContent.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); + }); + + // ...and is then moved outside, which should dismiss the popover. act(() => { outsideButton.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); }); @@ -73,6 +79,41 @@ describe('Popover', () => { document.body.removeChild(popoverContent); }); + it('should not close when focus moves outside but was never inside the popover', () => { + // Focus stays on an external element the whole time and is programmatically re-focused. + // Since focus never entered the popover, this outside -> outside change must not dismiss it. + const onOpenChange = jest.fn(); + const externalInput = document.createElement('input'); + const popoverContent = document.createElement('div'); + document.body.appendChild(externalInput); + 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; + }); + + // Focus lands outside the popover (and was never inside it). + act(() => { + externalInput.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); + }); + + expect(onOpenChange).not.toHaveBeenCalled(); + + document.body.removeChild(externalInput); + 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'); @@ -119,7 +160,7 @@ describe('Popover', () => { document.body.removeChild(outsideButton); }); - it('should also close when inertTrapFocus is enabled and focus moves to a page element outside', () => { + it('should also close when inertTrapFocus is enabled and focus moves from inside to a page element outside', () => { const onOpenChange = jest.fn(); const outsideButton = document.createElement('button'); const popoverContent = document.createElement('div'); @@ -140,6 +181,10 @@ describe('Popover', () => { (result.current.contentRef as React.RefObject).current = popoverContent; }); + act(() => { + popoverContent.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); + }); + act(() => { outsideButton.dispatchEvent(new FocusEvent('focusin', { bubbles: true })); }); 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 32320d2bdac39..333a3f45d25c7 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 @@ -126,13 +126,16 @@ export const usePopover_unstable = (props: PopoverProps): PopoverState => { 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. + // When trapFocus is enabled, dismiss the popover only on a genuine inside -> outside focus + // transition. Focus that was never inside - e.g. a consumer keeping focus on an external element + // via `unstable_disableAutoFocus` - must not dismiss it on an outside -> outside focus change. // Internal `closeOnFocusOutside` prop allows consumers to opt out during gradual rollout. const closeOnFocusOutside = (props as PopoverProps & { closeOnFocusOutside?: boolean }).closeOnFocusOutside ?? true; + const focusWasInsideRef = React.useRef(false); + const closeOnFocusOutCallback = useEventCallback((ev: FocusEvent) => { - const target = (ev.composedPath()[0] ?? ev.target) as HTMLElement; + const target = (ev.composedPath()[0] ?? ev.target) as HTMLElement | null; const contentElement = positioningRefs.contentRef.current; const triggerElement = positioningRefs.triggerRef.current ?? null; @@ -140,9 +143,16 @@ export const usePopover_unstable = (props: PopoverProps): PopoverState => { return; } - const isOutside = !elementContains(contentElement, target) && !elementContains(triggerElement, target); + const isInside = elementContains(contentElement, target) || elementContains(triggerElement, target); + + if (isInside) { + focusWasInsideRef.current = true; + return; + } - if (isOutside) { + // Only dismiss if focus previously lived inside the popover. + if (focusWasInsideRef.current) { + focusWasInsideRef.current = false; setOpen(ev, false); } }); @@ -152,6 +162,7 @@ export const usePopover_unstable = (props: PopoverProps): PopoverState => { return; } + focusWasInsideRef.current = false; targetDocument?.addEventListener('focusin', closeOnFocusOutCallback, true); return () => { targetDocument?.removeEventListener('focusin', closeOnFocusOutCallback, true);