From 1a9ad629741ce6d96e49498ccb353d3ae7911c73 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 3 Apr 2019 15:01:49 -0700 Subject: [PATCH 1/8] Add HoverProps type --- packages/react-events/src/Hover.js | 65 +++++++++++++++++++----------- 1 file changed, 41 insertions(+), 24 deletions(-) diff --git a/packages/react-events/src/Hover.js b/packages/react-events/src/Hover.js index 226430b838d..ff42aac232e 100644 --- a/packages/react-events/src/Hover.js +++ b/packages/react-events/src/Hover.js @@ -10,12 +10,14 @@ import type {EventResponderContext} from 'events/EventTypes'; import {REACT_EVENT_COMPONENT_TYPE} from 'shared/ReactSymbols'; -const targetEventTypes = [ - 'pointerover', - 'pointermove', - 'pointerout', - 'pointercancel', -]; +type HoverProps = { + disabled: boolean, + delayHoverEnd: number, + delayHoverStart: number, + onHoverChange: boolean => void, + onHoverEnd: (e: HoverEvent) => void, + onHoverStart: (e: HoverEvent) => void, +}; type HoverState = { isHovered: boolean, @@ -31,6 +33,21 @@ type HoverEvent = {| type: HoverEventType, |}; +// const DEFAULT_HOVER_END_DELAY_MS = 0; +// const DEFAULT_HOVER_START_DELAY_MS = 0; + +const targetEventTypes = [ + 'pointerover', + 'pointermove', + 'pointerout', + 'pointercancel', +]; + +// If PointerEvents is not supported (e.g., Safari), also listen to touch and mouse events. +if (typeof window !== 'undefined' && window.PointerEvent === undefined) { + targetEventTypes.push('touchstart', 'mouseover', 'mouseout'); +} + function createHoverEvent( type: HoverEventType, target: Element | Document, @@ -43,16 +60,9 @@ function createHoverEvent( }; } -// In the case we don't have PointerEvents (Safari), we listen to touch events -// too -if (typeof window !== 'undefined' && window.PointerEvent === undefined) { - targetEventTypes.push('touchstart', 'mouseover', 'mouseout'); -} - function dispatchHoverStartEvents( context: EventResponderContext, - props: Object, - state: HoverState, + props: HoverProps, ): void { const {event, eventTarget} = context; if (context.isTargetWithinEventComponent((event: any).relatedTarget)) { @@ -67,19 +77,22 @@ function dispatchHoverStartEvents( context.dispatchEvent(syntheticEvent, {discrete: true}); } if (props.onHoverChange) { - const hoverChangeEventListener = () => { + const listener = () => { props.onHoverChange(true); }; const syntheticEvent = createHoverEvent( 'hoverchange', eventTarget, - hoverChangeEventListener, + listener, ); context.dispatchEvent(syntheticEvent, {discrete: true}); } } -function dispatchHoverEndEvents(context: EventResponderContext, props: Object) { +function dispatchHoverEndEvents( + context: EventResponderContext, + props: HoverProps, +) { const {event, eventTarget} = context; if (context.isTargetWithinEventComponent((event: any).relatedTarget)) { return; @@ -93,13 +106,13 @@ function dispatchHoverEndEvents(context: EventResponderContext, props: Object) { context.dispatchEvent(syntheticEvent, {discrete: true}); } if (props.onHoverChange) { - const hoverChangeEventListener = () => { + const listener = () => { props.onHoverChange(false); }; const syntheticEvent = createHoverEvent( 'hoverchange', eventTarget, - hoverChangeEventListener, + listener, ); context.dispatchEvent(syntheticEvent, {discrete: true}); } @@ -116,18 +129,22 @@ const HoverResponder = { }, handleEvent( context: EventResponderContext, - props: Object, + props: HoverProps, state: HoverState, ): void { const {eventType, eventTarget, event} = context; switch (eventType) { - case 'touchstart': - // Touch devices don't have hover support + /** + * Prevent hover events when touch is being used. + */ + case 'touchstart': { if (!state.isTouched) { state.isTouched = true; } break; + } + case 'pointerover': case 'mouseover': { if ( @@ -148,7 +165,7 @@ const HoverResponder = { state.isInHitSlop = true; return; } - dispatchHoverStartEvents(context, props, state); + dispatchHoverStartEvents(context, props); state.isHovered = true; } break; @@ -172,7 +189,7 @@ const HoverResponder = { (event: any).y, ) ) { - dispatchHoverStartEvents(context, props, state); + dispatchHoverStartEvents(context, props); state.isHovered = true; state.isInHitSlop = false; } From 77468a3264aa1e6b9314583b56b267a7dd27d699 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 3 Apr 2019 15:02:29 -0700 Subject: [PATCH 2/8] Add more Hover event module tests --- .../src/__tests__/Hover-test.internal.js | 197 +++++++++++++----- 1 file changed, 142 insertions(+), 55 deletions(-) diff --git a/packages/react-events/src/__tests__/Hover-test.internal.js b/packages/react-events/src/__tests__/Hover-test.internal.js index 1250a49917a..87143c7d8a7 100644 --- a/packages/react-events/src/__tests__/Hover-test.internal.js +++ b/packages/react-events/src/__tests__/Hover-test.internal.js @@ -14,6 +14,12 @@ let ReactFeatureFlags; let ReactDOM; let Hover; +const createPointerEvent = type => { + const event = document.createEvent('Event'); + event.initEvent(type, true, true); + return event; +}; + describe('Hover event responder', () => { let container; @@ -34,69 +40,150 @@ describe('Hover event responder', () => { container = null; }); - it('should support onHover', () => { - let divRef = React.createRef(); - let events = []; - - function handleOnHover(e) { - if (e) { - events.push('hover in'); - } else { - events.push('hover out'); - } - } - - function Component() { - return ( - -
Hover me!
+ describe('onHoverStart', () => { + let onHoverStart, ref; + + beforeEach(() => { + onHoverStart = jest.fn(); + ref = React.createRef(); + const element = ( + +
); - } - - ReactDOM.render(, container); - - const mouseOverEvent = document.createEvent('Event'); - mouseOverEvent.initEvent('mouseover', true, true); - divRef.current.dispatchEvent(mouseOverEvent); - - const mouseOutEvent = document.createEvent('Event'); - mouseOutEvent.initEvent('mouseout', true, true); - divRef.current.dispatchEvent(mouseOutEvent); - - expect(events).toEqual(['hover in', 'hover out']); + ReactDOM.render(element, container); + }); + + it('is called after "pointerover" event', () => { + ref.current.dispatchEvent(createPointerEvent('pointerover')); + expect(onHoverStart).toHaveBeenCalledTimes(1); + }); + + it('is not called if "pointerover" pointerType is touch', () => { + const event = createPointerEvent('pointerover'); + event.pointerType = 'touch'; + ref.current.dispatchEvent(event); + expect(onHoverStart).not.toBeCalled(); + }); + + it('ignores browser emulated "mouseover" event', () => { + ref.current.dispatchEvent(createPointerEvent('pointerover')); + ref.current.dispatchEvent(createPointerEvent('mouseover')); + expect(onHoverStart).toHaveBeenCalledTimes(1); + }); + + // No PointerEvent fallbacks + it('is called after "mouseover" event', () => { + ref.current.dispatchEvent(createPointerEvent('mouseover')); + expect(onHoverStart).toHaveBeenCalledTimes(1); + }); + it('is not called after "touchstart"', () => { + ref.current.dispatchEvent(createPointerEvent('touchstart')); + ref.current.dispatchEvent(createPointerEvent('touchend')); + ref.current.dispatchEvent(createPointerEvent('mouseover')); + expect(onHoverStart).not.toBeCalled(); + }); + + // TODO: complete delayHoverStart tests + // describe('delayHoverStart', () => {}); }); - it('should support onHoverStart and onHoverEnd', () => { - let divRef = React.createRef(); - let events = []; - - function handleOnHoverStart() { - events.push('onHoverStart'); - } + describe('onHoverChange', () => { + let onHoverChange, ref; - function handleOnHoverEnd() { - events.push('onHoverEnd'); - } - - function Component() { - return ( - -
Hover me!
+ beforeEach(() => { + onHoverChange = jest.fn(); + ref = React.createRef(); + const element = ( + +
); - } - - ReactDOM.render(, container); - - const mouseOverEvent = document.createEvent('Event'); - mouseOverEvent.initEvent('mouseover', true, true); - divRef.current.dispatchEvent(mouseOverEvent); + ReactDOM.render(element, container); + }); + + it('is called after "pointerover" and "pointerout" events', () => { + ref.current.dispatchEvent(createPointerEvent('pointerover')); + expect(onHoverChange).toHaveBeenCalledTimes(1); + expect(onHoverChange).toHaveBeenCalledWith(true); + ref.current.dispatchEvent(createPointerEvent('pointerout')); + expect(onHoverChange).toHaveBeenCalledTimes(2); + expect(onHoverChange).toHaveBeenCalledWith(false); + }); + + // No PointerEvent fallbacks + it('is called after "mouseover" and "mouseout" events', () => { + ref.current.dispatchEvent(createPointerEvent('mouseover')); + expect(onHoverChange).toHaveBeenCalledTimes(1); + expect(onHoverChange).toHaveBeenCalledWith(true); + ref.current.dispatchEvent(createPointerEvent('mouseout')); + expect(onHoverChange).toHaveBeenCalledTimes(2); + expect(onHoverChange).toHaveBeenCalledWith(false); + }); + }); - const mouseOutEvent = document.createEvent('Event'); - mouseOutEvent.initEvent('mouseout', true, true); - divRef.current.dispatchEvent(mouseOutEvent); + describe('onHoverEnd', () => { + let onHoverEnd, ref; - expect(events).toEqual(['onHoverStart', 'onHoverEnd']); + beforeEach(() => { + onHoverEnd = jest.fn(); + ref = React.createRef(); + const element = ( + +
+ + ); + ReactDOM.render(element, container); + }); + + it('is called after "pointerout" event', () => { + ref.current.dispatchEvent(createPointerEvent('pointerover')); + ref.current.dispatchEvent(createPointerEvent('pointerout')); + expect(onHoverEnd).toHaveBeenCalledTimes(1); + }); + + it('is not called if "pointerover" pointerType is touch', () => { + const event = createPointerEvent('pointerover'); + event.pointerType = 'touch'; + ref.current.dispatchEvent(event); + ref.current.dispatchEvent(createPointerEvent('pointerout')); + expect(onHoverEnd).not.toBeCalled(); + }); + + it('ignores browser emulated "mouseout" event', () => { + ref.current.dispatchEvent(createPointerEvent('pointerover')); + ref.current.dispatchEvent(createPointerEvent('pointerout')); + ref.current.dispatchEvent(createPointerEvent('mouseout')); + expect(onHoverEnd).toHaveBeenCalledTimes(1); + }); + + it('is called after "pointercancel" event', () => { + ref.current.dispatchEvent(createPointerEvent('pointerover')); + ref.current.dispatchEvent(createPointerEvent('pointercancel')); + expect(onHoverEnd).toHaveBeenCalledTimes(1); + }); + + it('is not called again after "pointercancel" event if it follows "pointerout"', () => { + ref.current.dispatchEvent(createPointerEvent('pointerover')); + ref.current.dispatchEvent(createPointerEvent('pointerout')); + ref.current.dispatchEvent(createPointerEvent('pointercancel')); + expect(onHoverEnd).toHaveBeenCalledTimes(1); + }); + + // No PointerEvent fallbacks + it('is called after "mouseout" event', () => { + ref.current.dispatchEvent(createPointerEvent('mouseover')); + ref.current.dispatchEvent(createPointerEvent('mouseout')); + expect(onHoverEnd).toHaveBeenCalledTimes(1); + }); + it('is not called after "touchend"', () => { + ref.current.dispatchEvent(createPointerEvent('touchstart')); + ref.current.dispatchEvent(createPointerEvent('touchend')); + ref.current.dispatchEvent(createPointerEvent('mouseout')); + expect(onHoverEnd).not.toBeCalled(); + }); + + // TODO: complete delayHoverStart tests + // describe('delayHoverEnd', () => {}); }); }); From a4dbea23c704c79f0b324f41732df1f819812cbc Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 3 Apr 2019 15:02:58 -0700 Subject: [PATCH 3/8] Add more Press event module tests --- packages/react-events/src/Press.js | 41 +++++++++---------- .../src/__tests__/Press-test.internal.js | 23 +++++++++-- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index b79c38758b9..3a7b24c9c06 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -10,27 +10,6 @@ import type {EventResponderContext} from 'events/EventTypes'; import {REACT_EVENT_COMPONENT_TYPE} from 'shared/ReactSymbols'; -// const DEFAULT_PRESS_DELAY_MS = 0; -// const DEFAULT_PRESS_END_DELAY_MS = 0; -// const DEFAULT_PRESS_START_DELAY_MS = 0; -const DEFAULT_LONG_PRESS_DELAY_MS = 1000; - -const targetEventTypes = [ - {name: 'click', passive: false}, - {name: 'keydown', passive: false}, - 'pointerdown', - 'pointercancel', - 'contextmenu', -]; -const rootEventTypes = [{name: 'pointerup', passive: false}, 'scroll']; - -// In the case we don't have PointerEvents (Safari), we listen to touch events -// too -if (typeof window !== 'undefined' && window.PointerEvent === undefined) { - targetEventTypes.push('touchstart', 'touchend', 'mousedown', 'touchcancel'); - rootEventTypes.push({name: 'mouseup', passive: false}); -} - type PressProps = { disabled: boolean, delayLongPress: number, @@ -70,6 +49,26 @@ type PressEvent = {| type: PressEventType, |}; +// const DEFAULT_PRESS_DELAY_MS = 0; +// const DEFAULT_PRESS_END_DELAY_MS = 0; +// const DEFAULT_PRESS_START_DELAY_MS = 0; +const DEFAULT_LONG_PRESS_DELAY_MS = 1000; + +const targetEventTypes = [ + {name: 'click', passive: false}, + {name: 'keydown', passive: false}, + 'pointerdown', + 'pointercancel', + 'contextmenu', +]; +const rootEventTypes = [{name: 'pointerup', passive: false}, 'scroll']; + +// If PointerEvents is not supported (e.g., Safari), also listen to touch and mouse events. +if (typeof window !== 'undefined' && window.PointerEvent === undefined) { + targetEventTypes.push('touchstart', 'touchend', 'mousedown', 'touchcancel'); + rootEventTypes.push({name: 'mouseup', passive: false}); +} + function createPressEvent( type: PressEventType, target: Element | Document, diff --git a/packages/react-events/src/__tests__/Press-test.internal.js b/packages/react-events/src/__tests__/Press-test.internal.js index af3b93239e4..01bf2dc5f24 100644 --- a/packages/react-events/src/__tests__/Press-test.internal.js +++ b/packages/react-events/src/__tests__/Press-test.internal.js @@ -69,7 +69,7 @@ describe('Event responder: Press', () => { expect(onPressStart).toHaveBeenCalledTimes(1); }); - it('ignores emulated "mousedown" event', () => { + it('ignores browser emulated "mousedown" event', () => { ref.current.dispatchEvent(createPointerEvent('pointerdown')); ref.current.dispatchEvent(createPointerEvent('mousedown')); expect(onPressStart).toHaveBeenCalledTimes(1); @@ -109,7 +109,7 @@ describe('Event responder: Press', () => { expect(onPressEnd).toHaveBeenCalledTimes(1); }); - it('ignores emulated "mouseup" event', () => { + it('ignores browser emulated "mouseup" event', () => { ref.current.dispatchEvent(createPointerEvent('touchstart')); ref.current.dispatchEvent(createPointerEvent('touchend')); ref.current.dispatchEvent(createPointerEvent('mouseup')); @@ -122,7 +122,6 @@ describe('Event responder: Press', () => { ref.current.dispatchEvent(createPointerEvent('mouseup')); expect(onPressEnd).toHaveBeenCalledTimes(1); }); - it('is called after "touchend" event', () => { ref.current.dispatchEvent(createPointerEvent('touchstart')); ref.current.dispatchEvent(createPointerEvent('touchend')); @@ -155,6 +154,24 @@ describe('Event responder: Press', () => { expect(onPressChange).toHaveBeenCalledTimes(2); expect(onPressChange).toHaveBeenCalledWith(false); }); + + // No PointerEvent fallbacks + it('is called after "mousedown" and "mouseup" events', () => { + ref.current.dispatchEvent(createPointerEvent('mousedown')); + expect(onPressChange).toHaveBeenCalledTimes(1); + expect(onPressChange).toHaveBeenCalledWith(true); + ref.current.dispatchEvent(createPointerEvent('mouseup')); + expect(onPressChange).toHaveBeenCalledTimes(2); + expect(onPressChange).toHaveBeenCalledWith(false); + }); + it('is called after "touchstart" and "touchend" events', () => { + ref.current.dispatchEvent(createPointerEvent('touchstart')); + expect(onPressChange).toHaveBeenCalledTimes(1); + expect(onPressChange).toHaveBeenCalledWith(true); + ref.current.dispatchEvent(createPointerEvent('touchend')); + expect(onPressChange).toHaveBeenCalledTimes(2); + expect(onPressChange).toHaveBeenCalledWith(false); + }); }); describe('onPress', () => { From 3921e4521c174c25f2cb515b895341714c785485 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 3 Apr 2019 15:06:14 -0700 Subject: [PATCH 4/8] Change default longPress delay from 1000 to 500 The default longPress delay should be 500 to match Android and React Native - https://android.googlesource.com/platform/frameworks/base/+/master/core/java/android/view/ViewConfiguration.java#66. --- packages/react-events/src/Press.js | 2 +- packages/react-events/src/__tests__/Press-test.internal.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index 3a7b24c9c06..617389c6f7a 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -52,7 +52,7 @@ type PressEvent = {| // const DEFAULT_PRESS_DELAY_MS = 0; // const DEFAULT_PRESS_END_DELAY_MS = 0; // const DEFAULT_PRESS_START_DELAY_MS = 0; -const DEFAULT_LONG_PRESS_DELAY_MS = 1000; +const DEFAULT_LONG_PRESS_DELAY_MS = 500; const targetEventTypes = [ {name: 'click', passive: false}, diff --git a/packages/react-events/src/__tests__/Press-test.internal.js b/packages/react-events/src/__tests__/Press-test.internal.js index 01bf2dc5f24..b68b338fb7e 100644 --- a/packages/react-events/src/__tests__/Press-test.internal.js +++ b/packages/react-events/src/__tests__/Press-test.internal.js @@ -14,7 +14,7 @@ let ReactFeatureFlags; let ReactDOM; let Press; -const DEFAULT_LONG_PRESS_DELAY = 1000; +const DEFAULT_LONG_PRESS_DELAY = 500; const createPointerEvent = type => { const event = document.createEvent('Event'); From b27591458ebdecc96117a36aff19a6a6313c11a4 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 3 Apr 2019 15:11:50 -0700 Subject: [PATCH 5/8] Rename dispatchPressEvent -> dispatchEvent --- packages/react-events/src/Press.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index 617389c6f7a..0a2bf558884 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -81,7 +81,7 @@ function createPressEvent( }; } -function dispatchPressEvent( +function dispatchEvent( context: EventResponderContext, state: PressState, name: PressEventType, @@ -101,11 +101,11 @@ function dispatchPressStartEvents( const pressChangeEventListener = () => { props.onPressChange(bool); }; - dispatchPressEvent(context, state, 'presschange', pressChangeEventListener); + dispatchEvent(context, state, 'presschange', pressChangeEventListener); } if (props.onPressStart) { - dispatchPressEvent(context, state, 'pressstart', props.onPressStart); + dispatchEvent(context, state, 'pressstart', props.onPressStart); } if (props.onPressChange) { dispatchPressChangeEvent(true); @@ -131,7 +131,7 @@ function dispatchPressStartEvents( // state.defaultPrevented = true; // } }; - dispatchPressEvent( + dispatchEvent( context, state, 'longpress', @@ -143,7 +143,7 @@ function dispatchPressStartEvents( const longPressChangeEventListener = () => { props.onLongPressChange(true); }; - dispatchPressEvent( + dispatchEvent( context, state, 'longpresschange', @@ -166,19 +166,19 @@ function dispatchPressEndEvents( state.longPressTimeout = null; } if (props.onPressEnd) { - dispatchPressEvent(context, state, 'pressend', props.onPressEnd); + dispatchEvent(context, state, 'pressend', props.onPressEnd); } if (props.onPressChange) { const pressChangeEventListener = () => { props.onPressChange(false); }; - dispatchPressEvent(context, state, 'presschange', pressChangeEventListener); + dispatchEvent(context, state, 'presschange', pressChangeEventListener); } if (props.onLongPressChange && state.isLongPressed) { const longPressChangeEventListener = () => { props.onLongPressChange(false); }; - dispatchPressEvent( + dispatchEvent( context, state, 'longpresschange', @@ -230,7 +230,7 @@ const PressResponder = { ) { return; } - dispatchPressEvent(context, state, 'press', props.onPress); + dispatchEvent(context, state, 'press', props.onPress); break; } @@ -282,7 +282,7 @@ const PressResponder = { props.onLongPressShouldCancelPress() ) ) { - dispatchPressEvent(context, state, 'press', props.onPress); + dispatchEvent(context, state, 'press', props.onPress); } } } @@ -357,7 +357,7 @@ const PressResponder = { // state.defaultPrevented = true; // } }; - dispatchPressEvent(context, state, 'press', pressEventListener); + dispatchEvent(context, state, 'press', pressEventListener); } } } From 6a0e6b13b9a89f278ca73ea854a143ec472ea8a8 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 3 Apr 2019 15:23:51 -0700 Subject: [PATCH 6/8] Consolidate state updates in Press event module --- packages/react-events/src/Press.js | 128 ++++++++++++++--------------- 1 file changed, 61 insertions(+), 67 deletions(-) diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index 0a2bf558884..b2cb5bc47ef 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -92,23 +92,40 @@ function dispatchEvent( context.dispatchEvent(syntheticEvent, {discrete: true}); } +function dispatchPressChangeEvent( + context: EventResponderContext, + props: PressProps, + state: PressState, +): void { + const listener = () => { + props.onPressChange(state.isPressed); + }; + dispatchEvent(context, state, 'presschange', listener); +} + +function dispatchLongPressChangeEvent( + context: EventResponderContext, + props: PressProps, + state: PressState, +): void { + const listener = () => { + props.onLongPressChange(state.isLongPressed); + }; + dispatchEvent(context, state, 'longpresschange', listener); +} + function dispatchPressStartEvents( context: EventResponderContext, props: PressProps, state: PressState, ): void { - function dispatchPressChangeEvent(bool) { - const pressChangeEventListener = () => { - props.onPressChange(bool); - }; - dispatchEvent(context, state, 'presschange', pressChangeEventListener); - } + state.isPressed = true; if (props.onPressStart) { dispatchEvent(context, state, 'pressstart', props.onPressStart); } if (props.onPressChange) { - dispatchPressChangeEvent(true); + dispatchPressChangeEvent(context, props, state); } if ((props.onLongPress || props.onLongPressChange) && !state.isLongPressed) { const delayLongPress = calculateDelayMS( @@ -124,31 +141,18 @@ function dispatchPressStartEvents( state.longPressTimeout = null; if (props.onLongPress) { - const longPressEventListener = e => { + const listener = e => { props.onLongPress(e); // TODO address this again at some point // if (e.nativeEvent.defaultPrevented) { // state.defaultPrevented = true; // } }; - dispatchEvent( - context, - state, - 'longpress', - longPressEventListener, - ); + dispatchEvent(context, state, 'longpress', listener); } if (props.onLongPressChange) { - const longPressChangeEventListener = () => { - props.onLongPressChange(true); - }; - dispatchEvent( - context, - state, - 'longpresschange', - longPressChangeEventListener, - ); + dispatchLongPressChangeEvent(context, props, state); } }), delayLongPress, @@ -168,22 +172,19 @@ function dispatchPressEndEvents( if (props.onPressEnd) { dispatchEvent(context, state, 'pressend', props.onPressEnd); } - if (props.onPressChange) { - const pressChangeEventListener = () => { - props.onPressChange(false); - }; - dispatchEvent(context, state, 'presschange', pressChangeEventListener); + + if (state.isPressed) { + state.isPressed = false; + if (props.onPressChange) { + dispatchPressChangeEvent(context, props, state); + } } - if (props.onLongPressChange && state.isLongPressed) { - const longPressChangeEventListener = () => { - props.onLongPressChange(false); - }; - dispatchEvent( - context, - state, - 'longpresschange', - longPressChangeEventListener, - ); + + if (state.isLongPressed) { + state.isLongPressed = false; + if (props.onLongPressChange) { + dispatchLongPressChangeEvent(context, props, state); + } } } @@ -238,7 +239,7 @@ const PressResponder = { * Touch event implementations are only needed for Safari, which lacks * support for pointer events. */ - case 'touchstart': + case 'touchstart': { if (!state.isPressed && !context.isTargetOwned(eventTarget)) { // We bail out of polyfilling anchor tags, given the same heuristics // explained above in regards to needing to use click events. @@ -248,21 +249,22 @@ const PressResponder = { } state.pressTarget = eventTarget; dispatchPressStartEvents(context, props, state); - state.isPressed = true; context.addRootEventTypes(rootEventTypes); } - break; + } + case 'touchend': { if (state.isAnchorTouched) { + state.isAnchorTouched = false; return; } if (state.isPressed) { + const wasLongPressed = state.isLongPressed; + dispatchPressEndEvents(context, props, state); - if ( - eventType !== 'touchcancel' && - (props.onPress || props.onLongPress) - ) { + + if (eventType !== 'touchcancel' && props.onPress) { // Find if the X/Y of the end touch is still that of the original target const changedTouch = (event: any).changedTouches[0]; const doc = (eventTarget: any).ownerDocument; @@ -275,9 +277,8 @@ const PressResponder = { context.isTargetWithinEventComponent(target) ) { if ( - props.onPress && !( - state.isLongPressed && + wasLongPressed && props.onLongPressShouldCancelPress && props.onLongPressShouldCancelPress() ) @@ -286,8 +287,6 @@ const PressResponder = { } } } - state.isPressed = false; - state.isLongPressed = false; state.shouldSkipMouseAfterTouch = true; context.removeRootEventTypes(rootEventTypes); } @@ -308,8 +307,11 @@ const PressResponder = { (event: any).pointerType === 'mouse' || eventType === 'mousedown' ) { - // Ignore if we are pressing on hit slop area with mouse if ( + // Ignore right- and middle-clicks + event.button === 1 || + event.button === 2 || + // Ignore pressing on hit slop area with mouse context.isPositionWithinTouchHitTarget( (event: any).x, (event: any).y, @@ -317,14 +319,9 @@ const PressResponder = { ) { return; } - // Ignore middle- and right-clicks - if (event.button === 2 || event.button === 1) { - return; - } } state.pressTarget = eventTarget; dispatchPressStartEvents(context, props, state); - state.isPressed = true; context.addRootEventTypes(rootEventTypes); } break; @@ -336,33 +333,31 @@ const PressResponder = { state.shouldSkipMouseAfterTouch = false; return; } + + const wasLongPressed = state.isLongPressed; + dispatchPressEndEvents(context, props, state); - if ( - state.pressTarget !== null && - (props.onPress || props.onLongPress) - ) { + + if (state.pressTarget !== null && props.onPress) { if (context.isTargetWithinElement(eventTarget, state.pressTarget)) { if ( - props.onPress && !( - state.isLongPressed && + wasLongPressed && props.onLongPressShouldCancelPress && props.onLongPressShouldCancelPress() ) ) { - const pressEventListener = e => { + const listener = e => { props.onPress(e); // TODO address this again at some point // if (e.nativeEvent.defaultPrevented) { // state.defaultPrevented = true; // } }; - dispatchEvent(context, state, 'press', pressEventListener); + dispatchEvent(context, state, 'press', listener); } } } - state.isPressed = false; - state.isLongPressed = false; context.removeRootEventTypes(rootEventTypes); } state.isAnchorTouched = false; @@ -376,12 +371,11 @@ const PressResponder = { if (state.isPressed) { state.shouldSkipMouseAfterTouch = false; dispatchPressEndEvents(context, props, state); - state.isPressed = false; - state.isLongPressed = false; context.removeRootEventTypes(rootEventTypes); } break; } + case 'click': { if (state.defaultPrevented) { (event: any).preventDefault(); From 8d9278fed2b925d1f6dce5ccbb136d14594a2833 Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 3 Apr 2019 15:53:15 -0700 Subject: [PATCH 7/8] Add keyboard support for Press events --- packages/react-events/src/Press.js | 150 +++++++++++------- .../src/__tests__/Hover-test.internal.js | 4 + .../src/__tests__/Press-test.internal.js | 89 +++++++++-- 3 files changed, 174 insertions(+), 69 deletions(-) diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index b2cb5bc47ef..69251a1fa3e 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -61,7 +61,11 @@ const targetEventTypes = [ 'pointercancel', 'contextmenu', ]; -const rootEventTypes = [{name: 'pointerup', passive: false}, 'scroll']; +const rootEventTypes = [ + {name: 'keyup', passive: false}, + {name: 'pointerup', passive: false}, + 'scroll', +]; // If PointerEvents is not supported (e.g., Safari), also listen to touch and mouse events. if (typeof window !== 'undefined' && window.PointerEvent === undefined) { @@ -223,15 +227,74 @@ const PressResponder = { const {eventTarget, eventType, event} = context; switch (eventType) { - case 'keydown': { + /** + * Respond to pointer events and fall back to mouse. + */ + case 'pointerdown': + case 'mousedown': { if ( - !props.onPress || - context.isTargetOwned(eventTarget) || - !isValidKeyPress((event: any).key) + !state.isPressed && + !context.isTargetOwned(eventTarget) && + !state.shouldSkipMouseAfterTouch ) { - return; + if ( + (event: any).pointerType === 'mouse' || + eventType === 'mousedown' + ) { + if ( + // Ignore right- and middle-clicks + event.button === 1 || + event.button === 2 || + // Ignore pressing on hit slop area with mouse + context.isPositionWithinTouchHitTarget( + (event: any).x, + (event: any).y, + ) + ) { + return; + } + } + state.pressTarget = eventTarget; + dispatchPressStartEvents(context, props, state); + context.addRootEventTypes(rootEventTypes); } - dispatchEvent(context, state, 'press', props.onPress); + break; + } + case 'pointerup': + case 'mouseup': { + if (state.isPressed) { + if (state.shouldSkipMouseAfterTouch) { + state.shouldSkipMouseAfterTouch = false; + return; + } + + const wasLongPressed = state.isLongPressed; + + dispatchPressEndEvents(context, props, state); + + if (state.pressTarget !== null && props.onPress) { + if (context.isTargetWithinElement(eventTarget, state.pressTarget)) { + if ( + !( + wasLongPressed && + props.onLongPressShouldCancelPress && + props.onLongPressShouldCancelPress() + ) + ) { + const listener = e => { + props.onPress(e); + // TODO address this again at some point + // if (e.nativeEvent.defaultPrevented) { + // state.defaultPrevented = true; + // } + }; + dispatchEvent(context, state, 'press', listener); + } + } + } + context.removeRootEventTypes(rootEventTypes); + } + state.isAnchorTouched = false; break; } @@ -253,7 +316,6 @@ const PressResponder = { } break; } - case 'touchend': { if (state.isAnchorTouched) { state.isAnchorTouched = false; @@ -294,31 +356,19 @@ const PressResponder = { } /** - * Respond to pointer events and fall back to mouse. + * Keyboard interaction support + * TODO: determine UX for metaKey + validKeyPress interactions */ - case 'pointerdown': - case 'mousedown': { + case 'keydown': { if ( !state.isPressed && + !state.isLongPressed && !context.isTargetOwned(eventTarget) && - !state.shouldSkipMouseAfterTouch + isValidKeyPress((event: any).key) ) { - if ( - (event: any).pointerType === 'mouse' || - eventType === 'mousedown' - ) { - if ( - // Ignore right- and middle-clicks - event.button === 1 || - event.button === 2 || - // Ignore pressing on hit slop area with mouse - context.isPositionWithinTouchHitTarget( - (event: any).x, - (event: any).y, - ) - ) { - return; - } + // Prevent spacebar press from scrolling the window + if ((event: any).key === ' ') { + (event: any).preventDefault(); } state.pressTarget = eventTarget; dispatchPressStartEvents(context, props, state); @@ -326,48 +376,30 @@ const PressResponder = { } break; } - case 'pointerup': - case 'mouseup': { - if (state.isPressed) { - if (state.shouldSkipMouseAfterTouch) { - state.shouldSkipMouseAfterTouch = false; - return; - } - + case 'keyup': { + if (state.isPressed && isValidKeyPress((event: any).key)) { const wasLongPressed = state.isLongPressed; - dispatchPressEndEvents(context, props, state); - if (state.pressTarget !== null && props.onPress) { - if (context.isTargetWithinElement(eventTarget, state.pressTarget)) { - if ( - !( - wasLongPressed && - props.onLongPressShouldCancelPress && - props.onLongPressShouldCancelPress() - ) - ) { - const listener = e => { - props.onPress(e); - // TODO address this again at some point - // if (e.nativeEvent.defaultPrevented) { - // state.defaultPrevented = true; - // } - }; - dispatchEvent(context, state, 'press', listener); - } + if ( + !( + wasLongPressed && + props.onLongPressShouldCancelPress && + props.onLongPressShouldCancelPress() + ) + ) { + dispatchEvent(context, state, 'press', props.onPress); } } context.removeRootEventTypes(rootEventTypes); } - state.isAnchorTouched = false; break; } - case 'scroll': - case 'touchcancel': case 'contextmenu': - case 'pointercancel': { + case 'pointercancel': + case 'scroll': + case 'touchcancel': { if (state.isPressed) { state.shouldSkipMouseAfterTouch = false; dispatchPressEndEvents(context, props, state); diff --git a/packages/react-events/src/__tests__/Hover-test.internal.js b/packages/react-events/src/__tests__/Hover-test.internal.js index 87143c7d8a7..bd449d8f234 100644 --- a/packages/react-events/src/__tests__/Hover-test.internal.js +++ b/packages/react-events/src/__tests__/Hover-test.internal.js @@ -186,4 +186,8 @@ describe('Hover event responder', () => { // TODO: complete delayHoverStart tests // describe('delayHoverEnd', () => {}); }); + + it('expect displayName to show up for event component', () => { + expect(Hover.displayName).toBe('Hover'); + }); }); diff --git a/packages/react-events/src/__tests__/Press-test.internal.js b/packages/react-events/src/__tests__/Press-test.internal.js index b68b338fb7e..504f7e49d8d 100644 --- a/packages/react-events/src/__tests__/Press-test.internal.js +++ b/packages/react-events/src/__tests__/Press-test.internal.js @@ -75,6 +75,25 @@ describe('Event responder: Press', () => { expect(onPressStart).toHaveBeenCalledTimes(1); }); + it('is called once after "keydown" events for Enter', () => { + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); + expect(onPressStart).toHaveBeenCalledTimes(1); + }); + + it('is called once after "keydown" events for Spacebar', () => { + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: ' '})); + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: ' '})); + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: ' '})); + expect(onPressStart).toHaveBeenCalledTimes(1); + }); + + it('is not called after "keydown" for other keys', () => { + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'a'})); + expect(onPressStart).not.toBeCalled(); + }); + // No PointerEvent fallbacks it('is called after "mousedown" event', () => { ref.current.dispatchEvent(createPointerEvent('mousedown')); @@ -116,6 +135,24 @@ describe('Event responder: Press', () => { expect(onPressEnd).toHaveBeenCalledTimes(1); }); + 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); + }); + + it('is called after "keyup" event for Spacebar', () => { + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: ' '})); + ref.current.dispatchEvent(createKeyboardEvent('keyup', {key: ' '})); + expect(onPressEnd).toHaveBeenCalledTimes(1); + }); + + it('is not called after "keyup" event for other keys', () => { + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); + ref.current.dispatchEvent(createKeyboardEvent('keyup', {key: 'a'})); + expect(onPressEnd).not.toBeCalled(); + }); + // No PointerEvent fallbacks it('is called after "mouseup" event', () => { ref.current.dispatchEvent(createPointerEvent('mousedown')); @@ -155,6 +192,15 @@ describe('Event responder: Press', () => { expect(onPressChange).toHaveBeenCalledWith(false); }); + it('is called after valid "keydown" and "keyup" events', () => { + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); + expect(onPressChange).toHaveBeenCalledTimes(1); + expect(onPressChange).toHaveBeenCalledWith(true); + ref.current.dispatchEvent(createKeyboardEvent('keyup', {key: 'Enter'})); + expect(onPressChange).toHaveBeenCalledTimes(2); + expect(onPressChange).toHaveBeenCalledWith(false); + }); + // No PointerEvent fallbacks it('is called after "mousedown" and "mouseup" events', () => { ref.current.dispatchEvent(createPointerEvent('mousedown')); @@ -193,6 +239,20 @@ describe('Event responder: Press', () => { ref.current.dispatchEvent(createPointerEvent('pointerup')); expect(onPress).toHaveBeenCalledTimes(1); }); + + 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); + }); + + // No PointerEvent fallbacks + // TODO: jsdom missing APIs + //it('is called after "touchend" event', () => { + //ref.current.dispatchEvent(createPointerEvent('touchstart')); + //ref.current.dispatchEvent(createPointerEvent('touchend')); + //expect(onPress).toHaveBeenCalledTimes(1); + //}); }); describe('onLongPress', () => { @@ -209,7 +269,7 @@ describe('Event responder: Press', () => { ReactDOM.render(element, container); }); - it('is called if press lasts default delay', () => { + it('is called if "pointerdown" lasts default delay', () => { ref.current.dispatchEvent(createPointerEvent('pointerdown')); jest.advanceTimersByTime(DEFAULT_LONG_PRESS_DELAY - 1); expect(onLongPress).not.toBeCalled(); @@ -217,7 +277,7 @@ describe('Event responder: Press', () => { expect(onLongPress).toHaveBeenCalledTimes(1); }); - it('is not called if press is released before delay', () => { + it('is not called if "pointerup" is dispatched before delay', () => { ref.current.dispatchEvent(createPointerEvent('pointerdown')); jest.advanceTimersByTime(DEFAULT_LONG_PRESS_DELAY - 1); ref.current.dispatchEvent(createPointerEvent('pointerup')); @@ -225,6 +285,22 @@ describe('Event responder: Press', () => { expect(onLongPress).not.toBeCalled(); }); + it('is called if valid "keydown" lasts default delay', () => { + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); + jest.advanceTimersByTime(DEFAULT_LONG_PRESS_DELAY - 1); + expect(onLongPress).not.toBeCalled(); + jest.advanceTimersByTime(1); + expect(onLongPress).toHaveBeenCalledTimes(1); + }); + + it('is not called if valid "keyup" is dispatched before delay', () => { + ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); + jest.advanceTimersByTime(DEFAULT_LONG_PRESS_DELAY - 1); + ref.current.dispatchEvent(createKeyboardEvent('keyup', {key: 'Enter'})); + jest.advanceTimersByTime(1); + expect(onLongPress).not.toBeCalled(); + }); + describe('delayLongPress', () => { it('can be configured', () => { const element = ( @@ -356,7 +432,7 @@ describe('Event responder: Press', () => { describe('nested responders', () => { it('dispatch events in the correct order', () => { - let events = []; + const events = []; const ref = React.createRef(); const createEventHandler = msg => () => { events.push(msg); @@ -402,13 +478,6 @@ describe('Event responder: Press', () => { 'outer: onPressChange', 'outer: onPress', ]); - - events = []; - ref.current.dispatchEvent(createKeyboardEvent('keydown', {key: 'Enter'})); - // TODO update this test once we have a form of stopPropagation in - // the responder system again. This test had to be updated because - // we have removed stopPropagation() from synthetic events. - expect(events).toEqual(['keydown', 'inner: onPress', 'outer: onPress']); }); }); From dfad2b9aa979fbecfe5dfa834fc4702a4243a06a Mon Sep 17 00:00:00 2001 From: Nicolas Gallagher Date: Wed, 3 Apr 2019 16:48:24 -0700 Subject: [PATCH 8/8] Add FocusProps type and unit tests --- packages/react-events/src/Focus.js | 33 ++++-- .../src/__tests__/Focus-test.internal.js | 111 ++++++++++++++++++ 2 files changed, 134 insertions(+), 10 deletions(-) create mode 100644 packages/react-events/src/__tests__/Focus-test.internal.js diff --git a/packages/react-events/src/Focus.js b/packages/react-events/src/Focus.js index e56379f8871..0b26bde9757 100644 --- a/packages/react-events/src/Focus.js +++ b/packages/react-events/src/Focus.js @@ -10,10 +10,12 @@ import type {EventResponderContext} from 'events/EventTypes'; import {REACT_EVENT_COMPONENT_TYPE} from 'shared/ReactSymbols'; -const targetEventTypes = [ - {name: 'focus', passive: true, capture: true}, - {name: 'blur', passive: true, capture: true}, -]; +type FocusProps = { + disabled: boolean, + onBlur: (e: FocusEvent) => void, + onFocus: (e: FocusEvent) => void, + onFocusChange: boolean => void, +}; type FocusState = { isFocused: boolean, @@ -27,6 +29,11 @@ type FocusEvent = {| type: FocusEventType, |}; +const targetEventTypes = [ + {name: 'focus', passive: true, capture: true}, + {name: 'blur', passive: true, capture: true}, +]; + function createFocusEvent( type: FocusEventType, target: Element | Document, @@ -39,7 +46,10 @@ function createFocusEvent( }; } -function dispatchFocusInEvents(context: EventResponderContext, props: Object) { +function dispatchFocusInEvents( + context: EventResponderContext, + props: FocusProps, +) { const {event, eventTarget} = context; if (context.isTargetWithinEventComponent((event: any).relatedTarget)) { return; @@ -53,19 +63,22 @@ function dispatchFocusInEvents(context: EventResponderContext, props: Object) { context.dispatchEvent(syntheticEvent, {discrete: true}); } if (props.onFocusChange) { - const focusChangeEventListener = () => { + const listener = () => { props.onFocusChange(true); }; const syntheticEvent = createFocusEvent( 'focuschange', eventTarget, - focusChangeEventListener, + listener, ); context.dispatchEvent(syntheticEvent, {discrete: true}); } } -function dispatchFocusOutEvents(context: EventResponderContext, props: Object) { +function dispatchFocusOutEvents( + context: EventResponderContext, + props: FocusProps, +) { const {event, eventTarget} = context; if (context.isTargetWithinEventComponent((event: any).relatedTarget)) { return; @@ -75,13 +88,13 @@ function dispatchFocusOutEvents(context: EventResponderContext, props: Object) { context.dispatchEvent(syntheticEvent, {discrete: true}); } if (props.onFocusChange) { - const focusChangeEventListener = () => { + const listener = () => { props.onFocusChange(false); }; const syntheticEvent = createFocusEvent( 'focuschange', eventTarget, - focusChangeEventListener, + listener, ); context.dispatchEvent(syntheticEvent, {discrete: true}); } diff --git a/packages/react-events/src/__tests__/Focus-test.internal.js b/packages/react-events/src/__tests__/Focus-test.internal.js new file mode 100644 index 00000000000..0b1287773e7 --- /dev/null +++ b/packages/react-events/src/__tests__/Focus-test.internal.js @@ -0,0 +1,111 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let ReactFeatureFlags; +let ReactDOM; +let Focus; + +const createFocusEvent = type => { + const event = document.createEvent('Event'); + event.initEvent(type, true, true); + return event; +}; + +describe('Focus event responder', () => { + let container; + + beforeEach(() => { + jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.enableEventAPI = true; + React = require('react'); + ReactDOM = require('react-dom'); + Focus = require('react-events/focus'); + + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(() => { + document.body.removeChild(container); + container = null; + }); + + describe('onBlur', () => { + let onBlur, ref; + + beforeEach(() => { + onBlur = jest.fn(); + ref = React.createRef(); + const element = ( + +
+ + ); + ReactDOM.render(element, container); + }); + + it('is called after "blur" event', () => { + ref.current.dispatchEvent(createFocusEvent('focus')); + ref.current.dispatchEvent(createFocusEvent('blur')); + expect(onBlur).toHaveBeenCalledTimes(1); + }); + }); + + describe('onFocus', () => { + let onFocus, ref; + + beforeEach(() => { + onFocus = jest.fn(); + ref = React.createRef(); + const element = ( + +
+ + ); + ReactDOM.render(element, container); + }); + + it('is called after "focus" event', () => { + ref.current.dispatchEvent(createFocusEvent('focus')); + expect(onFocus).toHaveBeenCalledTimes(1); + }); + }); + + describe('onFocusChange', () => { + let onFocusChange, ref; + + beforeEach(() => { + onFocusChange = jest.fn(); + ref = React.createRef(); + const element = ( + +
+ + ); + ReactDOM.render(element, container); + }); + + it('is called after "blur" and "focus" events', () => { + ref.current.dispatchEvent(createFocusEvent('focus')); + expect(onFocusChange).toHaveBeenCalledTimes(1); + expect(onFocusChange).toHaveBeenCalledWith(true); + ref.current.dispatchEvent(createFocusEvent('blur')); + expect(onFocusChange).toHaveBeenCalledTimes(2); + expect(onFocusChange).toHaveBeenCalledWith(false); + }); + }); + + it('expect displayName to show up for event component', () => { + expect(Focus.displayName).toBe('Focus'); + }); +});