[Fabric] Implement the onPressOut property for the fabric implementation of TextInput
Description
Type of Change
- New feature (non-breaking change which adds functionality)
Why
Implement the onPressOut property for the fabric implementation of TextInput.
This property was available in RNW Paper via TextInputViewManager.
See https://reactnative.dev/docs/textinput#onpressout for details.
Resolves [https://github.com/microsoft/react-native-windows/issues/13128]
What
Implement the onPressOut property for the fabric implementation of TextInput.
Screenshots
https://github.com/user-attachments/assets/f0903a65-f869-4692-b8c2-52219522bee8
Testing
If you added tests that prove your changes are effective or that your feature works, add a few sentences here detailing the added test scenarios.
Changelog
yes
Add a brief summary of the change to use in the release notes for the next release.
Microsoft Reviewers: Open in CodeFlow
onPressIn and onPressOut are implemented using a pressable inside the JS in TextInput. https://github.com/facebook/react-native/blob/33aa7b91617bb6e2b9689850e8740c97e01464de/packages/react-native/Libraries/Components/TextInput/TextInput.js#L619
We shouldn't be emitting these events from native.
This looks like onPressIn was implemented incorrectly. Instead we should be removing these native events, and determining why the usePressability was/is not firing the onPressIn/Out on the TextInput.
onPressIn and onPressOut are implemented using a pressable inside the JS in TextInput. https://github.com/facebook/react-native/blob/33aa7b91617bb6e2b9689850e8740c97e01464de/packages/react-native/Libraries/Components/TextInput/TextInput.js#L619
We shouldn't be emitting these events from native.
This looks like onPressIn was implemented incorrectly. Instead we should be removing these native events, and determining why the usePressability was/is not firing the onPressIn/Out on the TextInput.
@acoates-ms We can track this as an enhancement task. I've created an issue to follow up. Currently, all touch-related events like onKeyUp, onKeyDown, onFocus, onBlur, and other touch handlers are being emitted from the native side. As part of future improvements, we plan to shift these to be handled through Pressability in JavaScript instead. https://github.com/microsoft/react-native-windows/issues/14798
onPressIn and onPressOut are implemented using a pressable inside the JS in TextInput. https://github.com/facebook/react-native/blob/33aa7b91617bb6e2b9689850e8740c97e01464de/packages/react-native/Libraries/Components/TextInput/TextInput.js#L619 We shouldn't be emitting these events from native. This looks like onPressIn was implemented incorrectly. Instead we should be removing these native events, and determining why the usePressability was/is not firing the onPressIn/Out on the TextInput.
@acoates-ms We can track this as an enhancement task. I've created an issue to follow up. Currently, all touch-related events like onKeyUp, onKeyDown, onFocus, onBlur, and other touch handlers are being emitted from the native side. As part of future improvements, we plan to shift these to be handled through Pressability in JavaScript instead. #14798
I dont think we should check this in as is. This is code that would all have to be removed to do the proper fix. We should be removing the onPressIn code that was already added, and looking at why pressable isn't working.
onPressIn and onPressOut are implemented using a pressable inside the JS in TextInput. https://github.com/facebook/react-native/blob/33aa7b91617bb6e2b9689850e8740c97e01464de/packages/react-native/Libraries/Components/TextInput/TextInput.js#L619
We shouldn't be emitting these events from native.
This looks like onPressIn was implemented incorrectly. Instead we should be removing these native events, and determining why the usePressability was/is not firing the onPressIn/Out on the TextInput.
Is this a change in Fabric? We fire omPressIn and onPressOut in TextInput ViewManager - just because they're defined in Pressable, don't we still need to fire them?
Edit: TextInput has its own onPressIn and onPressOut events that don't require you to wrap it in a Pressable: https://reactnative.dev/docs/textinput#onpressin
onPressIn and onPressOut are implemented using a pressable inside the JS in TextInput. https://github.com/facebook/react-native/blob/33aa7b91617bb6e2b9689850e8740c97e01464de/packages/react-native/Libraries/Components/TextInput/TextInput.js#L619 We shouldn't be emitting these events from native. This looks like onPressIn was implemented incorrectly. Instead we should be removing these native events, and determining why the usePressability was/is not firing the onPressIn/Out on the TextInput.
Is this a change in Fabric? We fire omPressIn and onPressOut in TextInput ViewManager - just because they're defined in Pressable, don't we still need to fire them?
Edit: TextInput has its own onPressIn and onPressOut events that don't require you to wrap it in a Pressable: https://reactnative.dev/docs/textinput#onpressin
onPressIn and onPressOut are implemented using a pressable inside the JS in TextInput. https://github.com/facebook/react-native/blob/33aa7b91617bb6e2b9689850e8740c97e01464de/packages/react-native/Libraries/Components/TextInput/TextInput.js#L619 We shouldn't be emitting these events from native. This looks like onPressIn was implemented incorrectly. Instead we should be removing these native events, and determining why the usePressability was/is not firing the onPressIn/Out on the TextInput.
Is this a change in Fabric? We fire omPressIn and onPressOut in TextInput ViewManager - just because they're defined in Pressable, don't we still need to fire them?
Edit: TextInput has its own onPressIn and onPressOut events that don't require you to wrap it in a Pressable: https://reactnative.dev/docs/textinput#onpressin
@acoates-ms
Since TextInput already supports onPressIn and onPressOut natively (as outlined here: https://reactnative.dev/docs/textinput#onpressin) and doesn't need to be wrapped in a Pressable, are we good to close this PR? Let me know if you have any additional thoughts on whether these events should still be triggered via Pressable.
@jonthysell @acoates-ms This pr has been open for a long time. Looking for a final signoff on what steps can be taken further
@jonthysell @acoates-ms This pr has been open for a long time. Looking for a final signoff on what steps can be taken further
Merging this pr for now. Will take this up further in enhancement task