fix: ensure esc key does not close modals that are blocking (#3033)

.github/pull_request_template.md

@@ -14,7 +14,7 @@ Include a direct link to your changes in this PR's deploy preview here (e.g., a
 * [ ] Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
 * [ ] Were your changes tested in the `example` app?
 * [ ] Is there adequate test coverage for your changes?
-* [ ] Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please add `wittjeff` and `adamstankiewicz` as reviewers on this PR.
+* [ ] Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please request an a11y review for the PR in the [#wg-paragon](https://openedx.slack.com/archives/C02NR285KV4) Open edX Slack channel.
 
 ## Post-merge Checklist
 

src/Modal/AlertModal.jsx

@@ -41,7 +41,7 @@ AlertModal.propTypes = {
   title: PropTypes.string.isRequired,
   /** Is the modal dialog open or closed */
   isOpen: PropTypes.bool,
-  /** Prevent clicking on the backdrop to close the modal */
+  /** Prevent clicking on the backdrop or pressing Esc to close the modal */
   isBlocking: PropTypes.bool,
   /** Specifies whether the dialog box should contain 'x' icon button in the top right */
   hasCloseButton: PropTypes.bool,

src/Modal/MarketingModal.jsx

@@ -36,7 +36,7 @@ MarketingModal.propTypes = {
   title: PropTypes.string.isRequired,
   /** Is the modal dialog open or closed */
   isOpen: PropTypes.bool,
-  /** Prevent clicking on the backdrop to close the modal */
+  /** Prevent clicking on the backdrop or pressing Esc to close the modal */
   isBlocking: PropTypes.bool,
   /** The close 'x' icon button in the top right corner */
   hasCloseButton: PropTypes.bool,

src/Modal/ModalDialog.jsx

@@ -115,7 +115,7 @@ ModalDialog.propTypes = {
    */
   isFullscreenOnMobile: PropTypes.bool,
   /**
-   * Prevent clicking on the backdrop to close the modal
+   * Prevent clicking on the backdrop or pressing Esc to close the modal
    */
   isBlocking: PropTypes.bool,
   /**

src/Modal/ModalLayer.jsx

@@ -63,7 +63,7 @@ function ModalLayer({
     return null;
   }
 
-  const onClickOutside = !isBlocking ? onClose : null;
+  const handleClose = isBlocking ? null : onClose;
 
   return (
     <ModalContextProvider onClose={onClose} isOpen={isOpen} isBlocking={isBlocking}>
@@ -72,15 +72,15 @@ function ModalLayer({
           allowPinchZoom
           scrollLock
           enabled={isOpen}
-          onEscapeKey={onClose}
-          onClickOutside={onClickOutside}
+          onEscapeKey={handleClose}
+          onClickOutside={handleClose}
           className={classNames(
             'pgn__modal-layer',
             zIndex ? `zindex-${zIndex}` : '',
           )}
         >
           <ModalContentContainer>
-            <ModalBackdrop onClick={onClickOutside} />
+            <ModalBackdrop onClick={handleClose} />
             {children}
           </ModalContentContainer>
         </FocusOn>
@@ -96,7 +96,7 @@ ModalLayer.propTypes = {
   onClose: PropTypes.func.isRequired,
   /** Is the modal dialog open or closed */
   isOpen: PropTypes.bool.isRequired,
-  /** Prevent clicking on the backdrop to close the modal */
+  /** Prevent clicking on the backdrop or pressing Esc to close the modal */
   isBlocking: PropTypes.bool,
   /** Specifies the z-index of the modal */
   zIndex: PropTypes.number,

src/Modal/ModalPopup.jsx

@@ -76,7 +76,7 @@ ModalPopup.propTypes = {
   onClose: PropTypes.func.isRequired,
   /** Is the modal dialog open or closed */
   isOpen: PropTypes.bool.isRequired,
-  /** Prevent clicking on the backdrop to close the modal */
+  /** Prevent clicking on the backdrop or pressing Esc to close the modal */
   isBlocking: PropTypes.bool,
   /** Insert modal into a different location in the DOM */
   withPortal: PropTypes.bool,

src/Modal/tests/ModalLayer.test.jsx

@@ -1,4 +1,5 @@
 import React from 'react';
+import { FocusOn } from 'react-focus-on';
 import { render, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 
@@ -15,12 +16,12 @@ jest.mock('../Portal', () => function PortalMock(props) {
 });
 
 jest.mock('react-focus-on', () => ({
-  FocusOn: (props) => {
+  FocusOn: jest.fn().mockImplementation((props) => {
     const { children, ...otherProps } = props;
     return (
       <focus-on data-testid="focus-on" {...otherProps}>{children}</focus-on>
     );
-  },
+  }),
 }));
 
 function Dialog() {
@@ -32,6 +33,10 @@ function Dialog() {
 }
 
 describe('<ModalLayer />', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
   describe('when isOpen', () => {
     const isOpen = true;
     const closeFn = jest.fn();
@@ -72,30 +77,64 @@ describe('<ModalLayer />', () => {
     expect(dialog).not.toBeInTheDocument();
   });
 
-  describe('Backdrop', () => {
-    it('closes a non-blocking modal layer when clicked', async () => {
+  describe('Dismiss modal', () => {
+    it('closes a non-blocking modal layer when backdrop is clicked', () => {
       const closeFn = jest.fn();
       render(
         <ModalLayer isOpen onClose={closeFn} isBlocking={false}>
           <Dialog />
         </ModalLayer>,
       );
-
       const backdrop = screen.getByTestId('modal-backdrop');
-      await userEvent.click(backdrop);
+      userEvent.click(backdrop);
       expect(closeFn).toHaveBeenCalled();
     });
 
-    it('does not close a blocking modal layer when clicked', async () => {
+    it('should configure FocusOn to close a non-blocking modal layer when Esc key is pressed', () => {
+      const closeFn = jest.fn();
+      render(
+        <ModalLayer isOpen onClose={closeFn} isBlocking={false}>
+          <Dialog />
+        </ModalLayer>,
+      );
+      expect(FocusOn).toHaveBeenCalledWith(
+        expect.objectContaining({
+          onEscapeKey: closeFn,
+        }),
+        // note: this 2nd function argument represents the
+        // `refOrContext` (in this case, the context value
+        // provided by `ModalContextProvider`).
+        {},
+      );
+    });
+
+    it('should not configure FocusOn to close a blocking modal layer when Esc key is pressed', () => {
       const closeFn = jest.fn();
       render(
         <ModalLayer isOpen onClose={closeFn} isBlocking>
           <Dialog />
         </ModalLayer>,
       );
+      expect(FocusOn).toHaveBeenCalledWith(
+        expect.objectContaining({
+          onEscapeKey: null,
+        }),
+        // note: this 2nd function argument represents the
+        // `refOrContext` (in this case, the context value
+        // provided by `ModalContextProvider`).
+        {},
+      );
+    });
 
+    it('does not close a blocking modal layer when backdrop is clicked', () => {
+      const closeFn = jest.fn();
+      render(
+        <ModalLayer isOpen onClose={closeFn} isBlocking>
+          <Dialog />
+        </ModalLayer>,
+      );
       const backdrop = screen.getByTestId('modal-backdrop');
-      await userEvent.click(backdrop);
+      userEvent.click(backdrop);
       expect(closeFn).not.toHaveBeenCalled();
     });
   });