From f47f34fec39cb3e805bf8e95055a42b772b5cbe5 Mon Sep 17 00:00:00 2001 From: petdud Date: Thu, 23 Jul 2026 21:59:23 +0200 Subject: [PATCH 1/2] fix(react-popover): only close on focus escape when focus was inside the surface The focusin dismiss handler added for trapFocus popovers previously closed the popover on any focus landing outside the surface/trigger, even when focus had never entered the popover. This regressed consumers (e.g. Editor SDK's AutoCorrectCard / CritiqueCard) that intentionally keep focus on an external input via unstable_disableAutoFocus: a programmatic re-focus of that external element dismissed the popover mid-interaction. Now the popover only dismisses on a genuine inside -> outside focus transition, preserving the original refocus bugfix while leaving popovers open when focus was never inside them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3f764f36-5770-4331-9c29-e96b9b4cf08c --- ...-bb8e7207-ae4d-4f2f-b158-35fd8b91a648.json | 7 +++ .../src/components/Popover/Popover.cy.tsx | 25 ++++++++++ .../src/components/Popover/Popover.test.tsx | 49 ++++++++++++++++++- .../src/components/Popover/usePopover.ts | 21 ++++++-- 4 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 change/@fluentui-react-popover-bb8e7207-ae4d-4f2f-b158-35fd8b91a648.json 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..d8da9dad36596 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,31 @@ describe('Popover', () => { cy.get(popoverInteractiveContentSelector).should('be.visible'); }); + it('should not close when focus moves outside but was never inside the popover', () => { + // Opening the popover does not steal focus (unstable_disableAutoFocus), so focus stays on + // the external input. A programmatic re-focus of it must not dismiss the popover. + mount( + <> + + + + + + + + + + , + ); + + cy.get('#outside').focus(); + cy.get(popoverTriggerSelector).click(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + // Re-focus the external element (focus never entered the popover). + cy.get('#outside').focus(); + cy.get(popoverInteractiveContentSelector).should('be.visible'); + }); + 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); From 523c22b8a49f203db1e1607d8a9cf28dfb5abf50 Mon Sep 17 00:00:00 2001 From: petdud Date: Fri, 31 Jul 2026 11:25:06 +0400 Subject: [PATCH 2/2] test(react-popover): add programmatic focus escape repro Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 90f4e743-9619-4c22-8707-20aeda09c96e --- .../src/components/Popover/Popover.cy.tsx | 81 +++++++++++++++---- 1 file changed, 64 insertions(+), 17 deletions(-) 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 d8da9dad36596..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 @@ -538,30 +538,77 @@ describe('Popover', () => { }); it('should not close when focus moves outside but was never inside the popover', () => { - // Opening the popover does not steal focus (unstable_disableAutoFocus), so focus stays on - // the external input. A programmatic re-focus of it must not dismiss the popover. - mount( - <> - - - - - - - - - - , - ); + const ControlledPopover = () => { + const [open, setOpen] = React.useState(false); + + return ( + <> + + + setOpen(data.open)} + trapFocus + unstable_disableAutoFocus + > + + + + + + + + + ); + }; + + mount(); cy.get('#outside').focus(); - cy.get(popoverTriggerSelector).click(); + cy.get('#open-programmatically').click(); cy.get(popoverInteractiveContentSelector).should('be.visible'); - // Re-focus the external element (focus never entered the popover). 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(