From abf61d2eda7c805946d0b4a69ca395acd832f2af Mon Sep 17 00:00:00 2001 From: Alan Kenyon Date: Thu, 8 Oct 2020 20:06:54 -0400 Subject: [PATCH 1/5] test showing the issue --- src/__tests__/fireEvent.test.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/__tests__/fireEvent.test.js b/src/__tests__/fireEvent.test.js index 74d97aa9e..02243212f 100644 --- a/src/__tests__/fireEvent.test.js +++ b/src/__tests__/fireEvent.test.js @@ -282,3 +282,26 @@ test('is not fooled by non-native disabled prop', () => { fireEvent.press(screen.getByText('Trigger Test')); expect(handlePress).toHaveBeenCalledTimes(1); }); + +function TestChildTouchableComponent({ onPress, someProp }) { + return ( + + + Trigger + + + ) +} + +test('is not fooled by non-responder wrapping host elements', () => { + const handlePress = jest.fn(); + + const screen = render( + + + + ); + + fireEvent.press(screen.getByText('Trigger')); + expect(handlePress).not.toHaveBeenCalled(); +}); From 83bbecea81e82f724d8c5c7126c393fa97808d91 Mon Sep 17 00:00:00 2001 From: Alan Kenyon Date: Thu, 8 Oct 2020 20:07:30 -0400 Subject: [PATCH 2/5] don't pass non-responder host elements as decsecendants --- src/fireEvent.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/fireEvent.js b/src/fireEvent.js index 08b9a65ec..5baa36c52 100644 --- a/src/fireEvent.js +++ b/src/fireEvent.js @@ -12,14 +12,15 @@ const findEventHandler = ( element: ReactTestInstance, eventName: string, callsite?: any, - nearestHostDescendent?: ReactTestInstance, + nearestHostResponderDescendent?: ReactTestInstance, hasDescendandHandler?: boolean ) => { const handler = getEventHandler(element, eventName); const hasHandler = handler != null || hasDescendandHandler; const isHostComponent = typeof element.type === 'string'; - const hostElement = isHostComponent ? element : nearestHostDescendent; + const hostElement = isHostComponent ? element : nearestHostResponderDescendent; + const isHostElementResponder = hostElement ? (isTextInputComponent(hostElement) || !!hostElement?.props.onStartShouldSetResponder) : false; const isEventEnabled = isTextInputComponent(element) ? element.props.editable !== false @@ -43,7 +44,7 @@ const findEventHandler = ( element.parent, eventName, callsite, - hostElement, + isHostElementResponder ? hostElement : (nearestHostResponderDescendent || element), hasHandler ); }; From cd849b26124188806681544a19c7258d21a22a26 Mon Sep 17 00:00:00 2001 From: Alan Kenyon Date: Thu, 8 Oct 2020 20:09:19 -0400 Subject: [PATCH 3/5] linter fixs --- src/__tests__/fireEvent.test.js | 2 +- src/fireEvent.js | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/__tests__/fireEvent.test.js b/src/__tests__/fireEvent.test.js index 02243212f..5fef1c806 100644 --- a/src/__tests__/fireEvent.test.js +++ b/src/__tests__/fireEvent.test.js @@ -290,7 +290,7 @@ function TestChildTouchableComponent({ onPress, someProp }) { Trigger - ) + ); } test('is not fooled by non-responder wrapping host elements', () => { diff --git a/src/fireEvent.js b/src/fireEvent.js index 5baa36c52..9712df661 100644 --- a/src/fireEvent.js +++ b/src/fireEvent.js @@ -19,8 +19,13 @@ const findEventHandler = ( const hasHandler = handler != null || hasDescendandHandler; const isHostComponent = typeof element.type === 'string'; - const hostElement = isHostComponent ? element : nearestHostResponderDescendent; - const isHostElementResponder = hostElement ? (isTextInputComponent(hostElement) || !!hostElement?.props.onStartShouldSetResponder) : false; + const hostElement = isHostComponent + ? element + : nearestHostResponderDescendent; + const isHostElementResponder = hostElement + ? isTextInputComponent(hostElement) || + !!hostElement?.props.onStartShouldSetResponder + : false; const isEventEnabled = isTextInputComponent(element) ? element.props.editable !== false @@ -44,7 +49,9 @@ const findEventHandler = ( element.parent, eventName, callsite, - isHostElementResponder ? hostElement : (nearestHostResponderDescendent || element), + isHostElementResponder + ? hostElement + : nearestHostResponderDescendent || element, hasHandler ); }; From ff1a473d43b80b796f0e8b3283000d44f5c59de2 Mon Sep 17 00:00:00 2001 From: Alan Kenyon Date: Mon, 12 Oct 2020 16:57:53 -0400 Subject: [PATCH 4/5] attempting to improve readability --- src/fireEvent.js | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/fireEvent.js b/src/fireEvent.js index 9712df661..6f14112c4 100644 --- a/src/fireEvent.js +++ b/src/fireEvent.js @@ -12,7 +12,7 @@ const findEventHandler = ( element: ReactTestInstance, eventName: string, callsite?: any, - nearestHostResponderDescendent?: ReactTestInstance, + nearestHostDescendent?: ReactTestInstance, hasDescendandHandler?: boolean ) => { const handler = getEventHandler(element, eventName); @@ -21,11 +21,7 @@ const findEventHandler = ( const isHostComponent = typeof element.type === 'string'; const hostElement = isHostComponent ? element - : nearestHostResponderDescendent; - const isHostElementResponder = hostElement - ? isTextInputComponent(hostElement) || - !!hostElement?.props.onStartShouldSetResponder - : false; + : nearestHostDescendent; const isEventEnabled = isTextInputComponent(element) ? element.props.editable !== false @@ -45,13 +41,25 @@ const findEventHandler = ( } } + // Check if the host element can receive touch events. Either it is a + // TextInput component or has a onStartShouldSetResponder prop + const isHostElementATouchResponder = hostElement + ? (isTextInputComponent(hostElement) || + !!hostElement?.props.onStartShouldSetResponder) + : false; + + // If the host element is a receiver of touch events than it is the new + // nearest host descendant. Otherwise, pass the previously received nearest + // host descendant (or current element if a nearest host descendent is falsy) + const targetDescendent = isHostElementATouchResponder + ? hostElement + : (nearestHostDescendent || element) + return findEventHandler( element.parent, eventName, callsite, - isHostElementResponder - ? hostElement - : nearestHostResponderDescendent || element, + targetDescendent, hasHandler ); }; From b77cbfaa4a4745fb98c89c764e2dc6fd6d9b6ad9 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Wed, 21 Oct 2020 11:25:31 +0200 Subject: [PATCH 5/5] refactor: re-structure code for clarity & redability --- src/fireEvent.js | 57 ++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/src/fireEvent.js b/src/fireEvent.js index 6f14112c4..c9156dd13 100644 --- a/src/fireEvent.js +++ b/src/fireEvent.js @@ -2,34 +2,47 @@ import act from './act'; import { ErrorWithStack } from './helpers/errors'; -const isTextInputComponent = (element: ReactTestInstance) => { +const isHostElement = (element?: ReactTestInstance) => { + return typeof element?.type === 'string'; +}; + +const isTextInput = (element?: ReactTestInstance) => { // eslint-disable-next-line import/no-extraneous-dependencies const { TextInput } = require('react-native'); - return element.type === TextInput; + return element?.type === TextInput; +}; + +const isTouchResponder = (element?: ReactTestInstance) => { + if (!isHostElement(element)) return false; + + return !!element?.props.onStartShouldSetResponder || isTextInput(element); +}; + +const isEventEnabled = ( + element?: ReactTestInstance, + touchResponder?: ReactTestInstance +) => { + if (isTextInput(element)) return element?.props.editable !== false; + + return touchResponder?.props.onStartShouldSetResponder?.() !== false; }; const findEventHandler = ( element: ReactTestInstance, eventName: string, callsite?: any, - nearestHostDescendent?: ReactTestInstance, + nearestTouchResponder?: ReactTestInstance, hasDescendandHandler?: boolean ) => { - const handler = getEventHandler(element, eventName); - const hasHandler = handler != null || hasDescendandHandler; - - const isHostComponent = typeof element.type === 'string'; - const hostElement = isHostComponent + const touchResponder = isTouchResponder(element) ? element - : nearestHostDescendent; - - const isEventEnabled = isTextInputComponent(element) - ? element.props.editable !== false - : hostElement?.props.onStartShouldSetResponder?.() !== false; + : nearestTouchResponder; - if (handler && isEventEnabled) return handler; + const handler = getEventHandler(element, eventName); + if (handler && isEventEnabled(element, touchResponder)) return handler; // Do not bubble event to the root element + const hasHandler = handler != null || hasDescendandHandler; if (element.parent === null || element.parent.parent === null) { if (hasHandler) { return null; @@ -41,25 +54,11 @@ const findEventHandler = ( } } - // Check if the host element can receive touch events. Either it is a - // TextInput component or has a onStartShouldSetResponder prop - const isHostElementATouchResponder = hostElement - ? (isTextInputComponent(hostElement) || - !!hostElement?.props.onStartShouldSetResponder) - : false; - - // If the host element is a receiver of touch events than it is the new - // nearest host descendant. Otherwise, pass the previously received nearest - // host descendant (or current element if a nearest host descendent is falsy) - const targetDescendent = isHostElementATouchResponder - ? hostElement - : (nearestHostDescendent || element) - return findEventHandler( element.parent, eventName, callsite, - targetDescendent, + touchResponder, hasHandler ); };