Repository: zeit-ui/react (ts)

Issue:
Don't close Modal onMouseUp
# About 'Modal' behavior 🛠️
Currently, the Modal will close not only on outside full click, but even if 'mouseDown' was pressed inside Modal and 'mouseUp' happened outside. This leading to some unexpected, unintentional closes of the dialog, which leads to bad user experience and potential input loss.

Look at the attached video to get a showcase of this behavior:
https://streamable.com/4zgl2f

In the attached example I'm trying to select text in my Input, but end up closing whole Dialog instead. 😥


Pull request:
fix(backdrop): fix click events from content to close modal unexpectedly
## PR Checklist

- [x] Fix linting errors
- [x] Label has been added


## Change information

fixed #264 


Patch:
diff --git a/components/shared/backdrop.tsx b/components/shared/backdrop.tsx
index d4482d7..a5ce5e8 100644
--- a/components/shared/backdrop.tsx
+++ b/components/shared/backdrop.tsx
@@ -2,6 +2,7 @@ import React, { MouseEvent, useCallback } from 'react'
 import withDefaults from '../utils/with-defaults'
 import useTheme from '../styles/use-theme'
 import CSSTransition from './css-transition'
+import useCurrentState from '../utils/use-current-state'
 
 interface Props {
   onClick?: (event: MouseEvent<HTMLElement>) => void
@@ -20,18 +21,30 @@ export type BackdropProps = Props & typeof defaultProps
 const Backdrop: React.FC<React.PropsWithChildren<BackdropProps>> = React.memo(
   ({ children, onClick, visible, offsetY }) => {
     const theme = useTheme()
-    const clickHandler = useCallback((event: MouseEvent<HTMLElement>) => {
+    const [, setIsContentMouseDown, IsContentMouseDownRef] = useCurrentState(false)
+    const clickHandler = (event: MouseEvent<HTMLElement>) => {
+      if (IsContentMouseDownRef.current) return
       onClick && onClick(event)
-    }, [])
+    }
     const childrenClickHandler = useCallback((event: MouseEvent<HTMLElement>) => {
       event.stopPropagation()
     }, [])
+    const mouseUpHandler = () => {
+      if (!IsContentMouseDownRef.current) return
+      const timer = setTimeout(() => {
+        setIsContentMouseDown(false)
+        clearTimeout(timer)
+      }, 0)
+    }
 
     return (
       <CSSTransition visible={visible} clearTime={300}>
-        <div className="backdrop" onClick={clickHandler}>
+        <div className="backdrop" onClick={clickHandler} onMouseUp={mouseUpHandler}>
           <div className="layer" />
-          <div onClick={childrenClickHandler} className="content">
+          <div
+            onClick={childrenClickHandler}
+            className="content"
+            onMouseDown={() => setIsContentMouseDown(true)}>
             {children}
           </div>
           <div onClick={childrenClickHandler} className="offset" />


Test patch:
diff --git a/components/shared/__tests__/backdrop.test.tsx b/components/shared/__tests__/backdrop.test.tsx
index 9d14ffd..d3de821 100644
--- a/components/shared/__tests__/backdrop.test.tsx
+++ b/components/shared/__tests__/backdrop.test.tsx
@@ -74,4 +74,29 @@ describe('Backdrop', () => {
     expect(wrapper.html()).toMatchSnapshot()
     expect(notOffset).not.toEqual(wrapper.html())
   })
+
+  it('backdrop handler should ignore click events from content', async () => {
+    const handler = jest.fn()
+    const wrapper = mount(
+      <Backdrop visible onClick={handler}>
+        <span>test-value</span>
+      </Backdrop>,
+    )
+
+    /**
+     * In simulation,`mousedown` and `mouseup`not directly triiger `click` event,
+     * the click event below is just for simulation.
+     */
+    wrapper.find('.content').simulate('mousedown')
+    wrapper.find('.backdrop').simulate('click', nativeEvent)
+    wrapper.find('.backdrop').simulate('mouseup')
+    await updateWrapper(wrapper)
+    expect(handler).not.toHaveBeenCalled()
+
+    wrapper.find('.backdrop').simulate('mousedown')
+    wrapper.find('.backdrop').simulate('click', nativeEvent)
+    wrapper.find('.backdrop').simulate('mouseup')
+    await updateWrapper(wrapper)
+    expect(handler).toHaveBeenCalled()
+  })
 })