diff --git a/packages/react-core/src/components/Modal/Modal.tsx b/packages/react-core/src/components/Modal/Modal.tsx index 25f6e2e2c68..a4e3f99174f 100644 --- a/packages/react-core/src/components/Modal/Modal.tsx +++ b/packages/react-core/src/components/Modal/Modal.tsx @@ -60,7 +60,6 @@ export enum ModalVariant { } interface ModalState { - container: HTMLElement; ouiaStateId: string; } @@ -68,6 +67,7 @@ class Modal extends React.Component { static displayName = 'Modal'; static currentId = 0; boxId = ''; + backdropId = ''; static defaultProps: PickOptional = { isOpen: false, @@ -80,10 +80,11 @@ class Modal extends React.Component { constructor(props: ModalProps) { super(props); const boxIdNum = Modal.currentId++; + const backdropId = boxIdNum + 1; this.boxId = props.id || `pf-modal-part-${boxIdNum}`; + this.backdropId = `pf-modal-part-${backdropId}`; this.state = { - container: undefined, ouiaStateId: getDefaultOUIAId(Modal.displayName, props.variant) }; } @@ -107,7 +108,7 @@ class Modal extends React.Component { const target: HTMLElement = this.getElement(appendTo); const bodyChildren = target.children; for (const child of Array.from(bodyChildren)) { - if (child !== this.state.container) { + if (child.id !== this.backdropId) { hide ? child.setAttribute('aria-hidden', '' + hide) : child.removeAttribute('aria-hidden'); } } @@ -118,36 +119,31 @@ class Modal extends React.Component { componentDidMount() { const { appendTo } = this.props; const target: HTMLElement = this.getElement(appendTo); - const container = document.createElement('div'); - this.setState({ container }); - target.appendChild(container); target.addEventListener('keydown', this.handleEscKeyClick, false); if (this.props.isOpen) { target.classList.add(css(styles.backdropOpen)); - } else { - target.classList.remove(css(styles.backdropOpen)); + this.toggleSiblingsFromScreenReaders(true); } } - componentDidUpdate() { + componentDidUpdate(prevProps: ModalProps) { const { appendTo } = this.props; const target: HTMLElement = this.getElement(appendTo); if (this.props.isOpen) { target.classList.add(css(styles.backdropOpen)); this.toggleSiblingsFromScreenReaders(true); } else { - target.classList.remove(css(styles.backdropOpen)); - this.toggleSiblingsFromScreenReaders(false); + if (prevProps.isOpen !== this.props.isOpen) { + target.classList.remove(css(styles.backdropOpen)); + this.toggleSiblingsFromScreenReaders(false); + } } } componentWillUnmount() { const { appendTo } = this.props; const target: HTMLElement = this.getElement(appendTo); - if (this.state.container) { - target.removeChild(this.state.container); - } target.removeEventListener('keydown', this.handleEscKeyClick, false); target.classList.remove(css(styles.backdropOpen)); this.toggleSiblingsFromScreenReaders(false); @@ -155,7 +151,6 @@ class Modal extends React.Component { render() { const { - // eslint-disable-next-line @typescript-eslint/no-unused-vars appendTo, // eslint-disable-next-line @typescript-eslint/no-unused-vars onEscapePress, @@ -168,15 +163,15 @@ class Modal extends React.Component { elementToFocus, ...props } = this.props; - const { container } = this.state; - if (!canUseDOM || !container) { + if (!canUseDOM || !this.getElement(appendTo)) { return null; } return ReactDOM.createPortal( { backdropClassName={props.backdropClassName} {...props} />, - container + this.getElement(appendTo) ) as React.ReactElement; } } diff --git a/packages/react-core/src/components/Modal/ModalContent.tsx b/packages/react-core/src/components/Modal/ModalContent.tsx index 761b0eee569..425a165c02b 100644 --- a/packages/react-core/src/components/Modal/ModalContent.tsx +++ b/packages/react-core/src/components/Modal/ModalContent.tsx @@ -16,6 +16,8 @@ export interface ModalContentProps extends OUIAProps { 'aria-labelledby'?: string; /** Id of the modal box container. */ boxId: string; + /** Id of the backdrop. */ + backdropId?: string; /** Content rendered inside the modal. */ children: React.ReactNode; /** Additional classes added to the modal box. */ @@ -63,6 +65,7 @@ export const ModalContent: React.FunctionComponent = ({ width, maxWidth, boxId, + backdropId, disableFocusTrap = false, ouiaId, ouiaSafe = true, @@ -109,7 +112,7 @@ export const ModalContent: React.FunctionComponent = ({ ); return ( - + { ); }; +const ModalWithAdjacentModal = () => { + const [isOpen, setIsOpen] = React.useState(true); + const [isModalMounted, setIsModalMounted] = React.useState(true); + const modalProps = { ...props, isOpen, appendTo: target, onClose: () => setIsOpen(false) }; + + return ( + <> + +
Section sibling
+ {isModalMounted && ( + <> + + + + {}}> + Modal closed for test + + {}}> + modal closed for test + + + )} + + ); +}; + describe('Modal', () => { test('Modal creates a container element once for div', () => { render(); @@ -128,4 +155,22 @@ describe('Modal', () => { expect(asideSibling).not.toHaveAttribute('aria-hidden'); expect(articleSibling).not.toHaveAttribute('aria-hidden'); }); + + test('modal siblings have the aria-hidden attribute when it has adjacent modals', async () => { + const user = userEvent.setup(); + + render(, { container: document.body.appendChild(target) }); + + const asideSibling = screen.getByRole('complementary', { hidden: true }); + const articleSibling = screen.getByRole('article', { hidden: true }); + const unmountButton = screen.getByRole('button', { name: 'Unmount Modal' }); + + expect(asideSibling).toHaveAttribute('aria-hidden'); + expect(articleSibling).toHaveAttribute('aria-hidden'); + + await user.click(unmountButton); + + expect(asideSibling).not.toHaveAttribute('aria-hidden'); + expect(articleSibling).not.toHaveAttribute('aria-hidden'); + }); }); diff --git a/packages/react-core/src/deprecated/components/Modal/Modal.tsx b/packages/react-core/src/deprecated/components/Modal/Modal.tsx index 73923f64539..a2ac8235d10 100644 --- a/packages/react-core/src/deprecated/components/Modal/Modal.tsx +++ b/packages/react-core/src/deprecated/components/Modal/Modal.tsx @@ -92,7 +92,6 @@ export enum ModalVariant { } interface ModalState { - container: HTMLElement; ouiaStateId: string; } @@ -102,6 +101,7 @@ class Modal extends React.Component { boxId = ''; labelId = ''; descriptorId = ''; + backdropId = ''; static defaultProps: PickOptional = { className: '', @@ -128,12 +128,13 @@ class Modal extends React.Component { const boxIdNum = Modal.currentId++; const labelIdNum = boxIdNum + 1; const descriptorIdNum = boxIdNum + 2; + const backdropIdNum = boxIdNum + 3; this.boxId = props.id || `pf-modal-part-${boxIdNum}`; this.labelId = `pf-modal-part-${labelIdNum}`; this.descriptorId = `pf-modal-part-${descriptorIdNum}`; + this.backdropId = `pf-modal-part-${backdropIdNum}`; this.state = { - container: undefined, ouiaStateId: getDefaultOUIAId(Modal.displayName, props.variant) }; } @@ -157,7 +158,7 @@ class Modal extends React.Component { const target: HTMLElement = this.getElement(appendTo); const bodyChildren = target.children; for (const child of Array.from(bodyChildren)) { - if (child !== this.state.container) { + if (child.id !== this.backdropId) { hide ? child.setAttribute('aria-hidden', '' + hide) : child.removeAttribute('aria-hidden'); } } @@ -175,15 +176,11 @@ class Modal extends React.Component { header } = this.props; const target: HTMLElement = this.getElement(appendTo); - const container = document.createElement('div'); - this.setState({ container }); - target.appendChild(container); target.addEventListener('keydown', this.handleEscKeyClick, false); if (this.props.isOpen) { target.classList.add(css(styles.backdropOpen)); - } else { - target.classList.remove(css(styles.backdropOpen)); + this.toggleSiblingsFromScreenReaders(true); } if (!title && this.isEmpty(ariaLabel) && this.isEmpty(ariaLabelledby)) { @@ -199,24 +196,23 @@ class Modal extends React.Component { } } - componentDidUpdate() { + componentDidUpdate(prevProps: ModalProps) { const { appendTo } = this.props; const target: HTMLElement = this.getElement(appendTo); if (this.props.isOpen) { target.classList.add(css(styles.backdropOpen)); this.toggleSiblingsFromScreenReaders(true); } else { - target.classList.remove(css(styles.backdropOpen)); - this.toggleSiblingsFromScreenReaders(false); + if (prevProps.isOpen !== this.props.isOpen) { + target.classList.remove(css(styles.backdropOpen)); + this.toggleSiblingsFromScreenReaders(false); + } } } componentWillUnmount() { const { appendTo } = this.props; const target: HTMLElement = this.getElement(appendTo); - if (this.state.container) { - target.removeChild(this.state.container); - } target.removeEventListener('keydown', this.handleEscKeyClick, false); target.classList.remove(css(styles.backdropOpen)); this.toggleSiblingsFromScreenReaders(false); @@ -224,7 +220,6 @@ class Modal extends React.Component { render() { const { - // eslint-disable-next-line @typescript-eslint/no-unused-vars appendTo, // eslint-disable-next-line @typescript-eslint/no-unused-vars onEscapePress, @@ -242,9 +237,8 @@ class Modal extends React.Component { elementToFocus, ...props } = this.props; - const { container } = this.state; - if (!canUseDOM || !container) { + if (!canUseDOM || !this.getElement(appendTo)) { return null; } @@ -254,6 +248,7 @@ class Modal extends React.Component { boxId={this.boxId} labelId={this.labelId} descriptorId={this.descriptorId} + backdropId={this.backdropId} title={title} titleIconVariant={titleIconVariant} titleLabel={titleLabel} @@ -267,7 +262,7 @@ class Modal extends React.Component { position={position} elementToFocus={elementToFocus} />, - container + this.getElement(appendTo) ) as React.ReactElement; } } diff --git a/packages/react-core/src/deprecated/components/Modal/ModalContent.tsx b/packages/react-core/src/deprecated/components/Modal/ModalContent.tsx index 4f0ddab0bae..6d5fc982c4b 100644 --- a/packages/react-core/src/deprecated/components/Modal/ModalContent.tsx +++ b/packages/react-core/src/deprecated/components/Modal/ModalContent.tsx @@ -25,6 +25,8 @@ export interface ModalContentProps extends OUIAProps { 'aria-label'?: string; /** Id to use for the modal box label. */ 'aria-labelledby'?: string | null; + /** Id of the backdrop. */ + backdropId?: string; /** Accessible label applied to the modal box body. This should be used to communicate * important information about the modal box body div element if needed, such as that it * is scrollable. @@ -117,6 +119,7 @@ export const ModalContent: React.FunctionComponent = ({ maxWidth, boxId, labelId, + backdropId, descriptorId, disableFocusTrap = false, hasNoBodyWrapper = false, @@ -202,7 +205,7 @@ export const ModalContent: React.FunctionComponent = ({ ); return ( - + { ); }; +const ModalWithAdjacentModal = () => { + const [isOpen, setIsOpen] = React.useState(true); + const [isModalMounted, setIsModalMounted] = React.useState(true); + const modalProps = { ...props, isOpen, appendTo: target, onClose: () => setIsOpen(false) }; + + return ( + <> + +
Section sibling
+ {isModalMounted && ( + <> + + + + {}}> + Modal closed for test + + {}}> + modal closed for test + + + )} + + ); +}; + describe('Modal', () => { test('Modal creates a container element once for div', () => { render(); @@ -154,6 +181,7 @@ describe('Modal', () => { expect(asideSibling).not.toHaveAttribute('aria-hidden'); expect(articleSibling).not.toHaveAttribute('aria-hidden'); }); + test('The modalBoxBody has no aria-label when bodyAriaLabel is not passed', () => { const props = { isOpen: true