-
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 6 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 |
---|---|---|
|
@@ -222,6 +222,43 @@ describe('TextInput Tests', () => { | |
const search = await app.findElementByTestID('example_search'); | ||
await search.setValue(''); | ||
}); | ||
test('TextInput triggers onPressOut and updates state text', async () => { | ||
// Scroll the example into view | ||
await searchBox('onPressIn'); | ||
const component = await app.findElementByTestID('textinput-press'); | ||
await component.waitForDisplayed({timeout: 5000}); | ||
const dump = await dumpVisualTree('textinput-press'); | ||
expect(dump).toMatchSnapshot(); | ||
|
||
// Get reference to state display element | ||
const stateText = await app.findElementByTestID('textinput-state-display'); | ||
|
||
// Trigger onPressIn followed by onPressOut (using touchAction for press and release) | ||
await component.touchAction([ | ||
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. touchAction is not triggering anything 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 right that |
||
{action: 'press', x: 0, y: 0}, | ||
{action: 'wait', ms: 100}, | ||
{action: 'release'}, | ||
]); | ||
|
||
// Wait for onPressOut to update the state text | ||
await app.waitUntil( | ||
async () => { | ||
const currentText = await stateText.getText(); | ||
return currentText === 'Released click/touch'; | ||
}, | ||
{ | ||
timeout: 5000, | ||
timeoutMsg: 'State text not updated after onPressOut.', | ||
}, | ||
); | ||
|
||
// Assertion | ||
expect(await stateText.getText()).toBe('Released click/touch'); | ||
|
||
// Clean up by unfocusing the input | ||
const search = await app.findElementByTestID('example_search'); | ||
await search.setValue(''); | ||
}); | ||
test('TextInputs can have attributed text', async () => { | ||
const component = await app.findElementByTestID('text-input'); | ||
await component.waitForDisplayed({timeout: 5000}); | ||
|
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 won't this hinder the test for onpressin as click instantly releases and then onpressout gets 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.
You're absolutely right about the test interference. The onPressIn test was using
click()
which triggers both press and release events, causing the state to immediately change to "Released click/touch" instead of staying at "Holding down the click/touch".I've fixed this by modifying the onPressIn test to use
touchAction
with separate press and release actions, ensuring proper isolation between the tests. Fixed in 4a94045.