feat(hooks): remove mouse and touch events on unmount (#1581)

* feat(hooks): remove mouse and touch events on unmount

* revert unrelated change

Author: silviuaavram

src/hooks/__tests__/utils.test.js

@@ -84,9 +84,7 @@ describe('utils', () => {
   describe('useMouseAndTouchTracker', () => {
     test('renders without error', () => {
       expect(() => {
-        renderHook(() =>
-          useMouseAndTouchTracker(false, [], undefined, jest.fn()),
-        )
+        renderHook(() => useMouseAndTouchTracker(undefined, [], jest.fn()))
       }).not.toThrowError()
     })
 
@@ -95,9 +93,11 @@ describe('utils', () => {
         addEventListener: jest.fn(),
         removeEventListener: jest.fn(),
       }
+      const refs = []
+      const handleBlur = jest.fn()
 
-      const {unmount, result} = renderHook(() =>
-        useMouseAndTouchTracker(false, [], environment, jest.fn()),
+      const {unmount, rerender, result} = renderHook(() =>
+        useMouseAndTouchTracker(environment, refs, handleBlur),
       )
 
       expect(environment.addEventListener).toHaveBeenCalledTimes(5)
@@ -123,6 +123,10 @@ describe('utils', () => {
       )
       expect(environment.removeEventListener).not.toHaveBeenCalled()
 
+      rerender()
+
+      expect(environment.removeEventListener).not.toHaveBeenCalled()
+
       unmount()
 
       expect(environment.addEventListener).toHaveBeenCalledTimes(5)
@@ -149,7 +153,9 @@ describe('utils', () => {
       )
 
       expect(result.current).toEqual({
-        current: {isMouseDown: false, isTouchMove: false, isTouchEnd: false},
+        isMouseDown: false,
+        isTouchMove: false,
+        isTouchEnd: false,
       })
     })
   })

src/hooks/useCombobox/index.js

@@ -116,17 +116,20 @@ function useCombobox(userProps = {}) {
       previousResultCountRef.current = items.length
     }
   })
-  // Add mouse/touch events to document.
-  const mouseAndTouchTrackersRef = useMouseAndTouchTracker(
-    isOpen,
-    [inputRef, menuRef, toggleButtonRef],
+  const mouseAndTouchTrackers = useMouseAndTouchTracker(
     environment,
-    () => {
-      dispatch({
-        type: stateChangeTypes.InputBlur,
-        selectItem: false,
-      })
-    },
+    [toggleButtonRef, menuRef, inputRef],
+    useCallback(
+      function handleBlur() {
+        if (latest.current.state.isOpen) {
+          dispatch({
+            type: stateChangeTypes.InputBlur,
+            selectItem: false,
+          })
+        }
+      },
+      [dispatch, latest],
+    ),
   )
   const setGetterPropCallInfo = useGetterPropsCalledChecker(
     'getInputProps',
@@ -309,7 +312,7 @@ function useCombobox(userProps = {}) {
 
       const itemHandleMouseMove = () => {
         if (
-          mouseAndTouchTrackersRef.current.isTouchEnd ||
+          mouseAndTouchTrackers.isTouchEnd ||
           index === latestState.highlightedIndex
         ) {
           return
@@ -353,7 +356,7 @@ function useCombobox(userProps = {}) {
       }
     },
 
-    [dispatch, elementIds, latest, mouseAndTouchTrackersRef, shouldScrollRef],
+    [dispatch, elementIds, latest, mouseAndTouchTrackers, shouldScrollRef],
   )
 
   const getToggleButtonProps = useCallback(
@@ -425,7 +428,7 @@ function useCombobox(userProps = {}) {
         if (
           environment?.document &&
           latestState.isOpen &&
-          !mouseAndTouchTrackersRef.current.isMouseDown
+          !mouseAndTouchTrackers.isMouseDown
         ) {
           const isBlurByTabChange =
             event.relatedTarget === null &&
@@ -501,13 +504,13 @@ function useCombobox(userProps = {}) {
       }
     },
     [
-      setGetterPropCallInfo,
-      latest,
-      elementIds,
-      inputKeyDownHandlers,
       dispatch,
-      mouseAndTouchTrackersRef,
+      elementIds,
       environment,
+      inputKeyDownHandlers,
+      latest,
+      mouseAndTouchTrackers,
+      setGetterPropCallInfo,
     ],
   )
 

src/hooks/useSelect/index.js

@@ -150,16 +150,20 @@ function useSelect(userProps = {}) {
     }
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [])
-  // Add mouse/touch events to document.
-  const mouseAndTouchTrackersRef = useMouseAndTouchTracker(
-    isOpen,
-    [menuRef, toggleButtonRef],
+
+  const mouseAndTouchTrackers = useMouseAndTouchTracker(
     environment,
-    () => {
-      dispatch({
-        type: stateChangeTypes.ToggleButtonBlur,
-      })
-    },
+    [toggleButtonRef, menuRef],
+    useCallback(
+      function handleBlur() {
+        if (latest.current.state.isOpen) {
+          dispatch({
+            type: stateChangeTypes.ToggleButtonBlur,
+          })
+        }
+      },
+      [dispatch, latest],
+    ),
   )
   const setGetterPropCallInfo = useGetterPropsCalledChecker(
     'getMenuProps',
@@ -365,10 +369,7 @@ function useSelect(userProps = {}) {
         })
       }
       const toggleButtonHandleBlur = () => {
-        if (
-          latestState.isOpen &&
-          !mouseAndTouchTrackersRef.current.isMouseDown
-        ) {
+        if (latestState.isOpen && !mouseAndTouchTrackers.isMouseDown) {
           dispatch({
             type: stateChangeTypes.ToggleButtonBlur,
           })
@@ -434,11 +435,11 @@ function useSelect(userProps = {}) {
       return toggleProps
     },
     [
-      latest,
+      dispatch,
       elementIds,
+      latest,
+      mouseAndTouchTrackers,
       setGetterPropCallInfo,
-      dispatch,
-      mouseAndTouchTrackersRef,
       toggleButtonKeyDownHandlers,
     ],
   )
@@ -472,7 +473,7 @@ function useSelect(userProps = {}) {
 
       const itemHandleMouseMove = () => {
         if (
-          mouseAndTouchTrackersRef.current.isTouchEnd ||
+          mouseAndTouchTrackers.isTouchEnd ||
           index === latestState.highlightedIndex
         ) {
           return
@@ -525,7 +526,7 @@ function useSelect(userProps = {}) {
 
       return itemProps
     },
-    [latest, elementIds, mouseAndTouchTrackersRef, shouldScrollRef, dispatch],
+    [latest, elementIds, mouseAndTouchTrackers, shouldScrollRef, dispatch],
   )
 
   return {

src/hooks/utils.js

@@ -74,9 +74,7 @@ function stateReducer(s, a) {
 function getA11ySelectionMessage(selectionParameters) {
   const {selectedItem, itemToString} = selectionParameters
 
-  return selectedItem
-    ? `${itemToString(selectedItem)} has been selected.`
-    : ''
+  return selectedItem ? `${itemToString(selectedItem)} has been selected.` : ''
 }
 
 /**
@@ -351,20 +349,17 @@ function getHighlightedIndexOnOpen(props, state, offset) {
   }
   return offset < 0 ? items.length - 1 : 0
 }
-
 /**
- * Reuse the movement tracking of mouse and touch events.
+ * Tracks mouse and touch events, such as mouseDown, touchMove and touchEnd.
  *
- * @param {boolean} isOpen Whether the dropdown is open or not.
- * @param {Array<Object>} downshiftElementRefs Downshift element refs to track movement (toggleButton, menu etc.)
- * @param {Object} environment Environment where component/hook exists.
- * @param {Function} handleBlur Handler on blur from mouse or touch.
- * @returns {Object} Ref containing whether mouseDown or touchMove event is happening
+ * @param {Object} environment The environment to add the event listeners to, for instance window.
+ * @param {Array<HTMLElement>} downshiftElementRefs The refs for the element that should not trigger a blur action from mouseDown or touchEnd.
+ * @param {Function} handleBlur The function that is called if mouseDown or touchEnd occured outside the downshiftElements.
+ * @returns {Object} The mouse and touch events information, if any of are happening.
  */
 function useMouseAndTouchTracker(
-  isOpen,
-  downshiftElementRefs,
   environment,
+  downshiftElementRefs,
   handleBlur,
 ) {
   const mouseAndTouchTrackersRef = useRef({
@@ -375,45 +370,39 @@ function useMouseAndTouchTracker(
 
   useEffect(() => {
     if (isReactNative || !environment) {
-      return
+      return noop
     }
 
-    // The same strategy for checking if a click occurred inside or outside downshift
-    // as in downshift.js.
-    const onMouseDown = () => {
+    const downshiftElements = downshiftElementRefs.map(ref => ref.current)
+
+    function onMouseDown() {
       mouseAndTouchTrackersRef.current.isTouchEnd = false // reset this one.
       mouseAndTouchTrackersRef.current.isMouseDown = true
     }
-    const onMouseUp = event => {
+    function onMouseUp(event) {
       mouseAndTouchTrackersRef.current.isMouseDown = false
 
       if (
-        isOpen &&
-        !targetWithinDownshift(
-          event.target,
-          downshiftElementRefs.map(ref => ref.current),
-          environment,
-        )
+        !targetWithinDownshift(event.target, downshiftElements, environment)
       ) {
         handleBlur()
       }
     }
-    const onTouchStart = () => {
+    function onTouchStart() {
       mouseAndTouchTrackersRef.current.isTouchEnd = false
       mouseAndTouchTrackersRef.current.isTouchMove = false
     }
-    const onTouchMove = () => {
+    function onTouchMove() {
       mouseAndTouchTrackersRef.current.isTouchMove = true
     }
-    const onTouchEnd = event => {
+    function onTouchEnd(event) {
       mouseAndTouchTrackersRef.current.isTouchEnd = true
 
       if (
-        isOpen &&
         !mouseAndTouchTrackersRef.current.isTouchMove &&
         !targetWithinDownshift(
           event.target,
-          downshiftElementRefs.map(ref => ref.current),
+          downshiftElements,
           environment,
           false,
         )
@@ -428,18 +417,17 @@ function useMouseAndTouchTracker(
     environment.addEventListener('touchmove', onTouchMove)
     environment.addEventListener('touchend', onTouchEnd)
 
-    // eslint-disable-next-line consistent-return
     return function cleanup() {
       environment.removeEventListener('mousedown', onMouseDown)
       environment.removeEventListener('mouseup', onMouseUp)
       environment.removeEventListener('touchstart', onTouchStart)
       environment.removeEventListener('touchmove', onTouchMove)
       environment.removeEventListener('touchend', onTouchEnd)
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [isOpen, environment])
+  // eslint-disable-next-line react-hooks/exhaustive-deps -- refs don't change
+  }, [environment, handleBlur])
 
-  return mouseAndTouchTrackersRef
+  return mouseAndTouchTrackersRef.current
 }
 
 /* istanbul ignore next */