necolas / react-native-web

Cross-platform React UI packages
https://necolas.github.io/react-native-web
MIT License
21.46k stars 1.77k forks source link

Fix onPress is called twice when activated with keyboard and set to disabled (blurred) #2632

Closed bernhardoj closed 2 months ago

bernhardoj commented 5 months ago

Description

Fixes https://github.com/necolas/react-native-web/issues/2605

We are attaching the keyup listener to the document. https://github.com/necolas/react-native-web/blob/5f9c32eba0f840bde7b462a57d0748358ff866f6/packages/react-native-web/src/modules/usePressEvents/PressResponder.js#L348-L350

If the Pressable (with a button role) is disabled after the first onPress is triggered, the focus is transferred to body and isNativeInteractiveElement will be false when releasing the press, thus the onPress is called for the second time. https://github.com/necolas/react-native-web/blob/5f9c32eba0f840bde7b462a57d0748358ff866f6/packages/react-native-web/src/modules/usePressEvents/PressResponder.js#L316-L326

Test plan

  1. On the pressable example code, disable the pressable inonPress
  2. Run the pressable example
  3. Hold down the pressable using a keyboard. Verify the onPress is called
  4. Release the press. Verify the onPress isn't called.

Currently, the 4th step fails and this PR is trying to fix it.

Before:

https://github.com/necolas/react-native-web/assets/50919443/26a9a3f2-d220-4b0f-8642-8cfdba2370d0

After:

https://github.com/necolas/react-native-web/assets/50919443/44ca0796-9a2e-4efa-98b8-485b2dd6a285

codesandbox-ci[bot] commented 5 months ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2ecdf1463d7a865f0e038899a4ae973d08e49a94:

Sandbox Source
react-native-web-examples Configuration
bernhardoj commented 3 months ago

@necolas I have updated the PR to use the first solution. With the new solution, releasing the enter key on a different element won't trigger onPress anymore as shown in the video below.

https://github.com/necolas/react-native-web/assets/50919443/cb87ad87-b0d6-487e-a9c6-dea8e1a2d21e

What I did in the video above is:

  1. Holding down enter on the top pressable
  2. Use tab key to change the focus to another button
  3. Release the enter

No onPress is called, compared to before the fix:

https://github.com/necolas/react-native-web/assets/50919443/328b0e8d-6723-474b-8a44-36a878ab70e4

Works fine for pressable with/without button role. Added a test too.

bernhardoj commented 2 months ago

@necolas hi, is there anything left to update?

bernhardoj commented 2 months ago

@necolas hi, is there anything I can do to help moving this PR forward? (sorry for keep bumping you)

necolas commented 2 months ago

Sorry for the delay. Merged

bernhardoj commented 2 months ago

Thanks!