[react-ui] Improve consistency of Tap responder

Makes sure that touch events with modifier keys behave the same way as other pointer types (i.e., does not call `onTapStart` if the gesture begins with a modifier key held down)
facebook · Sep 19, 2019 · 0123769 · 0123769
1 parent fd870e6
commit 0123769
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 31 deletions.
diff --git a/packages/react-ui/events/src/dom/Tap.js b/packages/react-ui/events/src/dom/Tap.js
@@ -350,23 +350,26 @@ function isActivePointer(
   }
 }
 
-function isAuxiliary(buttons: number, nativeEvent: any): boolean {
+function isAuxiliary(buttons: number, event: ReactDOMResponderEvent): boolean {
+  const nativeEvent: any = event.nativeEvent;
+  const isPrimaryPointer =
+    buttons === buttonsEnum.primary || event.pointerType === 'touch';
   return (
     // middle-click
     buttons === buttonsEnum.auxiliary ||
     // open-in-new-tab
-    (buttons === buttonsEnum.primary && nativeEvent.metaKey) ||
+    (isPrimaryPointer && nativeEvent.metaKey) ||
     // open-in-new-window
-    (buttons === buttonsEnum.primary && nativeEvent.shiftKey)
+    (isPrimaryPointer && nativeEvent.shiftKey)
   );
 }
 
 function shouldActivate(event: ReactDOMResponderEvent): boolean {
   const nativeEvent: any = event.nativeEvent;
-  const pointerType = event.pointerType;
-  const buttons = nativeEvent.buttons;
-  const isValidButton = buttons === buttonsEnum.primary;
-  return pointerType === 'touch' || (isValidButton && !hasModifierKey(event));
+  const isPrimaryPointer =
+    nativeEvent.buttons === buttonsEnum.primary ||
+    event.pointerType === 'touch';
+  return isPrimaryPointer && !hasModifierKey(event);
 }
 
 /**
@@ -514,10 +517,7 @@ const responderImpl = {
 
         if (!state.isActive) {
           const activate = shouldActivate(event);
-          const activateAuxiliary = isAuxiliary(
-            nativeEvent.buttons,
-            nativeEvent,
-          );
+          const activateAuxiliary = isAuxiliary(nativeEvent.buttons, event);
 
           if (activate || activateAuxiliary) {
             state.buttons = nativeEvent.buttons;
@@ -531,7 +531,9 @@ const responderImpl = {
             }
           }
 
-          if (activate) {
+          if (activateAuxiliary) {
+            state.isAuxiliaryActive = true;
+          } else if (activate) {
             const gestureState = createGestureState(
               context,
               props,
@@ -544,8 +546,6 @@ const responderImpl = {
             state.initialPosition.x = gestureState.x;
             state.initialPosition.y = gestureState.y;
             dispatchStart(context, props, state);
-          } else if (activateAuxiliary) {
-            state.isAuxiliaryActive = true;
           }
         }
         break;
@@ -611,7 +611,7 @@ const responderImpl = {
         if (state.isActive && isActivePointer(event, state)) {
           state.gestureState = createGestureState(context, props, state, event);
           state.isActive = false;
-          if (isAuxiliary(state.buttons, nativeEvent)) {
+          if (isAuxiliary(state.buttons, event)) {
             dispatchCancel(context, props, state);
             dispatchAuxiliaryTap(context, props, state);
             // Remove the root events here as no 'click' event is dispatched
@@ -626,7 +626,7 @@ const responderImpl = {
           }
         } else if (
           state.isAuxiliaryActive &&
-          isAuxiliary(state.buttons, nativeEvent)
+          isAuxiliary(state.buttons, event)
         ) {
           state.isAuxiliaryActive = false;
           state.gestureState = createGestureState(context, props, state, event);

diff --git a/packages/react-ui/events/src/dom/__tests__/Tap-test.internal.js b/packages/react-ui/events/src/dom/__tests__/Tap-test.internal.js
@@ -302,29 +302,31 @@ describeWithPointerEvent('Tap responder', hasPointerEvents => {
       expect(onTapStart).toHaveBeenCalledTimes(1);
     });
 
-    test('ignored buttons and modifiers', () => {
+    testWithPointerType('ignored buttons and modifiers', pointerType => {
       const target = createEventTarget(ref.current);
-      const primary = buttonsType.primary;
-      // right-click
-      target.pointerdown({buttons: buttonsType.secondary});
-      target.pointerup();
-      // middle-click
-      target.pointerdown({buttons: buttonsType.auxiliary});
-      target.pointerup();
-      // pen eraser
-      target.pointerdown({buttons: buttonsType.eraser});
-      target.pointerup();
+      const {auxiliary, eraser, primary, secondary} = buttonsType;
+      if (pointerType !== 'touch') {
+        // right-click
+        target.pointerdown({buttons: secondary, pointerType});
+        target.pointerup();
+        // middle-click
+        target.pointerdown({buttons: auxiliary, pointerType});
+        target.pointerup();
+        // pen eraser
+        target.pointerdown({buttons: eraser, pointerType});
+        target.pointerup();
+      }
       // alt-click
-      target.pointerdown({buttons: primary, altKey: true});
+      target.pointerdown({buttons: primary, altKey: true, pointerType});
       target.pointerup();
       // ctrl-click
-      target.pointerdown({buttons: primary, ctrlKey: true});
+      target.pointerdown({buttons: primary, ctrlKey: true, pointerType});
       target.pointerup();
       // meta-click
-      target.pointerdown({buttons: primary, metaKey: true});
+      target.pointerdown({buttons: primary, metaKey: true, pointerType});
       target.pointerup();
       // shift-click
-      target.pointerdown({buttons: primary, shiftKey: true});
+      target.pointerdown({buttons: primary, shiftKey: true, pointerType});
       target.pointerup();
 
       expect(onTapStart).toHaveBeenCalledTimes(0);