Skip to content

Commit 28a4c99

Browse files
authored
Do not call onPress on buttons while dismissing keyboard (#4281)
## Description Currently, when buttons are inside `ScrolView` with `keyboardShouldPersistTaps={never}` they still call `onPress` even though press was meant to dismiss keyboard. This PR fixes that behavior. It attaches listeners for keyboard state and based on that it blocks `onPress*` callbacks on JS side. Fixes #992 ## Test plan Tested on Keyboard Should Persist Taps example
1 parent e36d8d3 commit 28a4c99

5 files changed

Lines changed: 316 additions & 33 deletions

File tree

apps/common-app/src/new_api/tests/keyboardShouldPersistTaps/index.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,22 @@ import {
1313
Pressable as RNGHPressable,
1414
ScrollView as RNGHScrollView,
1515
TextInput as RNGHTextInput,
16+
Touchable as RNGHTouchable,
1617
useTapGesture,
1718
} from 'react-native-gesture-handler';
1819

1920
import type { FeedbackHandle } from '../../../common';
2021
import { COLORS, Feedback, InfoSection } from '../../../common';
2122

2223
type Mode = 'never' | 'handled' | 'always';
23-
type Example = 'pressable' | 'tap';
24+
type Example = 'pressable' | 'touchable' | 'tap';
2425

2526
const MODES: Mode[] = ['never', 'handled', 'always'];
26-
const EXAMPLES: Example[] = ['pressable', 'tap'];
27+
const EXAMPLES: Example[] = ['pressable', 'touchable', 'tap'];
2728

2829
const EXAMPLE_LABELS: Record<Example, string> = {
2930
pressable: 'GH Pressable',
31+
touchable: 'GH Touchable',
3032
tap: 'useTapGesture',
3133
};
3234

@@ -105,6 +107,12 @@ export default function KeyboardShouldPersistTapsExample() {
105107
onPress={() => report('GH Pressable onPress')}>
106108
<Text style={styles.buttonText}>Press me</Text>
107109
</RNGHPressable>
110+
) : example === 'touchable' ? (
111+
<RNGHTouchable
112+
style={[styles.button, { backgroundColor: COLORS.GREEN }]}
113+
onPress={() => report('GH Touchable onPress')}>
114+
<Text style={styles.buttonText}>Press me</Text>
115+
</RNGHTouchable>
108116
) : (
109117
<GestureTapButton
110118
onTap={() => report('useTapGesture onActivate')}

packages/react-native-gesture-handler/src/__tests__/api_v3.test.tsx

Lines changed: 164 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
11
import { render, renderHook } from '@testing-library/react-native';
22
import { act } from 'react';
3-
import { View } from 'react-native';
3+
import { Keyboard, View } from 'react-native';
44

55
import GestureHandlerRootView from '../components/GestureHandlerRootView';
66
import { fireGestureHandler, getByGestureTestId } from '../jestUtils';
77
import { State } from '../State';
88
import { Pressable, RectButton, ScrollView, Touchable } from '../v3/components';
9+
import {
10+
isKeyboardDismissingTap,
11+
type JSResponderContextValue,
12+
} from '../v3/components/ScrollViewResponderInterceptor';
913
import { GestureDetector } from '../v3/detectors';
1014
import { useSimultaneousGestures } from '../v3/hooks';
1115
import { usePanGesture, useTapGesture } from '../v3/hooks/gestures';
@@ -133,7 +137,7 @@ describe('[API v3] Components', () => {
133137

134138
describe('ScrollView', () => {
135139
test('handles responder event passed through Pressable for keyboardShouldPersistTaps handled', async () => {
136-
const { getByTestId, UNSAFE_getAllByType } = render(
140+
const { UNSAFE_getAllByType } = render(
137141
<GestureHandlerRootView>
138142
<ScrollView keyboardShouldPersistTaps="handled">
139143
<Pressable testID="pressable" />
@@ -143,22 +147,22 @@ describe('[API v3] Components', () => {
143147

144148
await act(flushImmediate);
145149

146-
const pressable = getByTestId('pressable');
150+
const nativeDetector = getNativeDetector(UNSAFE_getAllByType);
147151
const scrollViewResponder = getScrollViewResponder(UNSAFE_getAllByType);
148152

149153
expect(scrollViewResponder).toBeDefined();
150154
expect(
151155
scrollViewResponder?.props.onStartShouldSetResponderCapture()
152156
).toBe(false);
153-
expect(pressable.props.onStartShouldSetResponder()).toBe(false);
157+
expect(nativeDetector?.props.onStartShouldSetResponder()).toBe(false);
154158
expect(scrollViewResponder?.props.onStartShouldSetResponder()).toBe(true);
155159
expect(scrollViewResponder?.props.onStartShouldSetResponder()).toBe(
156160
false
157161
);
158162
});
159163

160164
test('does not handle responder event passed through Pressable without keyboardShouldPersistTaps handled', async () => {
161-
const { getByTestId, UNSAFE_getAllByType } = render(
165+
const { UNSAFE_getAllByType } = render(
162166
<GestureHandlerRootView>
163167
<ScrollView>
164168
<Pressable testID="pressable" />
@@ -168,14 +172,14 @@ describe('[API v3] Components', () => {
168172

169173
await act(flushImmediate);
170174

171-
const pressable = getByTestId('pressable');
175+
const nativeDetector = getNativeDetector(UNSAFE_getAllByType);
172176
const scrollViewResponder = getScrollViewResponder(UNSAFE_getAllByType);
173177

174178
expect(scrollViewResponder).toBeDefined();
175179
expect(
176180
scrollViewResponder?.props.onStartShouldSetResponderCapture()
177181
).toBe(false);
178-
expect(pressable.props.onStartShouldSetResponder()).toBe(false);
182+
expect(nativeDetector?.props.onStartShouldSetResponder()).toBe(false);
179183
expect(scrollViewResponder?.props.onStartShouldSetResponder()).toBe(
180184
false
181185
);
@@ -278,6 +282,159 @@ describe('[API v3] Components', () => {
278282
});
279283
});
280284

285+
describe('keyboardShouldPersistTaps="never" drop', () => {
286+
// The keyboard-visibility tracker subscribes via Keyboard.addListener when a
287+
// ScrollView mounts. We spy on it to grab the captured `keyboardDidShow`
288+
// handler and invoke it, simulating the keyboard being open.
289+
const showKeyboard = (addListenerSpy: jest.SpyInstance, height = 300) => {
290+
const showCall = addListenerSpy.mock.calls.find(
291+
(call) => call[0] === 'keyboardDidShow'
292+
);
293+
act(() => {
294+
showCall?.[1]?.({ endCoordinates: { height } });
295+
});
296+
};
297+
298+
const makeContext = (
299+
keyboardShouldPersistTaps: JSResponderContextValue['keyboardShouldPersistTaps']
300+
): JSResponderContextValue => ({
301+
isRNGHResponderEvent: { current: false },
302+
keyboardShouldPersistTaps,
303+
});
304+
305+
const fireTap = (testID: string) =>
306+
act(() => {
307+
fireGestureHandler(getByGestureTestId(testID), [
308+
{ state: State.BEGAN },
309+
{ state: State.ACTIVE },
310+
{ state: State.END },
311+
]);
312+
});
313+
314+
test('isKeyboardDismissingTap is true only in never mode while the keyboard is visible', async () => {
315+
const addListenerSpy = jest.spyOn(Keyboard, 'addListener');
316+
317+
render(
318+
<GestureHandlerRootView>
319+
<ScrollView keyboardShouldPersistTaps="never" />
320+
</GestureHandlerRootView>
321+
);
322+
await act(flushImmediate);
323+
324+
// Keyboard not shown yet -> nothing to dismiss.
325+
expect(isKeyboardDismissingTap(makeContext('never'))).toBe(false);
326+
327+
showKeyboard(addListenerSpy);
328+
329+
// `never` (and its default, undefined) drops the tap; the others never do.
330+
expect(isKeyboardDismissingTap(makeContext('never'))).toBe(true);
331+
expect(isKeyboardDismissingTap(makeContext(undefined))).toBe(true);
332+
expect(isKeyboardDismissingTap(makeContext('handled'))).toBe(false);
333+
expect(isKeyboardDismissingTap(makeContext('always'))).toBe(false);
334+
// Outside an RNGH ScrollView there is no context, so nothing is dropped.
335+
expect(isKeyboardDismissingTap(null)).toBe(false);
336+
337+
addListenerSpy.mockRestore();
338+
});
339+
340+
test('isKeyboardDismissingTap is false for a detached (height 0) keyboard', async () => {
341+
const addListenerSpy = jest.spyOn(Keyboard, 'addListener');
342+
343+
render(
344+
<GestureHandlerRootView>
345+
<ScrollView keyboardShouldPersistTaps="never" />
346+
</GestureHandlerRootView>
347+
);
348+
await act(flushImmediate);
349+
350+
// A detached/floating keyboard reports height 0 - nothing to dismiss.
351+
showKeyboard(addListenerSpy, 0);
352+
353+
expect(isKeyboardDismissingTap(makeContext('never'))).toBe(false);
354+
355+
addListenerSpy.mockRestore();
356+
});
357+
358+
test('Touchable does NOT fire any press callback on the keyboard-dismissing tap (never)', async () => {
359+
const addListenerSpy = jest.spyOn(Keyboard, 'addListener');
360+
const onPress = jest.fn();
361+
const onPressIn = jest.fn();
362+
const onPressOut = jest.fn();
363+
364+
render(
365+
<GestureHandlerRootView>
366+
<ScrollView keyboardShouldPersistTaps="never">
367+
<Touchable
368+
testID="touchable"
369+
onPress={onPress}
370+
onPressIn={onPressIn}
371+
onPressOut={onPressOut}
372+
/>
373+
</ScrollView>
374+
</GestureHandlerRootView>
375+
);
376+
await act(flushImmediate);
377+
showKeyboard(addListenerSpy);
378+
379+
// Includes a move (second ACTIVE) so the onUpdate path is exercised too.
380+
act(() => {
381+
fireGestureHandler(getByGestureTestId('touchable'), [
382+
{ state: State.BEGAN },
383+
{ state: State.ACTIVE },
384+
{ state: State.ACTIVE },
385+
{ state: State.END },
386+
]);
387+
});
388+
389+
// The whole interaction is swallowed - not just onPress, but the press-in/
390+
// out side effects too (incl. via onUpdate as the finger moves).
391+
expect(onPress).not.toHaveBeenCalled();
392+
expect(onPressIn).not.toHaveBeenCalled();
393+
expect(onPressOut).not.toHaveBeenCalled();
394+
addListenerSpy.mockRestore();
395+
});
396+
397+
test('Touchable fires onPress in never mode when the keyboard is not visible', async () => {
398+
const onPress = jest.fn();
399+
400+
render(
401+
<GestureHandlerRootView>
402+
<ScrollView keyboardShouldPersistTaps="never">
403+
<Touchable testID="touchable" onPress={onPress} />
404+
</ScrollView>
405+
</GestureHandlerRootView>
406+
);
407+
await act(flushImmediate);
408+
409+
fireTap('touchable');
410+
411+
expect(onPress).toHaveBeenCalledTimes(1);
412+
});
413+
414+
test.each(['handled', 'always'] as const)(
415+
'Touchable fires onPress in %s mode even while the keyboard is visible',
416+
async (mode) => {
417+
const addListenerSpy = jest.spyOn(Keyboard, 'addListener');
418+
const onPress = jest.fn();
419+
420+
render(
421+
<GestureHandlerRootView>
422+
<ScrollView keyboardShouldPersistTaps={mode}>
423+
<Touchable testID="touchable" onPress={onPress} />
424+
</ScrollView>
425+
</GestureHandlerRootView>
426+
);
427+
await act(flushImmediate);
428+
showKeyboard(addListenerSpy);
429+
430+
fireTap('touchable');
431+
432+
expect(onPress).toHaveBeenCalledTimes(1);
433+
addListenerSpy.mockRestore();
434+
}
435+
);
436+
});
437+
281438
describe('Touchable', () => {
282439
test('calls onPress on successful press', () => {
283440
const pressFn = jest.fn();

packages/react-native-gesture-handler/src/v3/components/Pressable.tsx

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ import {
4545
} from '../hooks';
4646
import { PureNativeButton } from './GestureButtons';
4747
import {
48+
isKeyboardDismissingTap,
4849
JSResponderContext,
49-
updateResponderEventValue,
5050
} from './ScrollViewResponderInterceptor';
5151

5252
const DEFAULT_LONG_PRESS_DURATION = 500;
@@ -92,6 +92,11 @@ const Pressable = (props: PressableProps) => {
9292
height: 0,
9393
});
9494

95+
// When the touch that begins a press is the one dismissing the keyboard
96+
// (keyboardShouldPersistTaps="never"), the press is swallowed to match RN's
97+
// touchables.
98+
const dropKeyboardTapRef = useRef<boolean | null>(null);
99+
95100
const normalizedHitSlop: Insets = useMemo(
96101
() =>
97102
typeof hitSlop === 'number'
@@ -153,11 +158,16 @@ const Pressable = (props: PressableProps) => {
153158

154159
const handleFinalize = useCallback(() => {
155160
isCurrentlyPressed.current = false;
161+
dropKeyboardTapRef.current = null;
156162
cancelLongPress();
157163
cancelDelayedPress();
158164
setPressedState(false);
159165
}, [cancelDelayedPress, cancelLongPress]);
160166

167+
const captureKeyboardDismiss = useCallback(() => {
168+
dropKeyboardTapRef.current ??= isKeyboardDismissingTap(jsResponderContext);
169+
}, [jsResponderContext]);
170+
161171
const handlePressIn = useCallback(
162172
(event: PressableEvent, skipBoundsCheck = false) => {
163173
if (
@@ -265,6 +275,12 @@ const Pressable = (props: PressableProps) => {
265275
maxDistance: INT32_MAX, // Stops long press from cancelling on touch move
266276
cancelsTouchesInView: false,
267277
onTouchesDown: (event) => {
278+
captureKeyboardDismiss();
279+
280+
if (dropKeyboardTapRef.current) {
281+
return;
282+
}
283+
268284
const pressableEvent = gestureTouchToPressableEvent(event);
269285
stateMachine.handleEvent(
270286
StateMachineEvent.LONG_PRESS_TOUCHES_DOWN,
@@ -314,6 +330,12 @@ const Pressable = (props: PressableProps) => {
314330
}
315331
},
316332
onBegin: () => {
333+
captureKeyboardDismiss();
334+
335+
if (dropKeyboardTapRef.current) {
336+
return;
337+
}
338+
317339
if (Platform.isTV) {
318340
// tvOS drives this native gesture from the focus-engine Select press.
319341
// The press state machine is touch-based and never
@@ -398,23 +420,11 @@ const Pressable = (props: PressableProps) => {
398420
[onLayout]
399421
);
400422

401-
// Let RN components higher in the tree handle JS responder negotiation.
402-
// RNGH ScrollView uses this marker to preserve keyboardShouldPersistTaps='handled'
403-
// when there are no RN responder components between it and this Pressable.
404-
const handleStartShouldSetResponder = useCallback(() => {
405-
if (!disabled) {
406-
updateResponderEventValue(jsResponderContext, true);
407-
}
408-
409-
return false;
410-
}, [disabled, jsResponderContext]);
411-
412423
const tvProps = getTVProps(remainingProps);
413424

414425
return (
415426
<GestureDetector gesture={gesture}>
416427
<PureNativeButton
417-
onStartShouldSetResponder={handleStartShouldSetResponder}
418428
{...remainingProps}
419429
{...tvProps}
420430
onLayout={setDimensions}

0 commit comments

Comments
 (0)