From 81f216b5eee738ecbcf9b6d0aca1d1036c904125 Mon Sep 17 00:00:00 2001 From: TMaszko Date: Thu, 13 Apr 2023 00:40:05 +0200 Subject: [PATCH 1/4] fix: editable event check --- src/__tests__/fireEvent-textInput.test.tsx | 131 +++++++++++++++++++++ src/__tests__/fireEvent.test.tsx | 38 ------ src/fireEvent.ts | 31 ++--- 3 files changed, 147 insertions(+), 53 deletions(-) create mode 100644 src/__tests__/fireEvent-textInput.test.tsx diff --git a/src/__tests__/fireEvent-textInput.test.tsx b/src/__tests__/fireEvent-textInput.test.tsx new file mode 100644 index 000000000..06414fdf4 --- /dev/null +++ b/src/__tests__/fireEvent-textInput.test.tsx @@ -0,0 +1,131 @@ +import React from 'react'; +import { Text, TextInput, TextInputProps } from 'react-native'; +import { render, fireEvent } from '..'; + +function WrappedTextInput(props: TextInputProps) { + return ; +} + +function DoubleWrappedTextInput(props: TextInputProps) { + return ; +} + +const layoutEvent = { nativeEvent: { layout: { width: 100, height: 100 } } }; + +test('should fire only non-touch-related events on non-editable TextInput', () => { + const onFocus = jest.fn(); + const onChangeText = jest.fn(); + const onSubmitEditing = jest.fn(); + const onLayout = jest.fn(); + + const view = render( + + ); + + const subject = view.getByTestId('subject'); + fireEvent(subject, 'focus'); + fireEvent.changeText(subject, 'Text'); + fireEvent(subject, 'submitEditing', { nativeEvent: { text: 'Text' } }); + fireEvent(subject, 'layout', layoutEvent); + + expect(onFocus).not.toHaveBeenCalled(); + expect(onChangeText).not.toHaveBeenCalled(); + expect(onSubmitEditing).not.toHaveBeenCalled(); + expect(onLayout).toHaveBeenCalledWith(layoutEvent); +}); + +test('should fire only non-touch-related events on non-editable TextInput with nested Text', () => { + const onFocus = jest.fn(); + const onChangeText = jest.fn(); + const onSubmitEditing = jest.fn(); + const onLayout = jest.fn(); + + const view = render( + + Nested Text + + ); + + const subject = view.getByText('Nested Text'); + fireEvent(subject, 'focus'); + fireEvent.changeText(subject, 'Text'); + fireEvent(subject, 'submitEditing', { nativeEvent: { text: 'Text' } }); + fireEvent(subject, 'layout', layoutEvent); + + expect(onFocus).not.toHaveBeenCalled(); + expect(onChangeText).not.toHaveBeenCalled(); + expect(onSubmitEditing).not.toHaveBeenCalled(); + expect(onLayout).toHaveBeenCalledWith(layoutEvent); +}); + +test('should fire only non-touch-related events on non-editable wrapped TextInput', () => { + const onFocus = jest.fn(); + const onChangeText = jest.fn(); + const onSubmitEditing = jest.fn(); + const onLayout = jest.fn(); + + const view = render( + + ); + + const subject = view.getByTestId('subject'); + fireEvent(subject, 'focus'); + fireEvent.changeText(subject, 'Text'); + fireEvent(subject, 'submitEditing', { nativeEvent: { text: 'Text' } }); + fireEvent(subject, 'layout', layoutEvent); + + expect(onFocus).not.toHaveBeenCalled(); + expect(onChangeText).not.toHaveBeenCalled(); + expect(onSubmitEditing).not.toHaveBeenCalled(); + expect(onLayout).toHaveBeenCalledWith(layoutEvent); +}); + +test('should fire only non-touch-related events on non-editable double wrapped TextInput', () => { + const onFocus = jest.fn(); + const onChangeText = jest.fn(); + const onSubmitEditing = jest.fn(); + const onLayout = jest.fn(); + + const view = render( + + ); + + const subject = view.getByTestId('subject'); + fireEvent(subject, 'focus'); + fireEvent.changeText(subject, 'Text'); + fireEvent(subject, 'submitEditing', { nativeEvent: { text: 'Text' } }); + fireEvent(subject, 'layout', layoutEvent); + + expect(onFocus).not.toHaveBeenCalled(); + expect(onChangeText).not.toHaveBeenCalled(); + expect(onSubmitEditing).not.toHaveBeenCalled(); + expect(onLayout).toHaveBeenCalledWith(layoutEvent); +}); diff --git a/src/__tests__/fireEvent.test.tsx b/src/__tests__/fireEvent.test.tsx index 0da46994e..ce1d66326 100644 --- a/src/__tests__/fireEvent.test.tsx +++ b/src/__tests__/fireEvent.test.tsx @@ -216,44 +216,6 @@ test('should not fire on disabled Pressable', () => { expect(handlePress).not.toHaveBeenCalled(); }); -test('should not fire on non-editable TextInput', () => { - const testID = 'my-text-input'; - const onChangeTextMock = jest.fn(); - const NEW_TEXT = 'New text'; - - const { getByTestId } = render( - - ); - - fireEvent.changeText(getByTestId(testID), NEW_TEXT); - expect(onChangeTextMock).not.toHaveBeenCalled(); -}); - -test('should not fire on non-editable TextInput with nested Text', () => { - const placeholder = 'Test placeholder'; - const onChangeTextMock = jest.fn(); - const NEW_TEXT = 'New text'; - - const { getByPlaceholderText } = render( - - - Test text - - - ); - - fireEvent.changeText(getByPlaceholderText(placeholder), NEW_TEXT); - expect(onChangeTextMock).not.toHaveBeenCalled(); -}); - test('should not fire inside View with pointerEvents="none"', () => { const onPress = jest.fn(); const screen = render( diff --git a/src/fireEvent.ts b/src/fireEvent.ts index f32ef6a37..db1197d4d 100644 --- a/src/fireEvent.ts +++ b/src/fireEvent.ts @@ -1,22 +1,13 @@ import { ReactTestInstance } from 'react-test-renderer'; -import { TextInput } from 'react-native'; import act from './act'; import { getHostParent, isHostElement } from './helpers/component-tree'; -import { filterNodeByType } from './helpers/filterNodeByType'; import { getHostComponentNames } from './helpers/host-component-names'; type EventHandler = (...args: unknown[]) => unknown; -function isTextInput(element: ReactTestInstance) { - // We have to test if the element type is either the `TextInput` component - // (for composite component) or the string "TextInput" (for host component) - // All queries return host components but since fireEvent bubbles up - // it would trigger the parent prop without the composite component check. - return ( - filterNodeByType(element, TextInput) || - filterNodeByType(element, getHostComponentNames().textInput) - ); -} +const isHostTextInput = (element?: ReactTestInstance) => { + return element?.type === getHostComponentNames().textInput; +}; function isTouchResponder(element: ReactTestInstance) { if (!isHostElement(element)) { @@ -24,7 +15,7 @@ function isTouchResponder(element: ReactTestInstance) { } return ( - Boolean(element.props.onStartShouldSetResponder) || isTextInput(element) + Boolean(element.props.onStartShouldSetResponder) || isHostTextInput(element) ); } @@ -57,13 +48,23 @@ function isTouchEvent(eventName: string) { return touchEventNames.includes(eventName); } +// Experimentally checked which events are called on non-editable TextInput +const textInputEventsNotAffectedByEditableProp = [ + 'contentSizeChange', + 'layout', + 'scroll', +]; + function isEventEnabled( element: ReactTestInstance, eventName: string, nearestTouchResponder?: ReactTestInstance ) { - if (isTextInput(element)) { - return element.props.editable !== false; + if (isHostTextInput(nearestTouchResponder)) { + return ( + nearestTouchResponder?.props.editable !== false || + textInputEventsNotAffectedByEditableProp.includes(eventName) + ); } if (isTouchEvent(eventName) && !isPointerEventEnabled(element)) { From 0b061fd2a2690a3c440039748f14e7377d440963 Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Wed, 3 May 2023 12:59:24 +0200 Subject: [PATCH 2/4] refactor: cleanup --- src/fireEvent.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/fireEvent.ts b/src/fireEvent.ts index db1197d4d..c9ddac132 100644 --- a/src/fireEvent.ts +++ b/src/fireEvent.ts @@ -49,7 +49,7 @@ function isTouchEvent(eventName: string) { } // Experimentally checked which events are called on non-editable TextInput -const textInputEventsNotAffectedByEditableProp = [ +const textInputEventsIgnoringEditableProp = [ 'contentSizeChange', 'layout', 'scroll', @@ -63,7 +63,7 @@ function isEventEnabled( if (isHostTextInput(nearestTouchResponder)) { return ( nearestTouchResponder?.props.editable !== false || - textInputEventsNotAffectedByEditableProp.includes(eventName) + textInputEventsIgnoringEditableProp.includes(eventName) ); } From 8aa0fbe427ec38ede715715d7d728f8a880aa8da Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Wed, 3 May 2023 20:23:31 +0200 Subject: [PATCH 3/4] chore: add code comments --- src/__tests__/fireEvent-textInput.test.tsx | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/__tests__/fireEvent-textInput.test.tsx b/src/__tests__/fireEvent-textInput.test.tsx index 06414fdf4..eb5261c3f 100644 --- a/src/__tests__/fireEvent-textInput.test.tsx +++ b/src/__tests__/fireEvent-textInput.test.tsx @@ -72,6 +72,22 @@ test('should fire only non-touch-related events on non-editable TextInput with n expect(onLayout).toHaveBeenCalledWith(layoutEvent); }); +/** + * Historically there were problems with custom TextInput wrappers, as they + * could creat a hierarchy of three or more text input views with very similar + * event props. + * + * Typical hierarchy would be: + * - User composite TextInput + * - UI library composite TextInput + * - RN composite TextInput + * - RN host TextInput + * + * Previous implementation of fireEvent only checked `editable` prop for + * RN TextInputs, both host & composite but did not check on the UI library or + * user composite TextInput level, hence invoking the event handlers that + * should be blocked by `editable={false}` prop. + */ test('should fire only non-touch-related events on non-editable wrapped TextInput', () => { const onFocus = jest.fn(); const onChangeText = jest.fn(); @@ -101,6 +117,9 @@ test('should fire only non-touch-related events on non-editable wrapped TextInpu expect(onLayout).toHaveBeenCalledWith(layoutEvent); }); +/** + * Ditto testing for even deeper hierarchy of TextInput wrappers. + */ test('should fire only non-touch-related events on non-editable double wrapped TextInput', () => { const onFocus = jest.fn(); const onChangeText = jest.fn(); From ab70817df3895e9a0d47558eff68a136a91f692a Mon Sep 17 00:00:00 2001 From: Maciej Jastrzebski Date: Thu, 4 May 2023 09:50:52 +0200 Subject: [PATCH 4/4] chore: tweaks --- src/__tests__/fireEvent-textInput.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/__tests__/fireEvent-textInput.test.tsx b/src/__tests__/fireEvent-textInput.test.tsx index eb5261c3f..f36a68f4b 100644 --- a/src/__tests__/fireEvent-textInput.test.tsx +++ b/src/__tests__/fireEvent-textInput.test.tsx @@ -74,8 +74,8 @@ test('should fire only non-touch-related events on non-editable TextInput with n /** * Historically there were problems with custom TextInput wrappers, as they - * could creat a hierarchy of three or more text input views with very similar - * event props. + * could creat a hierarchy of three or more composite text input views with + * very similar event props. * * Typical hierarchy would be: * - User composite TextInput