From 1431f73d48b30053c90c3273f62a76e16dbceb09 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 8 Sep 2023 13:36:01 +0200 Subject: [PATCH 1/3] fix: don't apply modalizer attributes when focus trap isn't configured Follow up from #28613. Not applying any of the `useModalAttributes()` resulted in focus restoration not working for popovers that don't trap focus. Now the modalizer attributes are handled from the `useModalAttributes()` hook. This way the popover will still apply the focus restoration attributes if there is no focus trap configured. --- .../src/components/Popover/Popover.cy.tsx | 27 +++++++++++++++++++ .../PopoverSurface/usePopoverSurface.ts | 2 +- .../src/hooks/useModalAttributes.ts | 14 +++++----- 3 files changed, 36 insertions(+), 7 deletions(-) diff --git a/packages/react-components/react-popover/src/components/Popover/Popover.cy.tsx b/packages/react-components/react-popover/src/components/Popover/Popover.cy.tsx index 4069d2bc9e2a7..4095204f8ba51 100644 --- a/packages/react-components/react-popover/src/components/Popover/Popover.cy.tsx +++ b/packages/react-components/react-popover/src/components/Popover/Popover.cy.tsx @@ -541,4 +541,31 @@ describe('Popover', () => { .should('have.length', 2); }); }); + + describe('Without trapFocus', () => { + it('should restore focus on close', () => { + mount( + + + + + + + + , + ); + + cy.get('#trigger') + .click() + .get(popoverContentSelector) + .should('exist') + .get('#button') + .focus() + .type('{esc}') + .get(popoverContentSelector) + .should('not.exist') + .get('#trigger') + .should('have.focus'); + }); + }); }); diff --git a/packages/react-components/react-popover/src/components/PopoverSurface/usePopoverSurface.ts b/packages/react-components/react-popover/src/components/PopoverSurface/usePopoverSurface.ts index ac0731e22d114..690121577c0f8 100644 --- a/packages/react-components/react-popover/src/components/PopoverSurface/usePopoverSurface.ts +++ b/packages/react-components/react-popover/src/components/PopoverSurface/usePopoverSurface.ts @@ -49,7 +49,7 @@ export const usePopoverSurface_unstable = ( ref: useMergedRefs(ref, contentRef), role: trapFocus ? 'dialog' : 'group', 'aria-modal': trapFocus ? true : undefined, - ...(trapFocus ? modalAttributes : {}), + ...modalAttributes, ...props, }), { elementType: 'div' }, diff --git a/packages/react-components/react-tabster/src/hooks/useModalAttributes.ts b/packages/react-components/react-tabster/src/hooks/useModalAttributes.ts index c4734b164d516..fc8cb2ec518b8 100644 --- a/packages/react-components/react-tabster/src/hooks/useModalAttributes.ts +++ b/packages/react-components/react-tabster/src/hooks/useModalAttributes.ts @@ -52,12 +52,14 @@ export const useModalAttributes = ( const id = useId('modal-', options.id); const modalAttributes = useTabsterAttributes({ restorer: { type: TabsterTypes.RestorerTypes.Source }, - modalizer: { - id, - isOthersAccessible: !trapFocus, - isAlwaysAccessible: alwaysFocusable, - isTrapped: legacyTrapFocus && trapFocus, - }, + ...(trapFocus && { + modalizer: { + id, + isOthersAccessible: !trapFocus, + isAlwaysAccessible: alwaysFocusable, + isTrapped: legacyTrapFocus && trapFocus, + }, + }), }); const triggerAttributes = useTabsterAttributes({ From 4198a734f4299000305474821f8c3b3a43332ccb Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 8 Sep 2023 13:39:12 +0200 Subject: [PATCH 2/3] changefile --- ...react-popover-a88a6d1a-e63c-4c03-8d58-35b1065c4396.json | 7 +++++++ ...react-tabster-74dae66a-2abe-42d2-87f7-515c21ea4a3e.json | 7 +++++++ 2 files changed, 14 insertions(+) create mode 100644 change/@fluentui-react-popover-a88a6d1a-e63c-4c03-8d58-35b1065c4396.json create mode 100644 change/@fluentui-react-tabster-74dae66a-2abe-42d2-87f7-515c21ea4a3e.json diff --git a/change/@fluentui-react-popover-a88a6d1a-e63c-4c03-8d58-35b1065c4396.json b/change/@fluentui-react-popover-a88a6d1a-e63c-4c03-8d58-35b1065c4396.json new file mode 100644 index 0000000000000..beff494b64b0b --- /dev/null +++ b/change/@fluentui-react-popover-a88a6d1a-e63c-4c03-8d58-35b1065c4396.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: don't apply modalizer attributes when focus trap isn't configured", + "packageName": "@fluentui/react-popover", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-tabster-74dae66a-2abe-42d2-87f7-515c21ea4a3e.json b/change/@fluentui-react-tabster-74dae66a-2abe-42d2-87f7-515c21ea4a3e.json new file mode 100644 index 0000000000000..37d9c875a9f9e --- /dev/null +++ b/change/@fluentui-react-tabster-74dae66a-2abe-42d2-87f7-515c21ea4a3e.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: don't apply modalizer attributes when focus trap isn't configured", + "packageName": "@fluentui/react-tabster", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} From 2ac101224cc6d815c0b967a2a4cc82990a906640 Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Fri, 8 Sep 2023 15:47:31 +0200 Subject: [PATCH 3/3] update snapshot --- .../PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react-components/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap b/packages/react-components/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap index 3bb7e10437395..d1661f0f1f593 100644 --- a/packages/react-components/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap +++ b/packages/react-components/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap @@ -4,6 +4,7 @@ exports[`PopoverSurface renders a default state 1`] = `