[react-ui] Add preventDefault+stopPropagation to Keyboard + update Focus components (#16833)

packages/react-ui/accessibility/src/FocusTable.js

@@ -146,7 +146,7 @@ export function createFocusTable(): Array<React.Component> {
   function Cell({children}): FocusCellProps {
     const scopeRef = useRef(null);
     const keyboard = useKeyboard({
-      onKeyDown(event: KeyboardEvent): boolean {
+      onKeyDown(event: KeyboardEvent): void {
         const currentCell = scopeRef.current;
         switch (event.key) {
           case 'UpArrow': {
@@ -162,7 +162,7 @@ export function createFocusTable(): Array<React.Component> {
                 }
               }
             }
-            return false;
+            return;
           }
           case 'DownArrow': {
             const [cells, rowIndex] = getRowCells(currentCell);
@@ -179,7 +179,7 @@ export function createFocusTable(): Array<React.Component> {
                 }
               }
             }
-            return false;
+            return;
           }
           case 'LeftArrow': {
             const [cells, rowIndex] = getRowCells(currentCell);
@@ -190,7 +190,7 @@ export function createFocusTable(): Array<React.Component> {
                 triggerNavigateOut(currentCell, 'left');
               }
             }
-            return false;
+            return;
           }
           case 'RightArrow': {
             const [cells, rowIndex] = getRowCells(currentCell);
@@ -203,10 +203,10 @@ export function createFocusTable(): Array<React.Component> {
                 }
               }
             }
-            return false;
+            return;
           }
         }
-        return true;
+        event.continuePropagation();
       },
     });
     return (

packages/react-ui/accessibility/src/TabFocus.js

@@ -53,10 +53,11 @@ function focusElem(elem: null | HTMLElement): void {
   }
 }
 
