-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Implement onPressOut property for TextInput in fabric #14754
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
base: main
Are you sure you want to change the base?
Changes from all commits
080419b
0c7ede5
456f8dc
1e2d12e
e8b9967
a72aa8a
4a94045
50da6f2
8d1fa43
ce1f79e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"type": "prerelease", | ||
"comment": "Implement onPressOut event emission in TextInput fabric component", | ||
"packageName": "react-native-windows", | ||
"email": "[email protected]", | ||
"dependentChangeType": "patch" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -700,8 +700,6 @@ void WindowsTextInputComponentView::OnPointerPressed( | |
auto emitter = std::static_pointer_cast<const facebook::react::WindowsTextInputEventEmitter>(m_eventEmitter); | ||
float offsetX = position.X - m_layoutMetrics.frame.origin.x; | ||
float offsetY = position.Y - m_layoutMetrics.frame.origin.y; | ||
float neutralX = m_layoutMetrics.frame.origin.x; | ||
float neutralY = m_layoutMetrics.frame.origin.y; | ||
|
||
facebook::react::GestureResponderEvent pressInArgs; | ||
pressInArgs.target = m_tag; | ||
|
@@ -759,6 +757,22 @@ void WindowsTextInputComponentView::OnPointerReleased( | |
auto hr = m_textServices->TxSendMessage(msg, static_cast<WPARAM>(wParam), static_cast<LPARAM>(lParam), &lresult); | ||
args.Handled(hr != S_FALSE); | ||
} | ||
|
||
// Emits the OnPressOut event | ||
if (m_eventEmitter && !m_comingFromJS) { | ||
auto emitter = std::static_pointer_cast<const facebook::react::WindowsTextInputEventEmitter>(m_eventEmitter); | ||
float offsetX = position.X - m_layoutMetrics.frame.origin.x; | ||
float offsetY = position.Y - m_layoutMetrics.frame.origin.y; | ||
|
||
Comment on lines
+764
to
+766
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comparing this to the OnPressIn event up at lines 698-715 I see this local variable initialized:
But it doesn't appear to be used. If that's dead code, can you clean it up as part of this PR, to keep the In/Out methods consistent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the unused neutralX and neutralY variables from the OnPressIn method to keep both In/Out methods consistent. The variables were indeed dead code that was not being used anywhere in the event structure. Fixed in e8b9967. |
||
facebook::react::GestureResponderEvent pressOutArgs; | ||
pressOutArgs.target = m_tag; | ||
pressOutArgs.pagePoint = {position.X, position.Y}; | ||
pressOutArgs.offsetPoint = {offsetX, offsetY}; | ||
pressOutArgs.timestamp = static_cast<double>(pp.Timestamp()) / 1000.0; | ||
pressOutArgs.identifier = pp.PointerId(); | ||
|
||
emitter->onPressOut(pressOutArgs); | ||
} | ||
} | ||
|
||
void WindowsTextInputComponentView::OnPointerMoved( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,6 +53,7 @@ class WindowsTextInputEventEmitter : public ViewEventEmitter { | |
void onKeyPress(OnKeyPress value) const; | ||
void onContentSizeChange(OnContentSizeChange value) const; | ||
void onPressIn(GestureResponderEvent event) const override; | ||
void onPressOut(GestureResponderEvent event) const; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I noticed the lack of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're correct that So the current implementation is correct: For consistency, |
||
void onEndEditing(OnEndEditing value) const; | ||
}; | ||
|
||
|
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.
@copilot I need to check if onpressin is working or not
Uh oh!
There was an error while loading. Please reload this page.
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.
@copilot Onpressin also has to be triggered
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.
I've enhanced the E2E tests to better verify that both onPressIn and onPressOut events are working correctly. The updated tests now:
The tests demonstrate that the complete press-release cycle works properly, confirming that both onPressIn and onPressOut events are being triggered correctly in the fabric implementation. Updated in e48e1a3.