diff --git a/packages/react-ui/accessibility/src/FocusTable.js b/packages/react-ui/accessibility/src/FocusTable.js index 30741fd876bd8..bae0520d7dbd0 100644 --- a/packages/react-ui/accessibility/src/FocusTable.js +++ b/packages/react-ui/accessibility/src/FocusTable.js @@ -146,7 +146,7 @@ export function createFocusTable(): Array { 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 { } } } - return false; + return; } case 'DownArrow': { const [cells, rowIndex] = getRowCells(currentCell); @@ -179,7 +179,7 @@ export function createFocusTable(): Array { } } } - return false; + return; } case 'LeftArrow': { const [cells, rowIndex] = getRowCells(currentCell); @@ -190,7 +190,7 @@ export function createFocusTable(): Array { triggerNavigateOut(currentCell, 'left'); } } - return false; + return; } case 'RightArrow': { const [cells, rowIndex] = getRowCells(currentCell); @@ -203,10 +203,10 @@ export function createFocusTable(): Array { } } } - return false; + return; } } - return true; + event.continuePropagation(); }, }); return ( diff --git a/packages/react-ui/accessibility/src/TabFocus.js b/packages/react-ui/accessibility/src/TabFocus.js index 8183072a8c112..812def1c00bf4 100644 --- a/packages/react-ui/accessibility/src/TabFocus.js +++ b/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 ( diff --git a/packages/react-ui/accessibility/src/__tests__/TabFocus-test.internal.js b/packages/react-ui/accessibility/src/__tests__/TabFocus-test.internal.js index db1491ca0e6e3..9a43205fb69bb 100644 --- a/packages/react-ui/accessibility/src/__tests__/TabFocus-test.internal.js +++ b/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); }); }); diff --git a/packages/react-ui/events/src/dom/Keyboard.js b/packages/react-ui/events/src/dom/Keyboard.js index 426e9950a4d0c..08cc19d1d3bfc 100644 --- a/packages/react-ui/events/src/dom/Keyboard.js +++ b/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>; - -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, ); } } diff --git a/packages/react-ui/events/src/dom/Press.js b/packages/react-ui/events/src/dom/Press.js index 4d38f3ae8f37e..92c9280371e93 100644 --- a/packages/react-ui/events/src/dom/Press.js +++ b/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; } }, }); diff --git a/packages/react-ui/events/src/dom/__tests__/Keyboard-test.internal.js b/packages/react-ui/events/src/dom/__tests__/Keyboard-test.internal.js index 792d4a2088241..45ab5a894c497 100644 --- a/packages/react-ui/events/src/dom/__tests__/Keyboard-test.internal.js +++ b/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,33 +367,33 @@ describe('Keyboard responder', () => { const target = createEventTarget(ref.current); target.virtualclick({preventDefault}); - expect(preventDefault).toBeCalled(); + expect(preventDefault).not.toBeCalled(); expect(onClick).toHaveBeenCalledTimes(1); expect(onClick).toHaveBeenCalledWith( expect.objectContaining({ - defaultPrevented: true, + defaultPrevented: false, }), ); }); - test('allows native behaviour if false', () => { - const onClick = jest.fn(); + test('prevents native behaviour with preventDefault', () => { + const onClick = jest.fn(e => e.preventDefault()); const preventDefault = jest.fn(); - const ref = render({onClick, preventClick: false}); + const ref = render({onClick}); const target = createEventTarget(ref.current); target.virtualclick({preventDefault}); - expect(preventDefault).not.toBeCalled(); + expect(preventDefault).toBeCalled(); expect(onClick).toHaveBeenCalledTimes(1); expect(onClick).toHaveBeenCalledWith( expect.objectContaining({ - defaultPrevented: false, + 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});