From 3f058debc29ccb05a47ac8a8d747c5a5b29a6ed3 Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 24 Apr 2019 17:56:21 +0100 Subject: [PATCH] Event API: various bug fixes (#15485) --- .../src/client/ReactDOMHostConfig.js | 11 +++- packages/react-events/src/Focus.js | 2 +- packages/react-events/src/Press.js | 25 +++++--- .../__tests__/TouchHitTarget-test.internal.js | 54 ++++++++++------ .../src/ReactFiberCommitWork.js | 64 ++++++++++--------- 5 files changed, 93 insertions(+), 63 deletions(-) diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index c829d0912f1cc..ee76c116e6ea2 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -966,6 +966,13 @@ export function handleEventTarget( rootContainerInstance: Container, internalInstanceHandle: Object, ): boolean { + if ( + __DEV__ && + type === REACT_EVENT_TARGET_TOUCH_HIT && + (props.left || props.right || props.top || props.bottom) + ) { + return true; + } return false; } @@ -992,9 +999,9 @@ export function commitEventTarget( 'value of "relative".', ); warning( - computedStyles.getPropertyValue('zIndex') !== '', + computedStyles.getPropertyValue('z-index') !== '', ' inserts an empty
with "z-index" of "-1". ' + - 'This requires its parent DOM node to have a "z-index" great than "-1",' + + 'This requires its parent DOM node to have a "z-index" greater than "-1",' + 'but the parent DOM node was found to no "z-index" value set.' + ' Try using a "z-index" value of "0" or greater.', ); diff --git a/packages/react-events/src/Focus.js b/packages/react-events/src/Focus.js index 7c8946520d2f5..c7b27772aee9a 100644 --- a/packages/react-events/src/Focus.js +++ b/packages/react-events/src/Focus.js @@ -108,7 +108,7 @@ const FocusResponder = { onEvent( event: ReactResponderEvent, context: ReactResponderContext, - props: Object, + props: FocusProps, state: FocusState, ): void { const {type, target} = event; diff --git a/packages/react-events/src/Press.js b/packages/react-events/src/Press.js index 14a6ee0e387cb..4b5dfcd623919 100644 --- a/packages/react-events/src/Press.js +++ b/packages/react-events/src/Press.js @@ -94,7 +94,9 @@ const targetEventTypes = [ {name: 'keydown', passive: false}, {name: 'keypress', passive: false}, {name: 'contextmenu', passive: false}, - 'pointerdown', + // We need to preventDefault on pointerdown for mouse/pen events + // that are in hit target area but not the element area. + {name: 'pointerdown', passive: false}, 'pointercancel', ]; const rootEventTypes = ['keyup', 'pointerup', 'pointermove', 'scroll']; @@ -447,15 +449,18 @@ const PressResponder = { } } - // Ignore emulated mouse events and mouse pressing on touch hit target - // area - if (type === 'mousedown') { - if ( - state.ignoreEmulatedMouseEvents || - isEventPositionWithinTouchHitTarget(event, context) - ) { - return; - } + // Ignore emulated mouse events + if (type === 'mousedown' && state.ignoreEmulatedMouseEvents) { + return; + } + // Ignore mouse/pen pressing on touch hit target area + if ( + (pointerType === 'mouse' || pointerType === 'pen') && + isEventPositionWithinTouchHitTarget(event, context) + ) { + // We need to prevent the native event to block the focus + nativeEvent.preventDefault(); + return; } // Ignore any device buttons except left-mouse and touch/pen contact diff --git a/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js b/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js index 672e0b8bfc5ff..4c00b96dbb249 100644 --- a/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js +++ b/packages/react-events/src/__tests__/TouchHitTarget-test.internal.js @@ -318,7 +318,7 @@ describe('TouchHitTarget', () => { const Test = () => ( -
+
{cond ? null : ( )} @@ -330,14 +330,16 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', ); cond = true; ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); - expect(container.innerHTML).toBe('
'); + expect(container.innerHTML).toBe( + '
', + ); }); it('should render a conditional TouchHitTarget correctly (true -> false)', () => { @@ -345,7 +347,7 @@ describe('TouchHitTarget', () => { const Test = () => ( -
+
{cond ? null : ( )} @@ -356,13 +358,15 @@ describe('TouchHitTarget', () => { const container = document.createElement('div'); ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); - expect(container.innerHTML).toBe('
'); + expect(container.innerHTML).toBe( + '
', + ); cond = false; ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', ); }); @@ -372,7 +376,7 @@ describe('TouchHitTarget', () => { const Test = () => ( -
+
{cond ? ( ) : ( @@ -386,14 +390,16 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', ); cond = true; ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); - expect(container.innerHTML).toBe('
'); + expect(container.innerHTML).toBe( + '
', + ); }); it('should render a conditional TouchHitTarget hit slop correctly (true -> false)', () => { @@ -401,7 +407,7 @@ describe('TouchHitTarget', () => { const Test = () => ( -
+
Random span 1 {cond ? ( @@ -417,14 +423,15 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1Random span 2
', + '
Random span 1Random span 2
', ); cond = false; ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 1' + + '
Random span 2
', ); }); @@ -434,7 +441,7 @@ describe('TouchHitTarget', () => { const Test = () => ( -
+
Random span 1 {cond ? ( @@ -455,7 +462,8 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 1' + + '
Random span 2
', ); @@ -463,7 +471,8 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 1' + + '
Random span 2
', ); }); @@ -473,7 +482,7 @@ describe('TouchHitTarget', () => { const Test = () => ( -
+
Random span 1 {cond ? ( @@ -494,7 +503,8 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 1' + + '
Random span 2
', ); @@ -502,7 +512,8 @@ describe('TouchHitTarget', () => { ReactDOM.render(, container); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
Random span 1
Random span 1' + + '
Random span 2
', ); }); @@ -510,14 +521,15 @@ describe('TouchHitTarget', () => { it('should hydrate TouchHitTarget hit slop elements correcty and patch them', () => { const Test = () => ( -
+
); const container = document.createElement('div'); - container.innerHTML = '
'; + container.innerHTML = + '
'; expect(() => { ReactDOM.hydrate(, container); expect(Scheduler).toFlushWithoutYielding(); @@ -527,7 +539,7 @@ describe('TouchHitTarget', () => { ); expect(Scheduler).toFlushWithoutYielding(); expect(container.innerHTML).toBe( - '
', ); }); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index b28401538ee96..5aaf20195b191 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -595,9 +595,41 @@ function commitLifeCycles( } case SuspenseComponent: case IncompleteClassComponent: - case EventTarget: - case EventComponent: - break; + return; + case EventTarget: { + if (enableEventAPI) { + const type = finishedWork.type.type; + const props = finishedWork.memoizedProps; + const instance = finishedWork.stateNode; + let parentInstance = null; + + let node = finishedWork.return; + // Traverse up the fiber tree until we find the parent host node. + while (node !== null) { + if (node.tag === HostComponent) { + parentInstance = node.stateNode; + break; + } else if (node.tag === HostRoot) { + parentInstance = node.stateNode.containerInfo; + break; + } + node = node.return; + } + invariant( + parentInstance !== null, + 'This should have a parent host component initialized. This error is likely ' + + 'caused by a bug in React. Please file an issue.', + ); + commitEventTarget(type, props, instance, parentInstance); + } + return; + } + case EventComponent: { + if (enableEventAPI) { + mountEventComponent(finishedWork.stateNode); + } + return; + } default: { invariant( false, @@ -1218,31 +1250,6 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { return; } case EventTarget: { - if (enableEventAPI) { - const type = finishedWork.type.type; - const props = finishedWork.memoizedProps; - const instance = finishedWork.stateNode; - let parentInstance = null; - - let node = finishedWork.return; - // Traverse up the fiber tree until we find the parent host node. - while (node !== null) { - if (node.tag === HostComponent) { - parentInstance = node.stateNode; - break; - } else if (node.tag === HostRoot) { - parentInstance = node.stateNode.containerInfo; - break; - } - node = node.return; - } - invariant( - parentInstance !== null, - 'This should have a parent host component initialized. This error is likely ' + - 'caused by a bug in React. Please file an issue.', - ); - commitEventTarget(type, props, instance, parentInstance); - } return; } case HostRoot: { @@ -1259,7 +1266,6 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { return; } case EventComponent: { - mountEventComponent(finishedWork.stateNode); return; } default: {