[Fabric] Implement onContentSizeChange in TextInput #14785
Conversation
anupriya13
added
Area: TextInput
Area: Fabric
| @@ -1247,6 +1247,12 @@ void WindowsTextInputComponentView::OnTextUpdated() noexcept { | |||
| onChangeArgs.text = GetTextFromRichEdit(); | |||
| onChangeArgs.eventCount = ++m_nativeEventCount; | |||
| emitter->onChange(onChangeArgs); | |||
| if (windowsTextInputProps().multiline) { | |||
| facebook::react::WindowsTextInputEventEmitter::OnContentSizeChange onContentSizeChangeArgs; | |||
| onContentSizeChangeArgs.contentSize.width = m_layoutMetrics.frame.size.width * m_layoutMetrics.pointScaleFactor; | |||
There was a problem hiding this comment.
This isn't what content size changed is supposed to report. m_layoutMetrics.frame.size is the size of the TextInput, onContentSizeChanged should be reporting the size of the text content within the textinput.
So if we have a textinput which scrolls, then onContentSizeChanged should include the whole scrollable content, not just the size of the textinput.
There was a problem hiding this comment.
Used this now please review https://learn.microsoft.com/en-us/windows/win32/api/textserv/nf-textserv-itextservices-txgetnaturalsize
Pull request was converted to draft
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the onContentSizeChange property in the Fabric TextInput, allowing consumers to receive updates on the rendered content’s dimensions.
- Added the GetContentSize method declaration and implementation in WindowsTextInputComponentView.
- Modified OnTextUpdated to emit onContentSizeChange events for multiline text inputs.
- Updated the playground sample to demonstrate usage of onContentSizeChange.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| vnext/Microsoft.ReactNative/Fabric/Composition/TextInput/WindowsTextInputComponentView.h | Declaration of GetContentSize added |
| vnext/Microsoft.ReactNative/Fabric/Composition/TextInput/WindowsTextInputComponentView.cpp | Implementation of GetContentSize and modification of OnTextUpdated to emit onContentSizeChange events |
| packages/playground/Samples/textinput.tsx | New sample demonstrating usage of onContentSizeChange |
| change/react-native-windows-f1078f0f-499f-408a-acd3-e969096b6dea.json | Change metadata for the release note |
Comments suppressed due to low confidence (2)
vnext/Microsoft.ReactNative/Fabric/Composition/TextInput/WindowsTextInputComponentView.cpp:1294
- [nitpick] Consider adding a comment here clarifying that the onContentSizeChange event is only emitted for multiline text inputs.
if (windowsTextInputProps().multiline) {
packages/playground/Samples/textinput.tsx:68
- [nitpick] Consider adding JSDoc comments to the getHeightAndWidth function to describe its purpose and parameters for improved code clarity.
getHeightAndWidth = (height: number, width: number) => {
| constexpr float HIMETRIC_PER_INCH = 2540.0f; | ||
|
|
||
| SIZE extentHimetric = { | ||
| static_cast<LONG>(availableWidth * scale * HIMETRIC_PER_INCH / dpi), |
There was a problem hiding this comment.
[nitpick] Consider adding inline comments to explain the conversion logic to HIMETRIC units and the use of std::numeric_limits::max() for clarity to future maintainers.
| constexpr float HIMETRIC_PER_INCH = 2540.0f; | |
| SIZE extentHimetric = { | |
| static_cast<LONG>(availableWidth * scale * HIMETRIC_PER_INCH / dpi), | |
| constexpr float HIMETRIC_PER_INCH = 2540.0f; // HIMETRIC units per inch, used for conversion. | |
| SIZE extentHimetric = { | |
| // Convert available width to HIMETRIC units using the formula: | |
| // HIMETRIC = (width in pixels) * (scale factor) * (HIMETRIC_PER_INCH) / (DPI) | |
| static_cast<LONG>(availableWidth * scale * HIMETRIC_PER_INCH / dpi), | |
| // Use the maximum value of LONG for height to indicate no vertical constraint. | |
| // This ensures the text layout engine calculates the natural height needed. |
Copilot uses AI. Check for mistakes.
| hdc, | ||
| nullptr, | ||
| nullptr, | ||
| static_cast<DWORD>(TXTNS_FITTOCONTENTWSP), |
There was a problem hiding this comment.
nit: I'm not sure on this vs TXTNS_FITTOCONTENT3. -- I dont know what "indented content" really means.
| @@ -1247,6 +1290,13 @@ void WindowsTextInputComponentView::OnTextUpdated() noexcept { | |||
| onChangeArgs.text = GetTextFromRichEdit(); | |||
| onChangeArgs.eventCount = ++m_nativeEventCount; | |||
| emitter->onChange(onChangeArgs); | |||
| if (windowsTextInputProps().multiline) { | |||
There was a problem hiding this comment.
Can we verify what single line textinputs do on the other platforms? Does it not fire at all? Is it just return the same as onLayout?
There was a problem hiding this comment.
As per this https://reactnative.dev/docs/textinput#oncontentsizechange
It's only called for multiline
|
It would be good to add an automation test for this event too. |
Description
Type of Change
Why
Implementation for onContentSizeChange property for TextInput for fabric
This property was available in RNW Paper via TextInputViewManager.
See https://reactnative.dev/docs/textinput#oncontentsizechange for details.
Resolves #13125
What
Implemented onContentSizeChange property for TextInput for fabric
Refer https://learn.microsoft.com/en-us/windows/win32/api/textserv/nf-textserv-itextservices-txgetnaturalsize
Screenshots
android:
android.content.size.mp4
windows:
contentsize.mp4
Testing
Tested using playground-composition App,
E2ETestApp already has this test case:
https://github.com/microsoft/react-native-windows/blob/main/packages/@react-native-windows/tester/src/js/examples/TextInput/TextInputExample.windows.js
Changelog
Should this change be included in the release notes: YES
[Fabric] Implemented onContentSizeChange in TextInput