-export function focusNext(
+function internalFocusNext(
   scope: ReactScopeMethods,
+  event?: KeyboardEvent,
   contain?: boolean,
-): boolean {
+): void {
   const [
     tabbableNodes,
     firstTabbableElem,
@@ -66,23 +67,31 @@ export function focusNext(
   ] = getTabbableNodes(scope);
 
   if (focusedElement === null) {
-    focusElem(firstTabbableElem);
+    if (event) {
+      event.continuePropagation();
+    }
   } else if (focusedElement === lastTabbableElem) {
-    if (contain === true) {
+    if (contain) {
       focusElem(firstTabbableElem);
-    } else {
-      return true;
+      if (event) {
+        event.preventDefault();
+      }
+    } else if (event) {
+      event.continuePropagation();
     }
   } else {
     focusElem((tabbableNodes: any)[currentIndex + 1]);
+    if (event) {
+      event.preventDefault();
+    }
   }
-  return false;
 }
 
-export function focusPrevious(
+function internalFocusPrevious(
   scope: ReactScopeMethods,
+  event?: KeyboardEvent,
   contain?: boolean,
-): boolean {
+): void {
   const [
     tabbableNodes,
     firstTabbableElem,
@@ -92,17 +101,32 @@ export function focusPrevious(
   ] = getTabbableNodes(scope);
 
   if (focusedElement === null) {
-    focusElem(firstTabbableElem);
+    if (event) {
+      event.continuePropagation();
+    }
   } else if (focusedElement === firstTabbableElem) {
-    if (contain === true) {
+    if (contain) {
       focusElem(lastTabbableElem);
-    } else {
-      return true;
+      if (event) {
+        event.preventDefault();
+      }
+    } else if (event) {
+      event.continuePropagation();
     }
   } else {
     focusElem((tabbableNodes: any)[currentIndex - 1]);
+    if (event) {
+      event.preventDefault();
+    }
   }
-  return false;
+}
+
+export function focusPrevious(scope: ReactScopeMethods): void {
+  internalFocusPrevious(scope);
+}
+
+export function focusNext(scope: ReactScopeMethods): void {
+  internalFocusNext(scope);
 }
 
 export function getNextController(
@@ -137,21 +161,20 @@ export const TabFocusController = React.forwardRef(
   ({children, contain}: TabFocusControllerProps, ref): React.Node => {
     const scopeRef = useRef(null);
     const keyboard = useKeyboard({
-      onKeyDown(event: KeyboardEvent): boolean {
+      onKeyDown(event: KeyboardEvent): void {
         if (event.key !== 'Tab') {
-          return true;
+          event.continuePropagation();
+          return;
         }
         const scope = scopeRef.current;
         if (scope !== null) {
           if (event.shiftKey) {
-            return focusPrevious(scope, contain);
+            internalFocusPrevious(scope, event, contain);
           } else {
-            return focusNext(scope, contain);
+            internalFocusNext(scope, event, contain);
           }
         }
-        return true;
       },
-      preventKeys: ['Tab', ['Tab', {shiftKey: true}]],
     });
 
     return (

packages/react-ui/accessibility/src/__tests__/TabFocus-test.internal.js

@@ -255,14 +255,14 @@ describe('TabFocusController', () => {
         firstFocusController,
       );
       expect(nextController).toBe(secondFocusController);
-      ReactTabFocus.focusNext(nextController);
+      ReactTabFocus.focusFirst(nextController);
       expect(document.activeElement).toBe(divRef.current);
 
       const previousController = ReactTabFocus.getPreviousController(
         nextController,
       );
       expect(previousController).toBe(firstFocusController);
-      ReactTabFocus.focusNext(previousController);
+      ReactTabFocus.focusFirst(previousController);
       expect(document.activeElement).toBe(buttonRef.current);
     });
   });

packages/react-ui/events/src/dom/Keyboard.js

@@ -24,15 +24,12 @@ type KeyboardEventType =
 
 type KeyboardProps = {|
   disabled?: boolean,
-  onClick?: (e: KeyboardEvent) => ?boolean,
-  onKeyDown?: (e: KeyboardEvent) => ?boolean,
-  onKeyUp?: (e: KeyboardEvent) => ?boolean,
-  preventClick?: boolean,
-  preventKeys?: PreventKeysArray,
+  onClick?: (e: KeyboardEvent) => void,
+  onKeyDown?: (e: KeyboardEvent) => void,
+  onKeyUp?: (e: KeyboardEvent) => void,
 |};
 
 type KeyboardState = {|
-  defaultPrevented: boolean,
   isActive: boolean,
 |};
 
@@ -48,20 +45,11 @@ export type KeyboardEvent = {|
   target: Element | Document,
   type: KeyboardEventType,
   timeStamp: number,
+  continuePropagation: () => void,
+  preventDefault: () => void,
 |};
 
-type ModifiersObject = {|
-  altKey?: boolean,
-  ctrlKey?: boolean,
-  metaKey?: boolean,
-  shiftKey?: boolean,
-|};
-
-type PreventKeysArray = Array<string | Array<string | ModifiersObject>>;
-
-const isArray = Array.isArray;
 const targetEventTypes = ['click_active', 'keydown_active', 'keyup'];
-const modifiers = ['altKey', 'ctrlKey', 'metaKey', 'shiftKey'];
 
 /**
  * Normalization of deprecated HTML5 `key` values
@@ -146,20 +134,31 @@ function createKeyboardEvent(
   event: ReactDOMResponderEvent,
   context: ReactDOMResponderContext,
   type: KeyboardEventType,
-  defaultPrevented: boolean,
 ): KeyboardEvent {
   const nativeEvent = (event: any).nativeEvent;
   const {altKey, ctrlKey, metaKey, shiftKey} = nativeEvent;
   let keyboardEvent = {
     altKey,
     ctrlKey,
-    defaultPrevented,
+    defaultPrevented: nativeEvent.defaultPrevented === true,
     metaKey,
     pointerType: 'keyboard',
     shiftKey,
     target: event.target,
     timeStamp: context.getTimeStamp(),
     type,
+    // We don't use stopPropagation, as the default behavior
+    // is to not propagate. Plus, there might be confusion
+    // using stopPropagation as we don't actually stop
+    // native propagation from working, but instead only
+    // allow propagation to the others keyboard responders.
+    continuePropagation() {
+      context.continuePropagation();
+    },
+    preventDefault() {
+      keyboardEvent.defaultPrevented = true;
+      nativeEvent.preventDefault();
+    },
   };
   if (type !== 'keyboard:click') {
     const key = getEventKey(nativeEvent);
@@ -171,32 +170,18 @@ function createKeyboardEvent(
 
 function dispatchKeyboardEvent(
   event: ReactDOMResponderEvent,
-  listener: KeyboardEvent => ?boolean,
+  listener: KeyboardEvent => void,
   context: ReactDOMResponderContext,
   type: KeyboardEventType,
-  defaultPrevented: boolean,
 ): void {
-  const syntheticEvent = createKeyboardEvent(
-    event,
-    context,
-    type,
-    defaultPrevented,
-  );
-  let shouldPropagate;
-  const listenerWithReturnValue = e => {
-    shouldPropagate = listener(e);
-  };
-  context.dispatchEvent(syntheticEvent, listenerWithReturnValue, DiscreteEvent);
-  if (shouldPropagate) {
-    context.continuePropagation();
-  }
+  const syntheticEvent = createKeyboardEvent(event, context, type);
+  context.dispatchEvent(syntheticEvent, listener, DiscreteEvent);
 }
 
 const keyboardResponderImpl = {
   targetEventTypes,
   getInitialState(): KeyboardState {
     return {
-      defaultPrevented: false,
       isActive: false,
     };
   },
@@ -207,71 +192,26 @@ const keyboardResponderImpl = {
     state: KeyboardState,
   ): void {
     const {type} = event;
-    const nativeEvent: any = event.nativeEvent;
 
     if (props.disabled) {
       return;
     }
 
     if (type === 'keydown') {
-      state.defaultPrevented = nativeEvent.defaultPrevented === true;
-
-      const preventKeys = ((props.preventKeys: any): PreventKeysArray);
-      if (!state.defaultPrevented && isArray(preventKeys)) {
-        preventKeyLoop: for (let i = 0; i < preventKeys.length; i++) {
-          const preventKey = preventKeys[i];
-          let key = preventKey;
-
-          if (isArray(preventKey)) {
-            key = preventKey[0];
-            const config = ((preventKey[1]: any): Object);
-            for (let s = 0; s < modifiers.length; s++) {
-              const modifier = modifiers[s];
-              const configModifier = config[modifier];
-              const eventModifier = nativeEvent[modifier];
-              if (
-                (configModifier && !eventModifier) ||
-                (!configModifier && eventModifier)
-              ) {
-                continue preventKeyLoop;
-              }
-            }
-          }
-
-          if (key === getEventKey(nativeEvent)) {
-            state.defaultPrevented = true;
-            nativeEvent.preventDefault();
-            break;
-          }
-        }
-      }
       state.isActive = true;
       const onKeyDown = props.onKeyDown;
       if (onKeyDown != null) {
         dispatchKeyboardEvent(
           event,
-          ((onKeyDown: any): (e: KeyboardEvent) => ?boolean),
+          ((onKeyDown: any): (e: KeyboardEvent) => void),
           context,
           'keyboard:keydown',
-          state.defaultPrevented,
         );
       }
     } else if (type === 'click' && isVirtualClick(event)) {
-      if (props.preventClick !== false) {
-        // 'click' occurs before or after 'keyup', and may need native
-        // behavior prevented
-        nativeEvent.preventDefault();
-        state.defaultPrevented = true;
-      }
       const onClick = props.onClick;
       if (onClick != null) {
-        dispatchKeyboardEvent(
-          event,
-          onClick,
-          context,
-          'keyboard:click',
-          state.defaultPrevented,
-        );
+        dispatchKeyboardEvent(event, onClick, context, 'keyboard:click');
       }
     } else if (type === 'keyup') {
       state.isActive = false;
@@ -279,10 +219,9 @@ const keyboardResponderImpl = {
       if (onKeyUp != null) {
         dispatchKeyboardEvent(
           event,
-          ((onKeyUp: any): (e: KeyboardEvent) => ?boolean),
+          ((onKeyUp: any): (e: KeyboardEvent) => void),
           context,
           'keyboard:keyup',
-          state.defaultPrevented,
         );
       }
     }

packages/react-ui/events/src/dom/Press.js

@@ -81,6 +81,13 @@ function isValidKey(e): boolean {
   );
 }
 
+function handlePreventDefault(preventDefault: ?boolean, e: any): void {
+  const key = e.key;
+  if (preventDefault !== false && (key === ' ' || key === 'Enter')) {
+    e.preventDefault();
+  }
+}
+
 /**
  * The lack of built-in composition for gesture responders means we have to
  * selectively ignore callbacks from useKeyboard or useTap if the other is
@@ -155,28 +162,30 @@ export function usePress(props: PressProps) {
 
   const keyboard = useKeyboard({
     disabled: disabled || active === 'tap',
-    preventClick: preventDefault !== false,
-    preventKeys: preventDefault !== false ? [' ', 'Enter'] : [],
     onClick(e) {
+      if (preventDefault !== false) {
+        e.preventDefault();
+      }
       if (active == null && onPress != null) {
         onPress(createGestureState(e, 'press'));
       }
     },
     onKeyDown(e) {
       if (active == null && isValidKey(e)) {
+        handlePreventDefault(preventDefault, e);
         updateActive('keyboard');
+
         if (onPressStart != null) {
           onPressStart(createGestureState(e, 'pressstart'));
         }
         if (onPressChange != null) {
           onPressChange(true);
         }
-        // stop propagation
-        return false;
       }
     },
     onKeyUp(e) {
       if (active === 'keyboard' && isValidKey(e)) {
+        handlePreventDefault(preventDefault, e);
         if (onPressChange != null) {
           onPressChange(false);
         }
@@ -187,8 +196,6 @@ export function usePress(props: PressProps) {
           onPress(createGestureState(e, 'press'));
         }
         updateActive(null);
-        // stop propagation
-        return false;
       }
     },
   });

packages/react-ui/events/src/dom/__tests__/Keyboard-test.internal.js

@@ -41,9 +41,9 @@ describe('Keyboard responder', () => {
   });
 
   function renderPropagationTest(propagates) {
-    const onClickInner = jest.fn(() => propagates);
-    const onKeyDownInner = jest.fn(() => propagates);
-    const onKeyUpInner = jest.fn(() => propagates);
+    const onClickInner = jest.fn(e => propagates && e.continuePropagation());
+    const onKeyDownInner = jest.fn(e => propagates && e.continuePropagation());
+    const onKeyUpInner = jest.fn(e => propagates && e.continuePropagation());
     const onClickOuter = jest.fn();
     const onKeyDownOuter = jest.fn();
     const onKeyUpOuter = jest.fn();
@@ -77,7 +77,7 @@ describe('Keyboard responder', () => {
     };
   }
 
-  test('propagates key event when a callback returns true', () => {
+  test('propagates key event when a continuePropagation() is used', () => {
     const {
       onClickInner,
       onKeyDownInner,
@@ -99,7 +99,7 @@ describe('Keyboard responder', () => {
     expect(onClickOuter).toBeCalled();
   });
 
-  test('does not propagate key event when a callback returns false', () => {
+  test('does not propagate key event by default', () => {
     const {
       onClickInner,
       onKeyDownInner,
@@ -148,7 +148,9 @@ describe('Keyboard responder', () => {
     let onClick, ref;
 
     beforeEach(() => {
-      onClick = jest.fn();
+      onClick = jest.fn(e => {
+        e.preventDefault();
+      });
       ref = React.createRef();
       const Component = () => {
         const listener = useKeyboard({onClick});
@@ -346,7 +348,7 @@ describe('Keyboard responder', () => {
     });
   });
 
-  describe('preventClick', () => {
+  describe('preventDefault for onClick', () => {
     function render(props) {
       const ref = React.createRef();
       const Component = () => {
@@ -357,7 +359,7 @@ describe('Keyboard responder', () => {
       return ref;
     }
 
-    test('prevents native click by default', () => {
+    test('does not prevent native click by default', () => {
       const onClick = jest.fn();
       const preventDefault = jest.fn();
       const ref = render({onClick});
@@ -365,22 +367,6 @@ describe('Keyboard responder', () => {
       const target = createEventTarget(ref.current);
       target.virtualclick({preventDefault});
 
-      expect(preventDefault).toBeCalled();
-      expect(onClick).toHaveBeenCalledTimes(1);
-      expect(onClick).toHaveBeenCalledWith(
-        expect.objectContaining({
-          defaultPrevented: true,
-        }),
-      );
-    });
-
-    test('allows native behaviour if false', () => {
-      const onClick = jest.fn();
-      const preventDefault = jest.fn();
-      const ref = render({onClick, preventClick: false});
-
-      const target = createEventTarget(ref.current);
-      target.virtualclick({preventDefault});
       expect(preventDefault).not.toBeCalled();
       expect(onClick).toHaveBeenCalledTimes(1);
       expect(onClick).toHaveBeenCalledWith(
@@ -389,9 +375,25 @@ describe('Keyboard responder', () => {
         }),
       );
     });
+
+    test('prevents native behaviour with preventDefault', () => {
+      const onClick = jest.fn(e => e.preventDefault());
+      const preventDefault = jest.fn();
+      const ref = render({onClick});
+
+      const target = createEventTarget(ref.current);
+      target.virtualclick({preventDefault});
+      expect(preventDefault).toBeCalled();
+      expect(onClick).toHaveBeenCalledTimes(1);
+      expect(onClick).toHaveBeenCalledWith(
+        expect.objectContaining({
+          defaultPrevented: true,
+        }),
+      );
+    });
   });
 
-  describe('preventKeys', () => {
+  describe('preventDefault for onKeyDown', () => {
     function render(props) {
       const ref = React.createRef();
       const Component = () => {
@@ -403,10 +405,14 @@ describe('Keyboard responder', () => {
     }
 
     test('key config matches', () => {
-      const onKeyDown = jest.fn();
+      const onKeyDown = jest.fn(e => {
+        if (e.key === 'Tab') {
+          e.preventDefault();
+        }
+      });
       const preventDefault = jest.fn();
       const preventDefaultClick = jest.fn();
-      const ref = render({onKeyDown, preventKeys: ['Tab']});
+      const ref = render({onKeyDown});
 
       const target = createEventTarget(ref.current);
       target.keydown({key: 'Tab', preventDefault});
@@ -424,9 +430,13 @@ describe('Keyboard responder', () => {
     });
 
     test('key config matches (modifier keys)', () => {
-      const onKeyDown = jest.fn();
+      const onKeyDown = jest.fn(e => {
+        if (e.key === 'Tab' && e.shiftKey) {
+          e.preventDefault();
+        }
+      });
       const preventDefault = jest.fn();
-      const ref = render({onKeyDown, preventKeys: [['Tab', {shiftKey: true}]]});
+      const ref = render({onKeyDown});
 
       const target = createEventTarget(ref.current);
       target.keydown({key: 'Tab', preventDefault, shiftKey: true});
@@ -443,9 +453,13 @@ describe('Keyboard responder', () => {
     });
 
     test('key config does not match (modifier keys)', () => {
-      const onKeyDown = jest.fn();
+      const onKeyDown = jest.fn(e => {
+        if (e.key === 'Tab' && e.shiftKey) {
+          e.preventDefault();
+        }
+      });
       const preventDefault = jest.fn();
-      const ref = render({onKeyDown, preventKeys: [['Tab', {shiftKey: true}]]});
+      const ref = render({onKeyDown});
 
       const target = createEventTarget(ref.current);
       target.keydown({key: 'Tab', preventDefault, shiftKey: false});