From 6e99ba6bb8bb7e0847e461b0c529a5b1f3215e85 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Mon, 27 Jul 2020 12:34:14 +0200 Subject: [PATCH 1/7] feat: add tests --- src/__tests__/fireEvent.test.js | 69 +++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/__tests__/fireEvent.test.js b/src/__tests__/fireEvent.test.js index 5a261dba7..f4058cfd2 100644 --- a/src/__tests__/fireEvent.test.js +++ b/src/__tests__/fireEvent.test.js @@ -3,6 +3,7 @@ import React from 'react'; import { View, TouchableOpacity, + Pressable, Text, ScrollView, TextInput, @@ -163,3 +164,71 @@ test('event with multiple handler parameters', () => { expect(handlePress).toHaveBeenCalledWith('param1', 'param2'); }); + +test('should not fire on disabled TouchableOpacity', () => { + const handlePress = jest.fn(); + const screen = render( + + Trigger + + ); + + expect(() => fireEvent.press(screen.getByText('Trigger'))).toThrow( + 'No handler function found for event: "press"' + ); + expect(handlePress).not.toHaveBeenCalled(); +}); + +test('should not fire on disabled Pressable', () => { + const handlePress = jest.fn(); + const screen = render( + + Trigger + + ); + + expect(() => fireEvent.press(screen.getByText('Trigger'))).toThrow( + 'No handler function found for event: "press"' + ); + expect(handlePress).not.toHaveBeenCalled(); +}); + +test('should pass event up on disabled TouchableOpacity', () => { + const handleInnerPress = jest.fn(); + const handleOuterPress = jest.fn(); + const screen = render( + + + Inner Trigger + + + ); + + fireEvent.press(screen.getByText('Inner Trigger')); + expect(handleInnerPress).not.toHaveBeenCalled(); + expect(handleOuterPress).toHaveBeenCalledTimes(1); +}); + +test.only('should pass event up on disabled Pressable', () => { + const handleInnerPress = jest.fn(); + const handleOuterPress = jest.fn(); + const screen = render( + + + Inner Trigger + + + ); + + fireEvent.press(screen.getByText('Inner Trigger')); + expect(handleInnerPress).not.toHaveBeenCalled(); + expect(handleOuterPress).toHaveBeenCalledTimes(1); +}); From fcd5245f56d48cf0a89b8e1b8873d4a1e57d42c1 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Mon, 27 Jul 2020 13:15:57 +0200 Subject: [PATCH 2/7] feat: prevent event handling on disabled element --- src/__tests__/fireEvent.test.js | 2 +- src/fireEvent.js | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/__tests__/fireEvent.test.js b/src/__tests__/fireEvent.test.js index f4058cfd2..34968b148 100644 --- a/src/__tests__/fireEvent.test.js +++ b/src/__tests__/fireEvent.test.js @@ -217,7 +217,7 @@ test('should pass event up on disabled TouchableOpacity', () => { expect(handleOuterPress).toHaveBeenCalledTimes(1); }); -test.only('should pass event up on disabled Pressable', () => { +test('should pass event up on disabled Pressable', () => { const handleInnerPress = jest.fn(); const handleOuterPress = jest.fn(); const screen = render( diff --git a/src/fireEvent.js b/src/fireEvent.js index 787620c5d..cbc25009c 100644 --- a/src/fireEvent.js +++ b/src/fireEvent.js @@ -9,9 +9,15 @@ const findEventHandler = ( ) => { const eventHandler = toEventHandlerName(eventName); - if (typeof element.props[eventHandler] === 'function') { + if ( + typeof element.props[eventHandler] === 'function' && + element.props.disabled !== true + ) { return element.props[eventHandler]; - } else if (typeof element.props[eventName] === 'function') { + } else if ( + typeof element.props[eventName] === 'function' && + element.props.disabled !== true + ) { return element.props[eventName]; } From ea3d7e6c036f548378dd29fc5142da4d2979866c Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Mon, 27 Jul 2020 13:23:43 +0200 Subject: [PATCH 3/7] refactor: code cleanup --- src/__tests__/fireEvent.test.js | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/__tests__/fireEvent.test.js b/src/__tests__/fireEvent.test.js index 34968b148..86b9ee99f 100644 --- a/src/__tests__/fireEvent.test.js +++ b/src/__tests__/fireEvent.test.js @@ -197,17 +197,9 @@ test('should pass event up on disabled TouchableOpacity', () => { const handleInnerPress = jest.fn(); const handleOuterPress = jest.fn(); const screen = render( - - - Inner Trigger + + + Inner Trigger ); @@ -221,9 +213,9 @@ test('should pass event up on disabled Pressable', () => { const handleInnerPress = jest.fn(); const handleOuterPress = jest.fn(); const screen = render( - - - Inner Trigger + + + Inner Trigger ); From 6abfbd02a1120086738fd8b0dede49a34029072e Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Tue, 28 Jul 2020 10:03:36 +0200 Subject: [PATCH 4/7] feat: use props.onStartShouldSetResponder to check for event handling --- src/fireEvent.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/fireEvent.js b/src/fireEvent.js index cbc25009c..613b71909 100644 --- a/src/fireEvent.js +++ b/src/fireEvent.js @@ -5,18 +5,24 @@ import { ErrorWithStack } from './helpers/errors'; const findEventHandler = ( element: ReactTestInstance, eventName: string, - callsite?: any + callsite?: any, + nearestHostDescendent?: ReactTestInstance ) => { + const isHostComponent = typeof element.type === 'string'; + const nearestHostComponent = isHostComponent + ? element + : nearestHostDescendent; + const eventHandler = toEventHandlerName(eventName); if ( typeof element.props[eventHandler] === 'function' && - element.props.disabled !== true + nearestHostComponent?.props.onStartShouldSetResponder?.() !== false ) { return element.props[eventHandler]; } else if ( typeof element.props[eventName] === 'function' && - element.props.disabled !== true + nearestHostComponent?.props.onStartShouldSetResponder?.() !== false ) { return element.props[eventName]; } @@ -29,7 +35,12 @@ const findEventHandler = ( ); } - return findEventHandler(element.parent, eventName, callsite); + return findEventHandler( + element.parent, + eventName, + callsite, + nearestHostComponent + ); }; const invokeEvent = ( From 44a9d27fa810f519e1279c810ecdc651f73bd909 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Tue, 28 Jul 2020 10:40:11 +0200 Subject: [PATCH 5/7] refactor: code review changes --- src/fireEvent.js | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/fireEvent.js b/src/fireEvent.js index 613b71909..c367679f5 100644 --- a/src/fireEvent.js +++ b/src/fireEvent.js @@ -9,21 +9,15 @@ const findEventHandler = ( nearestHostDescendent?: ReactTestInstance ) => { const isHostComponent = typeof element.type === 'string'; - const nearestHostComponent = isHostComponent - ? element - : nearestHostDescendent; + const hostElement = isHostComponent ? element : nearestHostDescendent; + const isEventEnabled = + hostElement?.props.onStartShouldSetResponder?.() !== false; - const eventHandler = toEventHandlerName(eventName); + const eventHandlerName = toEventHandlerName(eventName); - if ( - typeof element.props[eventHandler] === 'function' && - nearestHostComponent?.props.onStartShouldSetResponder?.() !== false - ) { - return element.props[eventHandler]; - } else if ( - typeof element.props[eventName] === 'function' && - nearestHostComponent?.props.onStartShouldSetResponder?.() !== false - ) { + if (typeof element.props[eventHandlerName] === 'function' && isEventEnabled) { + return element.props[eventHandlerName]; + } else if (typeof element.props[eventName] === 'function' && isEventEnabled) { return element.props[eventName]; } @@ -35,12 +29,7 @@ const findEventHandler = ( ); } - return findEventHandler( - element.parent, - eventName, - callsite, - nearestHostComponent - ); + return findEventHandler(element.parent, eventName, callsite, hostElement); }; const invokeEvent = ( From 01f8b876bb1598cf1c806eb27ee868c310acb508 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Tue, 28 Jul 2020 10:52:15 +0200 Subject: [PATCH 6/7] feat: updated docs --- website/docs/API.md | 4 ++++ website/docs/MigrationV7.md | 14 ++++++-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/website/docs/API.md b/website/docs/API.md index 728cbfd63..e027c6a27 100644 --- a/website/docs/API.md +++ b/website/docs/API.md @@ -188,6 +188,10 @@ test('fire changeText event', () => { }); ``` +:::note +Please note that from version `7.0` `fireEvent` performs checks that should prevent events firing on disabled elements. +::: + An example using `fireEvent` with native events that aren't already aliased by the `fireEvent` api. ```jsx diff --git a/website/docs/MigrationV7.md b/website/docs/MigrationV7.md index 2cadebc82..97852b7a5 100644 --- a/website/docs/MigrationV7.md +++ b/website/docs/MigrationV7.md @@ -44,6 +44,12 @@ To improve compatibility with React Testing Library, and to ease the migration f Please replace all occurrences of these queries in your codebase. +## `fireEvent` support for disabled components + +To improve compatibility with real React Native environment `fireEvent` now performs checks whether the component is disabled before firing an event on it. The checks internally uses `onStartShouldSetResponder` prop to establish should event fire, which should resemble the actual React Native runtime. + +If your code contained any workarounds for preventing events firing on disabled events, you should now be able to remove them. + # Guide for `@testing-library/react-native` users This guide describes steps necessary to migrate from `@testing-library/react-native` from `v6.0` to `v7.0`. Although the name stays the same, this is a different library, sourced at [Callstack GitHub repository](https://github.com/callstack/react-native-testing-library). We made sure the upgrade path is as easy for you as possible. @@ -88,14 +94,6 @@ Cleaning up (unmounting) components after each test is included by default in th You can opt-out of this behavior by running tests with `RNTL_SKIP_AUTO_CLEANUP=true` flag or importing from `@testing-library/react-native/pure`. We encourage you to keep the default though. -## No special handling for `disabled` prop - -The `disabled` prop on "Touchable\*" components is treated in the same manner as any other prop. We realize that with our library you can press "touchable" components even though they're in "disabled" state, however this is something that we strongly believe should be fixed upstream, in React Native core. - -If you feel strongly about this difference, please send a PR to React Native, adding JavaScript logic to "onPress" functions, making them aware of disabled state in JS logic as well (it's handled on native side for at least iOS, which is the default platform that tests are running in). - -As a mitigation, you'll likely need to modify the logic of "touchable" components to bail if they're pressed in disabled state. - ## No [NativeTestInstance](https://www.native-testing-library.com/docs/api-test-instance) abstraction We don't provide any abstraction over `ReactTestInstance` returned by queries, but allow to use it directly to access queried component's `props` or `type` for that example. From 7837f9ee47cacd8efb8ac7aa9372105d62ef21c7 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Tue, 28 Jul 2020 11:20:56 +0200 Subject: [PATCH 7/7] feat: added test for fake 'disabled' prop --- src/__tests__/fireEvent.test.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/__tests__/fireEvent.test.js b/src/__tests__/fireEvent.test.js index 86b9ee99f..fa0b9c255 100644 --- a/src/__tests__/fireEvent.test.js +++ b/src/__tests__/fireEvent.test.js @@ -224,3 +224,21 @@ test('should pass event up on disabled Pressable', () => { expect(handleInnerPress).not.toHaveBeenCalled(); expect(handleOuterPress).toHaveBeenCalledTimes(1); }); + +const TestComponent = ({ onPress }) => { + return ( + + Trigger Test + + ); +}; + +test('is not fooled by non-native disabled prop', () => { + const handlePress = jest.fn(); + const screen = render( + + ); + + fireEvent.press(screen.getByText('Trigger Test')); + expect(handlePress).toHaveBeenCalledTimes(1); +});