-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[Fabric] Implement the onPressOut property for the fabric implementation of TextInput #14784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fabric] Implement the onPressOut property for the fabric implementation of TextInput #14784
Conversation
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. |
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. |
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 |
vnext/Microsoft.ReactNative/Fabric/Composition/TextInput/WindowsTextInputEventEmitter.cpp
Show resolved
Hide resolved
packages/@react-native-windows/tester/src/js/examples/TextInput/TextInputExample.windows.js
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Fabric/Composition/TextInput/WindowsTextInputEventEmitter.cpp
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HariniMalothu17 please create 3 issues discussed in the PR comments that can be taken up in separate PRs. Rest looks good to me. :)
@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 |
Description
Type of Change
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//issues/13128]
What
Implement the onPressOut property for the fabric implementation of TextInput.
Screenshots
Recording.2025-06-16.154148.mp4
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
Should this change be included in the release notes: yes
Add a brief summary of the change to use in the release notes for the next release.
Implement the onPressOut property for the fabric implementation of TextInput
Microsoft Reviewers: Open in CodeFlow