refactor: Remove custom hit testing in usePress (#7427)

* refactor: Remove custom hit testing in usePress

* Only allow dragging to select an item with mouse, not touch

* Don't close when dragging outside submenu

Author: devongovett

packages/@react-aria/interactions/src/usePress.ts

@@ -433,7 +433,13 @@ export function usePress(props: PressHookProps): PressResult {
 
           shouldStopPropagation = triggerPressStart(e, state.pointerType);
 
-          addGlobalListener(getOwnerDocument(e.currentTarget), 'pointermove', onPointerMove, false);
+          // Release pointer capture so that touch interactions can leave the original target.
+          // This enables onPointerLeave and onPointerEnter to fire.
+          let target = e.target as Element;
+          if ('releasePointerCapture' in target) {
+            target.releasePointerCapture(e.pointerId);
+          }
+
           addGlobalListener(getOwnerDocument(e.currentTarget), 'pointerup', onPointerUp, false);
           addGlobalListener(getOwnerDocument(e.currentTarget), 'pointercancel', onPointerCancel, false);
         }
@@ -467,27 +473,20 @@ export function usePress(props: PressHookProps): PressResult {
         }
 
         // Only handle left clicks
-        // Safari on iOS sometimes fires pointerup events, even
-        // when the touch isn't over the target, so double check.
-        if (e.button === 0 && isOverTarget(e, e.currentTarget)) {
+        if (e.button === 0) {
           triggerPressUp(e, state.pointerType || e.pointerType);
         }
       };
 
-      // Safari on iOS < 13.2 does not implement pointerenter/pointerleave events correctly.
-      // Use pointer move events instead to implement our own hit testing.
-      // See https://bugs.webkit.org/show_bug.cgi?id=199803
-      let onPointerMove = (e: PointerEvent) => {
-        if (e.pointerId !== state.activePointerId) {
-          return;
+      pressProps.onPointerEnter = (e) => {
+        if (e.pointerId === state.activePointerId && state.target && !state.isOverTarget && state.pointerType != null) {
+          state.isOverTarget = true;
+          triggerPressStart(createEvent(state.target, e), state.pointerType);
         }
+      };
 
-        if (state.target && isOverTarget(e, state.target)) {
-          if (!state.isOverTarget && state.pointerType != null) {
-            state.isOverTarget = true;
-            triggerPressStart(createEvent(state.target, e), state.pointerType);
-          }
-        } else if (state.target && state.isOverTarget && state.pointerType != null) {
+      pressProps.onPointerLeave = (e) => {
+        if (e.pointerId === state.activePointerId && state.target && state.isOverTarget && state.pointerType != null) {
           state.isOverTarget = false;
           triggerPressEnd(createEvent(state.target, e), state.pointerType, false);
           cancelOnPointerExit(e);
@@ -496,7 +495,7 @@ export function usePress(props: PressHookProps): PressResult {
 
       let onPointerUp = (e: PointerEvent) => {
         if (e.pointerId === state.activePointerId && state.isPressed && e.button === 0 && state.target) {
-          if (isOverTarget(e, state.target) && state.pointerType != null) {
+          if (state.target.contains(e.target as Element) && state.pointerType != null) {
             triggerPressEnd(createEvent(state.target, e), state.pointerType);
           } else if (state.isOverTarget && state.pointerType != null) {
             triggerPressEnd(createEvent(state.target, e), state.pointerType, false);

packages/@react-aria/interactions/test/usePress.test.js

@@ -141,10 +141,15 @@ describe('usePress', function () {
       );
 
       let el = res.getByText('test');
+      el.releasePointerCapture = jest.fn();
       fireEvent(el, pointerEvent('pointerdown', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
+      expect(el.releasePointerCapture).toHaveBeenCalled();
       fireEvent(el, pointerEvent('pointermove', {pointerId: 1, pointerType: 'mouse', clientX: 100, clientY: 100}));
-      fireEvent(el, pointerEvent('pointerup', {pointerId: 1, pointerType: 'mouse', clientX: 100, clientY: 100}));
+      // react listens for pointerout and pointerover instead of pointerleave and pointerenter...
+      fireEvent(el, pointerEvent('pointerout', {pointerId: 1, pointerType: 'mouse', clientX: 100, clientY: 100}));
+      fireEvent(document, pointerEvent('pointerup', {pointerId: 1, pointerType: 'mouse', clientX: 100, clientY: 100}));
       fireEvent(el, pointerEvent('pointermove', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
+      fireEvent(el, pointerEvent('pointerover', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
 
       expect(events).toEqual([
         {
@@ -182,7 +187,10 @@ describe('usePress', function () {
       events = [];
       fireEvent(el, pointerEvent('pointerdown', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
       fireEvent(el, pointerEvent('pointermove', {pointerId: 1, pointerType: 'mouse', clientX: 100, clientY: 100}));
+      // react listens for pointerout and pointerover instead of pointerleave and pointerenter...
+      fireEvent(el, pointerEvent('pointerout', {pointerId: 1, pointerType: 'mouse', clientX: 100, clientY: 100}));
       fireEvent(el, pointerEvent('pointermove', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
+      fireEvent(el, pointerEvent('pointerover', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
       fireEvent(el, pointerEvent('pointerup', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
 
       expect(events).toEqual([
@@ -387,7 +395,9 @@ describe('usePress', function () {
       let el = res.getByText('test');
       fireEvent(el, pointerEvent('pointerdown', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
       fireEvent(el, pointerEvent('pointermove', {pointerId: 1, pointerType: 'mouse', clientX: 100, clientY: 100}));
+      fireEvent(el, pointerEvent('pointerout', {pointerId: 1, pointerType: 'mouse', clientX: 100, clientY: 100}));
       fireEvent(el, pointerEvent('pointermove', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
+      fireEvent(el, pointerEvent('pointerover', {pointerId: 1, pointerType: 'mouse', clientX: 0, clientY: 0}));
 
       expect(events).toEqual([
         {

packages/@react-aria/menu/src/useMenuItem.ts

@@ -110,7 +110,7 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
     'aria-haspopup': hasPopup,
     onPressStart: pressStartProp,
     onPressUp: pressUpProp,
-    onPress,
+    onPress: pressProp,
     onPressChange,
     onPressEnd,
     onHoverStart: hoverStartProp,
@@ -196,20 +196,34 @@ export function useMenuItem<T>(props: AriaMenuItemProps, state: TreeState<T>, re
     pressStartProp?.(e);
   };
 
+  let maybeClose = () => {
+    // Pressing a menu item should close by default in single selection mode but not multiple
+    // selection mode, except if overridden by the closeOnSelect prop.
+    if (!isTrigger && onClose && (closeOnSelect ?? (selectionManager.selectionMode !== 'multiple' || selectionManager.isLink(key)))) {
+      onClose();
+    }
+  };
+
   let onPressUp = (e: PressEvent) => {
-    if (e.pointerType !== 'keyboard') {
+    // If interacting with mouse, allow the user to mouse down on the trigger button,
+    // drag, and release over an item (matching native behavior).
+    if (e.pointerType === 'mouse') {
       performAction(e);
-
-      // Pressing a menu item should close by default in single selection mode but not multiple
-      // selection mode, except if overridden by the closeOnSelect prop.
-      if (!isTrigger && onClose && (closeOnSelect ?? (selectionManager.selectionMode !== 'multiple' || selectionManager.isLink(key)))) {
-        onClose();
-      }
+      maybeClose();
     }
 
     pressUpProp?.(e);
   };
 
+  let onPress = (e: PressEvent) => {
+    if (e.pointerType !== 'keyboard' && e.pointerType !== 'mouse') {
+      performAction(e);
+      maybeClose();
+    }
+
+    pressProp?.(e);
+  };
+
   let {itemProps, isFocused} = useSelectableItem({
     selectionManager: selectionManager,
     key,

packages/@react-aria/selection/src/useSelectableItem.ts

@@ -241,7 +241,7 @@ export function useSelectableItem(options: SelectableItemOptions): SelectableIte
       }
     };
 
-    // If allowsDifferentPressOrigin, make selection happen on pressUp (e.g. open menu on press down, selection on menu item happens on press up.)
+    // If allowsDifferentPressOrigin and interacting with mouse, make selection happen on pressUp (e.g. open menu on press down, selection on menu item happens on press up.)
     // Otherwise, have selection happen onPress (prevents listview row selection when clicking on interactable elements in the row)
     if (!allowsDifferentPressOrigin) {
       itemPressProps.onPress = (e) => {
@@ -257,12 +257,16 @@ export function useSelectableItem(options: SelectableItemOptions): SelectableIte
       };
     } else {
       itemPressProps.onPressUp = hasPrimaryAction ? undefined : (e) => {
-        if (e.pointerType !== 'keyboard' && allowsSelection) {
+        if (e.pointerType === 'mouse' && allowsSelection) {
           onSelect(e);
         }
       };
 
-      itemPressProps.onPress = hasPrimaryAction ? performAction : undefined;
+      itemPressProps.onPress = hasPrimaryAction ? performAction : (e) => {
+        if (e.pointerType !== 'keyboard' && e.pointerType !== 'mouse' && allowsSelection) {
+          onSelect(e);
+        }
+      };
     }
   } else {
     itemPressProps.onPressStart = (e) => {

packages/@react-spectrum/menu/src/Menu.tsx

@@ -24,7 +24,6 @@ import {mergeProps, useLayoutEffect, useSlotId, useSyncRef} from '@react-aria/ut
 import React, {ReactElement, useContext, useEffect, useRef, useState} from 'react';
 import {SpectrumMenuProps} from '@react-types/menu';
 import styles from '@adobe/spectrum-css-temp/components/menu/vars.css';
-import {useInteractOutside} from '@react-aria/interactions';
 import {useLocale, useLocalizedStringFormatter} from '@react-aria/i18n';
 import {useMenu} from '@react-aria/menu';
 import {useTreeState} from '@react-stately/tree';
@@ -70,15 +69,6 @@ export const Menu = React.forwardRef(function Menu<T extends object>(props: Spec
     let nextMenuLevel = state.collection.getItem(nextMenuLevelKey);
     hasOpenSubmenu = nextMenuLevel != null;
   }
-  useInteractOutside({
-    ref: domRef,
-    onInteractOutside: (e) => {
-      if (!popoverContainer?.contains(e.target as Node) && !trayContainerRef.current?.contains(e.target as Node)) {
-        rootMenuTriggerState?.close();
-      }
-    },
-    isDisabled: isSubmenu || !hasOpenSubmenu
-  });
 
   return (
     <MenuStateContext.Provider value={{popoverContainer, trayContainerRef, menu: domRef, submenu: submenuRef, rootMenuTriggerState, state}}>

packages/@react-spectrum/menu/src/MenuTrigger.tsx

@@ -10,12 +10,12 @@
  * governing permissions and limitations under the License.
  */
 
-import {classNames, SlotProvider, useDOMRef, useIsMobileDevice} from '@react-spectrum/utils';
+import {classNames, SlotProvider, unwrapDOMRef, useDOMRef, useIsMobileDevice} from '@react-spectrum/utils';
 import {DOMRef} from '@react-types/shared';
 import {MenuContext} from './context';
 import {Placement} from '@react-types/overlays';
 import {Popover, Tray} from '@react-spectrum/overlays';
-import {PressResponder} from '@react-aria/interactions';
+import {PressResponder, useInteractOutside} from '@react-aria/interactions';
 import React, {forwardRef, Fragment, useRef} from 'react';
 import {SpectrumMenuTriggerProps} from '@react-types/menu';
 import styles from '@adobe/spectrum-css-temp/components/menu/vars.css';
@@ -74,17 +74,29 @@ export const MenuTrigger = forwardRef(function MenuTrigger(props: SpectrumMenuTr
     state
   };
 
+  // Close when clicking outside the root menu when a submenu is open.
+  let rootOverlayRef = useRef(null);
+  let rootOverlayDomRef = unwrapDOMRef(rootOverlayRef);
+  useInteractOutside({
+    ref: rootOverlayDomRef,
+    onInteractOutside: () => {
+      state?.close();
+    },
+    isDisabled: !state.isOpen || state.expandedKeysStack.length === 0
+  });
+
   // On small screen devices, the menu is rendered in a tray, otherwise a popover.
   let overlay;
   if (isMobile) {
     overlay = (
-      <Tray state={state} isFixedHeight>
+      <Tray state={state} isFixedHeight ref={rootOverlayRef}>
         {menu}
       </Tray>
     );
   } else {
     overlay = (
       <Popover
+        ref={rootOverlayRef}
         UNSAFE_style={{clipPath: 'unset', overflow: 'visible', filter: 'unset', borderWidth: '0px'}}
         state={state}
         triggerRef={menuTriggerRef}