+
+ setIsOpen(false)}>
+ Inspect the div containing the modal to see the value of all root siblings having no aria-hidden attribute.
+
+ {}}>
+ This modal remains closed for this demo.
+
+ {}}>
+ This modal remains closed for this demo.
+
+
+ );
+ };
+
+ return (
+
+
+ When a modal is open, the root and all siblings should have aria-hidden="true" except for the modal container.
+
+
+
+ When the strict mode modal opens, inspect the DOM to see that the open modal has aria-hidden="true" when it
+ should be "false".
+
+
Also, every time the modal opens a new div is appened and is never removed until the page is refreshed.
+
+
+
+
When the non-strict mode modal opens, inspect the DOM to see the open modal has no aria-hidden attribute.
+
+
+
+
+ Opening a modal while another adjacent modal is closed causes the aria-hidden attribute to be removed from the
+ root and all siblings.
+
+
+
+ );
+};
From 92835cb43e8b7a55acd99ff668f75b83c731cb78 Mon Sep 17 00:00:00 2001
From: Titani
Date: Thu, 7 Nov 2024 16:20:13 -0500
Subject: [PATCH 2/3] update modal next
---
.../src/components/Modal/examples/Modal.md | 2 +-
.../src/next/components/Modal/Modal.tsx | 31 +++----
.../next/components/Modal/ModalContent.tsx | 5 +-
.../components/Modal/__tests__/Modal.test.tsx | 45 +++++++++++
.../next/components/Modal/examples/Modal.md | 8 ++
.../next/components/Modal/examples/Test.tsx | 80 +++++++++++++++++++
6 files changed, 151 insertions(+), 20 deletions(-)
create mode 100644 packages/react-core/src/next/components/Modal/examples/Test.tsx
diff --git a/packages/react-core/src/components/Modal/examples/Modal.md b/packages/react-core/src/components/Modal/examples/Modal.md
index 97c0932d18a..5973facff76 100644
--- a/packages/react-core/src/components/Modal/examples/Modal.md
+++ b/packages/react-core/src/components/Modal/examples/Modal.md
@@ -17,7 +17,7 @@ import formStyles from '@patternfly/react-styles/css/components/Form/form';
### TEST PR
-// this example is for testing the fix testing and will be deleted before PR merges.
+this example is for testing the fix testing and will be deleted before PR merges.
```ts file="./Test.tsx"
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');
+ });
});
diff --git a/packages/react-core/src/next/components/Modal/examples/Modal.md b/packages/react-core/src/next/components/Modal/examples/Modal.md
index c5b75813e96..4ce01f31eaa 100644
--- a/packages/react-core/src/next/components/Modal/examples/Modal.md
+++ b/packages/react-core/src/next/components/Modal/examples/Modal.md
@@ -15,6 +15,14 @@ import formStyles from '@patternfly/react-styles/css/components/Form/form';
## Examples
+### TEST PR
+
+this example is for testing the fix testing and will be deleted before PR merges.
+
+```ts file="./Test.tsx"
+
+```
+
### Basic modals
Basic modals give users the option to either confirm or cancel an action.
diff --git a/packages/react-core/src/next/components/Modal/examples/Test.tsx b/packages/react-core/src/next/components/Modal/examples/Test.tsx
new file mode 100644
index 00000000000..8459f2f559c
--- /dev/null
+++ b/packages/react-core/src/next/components/Modal/examples/Test.tsx
@@ -0,0 +1,80 @@
+// this is for testing purposed only. This file will be deleted before PR merges.
+
+import React from 'react';
+import { Button } from '@patternfly/react-core';
+import { Modal, ModalBody } from '@patternfly/react-core/next';
+
+export const Test: React.FunctionComponent = () => {
+ const YourIssue1 = ({ strict }) => {
+ const renderCount = React.useRef(0);
+ renderCount.current++;
+
+ const [isOpen, setIsOpen] = React.useState(false);
+
+ React.useEffect(() => {
+ // this use effect causes an extra re-render in strict mode
+ }, [isOpen]);
+
+ return (
+
+
+ {isOpen && (
+ <>
+ setIsOpen(false)}>
+
+ Inspect the div containing the modal to see the value of aria-hidden.
+
+
+ setIsOpen(false)}>
+
+ Inspect the div containing the modal to see the value of all root siblings having no aria-hidden attribute.
+
+
+ {}}>
+ This modal remains closed for this demo.
+
+ {}}>
+ This modal remains closed for this demo.
+
+
+ );
+ };
+
+ return (
+
+
+ When a modal is open, the root and all siblings should have aria-hidden="true" except for the modal container.
+
+
+
+ When the strict mode modal opens, inspect the DOM to see that the open modal has aria-hidden="true" when it
+ should be "false".
+
+
Also, every time the modal opens a new div is appened and is never removed until the page is refreshed.
+
+
+
+
When the non-strict mode modal opens, inspect the DOM to see the open modal has no aria-hidden attribute.
+
+
+
+
+ Opening a modal while another adjacent modal is closed causes the aria-hidden attribute to be removed from the
+ root and all siblings.
+
+
+
+ );
+};
From 1b2e38b9bb9808858747ebe71d576b2b25354e62 Mon Sep 17 00:00:00 2001
From: Titani
Date: Fri, 8 Nov 2024 15:28:00 -0500
Subject: [PATCH 3/3] update test example
---
.../src/components/Modal/examples/Test.tsx | 31 +++++++++++++-----
.../next/components/Modal/examples/Test.tsx | 32 ++++++++++++++-----
2 files changed, 47 insertions(+), 16 deletions(-)
diff --git a/packages/react-core/src/components/Modal/examples/Test.tsx b/packages/react-core/src/components/Modal/examples/Test.tsx
index ac17b765a7e..ab298d26572 100644
--- a/packages/react-core/src/components/Modal/examples/Test.tsx
+++ b/packages/react-core/src/components/Modal/examples/Test.tsx
@@ -20,8 +20,11 @@ export const Test: React.FunctionComponent = () => {
{isOpen && (
<>
setIsOpen(false)}>
- Inspect the div containing the modal to see the value of aria-hidden.
-
Render count: {renderCount.current}
+
+ Inspect the div containing the modal to see the value of aria-hidden.
+
Render count: {renderCount.current}
+
+
FIXED: Root div for portal has been removed. Only siblings of modal should have aria-hidden
setIsOpen(false)}>
- Inspect the div containing the modal to see the value of all root siblings having no aria-hidden attribute.
+
+ Inspect the div containing the modal to see the value of all root siblings having no aria-hidden attribute.
+
+
FIXED: Root siblings now have aria-hidden applied.
{}}>
This modal remains closed for this demo.
@@ -54,10 +60,16 @@ export const Test: React.FunctionComponent = () => {
- When the strict mode modal opens, inspect the DOM to see that the open modal has aria-hidden="true" when it
- should be "false".
+
+ When the strict mode modal opens, inspect the DOM to see that the open modal has aria-hidden="true" when it
+ should be "false".
+
-
Also, every time the modal opens a new div is appened and is never removed until the page is refreshed.
+
FIXED: root no longer has aria-hidden
+
+ Also, every time the modal opens a new div is appened and is never removed until the page is refreshed.
+
- Opening a modal while another adjacent modal is closed causes the aria-hidden attribute to be removed from the
- root and all siblings.
+
+ Opening a modal while another adjacent modal is closed causes the aria-hidden attribute to be removed from the
+ root and all siblings.
+
+
FIXED: aria hidden is no longer removed.
);
diff --git a/packages/react-core/src/next/components/Modal/examples/Test.tsx b/packages/react-core/src/next/components/Modal/examples/Test.tsx
index 8459f2f559c..3e02db07745 100644
--- a/packages/react-core/src/next/components/Modal/examples/Test.tsx
+++ b/packages/react-core/src/next/components/Modal/examples/Test.tsx
@@ -22,8 +22,11 @@ export const Test: React.FunctionComponent = () => {
<>
setIsOpen(false)}>
- Inspect the div containing the modal to see the value of aria-hidden.
-
Render count: {renderCount.current}
+
+ Inspect the div containing the modal to see the value of aria-hidden.
+
Render count: {renderCount.current}
+
+
FIXED: Root div for portal has been removed. Only siblings of modal should have aria-hidden
>
@@ -39,7 +42,11 @@ export const Test: React.FunctionComponent = () => {
setIsOpen(false)}>
- Inspect the div containing the modal to see the value of all root siblings having no aria-hidden attribute.
+
+ Inspect the div containing the modal to see the value of all root siblings having no aria-hidden
+ attribute.
+
+
FIXED: Root siblings now have aria-hidden applied.
- When the strict mode modal opens, inspect the DOM to see that the open modal has aria-hidden="true" when it
- should be "false".
+
+ When the strict mode modal opens, inspect the DOM to see that the open modal has aria-hidden="true" when it
+ should be "false".
+
-
Also, every time the modal opens a new div is appened and is never removed until the page is refreshed.
+
FIXED: root no longer has aria-hidden
+
+ Also, every time the modal opens a new div is appened and is never removed until the page is refreshed.
+
- Opening a modal while another adjacent modal is closed causes the aria-hidden attribute to be removed from the
- root and all siblings.
+
+ Opening a modal while another adjacent modal is closed causes the aria-hidden attribute to be removed from the
+ root and all siblings.
+