[fix] Change logic of PressResponder for keyboards

This patch removes the keyboard `onClick` shim from `unstable_createElement`
and adds it to the `PressResponder`.

Previously, the logic in `createElement` was calling `onClick` during
`onKeyDown`. This could result in React state update warnings if the host
element was no longer in the DOM during `keyup`.

Fix #2051

Author: necolas

packages/react-native-web/src/exports/Pressable/__tests__/__snapshots__/index-test.js.snap

@@ -62,14 +62,14 @@ exports[`components/Pressable hover interaction 3`] = `
 />
 `;
 
-exports[`components/Pressable press interaction 1`] = `
+exports[`components/Pressable press interaction (keyboard) 1`] = `
 <div
   class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
   tabindex="0"
 />
 `;
 
-exports[`components/Pressable press interaction 2`] = `
+exports[`components/Pressable press interaction (keyboard) 2`] = `
 <div
   class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
   style="outline: press-ring;"
@@ -81,7 +81,28 @@ exports[`components/Pressable press interaction 2`] = `
 </div>
 `;
 
-exports[`components/Pressable press interaction 3`] = `
+exports[`components/Pressable press interaction (keyboard) 3`] = `null`;
+
+exports[`components/Pressable press interaction (pointer) 1`] = `
+<div
+  class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
+  tabindex="0"
+/>
+`;
+
+exports[`components/Pressable press interaction (pointer) 2`] = `
+<div
+  class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
+  style="outline: press-ring;"
+  tabindex="0"
+>
+  <div
+    data-testid="press-content"
+  />
+</div>
+`;
+
+exports[`components/Pressable press interaction (pointer) 3`] = `
 <div
   class="css-view-1dbjc4n r-cursor-1loqt21 r-touchAction-1otgn73"
   style=""

packages/react-native-web/src/exports/Pressable/__tests__/index-test.js

@@ -121,7 +121,7 @@ describe('components/Pressable', () => {
     expect(container.firstChild).toMatchSnapshot();
   });
 
-  test('press interaction', () => {
+  test('press interaction (pointer)', () => {
     let container;
     const onContextMenu = jest.fn();
     const onPress = jest.fn();
@@ -162,6 +162,50 @@ describe('components/Pressable', () => {
     expect(onContextMenu).toBeCalled();
   });
 
+  test('press interaction (keyboard)', () => {
+    let container;
+    const onPress = jest.fn();
+    const onPressIn = jest.fn();
+    const onPressOut = jest.fn();
+    const ref = React.createRef();
+
+    function TestCase() {
+      const [shown, setShown] = React.useState(true);
+      return shown ? (
+        <Pressable
+          children={({ pressed }) => (pressed ? <div data-testid="press-content" /> : null)}
+          onPress={(e) => {
+            onPress(e);
+            setShown(false);
+          }}
+          onPressIn={onPressIn}
+          onPressOut={onPressOut}
+          ref={ref}
+          style={({ pressed }) => [pressed && { outline: 'press-ring' }]}
+        />
+      ) : null;
+    }
+
+    act(() => {
+      ({ container } = render(<TestCase />));
+    });
+    const target = createEventTarget(ref.current);
+    expect(container.firstChild).toMatchSnapshot();
+    act(() => {
+      target.keydown({ key: 'Enter' });
+      jest.runAllTimers();
+    });
+    expect(onPressIn).toBeCalled();
+    expect(container.firstChild).toMatchSnapshot();
+    act(() => {
+      target.keyup({ key: 'Enter' });
+      jest.runAllTimers();
+    });
+    expect(onPressOut).toBeCalled();
+    expect(onPress).toBeCalled();
+    expect(container.firstChild).toMatchSnapshot();
+  });
+
   describe('prop "ref"', () => {
     test('value is set', () => {
       const ref = jest.fn();

packages/react-native-web/src/modules/createDOMProps/__tests__/index-test.js

@@ -95,117 +95,6 @@ describe('modules/createDOMProps', () => {
     });
   });
 
-  describe('prop "onClick"', () => {
-    const callsOnClick = (component, accessibilityRole, accessibilityDisabled = false) => {
-      const onClick = jest.fn();
-      const event = { stopPropagation: jest.fn() };
-      const finalProps = createDOMProps(component, {
-        accessibilityRole,
-        accessibilityDisabled,
-        onClick
-      });
-      finalProps.onClick(event);
-      return onClick.mock.calls.length === 1;
-    };
-
-    test('is called for various roles', () => {
-      expect(callsOnClick('div', 'link')).toBe(true);
-      expect(callsOnClick('div', 'button')).toBe(true);
-      expect(callsOnClick('div', 'textbox')).toBe(true);
-      expect(callsOnClick('div', 'bogus')).toBe(true);
-      expect(callsOnClick('a')).toBe(true);
-      expect(callsOnClick('button')).toBe(true);
-      expect(callsOnClick('input')).toBe(true);
-      expect(callsOnClick('select')).toBe(true);
-      expect(callsOnClick('textarea')).toBe(true);
-      expect(callsOnClick('h1')).toBe(true);
-    });
-
-    test('is not called when disabled is true', () => {
-      expect(callsOnClick('div', 'link', true)).toBe(false);
-      expect(callsOnClick('div', 'button', true)).toBe(false);
-      expect(callsOnClick('a', undefined, true)).toBe(false);
-      expect(callsOnClick('button', undefined, true)).toBe(false);
-      expect(callsOnClick('input', undefined, true)).toBe(false);
-      expect(callsOnClick('select', undefined, true)).toBe(false);
-      expect(callsOnClick('textarea', undefined, true)).toBe(false);
-
-      expect(callsOnClick('div', 'textbox', true)).toBe(true);
-      expect(callsOnClick('div', 'bogus', true)).toBe(true);
-      expect(callsOnClick('h1', undefined, true)).toBe(true);
-    });
-  });
-
-  describe('prop "onKeyDown"', () => {
-    const callsOnClick = (key) => (component, accessibilityRole, accessibilityDisabled = false) => {
-      const onClick = jest.fn();
-      const onKeyDown = jest.fn();
-      const event = { key, preventDefault: jest.fn() };
-      const finalProps = createDOMProps(component, {
-        accessibilityRole,
-        accessibilityDisabled,
-        onClick,
-        onKeyDown
-      });
-      finalProps.onKeyDown(event);
-      // The original onKeyDown should always be called
-      expect(onKeyDown).toHaveBeenCalled();
-      return onClick.mock.calls.length === 1;
-    };
-
-    const respondsToEnter = callsOnClick('Enter');
-    const respondsToSpace = callsOnClick(' ');
-
-    test('does not emulate "onClick" when disabled', () => {
-      expect(respondsToEnter('div', 'link', true)).toBe(false);
-      expect(respondsToEnter('div', 'button', true)).toBe(false);
-      expect(respondsToEnter('div', 'textbox', true)).toBe(false);
-      expect(respondsToEnter('div', 'bogus', true)).toBe(false);
-    });
-
-    test('does not emulate "onClick" for native elements', () => {
-      expect(respondsToEnter('a')).toBe(false);
-      expect(respondsToEnter('button')).toBe(false);
-      expect(respondsToEnter('input')).toBe(false);
-      expect(respondsToEnter('select')).toBe(false);
-      expect(respondsToEnter('textarea')).toBe(false);
-      expect(respondsToEnter('h1')).toBe(false);
-      expect(respondsToEnter('div', 'link')).toBe(false);
-
-      expect(respondsToSpace('a')).toBe(false);
-      expect(respondsToSpace('button')).toBe(false);
-      expect(respondsToSpace('input')).toBe(false);
-      expect(respondsToSpace('select')).toBe(false);
-      expect(respondsToSpace('textarea')).toBe(false);
-      expect(respondsToSpace('h1')).toBe(false);
-      expect(respondsToSpace('div', 'link')).toBe(false);
-    });
-
-    test('emulates "onClick" for "Enter" for certain roles', () => {
-      expect(respondsToEnter('div', 'button')).toBe(true);
-      expect(respondsToEnter('div', 'textbox')).toBe(false);
-      expect(respondsToEnter('div', 'bogus')).toBe(false);
-    });
-
-    test('emulates "onClick" for "Enter" for items marked accessible', () => {
-      const onClick = jest.fn();
-      const event = { key: 'Enter', preventDefault: jest.fn() };
-      const finalProps = createDOMProps('div', {
-        accessibilityRole: 'article',
-        focusable: true,
-        onClick
-      });
-      finalProps.onKeyDown(event);
-      expect(onClick).toHaveBeenCalled();
-    });
-
-    test('emulates "onClick" for "Space" for certain roles', () => {
-      expect(respondsToSpace('div', 'button')).toBe(true);
-      expect(respondsToSpace('div', 'textbox')).toBe(false);
-      expect(respondsToSpace('div', 'bogus')).toBe(false);
-    });
-  });
-
   test('prop "accessibilityLabel" becomes "aria-label"', () => {
     const accessibilityLabel = 'accessibilityLabel';
     const props = createProps({ accessibilityLabel });

packages/react-native-web/src/modules/createDOMProps/index.js

@@ -136,15 +136,6 @@ const createDOMProps = (elementType, props) => {
 
   const role = AccessibilityUtil.propsToAriaRole(props);
 
-  const isNativeInteractiveElement =
-    role === 'link' ||
-    elementType === 'a' ||
-    elementType === 'button' ||
-    elementType === 'input' ||
-    elementType === 'select' ||
-    elementType === 'textarea' ||
-    domProps.contentEditable != null;
-
   // DEPRECATED
   if (accessibilityState != null) {
     for (const prop in accessibilityState) {
@@ -417,42 +408,6 @@ const createDOMProps = (elementType, props) => {
     domProps['data-testid'] = testID;
   }
 
-  // Keyboard accessibility
-  // Button-like roles should trigger 'onClick' if SPACE key is pressed.
-  // Button-like roles should not trigger 'onClick' if they are disabled.
-  if (isNativeInteractiveElement || role === 'button' || (_focusable === true && !disabled)) {
-    const onClick = domProps.onClick;
-    if (onClick != null) {
-      if (disabled) {
-        // Prevent click propagating if the element is disabled. See #1757
-        domProps.onClick = function (e) {
-          e.stopPropagation();
-        };
-      } else if (!isNativeInteractiveElement) {
-        // For native elements that are focusable but don't dispatch 'click' events
-        // for keyboards.
-        const onKeyDown = domProps.onKeyDown;
-        domProps.onKeyDown = function (e) {
-          const { key, repeat } = e;
-          const isSpacebarKey = key === ' ' || key === 'Spacebar';
-          const isButtonRole = role === 'button';
-          if (onKeyDown != null) {
-            onKeyDown(e);
-          }
-          if (!repeat && key === 'Enter') {
-            onClick(e);
-          } else if (isSpacebarKey && isButtonRole) {
-            if (!repeat) {
-              onClick(e);
-            }
-            // Prevent spacebar scrolling the window
-            e.preventDefault();
-          }
-        };
-      }
-    }
-  }
-
   return domProps;
 };
 

packages/react-native-web/src/modules/usePressEvents/PressResponder.js

@@ -130,11 +130,11 @@ const isTerminalSignal = (signal) =>
   signal === RESPONDER_TERMINATED || signal === RESPONDER_RELEASE;
 
 const isValidKeyPress = (event) => {
-  const key = event.key;
-  const target = event.currentTarget;
+  const { key, target } = event;
   const role = target.getAttribute('role');
   const isSpacebar = key === ' ' || key === 'Spacebar';
-  return !event.repeat && (key === 'Enter' || (isSpacebar && role === 'button'));
+
+  return key === 'Enter' || (isSpacebar && role === 'button');
 };
 
 const DEFAULT_LONG_PRESS_DELAY_MS = 450; // 500 - 50
@@ -298,9 +298,27 @@ export default class PressResponder {
     };
 
     const keyupHandler = (event: KeyboardEvent) => {
-      if (this._touchState !== NOT_RESPONDER) {
+      const { onPress } = this._config;
+      const { target } = event;
+
+      if (this._touchState !== NOT_RESPONDER && isValidKeyPress(event)) {
         end(event);
         document.removeEventListener('keyup', keyupHandler);
+
+        const role = target.getAttribute('role');
+        const elementType = target.tagName.toLowerCase();
+
+        const isNativeInteractiveElement =
+          role === 'link' ||
+          elementType === 'a' ||
+          elementType === 'button' ||
+          elementType === 'input' ||
+          elementType === 'select' ||
+          elementType === 'textarea';
+
+        if (onPress != null && !isNativeInteractiveElement) {
+          onPress(event);
+        }
       }
     };
 
@@ -317,13 +335,22 @@ export default class PressResponder {
       },
 
       onKeyDown: (event) => {
-        if (isValidKeyPress(event)) {
+        const { disabled } = this._config;
+        const { key, target } = event;
+        if (!disabled && isValidKeyPress(event)) {
           if (this._touchState === NOT_RESPONDER) {
             start(event, false);
             // Listen to 'keyup' on document to account for situations where
             // focus is moved to another element during 'keydown'.
             document.addEventListener('keyup', keyupHandler);
           }
+          const role = target.getAttribute('role');
+          const isSpacebarKey = key === ' ' || key === 'Spacebar';
+          const isButtonRole = role === 'button' || role === 'menuitem';
+          if (isSpacebarKey && isButtonRole) {
+            // Prevent spacebar scrolling the window
+            event.preventDefault();
+          }
           event.stopPropagation();
         }
       },