From 3402856fdcbeb89381dfdf03afac0f624fd4b655 Mon Sep 17 00:00:00 2001
From: Nick Taylor <nick@dev.to>
Date: Wed, 29 Jul 2020 10:35:42 -0400
Subject: [PATCH] Make <Modal /> component more accessible (#9563)

* Used <dialog /> for modal to stay on the semantic HTML train.

* Added tests and fixed a11y issues with <Modal /> component.
---
 app/javascript/crayons/Modal/Modal.jsx        | 41 +++++-----
 .../crayons/Modal/__tests__/Modal.test.jsx    | 77 +++++++++++++++++++
 2 files changed, 99 insertions(+), 19 deletions(-)
 create mode 100644 app/javascript/crayons/Modal/__tests__/Modal.test.jsx

diff --git a/app/javascript/crayons/Modal/Modal.jsx b/app/javascript/crayons/Modal/Modal.jsx
index dddaa85ee..b214e0f2f 100644
--- a/app/javascript/crayons/Modal/Modal.jsx
+++ b/app/javascript/crayons/Modal/Modal.jsx
@@ -17,6 +17,21 @@ function getAdditionalClassNames({ size, className }) {
   return additionalClassNames;
 }
 
+const CloseIcon = () => (
+  <svg
+    width="24"
+    height="24"
+    viewBox="0 0 24 24"
+    className="crayons-icon"
+    xmlns="http://www.w3.org/2000/svg"
+    role="img"
+    aria-labelledby="714d29e78a3867c79b07f310e075e824"
+  >
+    <title id="714d29e78a3867c79b07f310e075e824">Close</title>
+    <path d="M12 10.586l4.95-4.95 1.414 1.414-4.95 4.95 4.95 4.95-1.414 1.414-4.95-4.95-4.95 4.95-1.414-1.414 4.95-4.95-4.95-4.95L7.05 5.636l4.95 4.95z" />
+  </svg>
+);
+
 export const Modal = ({
   children,
   size = 'default',
@@ -25,44 +40,32 @@ export const Modal = ({
   overlay,
   onClose,
 }) => {
-  const Icon = () => (
-    <svg
-      width="24"
-      height="24"
-      viewBox="0 0 24 24"
-      className="crayons-icon"
-      xmlns="http://www.w3.org/2000/svg"
-      role="img"
-      aria-labelledby="714d29e78a3867c79b07f310e075e824"
-    >
-      <title id="714d29e78a3867c79b07f310e075e824">Close</title>
-      <path d="M12 10.586l4.95-4.95 1.414 1.414-4.95 4.95 4.95 4.95-1.414 1.414-4.95-4.95-4.95 4.95-1.414-1.414 4.95-4.95-4.95-4.95L7.05 5.636l4.95 4.95z" />
-    </svg>
-  );
-
   return (
     <div
+      data-testid="modal-container"
       className={`crayons-modal${getAdditionalClassNames({
         size,
         className,
       })}`}
     >
-      <div className="crayons-modal__box">
+      <div role="dialog" aria-modal="true" className="crayons-modal__box">
         {title.length > 0 && title && (
           <div className="crayons-modal__box__header">
             <h2>{title}</h2>
             <Button
-              icon={Icon}
+              icon={CloseIcon}
               variant="ghost"
               contentType="icon"
-              title="Close"
+              aria-label="Close"
               onClick={onClose}
             />
           </div>
         )}
         <div className="crayons-modal__box__body">{children}</div>
       </div>
-      {overlay && <div className="crayons-modal__overlay" />}
+      {overlay && (
+        <div data-testid="modal-overlay" className="crayons-modal__overlay" />
+      )}
     </div>
   );
 };
diff --git a/app/javascript/crayons/Modal/__tests__/Modal.test.jsx b/app/javascript/crayons/Modal/__tests__/Modal.test.jsx
new file mode 100644
index 000000000..1af598725
--- /dev/null
+++ b/app/javascript/crayons/Modal/__tests__/Modal.test.jsx
@@ -0,0 +1,77 @@
+import { h } from 'preact';
+import { axe } from 'jest-axe';
+import { render } from '@testing-library/preact';
+import { Modal } from '../Modal';
+
+it('should have no a11y violations', async () => {
+  const { container } = render(
+    <Modal title="This is a modal title">This is the modal body content</Modal>,
+  );
+  const results = await axe(container);
+
+  expect(results).toHaveNoViolations();
+});
+
+it('should close when the close button is clicked', async () => {
+  const onClose = jest.fn();
+  const { getByLabelText } = render(
+    <Modal title="This is a modal title" onClose={onClose}>
+      This is the modal body content
+    </Modal>,
+  );
+
+  const closeButton = getByLabelText('Close', { selector: 'button' });
+
+  closeButton.click();
+
+  expect(onClose).toHaveBeenCalledTimes(1);
+});
+
+it('should render with additional class names', async () => {
+  const { getByTestId } = render(
+    <Modal
+      title="This is a modal title"
+      className={'some-additional-class-name'}
+      onClose={jest.fn()}
+    >
+      This is the modal body content
+    </Modal>,
+  );
+
+  const modalContainer = getByTestId('modal-container');
+
+  expect(
+    modalContainer.classList.contains('some-additional-class-name'),
+  ).toEqual(true);
+});
+
+it('should render with an overlay', async () => {
+  const { getByTestId } = render(
+    <Modal title="This is a modal title" overlay onClose={jest.fn()}>
+      This is the modal body content
+    </Modal>,
+  );
+
+  const modalOverlay = getByTestId('modal-overlay');
+
+  expect(modalOverlay).not.toBeNull();
+});
+
+it('should render with a different size modal', async () => {
+  const { getByTestId } = render(
+    <Modal
+      title="This is a modal title"
+      size="large"
+      className={'some-additional-class-name'}
+      onClose={jest.fn()}
+    >
+      This is the modal body content
+    </Modal>,
+  );
+
+  const modalContainer = getByTestId('modal-container');
+
+  expect(modalContainer.classList.contains('crayons-modal--large')).toEqual(
+    true,
+  );
+});