From 4b8779ea00dc998dddaef14318fca5520631f68b Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 25 Jan 2022 09:22:47 +0100 Subject: [PATCH 1/4] fix: Use role=complementary for Popovers without focus traps --- .../PopoverSurface/PopoverSurface.test.tsx | 19 +++++++++++++++++++ .../PopoverSurface.test.tsx.snap | 2 +- .../PopoverSurface/usePopoverSurface.ts | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx b/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx index 1ad7065026070d..d998bd176ed01c 100644 --- a/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx +++ b/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx @@ -5,6 +5,7 @@ import { render, fireEvent } from '@testing-library/react'; import { isConformant } from '../../common/isConformant'; import { Portal } from '@fluentui/react-portal'; import { mockPopoverContext } from '../../common/mockUsePopoverContext'; +import { mount } from 'enzyme'; jest.mock('../../popoverContext'); @@ -70,4 +71,22 @@ describe('PopoverSurface', () => { // Assert expect(getByTestId(testid).getAttribute('aria-modal')).toEqual('true'); }); + + it('should set role dialog if focus trap is active', () => { + // Arrange + mockPopoverContext({ open: true, trapFocus: true }); + const { queryByRole } = render(Content); + + // Assert + expect(queryByRole('dialog')).not.toBeNull(); + }); + + it('should set role complementary if focus trap is not active', () => { + // Arrange + mockPopoverContext({ open: true, trapFocus: false }); + const { getByTestId } = render(Content); + + // Assert + expect(getByTestId(testid)).not.toBeNull(); + }); }); diff --git a/packages/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap b/packages/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap index 30cf4044a07be1..c1bb15e2e6c8eb 100644 --- a/packages/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap +++ b/packages/react-popover/src/components/PopoverSurface/__snapshots__/PopoverSurface.test.tsx.snap @@ -5,7 +5,7 @@ exports[`PopoverSurface renders a default state 1`] = ` class="fui-PopoverSurface" data-tabster="{\\"deloser\\":{},\\"modalizer\\":{\\"id\\":\\"modal-1\\",\\"isOthersAccessible\\":true}}" data-testid="component" - role="dialog" + role="complementary" >
Date: Tue, 25 Jan 2022 09:32:19 +0100 Subject: [PATCH 2/4] Change files --- ...react-popover-ca31294d-f8a3-4d50-bb6d-0d42a16bd4e2.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-popover-ca31294d-f8a3-4d50-bb6d-0d42a16bd4e2.json diff --git a/change/@fluentui-react-popover-ca31294d-f8a3-4d50-bb6d-0d42a16bd4e2.json b/change/@fluentui-react-popover-ca31294d-f8a3-4d50-bb6d-0d42a16bd4e2.json new file mode 100644 index 00000000000000..e4e1eb798040b6 --- /dev/null +++ b/change/@fluentui-react-popover-ca31294d-f8a3-4d50-bb6d-0d42a16bd4e2.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "fix: Use role=complementary for Popovers without focus traps", + "packageName": "@fluentui/react-popover", + "email": "lingfangao@hotmail.com", + "dependentChangeType": "patch" +} From ab05ecb41980b51d39f5e0c09257013a9bbb74fa Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 25 Jan 2022 11:09:14 +0100 Subject: [PATCH 3/4] update e2e selector --- packages/react-popover/e2e/Popover.e2e.ts | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/react-popover/e2e/Popover.e2e.ts b/packages/react-popover/e2e/Popover.e2e.ts index 957d17cd29b00b..ee3ae819b16985 100644 --- a/packages/react-popover/e2e/Popover.e2e.ts +++ b/packages/react-popover/e2e/Popover.e2e.ts @@ -1,5 +1,6 @@ const popoverTriggerSelector = '[aria-haspopup]'; -const popoverContentSelector = '[role="dialog"]'; +const popoverContentSelector = '[role="complementary"]'; +const popoverInteractiveContentSelector = '[role="dialog"]'; const popoverStoriesTitle = 'Components/Popover'; const popoverDefaultStory = 'Default'; @@ -95,21 +96,21 @@ describe('Popover', () => { }); it('should dismiss all nested popovers on outside click', () => { - cy.get('body').click('bottomRight').get(popoverContentSelector).should('not.exist'); + cy.get('body').click('bottomRight').get(popoverInteractiveContentSelector).should('not.exist'); }); it('should not dismiss when clicking on nested content', () => { - cy.contains('Second nested button').click().get(popoverContentSelector).should('have.length', 3); + cy.contains('Second nested button').click().get(popoverInteractiveContentSelector).should('have.length', 3); }); it('should dismiss child popovers when clicking on parents', () => { cy.contains('First nested button') .click() - .get(popoverContentSelector) + .get(popoverInteractiveContentSelector) .should('have.length', 2) .contains('Root button') .click() - .get(popoverContentSelector) + .get(popoverInteractiveContentSelector) .should('have.length', 1); }); @@ -119,22 +120,22 @@ describe('Popover', () => { cy.get(secondNestedTriggerSelector) .eq(1) .click() - .get(popoverContentSelector) + .get(popoverInteractiveContentSelector) .should('have.length', 3) .get(secondNestedTriggerSelector) .eq(0) .click() - .get(popoverContentSelector) + .get(popoverInteractiveContentSelector) .should('have.length', 3); }); it('should dismiss each popover in the stack with Escape keydown', () => { cy.focused().realPress('Escape'); - cy.get(popoverContentSelector).should('have.length', 2); + cy.get(popoverInteractiveContentSelector).should('have.length', 2); cy.focused().realPress('Escape'); - cy.get(popoverContentSelector).should('have.length', 1); + cy.get(popoverInteractiveContentSelector).should('have.length', 1); cy.focused().realPress('Escape'); - cy.get(popoverContentSelector).should('not.exist'); + cy.get(popoverInteractiveContentSelector).should('not.exist'); }); }); From 2450887488f2402b3efe05d7938ef57b557be10f Mon Sep 17 00:00:00 2001 From: Lingfan Gao Date: Tue, 25 Jan 2022 11:51:07 +0100 Subject: [PATCH 4/4] remove unused import --- .../src/components/PopoverSurface/PopoverSurface.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx b/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx index d998bd176ed01c..823c224ff29026 100644 --- a/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx +++ b/packages/react-popover/src/components/PopoverSurface/PopoverSurface.test.tsx @@ -5,7 +5,6 @@ import { render, fireEvent } from '@testing-library/react'; import { isConformant } from '../../common/isConformant'; import { Portal } from '@fluentui/react-portal'; import { mockPopoverContext } from '../../common/mockUsePopoverContext'; -import { mount } from 'enzyme'; jest.mock('../../popoverContext');