Skip to content

Commit

Permalink
Merge pull request #801 from openSUSE/remove-aria-hidden-patch
Browse files Browse the repository at this point in the history
[web] Remove Popup#aria-hidden patch added in #580
  • Loading branch information
dgdavid authored Oct 19, 2023
2 parents 1a9e0a3 + d5a1566 commit ccc3c8e
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 51 deletions.
30 changes: 1 addition & 29 deletions web/src/components/core/Popup.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* find current contact information at www.suse.com.
*/

import React, { useLayoutEffect } from "react";
import React from "react";
import { Button, Modal } from "@patternfly/react-core";

import { _ } from "~/i18n";
Expand Down Expand Up @@ -204,34 +204,6 @@ const Popup = ({
}) => {
const [actions, content] = partition(React.Children.toArray(children), child => child.type === Actions);

useLayoutEffect(() => {
/**
* A workaround for ensuring aria-hidden attributes added by a PF/Modal to
* its siblings are removed when the dialog is directly unmounted.
*
* To know more, read the following links
* - https://github.com/patternfly/patternfly-react/pull/9096
* - https://github.com/openSUSE/agama/pull/576
* - https://github.com/openSUSE/agama/pull/572
*
* Using body children because agama is not using the `appendTo` PF/Modal
* prop in its code base. Therefore, for this workaround it can be assumed
* that the default document.body is the parent of any modal always.
*
* See https://github.com/patternfly/patternfly-react/blob/a0f857c4de39dd415792a8701e7c6ac7fd853024/packages/react-core/src/components/Modal/Modal.tsx#L115
*
* Why not using a `ref` instead? Because PF/Modal is not forwarding its
* ref and which blocks us to do something similar to what we already did
* in core/Sidebar.
*
* Read https://react.dev/learn/manipulating-the-dom-with-refs#accessing-another-components-dom-node
*/
return () => {
const parent = document.body;
[...parent.children].forEach(n => n.removeAttribute("aria-hidden"));
};
}, []);

return (
<Modal
{ ...pfModalProps }
Expand Down
23 changes: 1 addition & 22 deletions web/src/components/core/Popup.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import React, { useState } from "react";

import { screen, within } from "@testing-library/react";
import { installerRender, plainRender } from "~/test-utils";
import { installerRender } from "~/test-utils";

import { Popup } from "~/components/core";

Expand Down Expand Up @@ -91,27 +91,6 @@ describe("Popup", () => {
within(footer).getByText("Confirm");
within(footer).getByText("Cancel");
});

it("removes aria-hidden attributes from body children (workaround for patternfly/patternfly-react#9096)", async () => {
const { user } = plainRender(
<>
<article>Popup Sibling</article>
<TestingPopup />
</>,
// Force React Testing Library to render the component directly in the
// body for emulating the default behavior of the PF/Modal when no
// appendTo prop is given.
// https://testing-library.com/docs/react-testing-library/api/#container
{ container: document.body }
);

await screen.findByRole("dialog");
const sibling = screen.getByText("Popup Sibling");
const unmountButton = screen.getByRole("button", { name: "Unmount Popup" });
expect(sibling).toHaveAttribute("aria-hidden");
await user.click(unmountButton);
expect(sibling).not.toHaveAttribute("aria-hidden");
});
});
});

Expand Down

0 comments on commit ccc3c8e

Please sign in to comment.