[iOS] Fix minimumFontScale in New Architecture - #54072
Closed
conner1reimers wants to merge 3 commits into
Closed
conner1reimers wants to merge 3 commits into
conner1reimers wants to merge 3 commits into
Conversation
meta-cla
Bot
added
the
CLA Signed
facebook-github-bot
added
the
Shared with Meta
|
Is there any plan on resolving this in the near future? This would help us a lot 🙏 |
Contributor
Author
meta-codesync Bot
pushed a commit
that referenced
this pull request
Oct 2, 2026
…cture (#58492) Summary: `minimumFontScale` has no effect when used with `adjustsFontSizeToFit` under the New Architecture on either platform (#50248). Text shrinks all the way down to the hard-coded floor regardless of the requested scale. The root cause is the same on both platforms. Fabric's `ParagraphAttributes` carries `minimumFontSize`, but no `<Text>` or `<TextInput>` prop ever sets it, so it is always `NaN`: - **iOS**: `RCTTextLayoutManager` falls back to a 4pt minimum whenever `minimumFontSize` is `NaN`, so the floor is always 4pt. - **Android**: the C++ side serializes `minimumFontSize` into MapBuffer key 6, and `TextLayoutManager.adjustSpannableFontToFit()` treats that value as an absolute minimum font size, falling back to 4dp when it is `NaN`. Same result: the floor is always 4dp. This PR serializes `minimumFontScale` (the field already existed) into a new MapBuffer key (10). Each platform then derives the minimum font size from that scale. `minimumFontSize` and MapBuffer key 6 are left as they are and deprecated in favor of `minimumFontScale`; an explicit `minimumFontSize` still takes precedence over the scale on both platforms, so existing callers keep working unchanged. The legacy renderers based the scale on the outermost text's font size; using the largest font size in the attributed string follows the approach from #43543. - **iOS**: `RCTTextLayoutManager` finds the largest font size in the attributed string and computes the minimum as `MAX(minimumFontScale * largestFontSize, 4.0)`. - **Android**: `adjustSpannableFontToFit()` finds the largest `ReactAbsoluteSizeSpan` first and uses `max(minimumFontScale * largestFontSize, 4dp)`. This uses the same scale-based calculation as the original Android implementation in #26389, although the legacy renderer based it on the outer text's effective font size rather than the largest span. A `NaN` or non-positive scale keeps the bare 4dp floor. Both the measurement path (`createLayout()`) and the view path (`ReactTextViewManager` -> `ReactTextView.onDraw()`) go through this one function, so only its interpretation of the value changes. `PA_KEY_MINIMUM_FONT_SCALE` is added as key 10, `ReactTextView.setMinimumFontSize()` is kept and marked `Deprecated`, and `setMinimumFontScale()` is added alongside it (the `ReactAndroid.api` dump was also updated). The 4pt/4dp floor is kept on both platforms. `minimumFontScale` still has an `platform ios` annotation in `TextProps.js`, so that annotation is removed since it's supported on Android as well. On Android, `ReactTextView.updateView()` still clears the ellipsize location whenever `adjustsFontSizeToFit` is set. Changing that is split out into #58749 as requested [here](#58492 (comment)). Builds on #58529, which removed the unused `maximumFontSize` attribute and introduced the largest-font-size helper this PR reuses on iOS. This supersedes #54072 (iOS only) and #43543. Fixes #50248. ## Changelog: [GENERAL] [FIXED] - Fix `minimumFontScale` with `adjustsFontSizeToFit` in the New Architecture Pull Request resolved: #58492 Test Plan: **Unit tests** - Updated `ParagraphAttributesTest.cpp` to cover `minimumFontScale`. - Added `TextLayoutManagerMinimumFontScaleTest.kt` covering the scale calculation, the 4dp floor, the precedence of an explicit `minimumFontSize` over the scale, and text that already fits. - Regenerated the C++ API snapshots (`yarn cxx-api-build`) for the new MapBuffer key. ```sh ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest --tests 'com.facebook.react.views.text.*' ``` All 46 `com.facebook.react.views.text` tests pass. **RNTester** Added a `minimumFontScale={0.5}` line to the `adjustsFontSizeToFit` section of the Text example on both platforms ("Can limit how small the text becomes with minimumFontScale"). Before this change the line shrinks past the requested floor; after it stops at 50% of the original font size. On iOS the overflow is ellipsized. On Android it is clipped, because `updateView()` still drops the ellipsize location with `adjustsFontSizeToFit` (see #58749). Each screenshot below was captured on the same machine, on an iPhone 17 simulator and an Android API 35 emulator. "Before" is this branch's base commit with only the new RNTester example applied. | | Before | After | | - | - | - | | iOS | <img width="465" height="446" alt="iPhone 17 2026-09-11 at 2 12 45 PM" src="https://github.com/user-attachments/assets/88a99587-de23-4751-b449-34f9ecfffedd" /> | <img width="464" height="465" alt="iPhone 17 2026-09-11 at 1 54 28 PM" src="https://github.com/user-attachments/assets/40251f9f-73c9-4b87-89fc-b836bd5c7c6b" /> | | Android | <img width="453" height="505" alt="Android Emulator - Pixel_8_API_355554 2026-09-11 at 2 12 26 PM" src="https://github.com/user-attachments/assets/2e7a466a-149b-4f4f-a1be-275d7032ed27" /> | <img width="406" height="457" alt="Android Emulator - Pixel_8_API_355554 2026-09-29 at 5 05 41 PM" src="https://github.com/user-attachments/assets/3b102bba-d027-4651-9d72-1bd124499893" /> | Reviewed By: christophpurrer Differential Revision: D120667665 Pulled By: javache fbshipit-source-id: b44f60fd14767651b1c212acf3e945962865123c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
This PR is meant to replace #43543 and attempts to address the issue of
minimumFontScalenot being respected on iOS when used withadjustsFontSizeToFit.When using
adjustsFontSizeToFiton Fabric, the behavior differs from the old renderer, asminimumFontScaleis not respected. The code currently usesminimumFontSizeandmaximumFontSize, but those props do not actually exist. This brings the behavior closer to the old renderer by removing the unused props and implementingminimumFontScaleinstead.From #43543:
Changelog:
[IOS] [FIXED] - Fix
minimumFontScaleon iOS FabricTest Plan:
This adds one example of the new iOS behavior in RNTester. You can see that, on iOS, the line "Can limit how small the text becomes with minimumFontScale" does not respect the
minimumFontScaleprop in the before screenshot, but does in the after screenshot: