[Modal] Should propagate event if disableEscapeKeyDown (#20786)

Author: Weslen Nascimento

docs/pages/api-docs/modal.md

@@ -42,9 +42,9 @@ This component shares many concepts with [react-overlays](https://react-bootstra
 | <span class="prop-name">closeAfterTransition</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | When set to true the Modal waits until a nested Transition is completed before closing. |
 | <span class="prop-name">container</span> | <span class="prop-type">HTML element<br>&#124;&nbsp;React.Component<br>&#124;&nbsp;func</span> |  | A HTML element, component instance, or function that returns either. The `container` will have the portal children appended to it.<br>By default, it uses the body of the top-level document object, so it's simply `document.body` most of the time. |
 | <span class="prop-name">disableAutoFocus</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the modal will not automatically shift focus to itself when it opens, and replace it to the last focused element when it closes. This also works correctly with any modal children that have the `disableAutoFocus` prop.<br>Generally this should never be set to `true` as it makes the modal less accessible to assistive technologies, like screen readers. |
-| <span class="prop-name">disableBackdropClick</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, clicking the backdrop will not fire any callback. |
+| <span class="prop-name">disableBackdropClick</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, clicking the backdrop will not fire `onClose`. |
 | <span class="prop-name">disableEnforceFocus</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the modal will not prevent focus from leaving the modal while open.<br>Generally this should never be set to `true` as it makes the modal less accessible to assistive technologies, like screen readers. |
-| <span class="prop-name">disableEscapeKeyDown</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, hitting escape will not fire any callback. |
+| <span class="prop-name">disableEscapeKeyDown</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, hitting escape will not fire `onClose`. |
 | <span class="prop-name">disablePortal</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Disable the portal behavior. The children stay within it's parent DOM hierarchy. |
 | <span class="prop-name">disableRestoreFocus</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | If `true`, the modal will not restore focus to previously focused element once modal is hidden. |
 | <span class="prop-name">disableScrollLock</span> | <span class="prop-type">bool</span> | <span class="prop-default">false</span> | Disable the scroll lock behavior. |

packages/material-ui/src/ButtonBase/ButtonBase.test.js

@@ -830,7 +830,7 @@ describe('<ButtonBase />', () => {
       });
 
       it('does not call onClick if Space was released on a child', () => {
-        const onClickSpy = spy((event) => event.defaultPrevented);
+        const onClickSpy = spy();
         const onKeyUpSpy = spy();
         render(
           <ButtonBase onClick={onClickSpy} onKeyUp={onKeyUpSpy} component="div">

packages/material-ui/src/Modal/Modal.js

@@ -193,15 +193,17 @@ const Modal = React.forwardRef(function Modal(inProps, ref) {
       return;
     }
 
-    // Swallow the event, in case someone is listening for the escape key on the body.
-    event.stopPropagation();
-
     if (onEscapeKeyDown) {
       onEscapeKeyDown(event);
     }
 
-    if (!disableEscapeKeyDown && onClose) {
-      onClose(event, 'escapeKeyDown');
+    if (!disableEscapeKeyDown) {
+      // Swallow the event, in case someone is listening for the escape key on the body.
+      event.stopPropagation();
+
+      if (onClose) {
+        onClose(event, 'escapeKeyDown');
+      }
     }
   };
 
@@ -294,7 +296,7 @@ Modal.propTypes = {
    */
   disableAutoFocus: PropTypes.bool,
   /**
-   * If `true`, clicking the backdrop will not fire any callback.
+   * If `true`, clicking the backdrop will not fire `onClose`.
    */
   disableBackdropClick: PropTypes.bool,
   /**
@@ -305,7 +307,7 @@ Modal.propTypes = {
    */
   disableEnforceFocus: PropTypes.bool,
   /**
-   * If `true`, hitting escape will not fire any callback.
+   * If `true`, hitting escape will not fire `onClose`.
    */
   disableEscapeKeyDown: PropTypes.bool,
   /**

packages/material-ui/src/Modal/Modal.test.js

@@ -270,12 +270,15 @@ describe('<Modal />', () => {
     });
 
     it('should call onEscapeKeyDown and onClose', () => {
+      const handleKeyDown = spy();
       const onEscapeKeyDownSpy = spy();
       const onCloseSpy = spy();
       const { getByTestId } = render(
-        <Modal data-testid="modal" open onEscapeKeyDown={onEscapeKeyDownSpy} onClose={onCloseSpy}>
-          <div />
-        </Modal>,
+        <div onKeyDown={handleKeyDown}>
+          <Modal data-testid="modal" open onEscapeKeyDown={onEscapeKeyDownSpy} onClose={onCloseSpy}>
+            <div />
+          </Modal>
+        </div>,
       );
       getByTestId('modal').focus();
 
@@ -285,21 +288,25 @@ describe('<Modal />', () => {
 
       expect(onEscapeKeyDownSpy).to.have.property('callCount', 1);
       expect(onCloseSpy).to.have.property('callCount', 1);
+      expect(handleKeyDown).to.have.property('callCount', 0);
     });
 
-    it('when disableEscapeKeyDown should call only onEscapeKeyDownSpy', () => {
+    it('should not call onChange when `disableEscapeKeyDown=true`', () => {
+      const handleKeyDown = spy();
       const onEscapeKeyDownSpy = spy();
       const onCloseSpy = spy();
       const { getByTestId } = render(
-        <Modal
-          data-testid="modal"
-          open
-          disableEscapeKeyDown
-          onEscapeKeyDown={onEscapeKeyDownSpy}
-          onClose={onCloseSpy}
-        >
-          <div />
-        </Modal>,
+        <div onKeyDown={handleKeyDown}>
+          <Modal
+            data-testid="modal"
+            open
+            disableEscapeKeyDown
+            onEscapeKeyDown={onEscapeKeyDownSpy}
+            onClose={onCloseSpy}
+          >
+            <div />
+          </Modal>
+        </div>,
       );
       getByTestId('modal').focus();
 
@@ -309,6 +316,7 @@ describe('<Modal />', () => {
 
       expect(onEscapeKeyDownSpy).to.have.property('callCount', 1);
       expect(onCloseSpy).to.have.property('callCount', 0);
+      expect(handleKeyDown).to.have.property('callCount', 1);
     });
   });