Skip to content

Commit 5adf148

Browse files
mdjastrzebskithymikee
authored andcommitted
feat: prevent events on disabled elements (#460)
* feat: add tests * feat: prevent event handling on disabled element * refactor: code cleanup * feat: use props.onStartShouldSetResponder to check for event handling * refactor: code review changes * feat: updated docs * feat: added test for fake 'disabled' prop
1 parent 37f0c42 commit 5adf148

File tree

4 files changed

+101
-14
lines changed

4 files changed

+101
-14
lines changed

src/__tests__/fireEvent.test.js

+79
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import React from 'react';
33
import {
44
View,
55
TouchableOpacity,
6+
Pressable,
67
Text,
78
ScrollView,
89
TextInput,
@@ -163,3 +164,81 @@ test('event with multiple handler parameters', () => {
163164

164165
expect(handlePress).toHaveBeenCalledWith('param1', 'param2');
165166
});
167+
168+
test('should not fire on disabled TouchableOpacity', () => {
169+
const handlePress = jest.fn();
170+
const screen = render(
171+
<TouchableOpacity onPress={handlePress} disabled={true}>
172+
<Text>Trigger</Text>
173+
</TouchableOpacity>
174+
);
175+
176+
expect(() => fireEvent.press(screen.getByText('Trigger'))).toThrow(
177+
'No handler function found for event: "press"'
178+
);
179+
expect(handlePress).not.toHaveBeenCalled();
180+
});
181+
182+
test('should not fire on disabled Pressable', () => {
183+
const handlePress = jest.fn();
184+
const screen = render(
185+
<Pressable onPress={handlePress} disabled={true}>
186+
<Text>Trigger</Text>
187+
</Pressable>
188+
);
189+
190+
expect(() => fireEvent.press(screen.getByText('Trigger'))).toThrow(
191+
'No handler function found for event: "press"'
192+
);
193+
expect(handlePress).not.toHaveBeenCalled();
194+
});
195+
196+
test('should pass event up on disabled TouchableOpacity', () => {
197+
const handleInnerPress = jest.fn();
198+
const handleOuterPress = jest.fn();
199+
const screen = render(
200+
<TouchableOpacity onPress={handleOuterPress}>
201+
<TouchableOpacity onPress={handleInnerPress} disabled={true}>
202+
<Text>Inner Trigger</Text>
203+
</TouchableOpacity>
204+
</TouchableOpacity>
205+
);
206+
207+
fireEvent.press(screen.getByText('Inner Trigger'));
208+
expect(handleInnerPress).not.toHaveBeenCalled();
209+
expect(handleOuterPress).toHaveBeenCalledTimes(1);
210+
});
211+
212+
test('should pass event up on disabled Pressable', () => {
213+
const handleInnerPress = jest.fn();
214+
const handleOuterPress = jest.fn();
215+
const screen = render(
216+
<Pressable onPress={handleOuterPress}>
217+
<Pressable onPress={handleInnerPress} disabled={true}>
218+
<Text>Inner Trigger</Text>
219+
</Pressable>
220+
</Pressable>
221+
);
222+
223+
fireEvent.press(screen.getByText('Inner Trigger'));
224+
expect(handleInnerPress).not.toHaveBeenCalled();
225+
expect(handleOuterPress).toHaveBeenCalledTimes(1);
226+
});
227+
228+
const TestComponent = ({ onPress }) => {
229+
return (
230+
<TouchableOpacity onPress={onPress}>
231+
<Text>Trigger Test</Text>
232+
</TouchableOpacity>
233+
);
234+
};
235+
236+
test('is not fooled by non-native disabled prop', () => {
237+
const handlePress = jest.fn();
238+
const screen = render(
239+
<TestComponent onPress={handlePress} disabled={true} />
240+
);
241+
242+
fireEvent.press(screen.getByText('Trigger Test'));
243+
expect(handlePress).toHaveBeenCalledTimes(1);
244+
});

src/fireEvent.js

+12-6
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,19 @@ import { ErrorWithStack } from './helpers/errors';
55
const findEventHandler = (
66
element: ReactTestInstance,
77
eventName: string,
8-
callsite?: any
8+
callsite?: any,
9+
nearestHostDescendent?: ReactTestInstance
910
) => {
10-
const eventHandler = toEventHandlerName(eventName);
11+
const isHostComponent = typeof element.type === 'string';
12+
const hostElement = isHostComponent ? element : nearestHostDescendent;
13+
const isEventEnabled =
14+
hostElement?.props.onStartShouldSetResponder?.() !== false;
15+
16+
const eventHandlerName = toEventHandlerName(eventName);
1117

12-
if (typeof element.props[eventHandler] === 'function') {
13-
return element.props[eventHandler];
14-
} else if (typeof element.props[eventName] === 'function') {
18+
if (typeof element.props[eventHandlerName] === 'function' && isEventEnabled) {
19+
return element.props[eventHandlerName];
20+
} else if (typeof element.props[eventName] === 'function' && isEventEnabled) {
1521
return element.props[eventName];
1622
}
1723

@@ -23,7 +29,7 @@ const findEventHandler = (
2329
);
2430
}
2531

26-
return findEventHandler(element.parent, eventName, callsite);
32+
return findEventHandler(element.parent, eventName, callsite, hostElement);
2733
};
2834

2935
const invokeEvent = (

website/docs/API.md

+4
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,10 @@ test('fire changeText event', () => {
188188
});
189189
```
190190

191+
:::note
192+
Please note that from version `7.0` `fireEvent` performs checks that should prevent events firing on disabled elements.
193+
:::
194+
191195
An example using `fireEvent` with native events that aren't already aliased by the `fireEvent` api.
192196

193197
```jsx

website/docs/MigrationV7.md

+6-8
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ To improve compatibility with React Testing Library, and to ease the migration f
4444

4545
Please replace all occurrences of these queries in your codebase.
4646

47+
## `fireEvent` support for disabled components
48+
49+
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.
50+
51+
If your code contained any workarounds for preventing events firing on disabled events, you should now be able to remove them.
52+
4753
# Guide for `@testing-library/react-native` users
4854

4955
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.
@@ -87,14 +93,6 @@ Cleaning up (unmounting) components after each test is included by default in th
8793

8894
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.
8995

90-
## No special handling for `disabled` prop
91-
92-
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.
93-
94-
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).
95-
96-
As a mitigation, you'll likely need to modify the logic of "touchable" components to bail if they're pressed in disabled state.
97-
9896
## No [NativeTestInstance](https://www.native-testing-library.com/docs/api-test-instance) abstraction
9997

10098
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.

0 commit comments

Comments
 (0)