From d7cd3da1e9f910846dc55896bbe40f1709144870 Mon Sep 17 00:00:00 2001 From: tringakrasniqi Date: Fri, 21 May 2021 07:31:28 +0200 Subject: [PATCH 1/5] Fix for blocking modal/dialog always having 'alertdialog' role --- .../react/src/components/Modal/Modal.base.tsx | 15 +- .../react/src/components/Modal/Modal.test.tsx | 45 +++ .../react/src/components/Modal/Modal.types.ts | 8 + .../Modal/__snapshots__/Modal.test.tsx.snap | 339 ++++++++++++++++++ 4 files changed, 406 insertions(+), 1 deletion(-) diff --git a/packages/react/src/components/Modal/Modal.base.tsx b/packages/react/src/components/Modal/Modal.base.tsx index 3fd2e09f81d7c..1d82f53525729 100644 --- a/packages/react/src/components/Modal/Modal.base.tsx +++ b/packages/react/src/components/Modal/Modal.base.tsx @@ -101,6 +101,7 @@ export const ModalBase: React.FunctionComponent = React.forwardRef< forceFocusInsideTrap, ignoreExternalFocusing, isBlocking, + isAlert, isClickableOutsideFocusTrap, isDarkOverlay, onDismiss, @@ -228,6 +229,18 @@ export const ModalBase: React.FunctionComponent = React.forwardRef< [keepInBounds, internalState], ); + /** + * Determines the modal role for accessibility + * The isModeless and isBlocking will be ignored only when isAlert is set + */ + const getDialogRole = () => { + if (isAlert || isModeless === false || isBlocking) { + return 'alertdialog'; + } + + return 'dialog'; + }; + const handleModalClose = (): void => { internalState.lastSetCoordinates = ZERO; @@ -446,7 +459,7 @@ export const ModalBase: React.FunctionComponent = React.forwardRef< (isModalOpen && modalResponsiveMode! >= (responsiveMode || ResponsiveMode.small) && ( { }, ); }); + + it('renders AlertDialog role correctly', () => { + // Mock createPortal to capture its component hierarchy in snapshot output. + const ReactDOM = require('react-dom'); + ReactDOM.createPortal = jest.fn(element => { + return element; + }); + + safeCreate( + + Test Content + , + component => { + expect(component!.toJSON()).toMatchSnapshot(); + + ReactDOM.createPortal.mockClear(); + }, + ); + }); + + it('renders AlertDialog role with isModeless, isBlocking props correctly', () => { + // Mock createPortal to capture its component hierarchy in snapshot output. + const ReactDOM = require('react-dom'); + ReactDOM.createPortal = jest.fn(element => { + return element; + }); + + safeCreate( + + Test Content + , + component => { + expect(component!.toJSON()).toMatchSnapshot(); + + ReactDOM.createPortal.mockClear(); + }, + ); + }); }); diff --git a/packages/react/src/components/Modal/Modal.types.ts b/packages/react/src/components/Modal/Modal.types.ts index 15e3f4bd56b73..468b29230ef32 100644 --- a/packages/react/src/components/Modal/Modal.types.ts +++ b/packages/react/src/components/Modal/Modal.types.ts @@ -124,6 +124,14 @@ export interface IModalProps extends React.RefAttributes, IAcces */ isModeless?: boolean; + /** + * Determines whether the dialog will be a Dialog or AlertDialog + * If this is true, then isBlocking and isModeless are ignored + * + * For more information regarding dialogs please see https://w3c.github.io/aria-practices/#alertdialog + */ + isAlert?: boolean; + /** * Optional class name to be added to the root class */ diff --git a/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap b/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap index 5dc4f45a14c1a..526a0dfed716c 100644 --- a/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap +++ b/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap @@ -1,5 +1,344 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Modal renders AlertDialog role correctly 1`] = ` + +
+
+
+
+
+
+
+ Test Content +
+
+
+
+
+
+ +`; + +exports[`Modal renders AlertDialog role with isModeless, isBlocking props correctly 1`] = ` + +
+
+
+
+
+
+ Test Content +
+
+
+
+
+
+ +`; + exports[`Modal renders Draggable Modal correctly 1`] = ` Date: Mon, 24 May 2021 12:28:51 +0200 Subject: [PATCH 2/5] Change files --- ...luentui-react-ac8491bb-f335-48e2-be2c-458dd83f06d9.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@fluentui-react-ac8491bb-f335-48e2-be2c-458dd83f06d9.json diff --git a/change/@fluentui-react-ac8491bb-f335-48e2-be2c-458dd83f06d9.json b/change/@fluentui-react-ac8491bb-f335-48e2-be2c-458dd83f06d9.json new file mode 100644 index 0000000000000..99936f775fe62 --- /dev/null +++ b/change/@fluentui-react-ac8491bb-f335-48e2-be2c-458dd83f06d9.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Blocking Dialog/Modal always gets assigned role \"alertdialog\" when IsBlocking is true", + "packageName": "@fluentui/react", + "email": "tkrasniqi@microsoft.com", + "dependentChangeType": "patch" +} From 3119de19bbd8daf896f816f5591916d8bb828fcc Mon Sep 17 00:00:00 2001 From: tringakrasniqi Date: Tue, 25 May 2021 18:55:17 +0200 Subject: [PATCH 3/5] Updated react.api --- packages/react/etc/react.api.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/react/etc/react.api.md b/packages/react/etc/react.api.md index 496df1ea8dd6a..940873d0cdaf1 100644 --- a/packages/react/etc/react.api.md +++ b/packages/react/etc/react.api.md @@ -5978,6 +5978,7 @@ export interface IModalProps extends React_2.RefAttributes, IAcc containerClassName?: string; dragOptions?: IDragOptions; enableAriaHiddenSiblings?: boolean; + isAlert?: boolean; isBlocking?: boolean; isDarkOverlay?: boolean; isModeless?: boolean; From ec0c69dd15a0c65fe8b4cba94b02dc4b2b9a43fb Mon Sep 17 00:00:00 2001 From: tringakrasniqi Date: Thu, 27 May 2021 14:50:27 +0200 Subject: [PATCH 4/5] Updated isAlert prop doc Changed the isAlert with default value --- .../react/src/components/Modal/Modal.base.tsx | 15 ++------------- .../react/src/components/Modal/Modal.types.ts | 4 ++-- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/packages/react/src/components/Modal/Modal.base.tsx b/packages/react/src/components/Modal/Modal.base.tsx index 1d82f53525729..9c428368efdf4 100644 --- a/packages/react/src/components/Modal/Modal.base.tsx +++ b/packages/react/src/components/Modal/Modal.base.tsx @@ -151,6 +151,7 @@ export const ModalBase: React.FunctionComponent = React.forwardRef< })); const { keepInBounds } = dragOptions || ({} as IDragOptions); + const isAlertRole = isAlert ?? (isBlocking && !isModeless); const layerClassName = layerProps === undefined ? '' : layerProps.className; const classNames = getClassNames(styles, { @@ -229,18 +230,6 @@ export const ModalBase: React.FunctionComponent = React.forwardRef< [keepInBounds, internalState], ); - /** - * Determines the modal role for accessibility - * The isModeless and isBlocking will be ignored only when isAlert is set - */ - const getDialogRole = () => { - if (isAlert || isModeless === false || isBlocking) { - return 'alertdialog'; - } - - return 'dialog'; - }; - const handleModalClose = (): void => { internalState.lastSetCoordinates = ZERO; @@ -459,7 +448,7 @@ export const ModalBase: React.FunctionComponent = React.forwardRef< (isModalOpen && modalResponsiveMode! >= (responsiveMode || ResponsiveMode.small) && ( , IAcces isModeless?: boolean; /** - * Determines whether the dialog will be a Dialog or AlertDialog - * If this is true, then isBlocking and isModeless are ignored + * Determines the ARIA role of the dialog (alertdialog/dialog) + * If this is set, it will override the ARIA role determined by isBlocking and isModeless * * For more information regarding dialogs please see https://w3c.github.io/aria-practices/#alertdialog */ From 5c6ae454405178692dbd90968215c928a4d7489d Mon Sep 17 00:00:00 2001 From: tringakrasniqi Date: Fri, 28 May 2021 13:16:13 +0200 Subject: [PATCH 5/5] Changed Modal tests and updated snapshots --- .../react/src/components/Modal/Modal.test.tsx | 60 +++- .../Modal/__snapshots__/Modal.test.tsx.snap | 339 ------------------ 2 files changed, 55 insertions(+), 344 deletions(-) diff --git a/packages/react/src/components/Modal/Modal.test.tsx b/packages/react/src/components/Modal/Modal.test.tsx index 09db1755d8e75..ceac53a0402bc 100644 --- a/packages/react/src/components/Modal/Modal.test.tsx +++ b/packages/react/src/components/Modal/Modal.test.tsx @@ -5,6 +5,7 @@ import * as path from 'path'; import { isConformant } from '../../common/isConformant'; import { safeCreate } from '@fluentui/test-utilities'; import { resetIds } from '../../Utilities'; +import { Popup } from '../Popup/Popup'; describe('Modal', () => { beforeEach(() => { @@ -90,7 +91,7 @@ describe('Modal', () => { ); }); - it('renders AlertDialog role correctly', () => { + it('renders a Modal with ARIA role alertDialog when isAlert is true ', () => { // Mock createPortal to capture its component hierarchy in snapshot output. const ReactDOM = require('react-dom'); ReactDOM.createPortal = jest.fn(element => { @@ -102,14 +103,14 @@ describe('Modal', () => { Test Content , component => { - expect(component!.toJSON()).toMatchSnapshot(); - + const componentInstance = component.root; + expect(componentInstance.findByType(Popup).props.role).toBe('alertdialog'); ReactDOM.createPortal.mockClear(); }, ); }); - it('renders AlertDialog role with isModeless, isBlocking props correctly', () => { + it('renders Modal with ARIA role dialog when isModeless and isBlocking are set to true', () => { // Mock createPortal to capture its component hierarchy in snapshot output. const ReactDOM = require('react-dom'); ReactDOM.createPortal = jest.fn(element => { @@ -128,8 +129,57 @@ describe('Modal', () => { Test Content , component => { - expect(component!.toJSON()).toMatchSnapshot(); + const componentInstance = component.root; + expect(componentInstance.findByType(Popup).props.role).toBe('dialog'); + ReactDOM.createPortal.mockClear(); + }, + ); + }); + it('renders Modal with ARIA role dialog when isAlert is false', () => { + // Mock createPortal to capture its component hierarchy in snapshot output. + const ReactDOM = require('react-dom'); + ReactDOM.createPortal = jest.fn(element => { + return element; + }); + + safeCreate( + + Test Content + , + component => { + const componentInstance = component.root; + expect(componentInstance.findByType(Popup).props.role).toBe('dialog'); + ReactDOM.createPortal.mockClear(); + }, + ); + }); + + it('renders Modal with ARIA role alertdialog when isBlocking is true', () => { + // Mock createPortal to capture its component hierarchy in snapshot output. + const ReactDOM = require('react-dom'); + ReactDOM.createPortal = jest.fn(element => { + return element; + }); + + safeCreate( + + Test Content + , + component => { + const componentInstance = component.root; + expect(componentInstance.findByType(Popup).props.role).toBe('alertdialog'); ReactDOM.createPortal.mockClear(); }, ); diff --git a/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap b/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap index 526a0dfed716c..5dc4f45a14c1a 100644 --- a/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap +++ b/packages/react/src/components/Modal/__snapshots__/Modal.test.tsx.snap @@ -1,344 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`Modal renders AlertDialog role correctly 1`] = ` - -
-
-
-
-
-
-
- Test Content -
-
-
-
-
-
- -`; - -exports[`Modal renders AlertDialog role with isModeless, isBlocking props correctly 1`] = ` - -
-
-
-
-
-
- Test Content -
-
-
-
-
-
- -`; - exports[`Modal renders Draggable Modal correctly 1`] = `