fix(Modal): improve the mouse scrolling experience on Modal (#2254)

* fix(Modal): improve the mouse scrolling experience on Modal

* fix(Modal): remove the deprecated onBackdropClick

* fix(Modal): remove redundant checks

src/Modal/Modal.tsx

@@ -1,11 +1,11 @@
-import React, { useRef, useMemo, useState, useEffect, useCallback } from 'react';
+import React, { useRef, useMemo, useState, useCallback } from 'react';
 import PropTypes from 'prop-types';
 import pick from 'lodash/pick';
 import on from 'dom-lib/on';
 import getAnimationEnd from 'dom-lib/getAnimationEnd';
 import BaseModal, { BaseModalProps, modalPropTypes } from '../Overlay/Modal';
 import Bounce from '../Animation/Bounce';
-import { useClassNames, mergeRefs, SIZE } from '../utils';
+import { useClassNames, mergeRefs, SIZE, useWillUnmount } from '../utils';
 import ModalDialog, { modalDialogPropTypes } from './ModalDialog';
 import { ModalContext, ModalContextProps } from './ModalContext';
 import ModalBody from './ModalBody';
@@ -85,17 +85,21 @@ const Modal: ModalComponent = React.forwardRef((props: ModalProps, ref) => {
     'aria-describedby': ariaDescribedby,
     ...rest
   } = props;
+
   const inClass = { in: open && !animation };
   const { merge, prefix } = useClassNames(classPrefix);
+  const [shake, setShake] = useState(false);
   const classes = merge(className, prefix(size, { full }));
   const dialogRef = useRef<HTMLElement>(null);
   const transitionEndListener = useRef<{ off: () => void } | null>();
+
   // The style of the Modal body will be updated with the size of the window or container.
   const [bodyStyles, onChangeBodyStyles, onDestroyEvents] = useBodyStyles(dialogRef, {
     overflow,
     drawer,
     prefix
   });
+
   const dialogId = useUniqueId('dialog-', idProp);
   const modalContextValue = useMemo<ModalContextProps>(
     () => ({
@@ -106,19 +110,7 @@ const Modal: ModalComponent = React.forwardRef((props: ModalProps, ref) => {
     }),
     [dialogId, onClose, bodyStyles, drawer]
   );
-  const [shake, setShake] = useState(false);
-  const handleBackdropClick = useCallback(() => {
-    // When the value of `backdrop` is `static`, a jitter animation will be added to the dialog when clicked.
-    if (backdrop === 'static') {
-      setShake(true);
-      if (!transitionEndListener.current && dialogRef.current) {
-        //fix: https://github.com/rsuite/rsuite/blob/a93d13c14fb20cc58204babe3331d3c3da3fe1fd/src/Modal/styles/index.less#L59
-        transitionEndListener.current = on(dialogRef.current, getAnimationEnd(), () => {
-          setShake(false);
-        });
-      }
-    }
-  }, [backdrop]);
+
   const handleExited = useCallback(
     (node: HTMLElement) => {
       onExited?.(node);
@@ -128,23 +120,59 @@ const Modal: ModalComponent = React.forwardRef((props: ModalProps, ref) => {
     },
     [onDestroyEvents, onExited]
   );
+
   const handleEntered = useCallback(
     (node: HTMLElement) => {
       onEntered?.(node);
       onChangeBodyStyles();
     },
     [onChangeBodyStyles, onEntered]
   );
+
   const handleEntering = useCallback(
     (node: HTMLElement) => {
       onEntering?.(node);
       onChangeBodyStyles(true);
     },
     [onChangeBodyStyles, onEntering]
   );
-  useEffect(() => {
+
+  const handleBackdropClick = useCallback(
+    e => {
+      if (e.target !== e.currentTarget) {
+        return;
+      }
+
+      // When the value of `backdrop` is `static`, a jitter animation will be added to the dialog when clicked.
+      if (backdrop === 'static') {
+        setShake(true);
+        if (!transitionEndListener.current && dialogRef.current) {
+          //fix: https://github.com/rsuite/rsuite/blob/a93d13c14fb20cc58204babe3331d3c3da3fe1fd/src/Modal/styles/index.less#L59
+          transitionEndListener.current = on(dialogRef.current, getAnimationEnd(), () => {
+            setShake(false);
+          });
+        }
+        return;
+      }
+
+      onClose?.(e);
+    },
+    [backdrop, onClose]
+  );
+
+  const handleClick = useCallback(
+    e => {
+      if (dialogRef.current && e.target !== dialogRef.current) {
+        handleBackdropClick(e);
+      }
+    },
+    [handleBackdropClick]
+  );
+
+  useWillUnmount(() => {
     transitionEndListener.current?.off();
-  }, []);
+  });
+
   return (
     <ModalContext.Provider value={modalContextValue}>
       <BaseModal
@@ -153,7 +181,6 @@ const Modal: ModalComponent = React.forwardRef((props: ModalProps, ref) => {
         backdrop={backdrop}
         open={open}
         onClose={onClose}
-        onBackdropClick={handleBackdropClick}
         className={prefix`wrapper`}
         onEntered={handleEntered}
         onEntering={handleEntering}
@@ -164,6 +191,7 @@ const Modal: ModalComponent = React.forwardRef((props: ModalProps, ref) => {
         animationProps={animationProps}
         dialogTransitionTimeout={animationTimeout}
         backdropTransitionTimeout={150}
+        onClick={backdrop ? handleClick : undefined}
       >
         {(transitionProps, transitionRef) => {
           const { className: transitionClassName, ...transitionRest } = transitionProps;

src/Modal/styles/index.less

@@ -5,6 +5,27 @@
 // Modals
 // --------------------------------------------------
 
+// Modal background
+.rs-modal-backdrop {
+  position: fixed;
+  top: 0;
+  left: 0;
+  width: 100vw;
+  height: 100vh;
+  z-index: @zindex-modal - 1;
+  background-color: var(--rs-bg-backdrop);
+
+  // Fade for backdrop
+  &.rs-anim-fade {
+    opacity: 0;
+    transition: opacity 0.3s ease-in;
+  }
+
+  &.rs-anim-in {
+    opacity: 1;
+  }
+}
+
 // Kill the scroll on the body
 .rs-modal-open {
   overflow: hidden;
@@ -15,9 +36,9 @@
   overflow: auto;
   z-index: @zindex-modal;
   top: 0;
-  bottom: 0;
-  right: 0;
   left: 0;
+  width: 100%;
+  height: 100%;
 }
 
 // Container that the modal scrolls within
@@ -71,26 +92,6 @@
   padding: @modal-content-padding;
 }
 
-// Modal background
-.rs-modal-backdrop {
-  position: fixed;
-  top: 0;
-  right: 0;
-  bottom: 0;
-  left: 0;
-  background-color: var(--rs-bg-backdrop);
-
-  // Fade for backdrop
-  &.rs-anim-fade {
-    opacity: 0;
-    transition: opacity 0.3s ease-in;
-  }
-
-  &.rs-anim-in {
-    opacity: 1;
-  }
-}
-
 // Modal header
 // Top section of the modal w/ title and dismiss
 .rs-modal-header {

src/Modal/test/ModalSpec.js

@@ -1,6 +1,6 @@
 import React from 'react';
 import ReactTestUtils from 'react-dom/test-utils';
-import { render, screen } from '@testing-library/react';
+import { render, screen, fireEvent } from '@testing-library/react';
 import { getInstance } from '@test/testUtils';
 import Modal from '../Modal';
 import SelectPicker from '../../SelectPicker';
@@ -17,17 +17,31 @@ describe('Modal', () => {
 
   it('Should close the modal when the modal dialog is clicked', () => {
     const onCloseSpy = sinon.spy();
-    const instance = getInstance(<Modal open onClose={onCloseSpy} />);
+    const { getByTestId } = render(<Modal data-testid="wrapper" open onClose={onCloseSpy} />);
 
-    ReactTestUtils.Simulate.click(instance.querySelector('.rs-modal-backdrop'));
+    fireEvent.click(getByTestId('wrapper'));
 
     assert.isTrue(onCloseSpy.calledOnce);
   });
 
   it('Should not close the modal when the "static" dialog is clicked', () => {
     const onCloseSpy = sinon.spy();
-    const instance = getInstance(<Modal open onClose={onCloseSpy} backdrop="static" />);
-    ReactTestUtils.Simulate.click(instance.querySelector('.rs-modal-backdrop'));
+    const { getByTestId } = render(
+      <Modal data-testid="wrapper" open onClose={onCloseSpy} backdrop="static" />
+    );
+
+    fireEvent.click(getByTestId('wrapper'));
+
+    assert.isFalse(onCloseSpy.calledOnce);
+  });
+
+  it('Should not close the modal when clicking inside the dialog', () => {
+    const onCloseSpy = sinon.spy();
+    const { getByRole } = render(<Modal open onClose={onCloseSpy} />);
+
+    fireEvent.click(getByRole('dialog'));
+    fireEvent.click(getByRole('document'));
+
     assert.isFalse(onCloseSpy.calledOnce);
   });
 

src/Overlay/Modal.tsx

@@ -74,6 +74,8 @@ interface ModalProps extends BaseModalProps {
   dialogTransitionTimeout?: number;
   transition?: React.ElementType;
   onEsc?: React.KeyboardEventHandler;
+
+  // @deprecated
   onBackdropClick?: React.MouseEventHandler;
 }
 
@@ -127,7 +129,6 @@ const Modal: RsRefForwardingComponent<'div', ModalProps> = React.forwardRef<
     backdropClassName,
     open,
     autoFocus = true,
-    onBackdropClick,
     onEsc,
     onExit,
     onExiting,
@@ -191,18 +192,6 @@ const Modal: RsRefForwardingComponent<'div', ModalProps> = React.forwardRef<
     handleFocusDialog();
   });
 
-  const handleBackdropClick = useEventCallback((event: React.MouseEvent) => {
-    if (event.target !== event.currentTarget) {
-      return;
-    }
-
-    onBackdropClick?.(event);
-
-    if (backdrop === true) {
-      onClose?.(event);
-    }
-  });
-
   const documentKeyDownListener = useRef<{ off: () => void } | null>();
   const documentFocusListener = useRef<{ off: () => void } | null>();
 
@@ -276,11 +265,6 @@ const Modal: RsRefForwardingComponent<'div', ModalProps> = React.forwardRef<
   }
 
   const renderBackdrop = () => {
-    const backdropPorps = {
-      style: backdropStyle,
-      onClick: handleBackdropClick
-    };
-
     if (Transition) {
       return (
         <Fade transitionAppear in={open} timeout={backdropTransitionTimeout}>
@@ -290,7 +274,7 @@ const Modal: RsRefForwardingComponent<'div', ModalProps> = React.forwardRef<
               <div
                 aria-hidden
                 {...rest}
-                {...backdropPorps}
+                style={backdropStyle}
                 ref={mergeRefs(modal.setBackdropRef, ref)}
                 className={classNames(backdropClassName, className)}
               />
@@ -300,7 +284,7 @@ const Modal: RsRefForwardingComponent<'div', ModalProps> = React.forwardRef<
       );
     }
 
-    return <div aria-hidden {...backdropPorps} className={backdropClassName} />;
+    return <div aria-hidden style={backdropStyle} className={backdropClassName} />;
   };
 
   const dialogElement = Transition ? (
@@ -326,14 +310,14 @@ const Modal: RsRefForwardingComponent<'div', ModalProps> = React.forwardRef<
   return (
     <OverlayContext.Provider value={contextValue}>
       <Portal>
+        {backdrop && renderBackdrop()}
         <Component
           {...rest}
           ref={mergeRefs(modal.setDialogRef, ref as any)}
           style={style}
           className={className}
           tabIndex={-1}
         >
-          {backdrop && renderBackdrop()}
           {dialogElement}
         </Component>
       </Portal>
@@ -367,8 +351,7 @@ Modal.propTypes = {
   dialogTransitionTimeout: PropTypes.number,
   backdropTransitionTimeout: PropTypes.number,
   transition: PropTypes.any,
-  onEsc: PropTypes.func,
-  onBackdropClick: PropTypes.func
+  onEsc: PropTypes.func
 };
 
 export default Modal;