Pressable click fix (facebook#18625)

nmn · web-flow · commit 5f7b175b35fa · 2020-04-20T23:13:17.000+01:00
* Update press-legacy to use native click events

* update tests for pressable change

* fix formatting issue

* Address comments. Bring back some tests, remove others. Cleanup

* Fix flow and lint errors

* formatting fix missed by yarn lint
diff --git a/packages/react-dom/src/shared/ReactDOMTypes.js b/packages/react-dom/src/shared/ReactDOMTypes.js
@@ -16,7 +16,7 @@ import type {
 } from 'shared/ReactTypes';
 import type {DOMTopLevelEventType} from 'legacy-events/TopLevelEventTypes';
 
-type AnyNativeEvent = Event | KeyboardEvent | MouseEvent | Touch;
+type AnyNativeEvent = Event | KeyboardEvent | MouseEvent | TouchEvent;
 
 export type PointerType =
   | ''
diff --git a/packages/react-interactions/events/src/dom/PressLegacy.js b/packages/react-interactions/events/src/dom/PressLegacy.js
@@ -64,7 +64,6 @@ type PressState = {
   |}>,
   ignoreEmulatedMouseEvents: boolean,
   activePointerId: null | number,
-  shouldPreventClick: boolean,
   touchEvent: null | Touch,
   ...
 };
