fix(onMouseMove): only call handler on non-touch devices (#1571)

downshift-js · Feb 21, 2024 · dc9da74 · dc9da74
1 parent a5d6310
commit dc9da74
Show file tree

Hide file tree

Showing 10 changed files with 172 additions and 92 deletions.
diff --git a/src/hooks/__tests__/utils.test.js b/src/hooks/__tests__/utils.test.js
@@ -149,7 +149,7 @@ describe('utils', () => {
       )
 
       expect(result.current).toEqual({
-        current: {isMouseDown: false, isTouchMove: false},
+        current: {isMouseDown: false, isTouchMove: false, isTouchEnd: false},
       })
     })
   })

diff --git a/src/hooks/useCombobox/__tests__/getItemProps.test.js b/src/hooks/useCombobox/__tests__/getItemProps.test.js
@@ -280,6 +280,37 @@ describe('getItemProps', () => {
         await mouseMoveItemAtIndex(disabledIndex)
         expect(input).toHaveAttribute('aria-activedescendant', '')
       })
+
+      // Test that we don't call the mouse move handler on mobile.
+      test('does not set highlight the item if a touch event ocurred', async () => {
+        let touchEndHandler
+        const index = 1
+
+        renderCombobox({
+          isOpen: true,
+          environment: {
+            addEventListener: (name, handler) => {
+              // eslint-disable-next-line jest/no-conditional-in-test
+              if (name === 'touchend') {
+                touchEndHandler = handler
+              }
+            },
+            removeEventListener: () => {},
+            document: {
+              createElement: () => {},
+              getElementById: () => {},
+              activeElement: () => {},
+              body: {},
+            },
+            Node: () => {},
+          },
+        })
+
+        act(() => touchEndHandler({target: null}))
+        await mouseMoveItemAtIndex(index)
+
+        expect(getInput()).toHaveAttribute('aria-activedescendant', '')
+      })
     })
 
     describe('on click', () => {

diff --git a/src/hooks/useCombobox/__tests__/props.test.js b/src/hooks/useCombobox/__tests__/props.test.js
@@ -93,6 +93,7 @@ describe('props', () => {
 
   describe('selectedItemChanged', () => {
     test('props update of selectedItem will update inputValue state with default selectedItemChanged referential equality check', () => {
+      const initialSelectedItem = {id: 3, value: 'init'}
       const selectedItem = {id: 1, value: 'wow'}
       const newSelectedItem = {id: 1, value: 'not wow'}
       function itemToString(item) {
@@ -103,6 +104,14 @@ describe('props', () => {
         .mockImplementation((_state, {changes}) => changes)
 
       const {rerender} = renderCombobox({
+        stateReducer,
+        itemToString,
+        selectedItem: initialSelectedItem,
+      })
+
+      expect(stateReducer).not.toHaveBeenCalled() // won't get called on first render
+
+      rerender({
         stateReducer,
         itemToString,
         selectedItem,
@@ -111,7 +120,7 @@ describe('props', () => {
       expect(stateReducer).toHaveBeenCalledTimes(1)
       expect(stateReducer).toHaveBeenCalledWith(
         {
-          inputValue: itemToString(selectedItem),
+          inputValue: itemToString(initialSelectedItem),
           selectedItem,
           highlightedIndex: -1,
           isOpen: false,
@@ -155,46 +164,39 @@ describe('props', () => {
       expect(getInput()).toHaveValue(itemToString(newSelectedItem))
     })
 
-    test('props update of selectedItem will not update inputValue state', () => {
+    test('props update of selectedItem will not update inputValue state if selectedItemChanged returns false', () => {
+      const initialSelectedItem = {id: 1, value: 'hmm'}
       const selectedItem = {id: 1, value: 'wow'}
-      const newSelectedItem = {id: 1, value: 'not wow'}
       function itemToString(item) {
         return item.value
       }
       const selectedItemChanged = jest
         .fn()
-        .mockReturnValue((prev, next) => prev.id !== next.id)
+        .mockImplementation((prev, next) => prev.id !== next.id)
       const stateReducer = jest
         .fn()
         .mockImplementation((_state, {changes}) => changes)
 
       const {rerender} = renderCombobox({
         selectedItemChanged,
         stateReducer,
-        selectedItem,
+        selectedItem: initialSelectedItem,
         itemToString,
       })
 
-      expect(getInput()).toHaveValue(itemToString(selectedItem))
-      expect(selectedItemChanged).toHaveBeenCalledTimes(1)
-      expect(selectedItemChanged).toHaveBeenCalledWith(undefined, selectedItem)
-
-      stateReducer.mockReset()
-      selectedItemChanged.mockReset()
       rerender({
+        selectedItemChanged,
         stateReducer,
+        selectedItem,
         itemToString,
-        selectedItem: newSelectedItem,
-        selectedItemChanged,
       })
 
+      expect(getInput()).toHaveValue(itemToString(initialSelectedItem))
       expect(selectedItemChanged).toHaveBeenCalledTimes(1)
       expect(selectedItemChanged).toHaveBeenCalledWith(
+        initialSelectedItem,
         selectedItem,
-        newSelectedItem,
       )
-      expect(stateReducer).not.toHaveBeenCalled()
-      expect(getInput()).toHaveValue(itemToString(selectedItem))
     })
   })
 

diff --git a/src/hooks/useCombobox/index.js b/src/hooks/useCombobox/index.js
@@ -11,7 +11,8 @@ import {
   useElementIds,
   getItemAndIndex,
   getInitialValue,
-  isDropdownsStateEqual
+  isDropdownsStateEqual,
+  useIsInitialMount,
 } from '../utils'
 import {
   getInitialState,
@@ -44,7 +45,7 @@ function useCombobox(userProps = {}) {
     downshiftUseComboboxReducer,
     props,
     getInitialState,
-    isDropdownsStateEqual
+    isDropdownsStateEqual,
   )
   const {isOpen, highlightedIndex, selectedItem, inputValue} = state
 
@@ -53,7 +54,8 @@ function useCombobox(userProps = {}) {
   const itemRefs = useRef({})
   const inputRef = useRef(null)
   const toggleButtonRef = useRef(null)
-  const isInitialMountRef = useRef(true)
+  const isInitialMount = useIsInitialMount()
+
   // prevent id re-generation between renders.
   const elementIds = useElementIds(props)
   // used to keep track of how many items we had on previous cycle.
@@ -72,7 +74,6 @@ function useCombobox(userProps = {}) {
     getA11yStatusMessage,
     [isOpen, highlightedIndex, inputValue, items],
     {
-      isInitialMount: isInitialMountRef.current,
       previousResultCount: previousResultCountRef.current,
       items,
       environment,
@@ -82,7 +83,6 @@ function useCombobox(userProps = {}) {
   )
   // Sets a11y status message on changes in selectedItem.
   useA11yMessageSetter(getA11ySelectionMessage, [selectedItem], {
-    isInitialMount: isInitialMountRef.current,
     previousResultCount: previousResultCountRef.current,
     items,
     environment,
@@ -99,7 +99,6 @@ function useCombobox(userProps = {}) {
     getItemNodeFromIndex,
   })
   useControlPropsValidator({
-    isInitialMount: isInitialMountRef.current,
     props,
     state,
   })
@@ -113,11 +112,9 @@ function useCombobox(userProps = {}) {
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [])
   useEffect(() => {
-    if (isInitialMountRef.current) {
-      return
+    if (!isInitialMount) {
+      previousResultCountRef.current = items.length
     }
-
-    previousResultCountRef.current = items.length
   })
   // Add mouse/touch events to document.
   const mouseAndTouchTrackersRef = useMouseAndTouchTracker(
@@ -135,14 +132,6 @@ function useCombobox(userProps = {}) {
     'getInputProps',
     'getMenuProps',
   )
-  // Make initial ref false.
-  useEffect(() => {
-    isInitialMountRef.current = false
-
-    return () => {
-      isInitialMountRef.current = true
-    }
-  }, [])
   // Reset itemRefs on close.
   useEffect(() => {
     if (!isOpen) {
@@ -319,10 +308,15 @@ function useCombobox(userProps = {}) {
         : onClick
 
       const itemHandleMouseMove = () => {
-        if (index === latestState.highlightedIndex) {
+        if (
+          mouseAndTouchTrackersRef.current.isTouchEnd ||
+          index === latestState.highlightedIndex
+        ) {
           return
         }
+
         shouldScrollRef.current = false
+
         dispatch({
           type: stateChangeTypes.ItemMouseMove,
           index,
@@ -358,7 +352,8 @@ function useCombobox(userProps = {}) {
         ...rest,
       }
     },
-    [dispatch, latest, shouldScrollRef, elementIds],
+
+    [dispatch, elementIds, latest, mouseAndTouchTrackersRef, shouldScrollRef],
   )
 
   const getToggleButtonProps = useCallback(

diff --git a/src/hooks/useCombobox/utils.js b/src/hooks/useCombobox/utils.js
@@ -11,6 +11,7 @@ import {
   defaultProps as defaultPropsCommon,
   getInitialState as getInitialStateCommon,
   useEnhancedReducer,
+  useIsInitialMount,
 } from '../utils'
 import {ControlledPropUpdatedSelectedItem} from './stateChangeTypes'
 
@@ -61,22 +62,28 @@ const propTypes = {
  * @param {Function} isStateEqual Function that checks if a previous state is equal to the next.
  * @returns {Array} An array with the state and an action dispatcher.
  */
-export function useControlledReducer(reducer, props, createInitialState, isStateEqual) {
+export function useControlledReducer(
+  reducer,
+  props,
+  createInitialState,
+  isStateEqual,
+) {
   const previousSelectedItemRef = useRef()
   const [state, dispatch] = useEnhancedReducer(
     reducer,
     props,
     createInitialState,
-    isStateEqual
+    isStateEqual,
   )
+  const isInitialMount = useIsInitialMount()
 
-  // ToDo: if needed, make same approach as selectedItemChanged from Downshift.
   useEffect(() => {
     if (!isControlledProp(props, 'selectedItem')) {
       return
     }
 
     if (
+      !isInitialMount && // on first mount we already have the proper inputValue for a initial selected item.
       props.selectedItemChanged(
         previousSelectedItemRef.current,
         props.selectedItem,

diff --git a/src/hooks/useMultipleSelection/index.js b/src/hooks/useMultipleSelection/index.js
@@ -8,13 +8,14 @@ import {
   useLatestRef,
   useControlPropsValidator,
   getItemAndIndex,
+  useIsInitialMount,
 } from '../utils'
 import {
   getInitialState,
   defaultProps,
   isKeyDownOperationPermitted,
   validatePropTypes,
-  isStateEqual
+  isStateEqual,
 } from './utils'
 import downshiftMultipleSelectionReducer from './reducer'
 import * as stateChangeTypes from './stateChangeTypes'
@@ -41,12 +42,12 @@ function useMultipleSelection(userProps = {}) {
     downshiftMultipleSelectionReducer,
     props,
     getInitialState,
-    isStateEqual
+    isStateEqual,
   )
   const {activeIndex, selectedItems} = state
 
   // Refs.
-  const isInitialMountRef = useRef(true)
+  const isInitialMount = useIsInitialMount()
   const dropdownRef = useRef(null)
   const previousSelectedItemsRef = useRef(selectedItems)
   const selectedItemRefs = useRef()
@@ -56,7 +57,7 @@ function useMultipleSelection(userProps = {}) {
   // Effects.
   /* Sets a11y status message on changes in selectedItem. */
   useEffect(() => {
-    if (isInitialMountRef.current || isReactNative || !environment?.document) {
+    if (isInitialMount || isReactNative || !environment?.document) {
       return
     }
 
@@ -83,7 +84,7 @@ function useMultipleSelection(userProps = {}) {
   }, [selectedItems.length])
   // Sets focus on active item.
   useEffect(() => {
-    if (isInitialMountRef.current) {
+    if (isInitialMount) {
       return
     }
 
@@ -92,21 +93,12 @@ function useMultipleSelection(userProps = {}) {
     } else if (selectedItemRefs.current[activeIndex]) {
       selectedItemRefs.current[activeIndex].focus()
     }
-  }, [activeIndex])
+  }, [activeIndex, isInitialMount])
   useControlPropsValidator({
-    isInitialMount: isInitialMountRef.current,
     props,
     state,
   })
   const setGetterPropCallInfo = useGetterPropsCalledChecker('getDropdownProps')
-  // Make initial ref false.
-  useEffect(() => {
-    isInitialMountRef.current = false
-
-    return () => {
-      isInitialMountRef.current = true
-    }
-  }, [])
 
   // Event handler functions.
   const selectedItemKeyDownHandlers = useMemo(