From 45473c94cdb5d5d9642e5cde55ed3a5795720801 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Thu, 11 Apr 2019 13:20:21 -0700 Subject: [PATCH] React events: Press event fixes (#15386) 1. Fix hiding context menu for longpress via touch. 2. Fix scrolling of viewport for longpress via spacebar key. 3. Add tests for anchor-related behaviour and preventDefault. 4. Add a deactivation delay for forced activation 5. Add pointerType to Press events. NOTE: this currently extends pointerType to include `keyboard`. NOTE: React Native doesn't have a deactivation delay for forced activation, but this is possibly because of the async bridge meaning that the events aren't dispatched sync. --- packages/react-events/src/Hover.js | 3 +- packages/react-events/src/Press.js | 104 +++++++++--- .../src/__tests__/Press-test.internal.js | 160 +++++++++++++++++- 3 files changed, 240 insertions(+), 27 deletions(-) diff --git a/packages/react-events/src/Hover.js b/packages/react-events/src/Hover.js index 1eebab060f5e7..de4bcd2f41c18 100644 --- a/packages/react-events/src/Hover.js +++ b/packages/react-events/src/Hover.js @@ -72,8 +72,9 @@ function dispatchHoverChangeEvent( props: HoverProps, state: HoverState, ): void { + const bool = state.isActiveHovered; const listener = () => { - props.onHoverChange(state.isActiveHovered); + props.onHoverChange(bool); }; const syntheticEvent = createHoverEvent( 'hoverchange', diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index ce58d7a8c6308..74d3fcc78f3de 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -36,6 +36,8 @@ type PressProps = { stopPropagation: boolean, }; +type PointerType = '' | 'mouse' | 'keyboard' | 'pen' | 'touch'; + type PressState = { didDispatchEvent: boolean, isActivePressed: boolean, @@ -45,6 +47,7 @@ type PressState = { isPressed: boolean, isPressWithinResponderRegion: boolean, longPressTimeout: null | Symbol, + pointerType: PointerType, pressTarget: null | Element | Document, pressEndTimeout: null | Symbol, pressStartTimeout: null | Symbol, @@ -70,6 +73,7 @@ type PressEvent = {| listener: PressEvent => void, target: Element | Document, type: PressEventType, + pointerType: PointerType, |}; const DEFAULT_PRESS_END_DELAY_MS = 0; @@ -85,9 +89,10 @@ const DEFAULT_PRESS_RETENTION_OFFSET = { const targetEventTypes = [ {name: 'click', passive: false}, {name: 'keydown', passive: false}, + {name: 'keypress', passive: false}, + {name: 'contextmenu', passive: false}, 'pointerdown', 'pointercancel', - 'contextmenu', ]; const rootEventTypes = [ {name: 'keyup', passive: false}, @@ -110,11 +115,13 @@ function createPressEvent( type: PressEventType, target: Element | Document, listener: PressEvent => void, + pointerType: PointerType, ): PressEvent { return { listener, target, type, + pointerType, }; } @@ -125,7 +132,8 @@ function dispatchEvent( listener: (e: Object) => void, ): void { const target = ((state.pressTarget: any): Element | Document); - const syntheticEvent = createPressEvent(name, target, listener); + const pointerType = state.pointerType; + const syntheticEvent = createPressEvent(name, target, listener, pointerType); context.dispatchEvent(syntheticEvent, { discrete: true, }); @@ -137,8 +145,9 @@ function dispatchPressChangeEvent( props: PressProps, state: PressState, ): void { + const bool = state.isActivePressed; const listener = () => { - props.onPressChange(state.isActivePressed); + props.onPressChange(bool); }; dispatchEvent(context, state, 'presschange', listener); } @@ -148,8 +157,9 @@ function dispatchLongPressChangeEvent( props: PressProps, state: PressState, ): void { + const bool = state.isLongPressed; const listener = () => { - props.onLongPressChange(state.isLongPressed); + props.onLongPressChange(bool); }; dispatchEvent(context, state, 'longpresschange', listener); } @@ -251,6 +261,7 @@ function dispatchPressEndEvents( state: PressState, ): void { const wasActivePressStart = state.isActivePressStart; + let activationWasForced = false; state.isActivePressStart = false; state.isPressed = false; @@ -267,13 +278,17 @@ function dispatchPressEndEvents( if (state.isPressWithinResponderRegion) { // if we haven't yet activated (due to delays), activate now activate(context, props, state); + activationWasForced = true; } } if (state.isActivePressed) { const delayPressEnd = calculateDelayMS( props.delayPressEnd, - 0, + // if activation and deactivation occur during the same event there's no + // time for visual user feedback therefore a small delay is added before + // deactivating. + activationWasForced ? 10 : 0, DEFAULT_PRESS_END_DELAY_MS, ); if (delayPressEnd > 0) { @@ -338,6 +353,23 @@ function calculateResponderRegion(target, props) { }; } +function getPointerType(nativeEvent: any) { + const {type, pointerType} = nativeEvent; + if (pointerType != null) { + return pointerType; + } + if (type.indexOf('mouse') > -1) { + return 'mouse'; + } + if (type.indexOf('touch') > -1) { + return 'touch'; + } + if (type.indexOf('key') > -1) { + return 'keyboard'; + } + return ''; +} + function isPressWithinResponderRegion( nativeEvent: $PropertyType, state: PressState, @@ -377,6 +409,7 @@ const PressResponder = { isPressed: false, isPressWithinResponderRegion: true, longPressTimeout: null, + pointerType: '', pressEndTimeout: null, pressStartTimeout: null, pressTarget: null, @@ -403,10 +436,10 @@ const PressResponder = { !context.hasOwnership() && !state.shouldSkipMouseAfterTouch ) { - if ( - (nativeEvent: any).pointerType === 'mouse' || - type === 'mousedown' - ) { + const pointerType = getPointerType(nativeEvent); + state.pointerType = pointerType; + + if (pointerType === 'mouse' || type === 'mousedown') { if ( // Ignore right- and middle-clicks nativeEvent.button === 1 || @@ -436,6 +469,9 @@ const PressResponder = { return; } + const pointerType = getPointerType(nativeEvent); + state.pointerType = pointerType; + if (state.responderRegion == null) { let currentTarget = (target: any); while ( @@ -470,6 +506,9 @@ const PressResponder = { return; } + const pointerType = getPointerType(nativeEvent); + state.pointerType = pointerType; + const wasLongPressed = state.isLongPressed; dispatchPressEndEvents(context, props, state); @@ -506,6 +545,8 @@ const PressResponder = { state.isAnchorTouched = true; return; } + const pointerType = getPointerType(nativeEvent); + state.pointerType = pointerType; state.pressTarget = target; state.isPressWithinResponderRegion = true; dispatchPressStartEvents(context, props, state); @@ -519,6 +560,9 @@ const PressResponder = { return; } if (state.isPressed) { + const pointerType = getPointerType(nativeEvent); + state.pointerType = pointerType; + const wasLongPressed = state.isLongPressed; dispatchPressEndEvents(context, props, state); @@ -556,20 +600,24 @@ const PressResponder = { * Keyboard interaction support * TODO: determine UX for metaKey + validKeyPress interactions */ - case 'keydown': { + case 'keydown': + case 'keypress': { if ( - !state.isPressed && - !state.isLongPressed && !context.hasOwnership() && isValidKeyPress((nativeEvent: any).key) ) { - // Prevent spacebar press from scrolling the window - if ((nativeEvent: any).key === ' ') { - (nativeEvent: any).preventDefault(); + if (state.isPressed) { + // Prevent spacebar press from scrolling the window + if ((nativeEvent: any).key === ' ') { + (nativeEvent: any).preventDefault(); + } + } else { + const pointerType = getPointerType(nativeEvent); + state.pointerType = pointerType; + state.pressTarget = target; + dispatchPressStartEvents(context, props, state); + context.addRootEventTypes(target.ownerDocument, rootEventTypes); } - state.pressTarget = target; - dispatchPressStartEvents(context, props, state); - context.addRootEventTypes(target.ownerDocument, rootEventTypes); } break; } @@ -593,7 +641,6 @@ const PressResponder = { break; } - case 'contextmenu': case 'pointercancel': case 'scroll': case 'touchcancel': { @@ -608,14 +655,29 @@ const PressResponder = { case 'click': { if (isAnchorTagElement(target)) { const {ctrlKey, metaKey, shiftKey} = ((nativeEvent: any): MouseEvent); + // Check "open in new window/tab" and "open context menu" key modifiers const preventDefault = props.preventDefault; - // Check "open in new window/tab" key modifiers - if (preventDefault !== false && !shiftKey && !ctrlKey && !metaKey) { + if (preventDefault !== false && !shiftKey && !metaKey && !ctrlKey) { (nativeEvent: any).preventDefault(); } } + break; + } + + case 'contextmenu': { + if (state.isPressed) { + if (props.preventDefault !== false) { + (nativeEvent: any).preventDefault(); + } else { + state.shouldSkipMouseAfterTouch = false; + dispatchPressEndEvents(context, props, state); + context.removeRootEventTypes(rootEventTypes); + } + } + break; } } + if (state.didDispatchEvent) { const shouldStopPropagation = props.stopPropagation === undefined ? true : props.stopPropagation; diff --git a/packages/react-events/src/__tests__/Press-test.internal.js b/packages/react-events/src/__tests__/Press-test.internal.js index 331685eb4d1c9..f82ecb287ac93 100644 --- a/packages/react-events/src/__tests__/Press-test.internal.js +++ b/packages/react-events/src/__tests__/Press-test.internal.js @@ -70,8 +70,13 @@ describe('Event responder: Press', () => { }); it('is called after "pointerdown" event', () => { - ref.current.dispatchEvent(createPointerEvent('pointerdown')); + ref.current.dispatchEvent( + createPointerEvent('pointerdown', {pointerType: 'pen'}), + ); expect(onPressStart).toHaveBeenCalledTimes(1); + expect(onPressStart).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'pen', type: 'pressstart'}), + ); }); it('ignores browser emulated "mousedown" event', () => { @@ -85,13 +90,20 @@ describe('Event responder: Press', () => { ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); expect(onPressStart).toHaveBeenCalledTimes(1); + expect(onPressStart).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'keyboard', type: 'pressstart'}), + ); }); it('is called once after "keydown" events for Spacebar', () => { ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: ' '})); + ref.current.dispatchEvent(createKeyboardEvent('keypress', {key: ' '})); ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: ' '})); - ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: ' '})); + ref.current.dispatchEvent(createKeyboardEvent('keypress', {key: ' '})); expect(onPressStart).toHaveBeenCalledTimes(1); + expect(onPressStart).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'keyboard', type: 'pressstart'}), + ); }); it('is not called after "keydown" for other keys', () => { @@ -103,10 +115,16 @@ describe('Event responder: Press', () => { it('is called after "mousedown" event', () => { ref.current.dispatchEvent(createPointerEvent('mousedown')); expect(onPressStart).toHaveBeenCalledTimes(1); + expect(onPressStart).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'mouse', type: 'pressstart'}), + ); }); it('is called after "touchstart" event', () => { ref.current.dispatchEvent(createPointerEvent('touchstart')); expect(onPressStart).toHaveBeenCalledTimes(1); + expect(onPressStart).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'touch', type: 'pressstart'}), + ); }); describe('delayPressStart', () => { @@ -191,8 +209,13 @@ describe('Event responder: Press', () => { it('is called after "pointerup" event', () => { ref.current.dispatchEvent(createPointerEvent('pointerdown')); - ref.current.dispatchEvent(createPointerEvent('pointerup')); + ref.current.dispatchEvent( + createPointerEvent('pointerup', {pointerType: 'pen'}), + ); expect(onPressEnd).toHaveBeenCalledTimes(1); + expect(onPressEnd).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'pen', type: 'pressend'}), + ); }); it('ignores browser emulated "mouseup" event', () => { @@ -200,18 +223,27 @@ describe('Event responder: Press', () => { ref.current.dispatchEvent(createPointerEvent('touchend')); ref.current.dispatchEvent(createPointerEvent('mouseup')); expect(onPressEnd).toHaveBeenCalledTimes(1); + expect(onPressEnd).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'touch', type: 'pressend'}), + ); }); it('is called after "keyup" event for Enter', () => { ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); ref.current.dispatchEvent(createKeyboardEvent('keyup', {key: 'Enter'})); expect(onPressEnd).toHaveBeenCalledTimes(1); + expect(onPressEnd).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'keyboard', type: 'pressend'}), + ); }); it('is called after "keyup" event for Spacebar', () => { ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: ' '})); ref.current.dispatchEvent(createKeyboardEvent('keyup', {key: ' '})); expect(onPressEnd).toHaveBeenCalledTimes(1); + expect(onPressEnd).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'keyboard', type: 'pressend'}), + ); }); it('is not called after "keyup" event for other keys', () => { @@ -225,11 +257,17 @@ describe('Event responder: Press', () => { ref.current.dispatchEvent(createPointerEvent('mousedown')); ref.current.dispatchEvent(createPointerEvent('mouseup')); expect(onPressEnd).toHaveBeenCalledTimes(1); + expect(onPressEnd).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'mouse', type: 'pressend'}), + ); }); it('is called after "touchend" event', () => { ref.current.dispatchEvent(createPointerEvent('touchstart')); ref.current.dispatchEvent(createPointerEvent('touchend')); expect(onPressEnd).toHaveBeenCalledTimes(1); + expect(onPressEnd).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'touch', type: 'pressend'}), + ); }); describe('delayPressEnd', () => { @@ -342,6 +380,9 @@ describe('Event responder: Press', () => { ref.current.dispatchEvent(createPointerEvent('pointerdown')); jest.advanceTimersByTime(100); ref.current.dispatchEvent(createPointerEvent('pointerup')); + jest.advanceTimersByTime(10); + expect(onPressChange).toHaveBeenCalledWith(true); + expect(onPressChange).toHaveBeenCalledWith(false); expect(onPressChange).toHaveBeenCalledTimes(2); }); @@ -431,14 +472,22 @@ describe('Event responder: Press', () => { it('is called after "pointerup" event', () => { ref.current.dispatchEvent(createPointerEvent('pointerdown')); - ref.current.dispatchEvent(createPointerEvent('pointerup')); + ref.current.dispatchEvent( + createPointerEvent('pointerup', {pointerType: 'pen'}), + ); expect(onPress).toHaveBeenCalledTimes(1); + expect(onPress).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'pen', type: 'press'}), + ); }); it('is called after valid "keyup" event', () => { ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); ref.current.dispatchEvent(createKeyboardEvent('keyup', {key: 'Enter'})); expect(onPress).toHaveBeenCalledTimes(1); + expect(onPress).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'keyboard', type: 'press'}), + ); }); it('is always called immediately after press is released', () => { @@ -508,11 +557,16 @@ describe('Event responder: Press', () => { }); it('is called if "pointerdown" lasts default delay', () => { - ref.current.dispatchEvent(createPointerEvent('pointerdown')); + ref.current.dispatchEvent( + createPointerEvent('pointerdown', {pointerType: 'pen'}), + ); jest.advanceTimersByTime(DEFAULT_LONG_PRESS_DELAY - 1); expect(onLongPress).not.toBeCalled(); jest.advanceTimersByTime(1); expect(onLongPress).toHaveBeenCalledTimes(1); + expect(onLongPress).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'pen', type: 'longpress'}), + ); }); it('is not called if "pointerup" is dispatched before delay', () => { @@ -529,6 +583,9 @@ describe('Event responder: Press', () => { expect(onLongPress).not.toBeCalled(); jest.advanceTimersByTime(1); expect(onLongPress).toHaveBeenCalledTimes(1); + expect(onLongPress).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'keyboard', type: 'longpress'}), + ); }); it('is not called if valid "keyup" is dispatched before delay', () => { @@ -691,6 +748,38 @@ describe('Event responder: Press', () => { }); }); + describe('onPressMove', () => { + it('is called after "pointermove"', () => { + const onPressMove = jest.fn(); + const ref = React.createRef(); + const element = ( + +
+ + ); + ReactDOM.render(element, container); + + ref.current.getBoundingClientRect = () => ({ + top: 50, + left: 50, + bottom: 500, + right: 500, + }); + ref.current.dispatchEvent(createPointerEvent('pointerdown')); + ref.current.dispatchEvent( + createPointerEvent('pointermove', { + pointerType: 'touch', + pageX: 55, + pageY: 55, + }), + ); + expect(onPressMove).toHaveBeenCalledTimes(1); + expect(onPressMove).toHaveBeenCalledWith( + expect.objectContaining({pointerType: 'touch', type: 'pressmove'}), + ); + }); + }); + describe('press with movement', () => { const rectMock = { width: 100, @@ -1042,6 +1131,67 @@ describe('Event responder: Press', () => { }); }); + describe('link components', () => { + it('prevents native behaviour by default', () => { + const onPress = jest.fn(); + const preventDefault = jest.fn(); + const ref = React.createRef(); + const element = ( + + + + ); + ReactDOM.render(element, container); + + ref.current.dispatchEvent(createPointerEvent('pointerdown')); + ref.current.dispatchEvent(createPointerEvent('pointerup')); + ref.current.dispatchEvent(createPointerEvent('click', {preventDefault})); + expect(preventDefault).toBeCalled(); + }); + + it('uses native behaviour for interactions with modifier keys', () => { + const onPress = jest.fn(); + const preventDefault = jest.fn(); + const ref = React.createRef(); + const element = ( + + + + ); + ReactDOM.render(element, container); + + ['metaKey', 'ctrlKey', 'shiftKey'].forEach(modifierKey => { + ref.current.dispatchEvent( + createPointerEvent('pointerdown', {[modifierKey]: true}), + ); + ref.current.dispatchEvent( + createPointerEvent('pointerup', {[modifierKey]: true}), + ); + ref.current.dispatchEvent( + createPointerEvent('click', {[modifierKey]: true, preventDefault}), + ); + expect(preventDefault).not.toBeCalled(); + }); + }); + + it('uses native behaviour if preventDefault is false', () => { + const onPress = jest.fn(); + const preventDefault = jest.fn(); + const ref = React.createRef(); + const element = ( + + + + ); + ReactDOM.render(element, container); + + ref.current.dispatchEvent(createPointerEvent('pointerdown')); + ref.current.dispatchEvent(createPointerEvent('pointerup')); + ref.current.dispatchEvent(createPointerEvent('click', {preventDefault})); + expect(preventDefault).not.toBeCalled(); + }); + }); + it('expect displayName to show up for event component', () => { expect(Press.displayName).toBe('Press'); });