@@ -199,7 +198,6 @@ function createPressEvent(
     x: clientX,
     y: clientY,
     preventDefault() {
-      state.shouldPreventClick = true;
       if (nativeEvent) {
         pressEvent.defaultPrevented = true;
         nativeEvent.preventDefault();
@@ -229,8 +227,7 @@ function dispatchEvent(
   const target = ((state.pressTarget: any): Element | Document);
   const pointerType = state.pointerType;
   const defaultPrevented =
-    (event != null && event.nativeEvent.defaultPrevented === true) ||
-    (name === 'press' && state.shouldPreventClick);
+    event != null && event.nativeEvent.defaultPrevented === true;
   const touchEvent = state.touchEvent;
   const syntheticEvent = createPressEvent(
     context,
@@ -529,7 +526,6 @@ const pressResponderImpl = {
       responderRegionOnDeactivation: null,
       ignoreEmulatedMouseEvents: false,
       activePointerId: null,
-      shouldPreventClick: false,
       touchEvent: null,
     };
   },
@@ -567,7 +563,6 @@ const pressResponderImpl = {
             return;
           }
 
-          state.shouldPreventClick = false;
           if (isTouchEvent) {
             state.ignoreEmulatedMouseEvents = true;
           } else if (isKeyboardEvent) {
@@ -587,7 +582,6 @@ const pressResponderImpl = {
                 !altKey
               ) {
                 nativeEvent.preventDefault();
-                state.shouldPreventClick = true;
               }
             } else {
               return;
@@ -645,9 +639,6 @@ const pressResponderImpl = {
       }
 
       case 'click': {
-        if (state.shouldPreventClick) {
-          nativeEvent.preventDefault();
-        }
         const onPress = props.onPress;
 
         if (isFunction(onPress) && isScreenReaderVirtualClick(nativeEvent)) {
@@ -751,7 +742,6 @@ const pressResponderImpl = {
       case 'touchend': {
         if (isPressed) {
           const buttons = state.buttons;
-          let isKeyboardEvent = false;
           let touchEvent;
           if (
             type === 'pointerup' &&
@@ -770,79 +760,13 @@ const pressResponderImpl = {
             if (!isValidKeyboardEvent(nativeEvent)) {
               return;
             }
-            isKeyboardEvent = true;
             removeRootEventTypes(context, state);
           } else if (buttons === 4) {
             // Remove the root events here as no 'click' event is dispatched when this 'button' is pressed.
             removeRootEventTypes(context, state);
           }
 
-          // Determine whether to call preventDefault on subsequent native events.
-          if (
-            target !== null &&
-            context.isTargetWithinResponder(target) &&
-            context.isTargetWithinHostComponent(target, 'a')
-          ) {
-            const {
-              altKey,
-              ctrlKey,
-              metaKey,
-              shiftKey,
-            } = (nativeEvent: MouseEvent);
-            // Check "open in new window/tab" and "open context menu" key modifiers
-            const preventDefault = props.preventDefault;
-
-            if (
-              preventDefault !== false &&
-              !shiftKey &&
-              !metaKey &&
-              !ctrlKey &&
-              !altKey
-            ) {
-              state.shouldPreventClick = true;
-            }
-          }
-
-          const pressTarget = state.pressTarget;
           dispatchPressEndEvents(event, context, props, state);
-          const onPress = props.onPress;
-
-          if (pressTarget !== null && isFunction(onPress)) {
-            if (
-              !isKeyboardEvent &&
-              pressTarget !== null &&
-              target !== null &&
-              !targetIsDocument(pressTarget)
-            ) {
-              if (
-                pointerType === 'mouse' &&
-                context.isTargetWithinNode(target, pressTarget)
-              ) {
-                state.isPressWithinResponderRegion = true;
-              } else {
-                // If the event target isn't within the press target, check if we're still
-                // within the responder region. The region may have changed if the
-                // element's layout was modified after activation.
-                updateIsPressWithinResponderRegion(
-                  touchEvent || nativeEvent,
-                  context,
-                  props,
-                  state,
-                );
-              }
-            }
-
-            if (state.isPressWithinResponderRegion && buttons !== 4) {
-              dispatchEvent(
-                event,
-                onPress,
-                context,
-                state,
-                'press',
-                DiscreteEvent,
-              );
-            }
-          }
           state.touchEvent = null;
         } else if (type === 'mouseup') {
           state.ignoreEmulatedMouseEvents = false;
@@ -855,6 +779,12 @@ const pressResponderImpl = {
         if (previousPointerType !== 'keyboard') {
           removeRootEventTypes(context, state);
         }
+
+        const pressTarget = state.pressTarget;
+        const onPress = props.onPress;
+        if (pressTarget !== null && isFunction(onPress)) {
+          dispatchEvent(event, onPress, context, state, 'press', DiscreteEvent);
+        }
         break;
       }
 
diff --git a/packages/react-interactions/events/src/dom/__tests__/PressLegacy-test.internal.js b/packages/react-interactions/events/src/dom/__tests__/PressLegacy-test.internal.js
@@ -469,15 +469,12 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => {
     });
 
     // @gate experimental
-    it('is called after valid "keyup" event', () => {
+    it('is called after valid "click" event', () => {
       componentInit();
       const target = createEventTarget(ref.current);
-      target.keydown({key: 'Enter'});
-      target.keyup({key: 'Enter'});
+      target.pointerdown();
+      target.pointerup();
       expect(onPress).toHaveBeenCalledTimes(1);
-      expect(onPress).toHaveBeenCalledWith(
-        expect.objectContaining({pointerType: 'keyboard', type: 'press'}),
-      );
     });
 
     // @gate experimental
@@ -804,40 +801,6 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => {
       });
     });
 
-    describe('beyond bounds of hit rect', () => {
-      /** ┌──────────────────┐
-       *  │  ┌────────────┐  │
-       *  │  │ VisualRect │  │
-       *  │  └────────────┘  │
-       *  │     HitRect      │
-       *  └──────────────────┘
-       *                   X   <= Move to X and release
-       */
-      // @gate experimental
-      it('"onPress" is not called on release', () => {
-        componentInit();
-        const target = createEventTarget(ref.current);
-        const targetContainer = createEventTarget(container);
-        target.setBoundingClientRect(rectMock);
-        target.pointerdown({pointerType});
-        target.pointermove({pointerType, ...coordinatesInside});
-        if (pointerType === 'mouse') {
-          // TODO: use setPointerCapture so this is only true for fallback mouse events.
-          targetContainer.pointermove({pointerType, ...coordinatesOutside});
-          targetContainer.pointerup({pointerType, ...coordinatesOutside});
-        } else {
-          target.pointermove({pointerType, ...coordinatesOutside});
-          target.pointerup({pointerType, ...coordinatesOutside});
-        }
-        expect(events.filter(removePressMoveStrings)).toEqual([
-          'onPressStart',
-          'onPressChange',
-          'onPressEnd',
-          'onPressChange',
-        ]);
-      });
-    });
-
     // @gate experimental
     it('"onPress" is called on re-entry to hit rect', () => {
       componentInit();
@@ -926,8 +889,8 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => {
           'pointerdown',
           'inner: onPressEnd',
           'inner: onPressChange',
-          'inner: onPress',
           'pointerup',
+          'inner: onPress',
         ]);
       });
     }
@@ -1023,28 +986,6 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => {
     // @gate experimental
     it('prevents native behavior by default', () => {
       const onPress = jest.fn();
-      const preventDefault = jest.fn();
-      const ref = React.createRef();
-
-      const Component = () => {
-        const listener = usePress({onPress});
-        return <a href="#" ref={ref} DEPRECATED_flareListeners={listener} />;
-      };
-      ReactDOM.render(<Component />, container);
-
-      const target = createEventTarget(ref.current);
-      target.pointerdown();
-      target.pointerup({preventDefault});
-      expect(preventDefault).toBeCalled();
-      expect(onPress).toHaveBeenCalledWith(
-        expect.objectContaining({defaultPrevented: true}),
-      );
-    });
-
-    // @gate experimental
-    it('prevents native behaviour for keyboard events by default', () => {
-      const onPress = jest.fn();
-      const preventDefault = jest.fn();
       const ref = React.createRef();
 
       const Component = () => {
@@ -1054,59 +995,9 @@ describe.each(environmentTable)('Press responder', hasPointerEvents => {
       ReactDOM.render(<Component />, container);
 
       const target = createEventTarget(ref.current);
-      target.keydown({key: 'Enter', preventDefault});
-      target.keyup({key: 'Enter'});
-      expect(preventDefault).toBeCalled();
-      expect(onPress).toHaveBeenCalledWith(
-        expect.objectContaining({defaultPrevented: true}),
-      );
-    });
-
-    // @gate experimental
-    it('deeply prevents native behaviour by default', () => {
-      const onPress = jest.fn();
-      const preventDefault = jest.fn();
-      const buttonRef = React.createRef();
-
-      const Component = () => {
-        const listener = usePress({onPress});
-        return (
-          <a href="#">
-            <button ref={buttonRef} DEPRECATED_flareListeners={listener} />
-          </a>
-        );
-      };
-      ReactDOM.render(<Component />, container);
-
-      const target = createEventTarget(buttonRef.current);
       target.pointerdown();
-      target.pointerup({preventDefault});
-      expect(preventDefault).toBeCalled();
-    });
-
-    // @gate experimental
-    it('prevents native behaviour by default with nested elements', () => {
-      const onPress = jest.fn();
-      const preventDefault = jest.fn();
-      const ref = React.createRef();
-
-      const Component = () => {
-        const listener = usePress({onPress});
-        return (
-          <a href="#" DEPRECATED_flareListeners={listener}>
-            <div ref={ref} />
-          </a>
-        );
-      };
-      ReactDOM.render(<Component />, container);
-
-      const target = createEventTarget(ref.current);
-      target.pointerdown();
-      target.pointerup({preventDefault});
-      expect(preventDefault).toBeCalled();
-      expect(onPress).toHaveBeenCalledWith(
-        expect.objectContaining({defaultPrevented: true}),
-      );
+      target.pointerup();
+      expect(onPress).toBeCalled();
     });
 
     // @gate experimental
