Skip to content

[Fabric] Implement maxFontSizeMultiplier in Text Input #14639

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

anupriya13
Copy link
Contributor

@anupriya13 anupriya13 commented May 6, 2025

Description

Type of Change

  • New feature (non-breaking change which adds functionality)

Why

What is the motivation for this change? Add a few sentences describing the context and overall goals of the pull request's commits.
[Fabric] Implement maxFontSizeMultiplier in Text Input

Resolves #14638

What

What changes were made to the codebase to solve the bug, add the functionality, etc. that you specified above.
[Fabric] Implement maxFontSizeMultiplier in Text Input

https://reactnative.dev/docs/textinput#maxfontsizemultiplier

Please note waiting for this PR to be merged from upstream to uncomment commented code: facebook/react-native@97cf42f

Implemented as per IOS and Android.

Refer https://github.com/facebook/react-native/blob/e69e35e370a7751d258905e3348fd2b0bb2e93c1/packages/react-native/Libraries/Text/RCTTextAttributes.mm#L249

Screenshots

Add any relevant screen captures here from before or after your changes.

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.
Tested in playground and manipulating node_modules file code in TextAttributes.h

Changelog

Should this change be included in the release notes: indicate yes or no Yes

Add a brief summary of the change to use in the release notes for the next release.
[Fabric] Implement maxFontSizeMultiplier in Text Input

@anupriya13 anupriya13 marked this pull request as ready for review May 6, 2025 06:31
@anupriya13 anupriya13 requested a review from a team as a code owner May 6, 2025 06:31
@anupriya13 anupriya13 added Area: TextInput Area: Fabric Support Facebook Fabric New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric Workstream: Component Parity Close the parity gap between RNW and RN for core RN components and their supporting APIs. API: Completion labels May 6, 2025
@anupriya13 anupriya13 added the AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) label May 6, 2025
@anupriya13 anupriya13 requested review from acoates-ms and a team May 14, 2025 06:08

// Apply maxFontSizeMultiplier if specified
auto maxFontSizeMultiplier = windowsTextInputProps().textAttributes.maxFontSizeMultiplier;
fontSize *=
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This presumably changes the size that the text actually renders with. Is this accounted for in the layout? -- Basically I think TextLayoutManager needs to also account for this property to correctly calculate the layout of this text.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh i see. Thats part of the other PR.
Dont we need to get the font size from the layout? How do we get get the actual font size that the layout determined was appropriate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Text we reuse the same TextLayout object that was modified by TextLayoutManager. For TextInput, I think we'd have to extract the size that was calculated from the TextLayout (or otherwise get it from the TextLayoutManager), so that we can set it on the CHARFORMAT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I referred to Android / IOS implementations. Let me get back on this tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API: Completion Area: Fabric Support Facebook Fabric Area: TextInput AutoMerge Causes a PR to be automatically merged once all requirements are passed (label drives bot activity) New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric Workstream: Component Parity Close the parity gap between RNW and RN for core RN components and their supporting APIs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement maxFontSizeMultiplier property for TextInput in Fabric
4 participants