Skip to content

Commit 7d60ff8

Browse files
bernhardojnecolas
authored andcommittedApr 22, 2024·
[fix] Pressable keyboard onPress bug
Don't call onPress when keyup is called on a different element than the target. Fix #2605 Close #2632
1 parent a0cd8ff commit 7d60ff8

File tree

3 files changed

+79
-44
lines changed

3 files changed

+79
-44
lines changed
 

‎packages/react-native-web/src/exports/Pressable/__tests__/__snapshots__/index-test.js.snap

+3-3
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,14 @@ exports[`components/Pressable hover interaction 3`] = `
6161
/>
6262
`;
6363

64-
exports[`components/Pressable press interaction (keyboard) 1`] = `
64+
exports[`components/Pressable press interaction (keyboard) trigger press when keyup is on the same element 1`] = `
6565
<div
6666
class="css-view-175oi2r r-cursor-1loqt21 r-touchAction-1otgn73"
6767
tabindex="0"
6868
/>
6969
`;
7070

71-
exports[`components/Pressable press interaction (keyboard) 2`] = `
71+
exports[`components/Pressable press interaction (keyboard) trigger press when keyup is on the same element 2`] = `
7272
<div
7373
class="css-view-175oi2r r-cursor-1loqt21 r-touchAction-1otgn73"
7474
style="outline: press-ring;"
@@ -80,7 +80,7 @@ exports[`components/Pressable press interaction (keyboard) 2`] = `
8080
</div>
8181
`;
8282

83-
exports[`components/Pressable press interaction (keyboard) 3`] = `null`;
83+
exports[`components/Pressable press interaction (keyboard) trigger press when keyup is on the same element 3`] = `null`;
8484

8585
exports[`components/Pressable press interaction (pointer) 1`] = `
8686
<div

‎packages/react-native-web/src/exports/Pressable/__tests__/index-test.js

+70-40
Original file line numberDiff line numberDiff line change
@@ -200,50 +200,80 @@ describe('components/Pressable', () => {
200200
expect(onContextMenu).toBeCalled();
201201
});
202202

203-
test('press interaction (keyboard)', () => {
204-
let container;
205-
const onPress = jest.fn();
206-
const onPressIn = jest.fn();
207-
const onPressOut = jest.fn();
208-
const ref = React.createRef();
203+
describe('press interaction (keyboard)', () => {
204+
test('trigger press when keyup is on the same element', () => {
205+
let container;
206+
const onPress = jest.fn();
207+
const onPressIn = jest.fn();
208+
const onPressOut = jest.fn();
209+
const ref = React.createRef();
209210

210-
function TestCase() {
211-
const [shown, setShown] = React.useState(true);
212-
return shown ? (
213-
<Pressable
214-
children={({ pressed }) =>
215-
pressed ? <div data-testid="press-content" /> : null
216-
}
217-
onPress={(e) => {
218-
onPress(e);
219-
setShown(false);
220-
}}
221-
onPressIn={onPressIn}
222-
onPressOut={onPressOut}
223-
ref={ref}
224-
style={({ pressed }) => [pressed && { outline: 'press-ring' }]}
225-
/>
226-
) : null;
227-
}
211+
function TestCase() {
212+
const [shown, setShown] = React.useState(true);
213+
return shown ? (
214+
<Pressable
215+
children={({ pressed }) =>
216+
pressed ? <div data-testid="press-content" /> : null
217+
}
218+
onPress={(e) => {
219+
onPress(e);
220+
setShown(false);
221+
}}
222+
onPressIn={onPressIn}
223+
onPressOut={onPressOut}
224+
ref={ref}
225+
style={({ pressed }) => [pressed && { outline: 'press-ring' }]}
226+
/>
227+
) : null;
228+
}
228229

229-
act(() => {
230-
({ container } = render(<TestCase />));
231-
});
232-
const target = createEventTarget(ref.current);
233-
expect(container.firstChild).toMatchSnapshot();
234-
act(() => {
235-
target.keydown({ key: 'Enter' });
236-
jest.runAllTimers();
230+
act(() => {
231+
({ container } = render(<TestCase />));
232+
});
233+
const target = createEventTarget(ref.current);
234+
expect(container.firstChild).toMatchSnapshot();
235+
act(() => {
236+
target.keydown({ key: 'Enter' });
237+
jest.runAllTimers();
238+
});
239+
expect(onPressIn).toBeCalled();
240+
expect(container.firstChild).toMatchSnapshot();
241+
act(() => {
242+
target.keyup({ key: 'Enter' });
243+
jest.runAllTimers();
244+
});
245+
expect(onPressOut).toBeCalled();
246+
expect(onPress).toBeCalled();
247+
expect(container.firstChild).toMatchSnapshot();
237248
});
238-
expect(onPressIn).toBeCalled();
239-
expect(container.firstChild).toMatchSnapshot();
240-
act(() => {
241-
target.keyup({ key: 'Enter' });
242-
jest.runAllTimers();
249+
250+
test('ignore press when keyup is on a different element', () => {
251+
const onPress = jest.fn();
252+
const firstRef = React.createRef();
253+
254+
function TestCase() {
255+
return (
256+
<Pressable
257+
onPress={(e) => {
258+
onPress(e);
259+
}}
260+
ref={firstRef}
261+
/>
262+
);
263+
}
264+
265+
act(() => {
266+
render(<TestCase />);
267+
});
268+
const target = createEventTarget(firstRef.current);
269+
const body = createEventTarget(document.body);
270+
act(() => {
271+
target.keydown({ key: 'Enter' });
272+
body.keyup({ key: 'Enter' });
273+
jest.runAllTimers();
274+
});
275+
expect(onPress).not.toBeCalled();
243276
});
244-
expect(onPressOut).toBeCalled();
245-
expect(onPress).toBeCalled();
246-
expect(container.firstChild).toMatchSnapshot();
247277
});
248278

249279
test('press interaction as button (keyboard)', () => {

‎packages/react-native-web/src/modules/usePressEvents/PressResponder.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ export default class PressResponder {
233233
pageY: number
234234
|}>;
235235
_touchState: TouchState = NOT_RESPONDER;
236+
_responderElement: ?HTMLElement = null;
236237

237238
constructor(config: PressResponderConfig) {
238239
this.configure(config);
@@ -320,10 +321,13 @@ export default class PressResponder {
320321
elementType === 'input' ||
321322
elementType === 'select' ||
322323
elementType === 'textarea';
324+
const isActiveElement = this._responderElement === target;
323325

324-
if (onPress != null && !isNativeInteractiveElement) {
326+
if (onPress != null && !isNativeInteractiveElement && isActiveElement) {
325327
onPress(event);
326328
}
329+
330+
this._responderElement = null;
327331
}
328332
};
329333

@@ -345,6 +349,7 @@ export default class PressResponder {
345349
if (!disabled && isValidKeyPress(event)) {
346350
if (this._touchState === NOT_RESPONDER) {
347351
start(event, false);
352+
this._responderElement = target;
348353
// Listen to 'keyup' on document to account for situations where
349354
// focus is moved to another element during 'keydown'.
350355
document.addEventListener('keyup', keyupHandler);

0 commit comments

Comments
 (0)
Please sign in to comment.