From f3f730976fddf7015ca593d70e52f5bcedcd56bb Mon Sep 17 00:00:00 2001
From: Half-Shot <will@half-shot.uk>
Date: Thu, 7 Nov 2024 16:58:35 +0000
Subject: [PATCH] apply review feedback

---
 src/Modal.test.tsx | 24 +++++++++++++-----------
 src/Modal.tsx      |  6 +++---
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/Modal.test.tsx b/src/Modal.test.tsx
index 5c1182e5..4217b6ce 100644
--- a/src/Modal.test.tsx
+++ b/src/Modal.test.tsx
@@ -5,11 +5,18 @@ SPDX-License-Identifier: AGPL-3.0-only
 Please see LICENSE in the repository root for full details.
 */
 
-import { afterAll, expect, test } from "vitest";
+import { expect, test } from "vitest";
 import { render } from "@testing-library/react";
 import { act, ReactNode, useState } from "react";
 
 import { Modal } from "./Modal";
+import { afterEach } from "node:test";
+import userEvent from "@testing-library/user-event";
+
+const originalMatchMedia = window.matchMedia;
+afterEach(() => {
+  window.matchMedia = originalMatchMedia;
+});
 
 test("that nothing is rendered when the modal is closed", () => {
   const { queryByRole } = render(
@@ -29,7 +36,7 @@ test("the content is rendered when the modal is open", () => {
   expect(queryByRole("dialog")).toMatchSnapshot();
 });
 
-test("the modal can be closed by clicking the close button", () => {
+test("the modal can be closed by clicking the close button", async () => {
   function ModalFn(): ReactNode {
     const [isOpen, setOpen] = useState(true);
     return (
@@ -38,19 +45,14 @@ test("the modal can be closed by clicking the close button", () => {
       </Modal>
     );
   }
-  const { queryByRole, getByLabelText } = render(<ModalFn />);
-  act(() => {
-    getByLabelText("action.close").click();
+  const user = userEvent.setup();
+  const { queryByRole, getByRole } = render(<ModalFn />);
+  await act(async () => {
+    await user.click(getByRole("button", { name: "action.close" }));
   });
   expect(queryByRole("dialog")).toBeNull();
 });
 
-const originalMatchMedia = window.matchMedia;
-
-afterAll(() => {
-  window.matchMedia = originalMatchMedia;
-});
-
 test("the modal renders as a drawer in mobile viewports", () => {
   window.matchMedia = function (query): MediaQueryList {
     return {
diff --git a/src/Modal.tsx b/src/Modal.tsx
index 195b6592..6e9de90b 100644
--- a/src/Modal.tsx
+++ b/src/Modal.tsx
@@ -89,9 +89,9 @@ export const Modal: FC<Props> = ({
               styles.drawer,
               { [styles.tabbed]: tabbed },
             )}
+            role="dialog"
             // Suppress the warning about there being no description; the modal
             // has an accessible title
-            role="dialog"
             aria-describedby={undefined}
             {...rest}
           >
@@ -115,10 +115,10 @@ export const Modal: FC<Props> = ({
           <DialogOverlay
             className={classNames(overlayStyles.bg, overlayStyles.animate)}
           />
-          {/* Suppress the warning about there being no description; the modal
-          has an accessible title */}
           <DialogContent
             asChild
+            // Suppress the warning about there being no description; the modal
+            // has an accessible title
             aria-describedby={undefined}
             role="dialog"
             {...rest}