diff --git a/eslint.config.mjs b/eslint.config.mjs index df92a6bbf23..122bfe7aefc 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -16,7 +16,8 @@ export default [ 'packages/react-core/src/helpers/Popper/thirdparty', 'packages/react-docs/patternfly-docs/generated', 'packages/react-docs/static', - '**/.cache' + '**/.cache', + '.history' ] }, js.configs.recommended, diff --git a/packages/react-core/src/components/Modal/Modal.tsx b/packages/react-core/src/components/Modal/Modal.tsx index e2517578485..43bbdc0a540 100644 --- a/packages/react-core/src/components/Modal/Modal.tsx +++ b/packages/react-core/src/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,24 @@ 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 +221,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 +238,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 +249,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 +263,7 @@ class Modal extends React.Component { position={position} elementToFocus={elementToFocus} />, - 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 dd9cda7ac4f..1a6672b9a95 100644 --- a/packages/react-core/src/components/Modal/ModalContent.tsx +++ b/packages/react-core/src/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 ( - + { ); }; -describe('Modal', () => { - test('Modal creates a container element once for div', () => { - render(); - expect(document.createElement).toHaveBeenCalledWith('div'); - }); +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 closes with escape', async () => { const user = userEvent.setup(); @@ -137,10 +159,10 @@ describe('Modal', () => { expect(asideSibling).not.toHaveAttribute('aria-hidden'); }); - test('modal removes the aria-hidden attribute from its siblings when unmounted', async () => { + test('modal siblings have the aria-hidden attribute when it has adjacent modals', async () => { const user = userEvent.setup(); - render(, { container: document.body.appendChild(target) }); + render(, { container: document.body.appendChild(target) }); const asideSibling = screen.getByRole('complementary', { hidden: true }); const articleSibling = screen.getByRole('article', { hidden: true }); @@ -154,6 +176,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 diff --git a/packages/react-core/src/next/components/Modal/Modal.tsx b/packages/react-core/src/next/components/Modal/Modal.tsx index 380e73b0914..0ed05ab901a 100644 --- a/packages/react-core/src/next/components/Modal/Modal.tsx +++ b/packages/react-core/src/next/components/Modal/Modal.tsx @@ -58,7 +58,6 @@ export enum ModalVariant { } interface ModalState { - container: HTMLElement; ouiaStateId: string; } @@ -66,6 +65,7 @@ class Modal extends React.Component { static displayName = 'Modal'; static currentId = 0; boxId = ''; + backdropId = ''; static defaultProps: PickOptional = { isOpen: false, @@ -78,10 +78,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) }; } @@ -105,7 +106,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'); } } @@ -116,36 +117,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); @@ -153,7 +149,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, @@ -166,15 +161,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( { elementToFocus={elementToFocus} {...props} />, - container + this.getElement(appendTo) ) as React.ReactElement; } } diff --git a/packages/react-core/src/next/components/Modal/ModalContent.tsx b/packages/react-core/src/next/components/Modal/ModalContent.tsx index f96d53b054a..a6d62322f23 100644 --- a/packages/react-core/src/next/components/Modal/ModalContent.tsx +++ b/packages/react-core/src/next/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. */ @@ -60,6 +62,7 @@ export const ModalContent: React.FunctionComponent = ({ width, maxWidth, boxId, + backdropId, disableFocusTrap = false, ouiaId, ouiaSafe = true, @@ -106,7 +109,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'); + }); });