From 87eaa90ef8dabd3d557085586d524016cd946d30 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Fri, 13 Sep 2019 13:19:18 -0700 Subject: [PATCH] [react-events] Keyboard calls preventDefault on 'click' events (#16779) Make sure to call preventDefault for any 'click' events that follow a 'keydown' event that matches 'preventKeys' --- packages/react-events/src/dom/Keyboard.js | 46 +- .../dom/__tests__/Keyboard-test.internal.js | 406 +++++++++--------- .../src/dom/testing-library/domEvents.js | 2 + .../src/__tests__/ReactScope-test.internal.js | 6 - 4 files changed, 237 insertions(+), 223 deletions(-) diff --git a/packages/react-events/src/dom/Keyboard.js b/packages/react-events/src/dom/Keyboard.js index b8f024cbd5809..17da68bc6681d 100644 --- a/packages/react-events/src/dom/Keyboard.js +++ b/packages/react-events/src/dom/Keyboard.js @@ -16,26 +16,32 @@ import React from 'react'; import {DiscreteEvent} from 'shared/ReactTypes'; import type {ReactEventResponderListener} from 'shared/ReactTypes'; -type KeyboardEventType = 'keydown' | 'keyup'; +type KeyboardEventType = 'keyboard:keydown' | 'keyboard:keyup'; -type KeyboardProps = { +type KeyboardProps = {| disabled?: boolean, onKeyDown?: (e: KeyboardEvent) => ?boolean, onKeyUp?: (e: KeyboardEvent) => ?boolean, preventKeys?: PreventKeysArray, -}; +|}; + +type KeyboardState = {| + defaultPrevented: boolean, + isActive: boolean, +|}; export type KeyboardEvent = {| altKey: boolean, ctrlKey: boolean, + defaultPrevented: boolean, isComposing: boolean, key: string, metaKey: boolean, + pointerType: 'keyboard', shiftKey: boolean, target: Element | Document, type: KeyboardEventType, timeStamp: number, - defaultPrevented: boolean, |}; type ModifiersObject = {| @@ -48,7 +54,7 @@ type ModifiersObject = {| type PreventKeysArray = Array>; const isArray = Array.isArray; -const targetEventTypes = ['keydown_active', 'keyup']; +const targetEventTypes = ['click_active', 'keydown_active', 'keyup']; const modifiers = ['altKey', 'ctrlKey', 'metaKey', 'shiftKey']; /** @@ -150,6 +156,7 @@ function createKeyboardEvent( isComposing, key: getEventKey(nativeEvent), metaKey, + pointerType: 'keyboard', shiftKey, target: event.target, timeStamp: context.getTimeStamp(), @@ -182,10 +189,17 @@ function dispatchKeyboardEvent( const keyboardResponderImpl = { targetEventTypes, + getInitialState(): KeyboardState { + return { + defaultPrevented: false, + isActive: false, + }; + }, onEvent( event: ReactDOMResponderEvent, context: ReactDOMResponderContext, props: KeyboardProps, + state: KeyboardState, ): void { const {type} = event; const nativeEvent: any = event.nativeEvent; @@ -193,10 +207,12 @@ const keyboardResponderImpl = { if (props.disabled) { return; } - let defaultPrevented = nativeEvent.defaultPrevented === true; + if (type === 'keydown') { + state.defaultPrevented = nativeEvent.defaultPrevented === true; + const preventKeys = ((props.preventKeys: any): PreventKeysArray); - if (!defaultPrevented && isArray(preventKeys)) { + if (!state.defaultPrevented && isArray(preventKeys)) { preventKeyLoop: for (let i = 0; i < preventKeys.length; i++) { const preventKey = preventKeys[i]; let key = preventKey; @@ -216,32 +232,38 @@ const keyboardResponderImpl = { } } } + if (key === getEventKey(nativeEvent)) { - defaultPrevented = true; + state.defaultPrevented = true; nativeEvent.preventDefault(); break; } } } + state.isActive = true; const onKeyDown = props.onKeyDown; if (isFunction(onKeyDown)) { dispatchKeyboardEvent( event, ((onKeyDown: any): (e: KeyboardEvent) => ?boolean), context, - 'keydown', - defaultPrevented, + 'keyboard:keydown', + state.defaultPrevented, ); } + } else if (type === 'click' && state.isActive && state.defaultPrevented) { + // 'click' occurs before 'keyup' and may need native behavior prevented + nativeEvent.preventDefault(); } else if (type === 'keyup') { + state.isActive = false; const onKeyUp = props.onKeyUp; if (isFunction(onKeyUp)) { dispatchKeyboardEvent( event, ((onKeyUp: any): (e: KeyboardEvent) => ?boolean), context, - 'keyup', - defaultPrevented, + 'keyboard:keyup', + state.defaultPrevented, ); } } diff --git a/packages/react-events/src/dom/__tests__/Keyboard-test.internal.js b/packages/react-events/src/dom/__tests__/Keyboard-test.internal.js index fdfc9b40dc997..237d35df009a0 100644 --- a/packages/react-events/src/dom/__tests__/Keyboard-test.internal.js +++ b/packages/react-events/src/dom/__tests__/Keyboard-test.internal.js @@ -25,7 +25,7 @@ function initializeModules(hasPointerEvents) { useKeyboard = require('react-events/keyboard').useKeyboard; } -describe('Keyboard event responder', () => { +describe('Keyboard responder', () => { let container; beforeEach(() => { @@ -40,6 +40,71 @@ describe('Keyboard event responder', () => { container = null; }); + function renderPropagationTest(propagates) { + const onKeyDownInner = jest.fn(() => propagates); + const onKeyDownOuter = jest.fn(); + const onKeyUpInner = jest.fn(() => propagates); + const onKeyUpOuter = jest.fn(); + const ref = React.createRef(); + const Component = () => { + const listenerInner = useKeyboard({ + onKeyDown: onKeyDownInner, + onKeyUp: onKeyUpInner, + }); + const listenerOuter = useKeyboard({ + onKeyDown: onKeyDownOuter, + onKeyUp: onKeyUpOuter, + }); + return ( +
+
+
+ ); + }; + ReactDOM.render(, container); + return { + onKeyDownInner, + onKeyDownOuter, + onKeyUpInner, + onKeyUpOuter, + ref, + }; + } + + test('propagates event when a callback returns true', () => { + const { + onKeyDownInner, + onKeyDownOuter, + onKeyUpInner, + onKeyUpOuter, + ref, + } = renderPropagationTest(true); + const target = createEventTarget(ref.current); + target.keydown(); + expect(onKeyDownInner).toBeCalled(); + expect(onKeyDownOuter).toBeCalled(); + target.keyup(); + expect(onKeyUpInner).toBeCalled(); + expect(onKeyUpOuter).toBeCalled(); + }); + + test('does not propagate event when a callback returns false', () => { + const { + onKeyDownInner, + onKeyDownOuter, + onKeyUpInner, + onKeyUpOuter, + ref, + } = renderPropagationTest(false); + const target = createEventTarget(ref.current); + target.keydown(); + expect(onKeyDownInner).toBeCalled(); + expect(onKeyDownOuter).not.toBeCalled(); + target.keyup(); + expect(onKeyUpInner).toBeCalled(); + expect(onKeyUpOuter).not.toBeCalled(); + }); + describe('disabled', () => { let onKeyDown, onKeyUp, ref; @@ -48,17 +113,13 @@ describe('Keyboard event responder', () => { onKeyUp = jest.fn(); ref = React.createRef(); const Component = () => { - const listener = useKeyboard({ - disabled: true, - onKeyDown, - onKeyUp, - }); + const listener = useKeyboard({disabled: true, onKeyDown, onKeyUp}); return
; }; ReactDOM.render(, container); }); - it('prevents custom events being dispatched', () => { + test('does not call callbacks', () => { const target = createEventTarget(ref.current); target.keydown(); target.keyup(); @@ -74,265 +135,200 @@ describe('Keyboard event responder', () => { onKeyDown = jest.fn(); ref = React.createRef(); const Component = () => { - const listener = useKeyboard({ - onKeyDown, - }); + const listener = useKeyboard({onKeyDown}); return
; }; ReactDOM.render(, container); }); - it('is called after "keydown" event', () => { + test('key down', () => { const target = createEventTarget(ref.current); target.keydown({key: 'Q'}); expect(onKeyDown).toHaveBeenCalledTimes(1); expect(onKeyDown).toHaveBeenCalledWith( - expect.objectContaining({key: 'Q', type: 'keydown'}), + expect.objectContaining({ + altKey: false, + ctrlKey: false, + defaultPrevented: false, + isComposing: false, + key: 'Q', + metaKey: false, + pointerType: 'keyboard', + shiftKey: false, + target: target.node, + timeStamp: expect.any(Number), + type: 'keyboard:keydown', + }), ); }); - }); - describe('preventKeys', () => { - it('onKeyDown is default prevented', () => { - const onKeyDown = jest.fn(); - const ref = React.createRef(); - const Component = () => { - const listener = useKeyboard({ - onKeyDown, - preventKeys: ['Tab'], - }); - return
; - }; - ReactDOM.render(, container); - - const preventDefault = jest.fn(); + test('modified key down', () => { const target = createEventTarget(ref.current); - target.keydown({key: 'Tab', preventDefault}); - expect(onKeyDown).toHaveBeenCalledTimes(1); - expect(preventDefault).toBeCalled(); + target.keydown({ + key: 'Q', + altKey: true, + ctrlKey: true, + shiftKey: true, + metaKey: true, + }); expect(onKeyDown).toHaveBeenCalledWith( expect.objectContaining({ - key: 'Tab', - type: 'keydown', - defaultPrevented: true, + altKey: true, + ctrlKey: true, + defaultPrevented: false, + isComposing: false, + key: 'Q', + metaKey: true, + pointerType: 'keyboard', + shiftKey: true, + target: target.node, + timeStamp: expect.any(Number), + type: 'keyboard:keydown', }), ); }); + }); - it('onKeyDown is default prevented (falsy modifier keys)', () => { - let onKeyDown = jest.fn(); - let ref = React.createRef(); - let Component = () => { - const listener = useKeyboard({ - onKeyDown, - preventKeys: [['Tab', {metaKey: false}]], - }); + describe('onKeyUp', () => { + let onKeyUp, ref; + + beforeEach(() => { + onKeyUp = jest.fn(); + ref = React.createRef(); + const Component = () => { + const listener = useKeyboard({onKeyUp}); return
; }; ReactDOM.render(, container); + }); - let preventDefault = jest.fn(); - let target = createEventTarget(ref.current); - target.keydown({key: 'Tab', preventDefault, metaKey: true}); - expect(onKeyDown).toHaveBeenCalledTimes(1); - expect(preventDefault).not.toBeCalled(); - expect(onKeyDown).toHaveBeenCalledWith( + test('key up', () => { + const target = createEventTarget(ref.current); + target.keydown({key: 'Q'}); + target.keyup({key: 'Q'}); + expect(onKeyUp).toHaveBeenCalledTimes(1); + expect(onKeyUp).toHaveBeenCalledWith( expect.objectContaining({ - key: 'Tab', - type: 'keydown', + altKey: false, + ctrlKey: false, defaultPrevented: false, + isComposing: false, + key: 'Q', + metaKey: false, + pointerType: 'keyboard', + shiftKey: false, + target: target.node, + timeStamp: expect.any(Number), + type: 'keyboard:keyup', }), ); + }); - onKeyDown = jest.fn(); - ref = React.createRef(); - Component = () => { - const listener = useKeyboard({ - onKeyDown, - preventKeys: [['Tab', {metaKey: true}]], - }); - return
; - }; - ReactDOM.render(, container); - - preventDefault = jest.fn(); - target = createEventTarget(ref.current); - target.keydown({key: 'Tab', preventDefault, metaKey: false}); - expect(onKeyDown).toHaveBeenCalledTimes(1); - expect(preventDefault).not.toBeCalled(); - expect(onKeyDown).toHaveBeenCalledWith( + test('modified key up', () => { + const target = createEventTarget(ref.current); + target.keydown({key: 'Q'}); + target.keyup({ + key: 'Q', + altKey: true, + ctrlKey: true, + shiftKey: true, + metaKey: true, + }); + expect(onKeyUp).toHaveBeenCalledWith( expect.objectContaining({ - key: 'Tab', - type: 'keydown', + altKey: true, + ctrlKey: true, defaultPrevented: false, + isComposing: false, + key: 'Q', + metaKey: true, + pointerType: 'keyboard', + shiftKey: true, + target: target.node, + timeStamp: expect.any(Number), + type: 'keyboard:keyup', }), ); }); + }); - it('onKeyDown is default prevented (truthy modifier keys)', () => { - let onKeyDown = jest.fn(); - let ref = React.createRef(); - let Component = () => { - const listener = useKeyboard({ - onKeyDown, - preventKeys: [['Tab', {metaKey: true}]], - }); + describe('preventKeys', () => { + function render(props) { + const ref = React.createRef(); + const Component = () => { + const listener = useKeyboard(props); return
; }; ReactDOM.render(, container); + return ref; + } + + test('key config matches', () => { + const onKeyDown = jest.fn(); + const preventDefault = jest.fn(); + const preventDefaultClick = jest.fn(); + const ref = render({onKeyDown, preventKeys: ['Tab']}); + + const target = createEventTarget(ref.current); + target.keydown({key: 'Tab', preventDefault}); + target.click({preventDefault: preventDefaultClick}); - let preventDefault = jest.fn(); - let target = createEventTarget(ref.current); - target.keydown({key: 'Tab', preventDefault, metaKey: true}); expect(onKeyDown).toHaveBeenCalledTimes(1); expect(preventDefault).toBeCalled(); + expect(preventDefaultClick).toBeCalled(); expect(onKeyDown).toHaveBeenCalledWith( expect.objectContaining({ - key: 'Tab', - type: 'keydown', defaultPrevented: true, + key: 'Tab', + type: 'keyboard:keydown', }), ); + }); - onKeyDown = jest.fn(); - ref = React.createRef(); - Component = () => { - const listener = useKeyboard({ - onKeyDown, - preventKeys: [['Tab', {metaKey: false}]], - }); - return
; - }; - ReactDOM.render(, container); + test('key config matches (modifier keys)', () => { + const onKeyDown = jest.fn(); + const preventDefault = jest.fn(); + const preventDefaultClick = jest.fn(); + const ref = render({onKeyDown, preventKeys: [['Tab', {shiftKey: true}]]}); + + const target = createEventTarget(ref.current); + target.keydown({key: 'Tab', preventDefault, shiftKey: true}); + target.click({preventDefault: preventDefaultClick, shiftKey: true}); - preventDefault = jest.fn(); - target = createEventTarget(ref.current); - target.keydown({key: 'Tab', preventDefault, metaKey: false}); expect(onKeyDown).toHaveBeenCalledTimes(1); expect(preventDefault).toBeCalled(); + expect(preventDefaultClick).toBeCalled(); expect(onKeyDown).toHaveBeenCalledWith( expect.objectContaining({ - key: 'Tab', - type: 'keydown', defaultPrevented: true, + key: 'Tab', + shiftKey: true, + type: 'keyboard:keydown', }), ); }); - }); - describe('onKeyUp', () => { - let onKeyDown, onKeyUp, ref; - - beforeEach(() => { - onKeyDown = jest.fn(); - onKeyUp = jest.fn(); - ref = React.createRef(); - const Component = () => { - const listener = useKeyboard({ - onKeyDown, - onKeyUp, - }); - return
; - }; - ReactDOM.render(, container); - }); + test('key config does not match (modifier keys)', () => { + const onKeyDown = jest.fn(); + const preventDefault = jest.fn(); + const preventDefaultClick = jest.fn(); + const ref = render({onKeyDown, preventKeys: [['Tab', {shiftKey: true}]]}); - it('is called after "keydown" event', () => { const target = createEventTarget(ref.current); - target.keydown({key: 'Q'}); - target.keyup({key: 'Q'}); + target.keydown({key: 'Tab', preventDefault, shiftKey: false}); + target.click({preventDefault: preventDefaultClick, shiftKey: false}); + expect(onKeyDown).toHaveBeenCalledTimes(1); + expect(preventDefault).not.toBeCalled(); + expect(preventDefaultClick).not.toBeCalled(); expect(onKeyDown).toHaveBeenCalledWith( - expect.objectContaining({key: 'Q', type: 'keydown'}), - ); - expect(onKeyUp).toHaveBeenCalledTimes(1); - expect(onKeyUp).toHaveBeenCalledWith( - expect.objectContaining({key: 'Q', type: 'keyup'}), + expect.objectContaining({ + defaultPrevented: false, + key: 'Tab', + shiftKey: false, + type: 'keyboard:keydown', + }), ); }); }); - - describe('correctly handles responder propagation', () => { - describe('onKeyDown', () => { - let onKeyDownInner, onKeyDownOuter, ref; - - function renderPropagationTest(propagates) { - onKeyDownInner = jest.fn(() => propagates); - onKeyDownOuter = jest.fn(); - ref = React.createRef(); - const Component = () => { - const listenerInner = useKeyboard({ - onKeyDown: onKeyDownInner, - }); - const listenerOuter = useKeyboard({ - onKeyDown: onKeyDownOuter, - }); - return ( -
-
-
- ); - }; - ReactDOM.render(, container); - } - - it('propagates when cb returns true', () => { - renderPropagationTest(true); - const target = createEventTarget(ref.current); - target.keydown(); - expect(onKeyDownInner).toBeCalled(); - expect(onKeyDownOuter).toBeCalled(); - }); - - it('does not propagate when cb returns false', () => { - renderPropagationTest(false); - const target = createEventTarget(ref.current); - target.keydown(); - expect(onKeyDownInner).toBeCalled(); - expect(onKeyDownOuter).not.toBeCalled(); - }); - }); - - describe('onKeyUp', () => { - let onKeyUpInner, onKeyUpOuter, ref; - - function renderPropagationTest(propagates) { - onKeyUpInner = jest.fn(() => propagates); - onKeyUpOuter = jest.fn(); - ref = React.createRef(); - const Component = () => { - const listenerInner = useKeyboard({ - onKeyUp: onKeyUpInner, - }); - const listenerOuter = useKeyboard({ - onKeyUp: onKeyUpOuter, - }); - return ( -
-
-
- ); - }; - ReactDOM.render(, container); - } - - it('propagates when cb returns true', () => { - renderPropagationTest(true); - const target = createEventTarget(ref.current); - target.keydown(); - target.keyup(); - expect(onKeyUpInner).toBeCalled(); - expect(onKeyUpOuter).toBeCalled(); - }); - - it('does not propagate when cb returns false', () => { - renderPropagationTest(false); - const target = createEventTarget(ref.current); - target.keydown(); - target.keyup(); - expect(onKeyUpInner).toBeCalled(); - expect(onKeyUpOuter).not.toBeCalled(); - }); - }); - }); }); diff --git a/packages/react-events/src/dom/testing-library/domEvents.js b/packages/react-events/src/dom/testing-library/domEvents.js index e3ce897cd2708..963cbc2086fbb 100644 --- a/packages/react-events/src/dom/testing-library/domEvents.js +++ b/packages/react-events/src/dom/testing-library/domEvents.js @@ -137,6 +137,7 @@ function createKeyboardEvent( { altKey = false, ctrlKey = false, + isComposing = false, key = '', metaKey = false, preventDefault = emptyFunction, @@ -151,6 +152,7 @@ function createKeyboardEvent( getModifierState(keyArg) { createGetModifierState(keyArg, modifierState); }, + isComposing, key, metaKey, preventDefault, diff --git a/packages/react-reconciler/src/__tests__/ReactScope-test.internal.js b/packages/react-reconciler/src/__tests__/ReactScope-test.internal.js index b55422c3d531a..ee0af6be0efa3 100644 --- a/packages/react-reconciler/src/__tests__/ReactScope-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactScope-test.internal.js @@ -220,9 +220,6 @@ describe('ReactScope', () => { let target = createEventTarget(ref.current); target.keydown({key: 'Q'}); expect(onKeyDown).toHaveBeenCalledTimes(1); - expect(onKeyDown).toHaveBeenCalledWith( - expect.objectContaining({key: 'Q', type: 'keydown'}), - ); onKeyDown = jest.fn(); Component = () => { @@ -242,9 +239,6 @@ describe('ReactScope', () => { target = createEventTarget(ref.current); target.keydown({key: 'Q'}); expect(onKeyDown).toHaveBeenCalledTimes(1); - expect(onKeyDown).toHaveBeenCalledWith( - expect.objectContaining({key: 'Q', type: 'keydown'}), - ); }); });