From 27bda05513e17583a2f357f385ca4d3c74843af0 Mon Sep 17 00:00:00 2001 From: sivefunc Date: Sat, 10 Oct 2026 18:36:34 -0400 Subject: [PATCH] Fix lineHeight and letterSpacing ignoring maxFontSizeMultiplier on Android TextAttributeProps capped fontSize with maxFontSizeMultiplier but converted lineHeight and letterSpacing without the cap, unlike TextAttributes (used by TextInput) and iOS. Pass the cap in both conversions, add unit tests, and add a lineHeight case to the maxFontSizeMultiplier example in RNTester. Co-Authored-By: Claude Opus 5.5 --- .../react/views/text/TextAttributeProps.kt | 5 +- .../views/text/TextAttributePropsTest.kt | 127 ++++++++++++++++++ .../js/examples/Text/TextExample.android.js | 12 ++ .../js/examples/Text/TextExample.ios.js | 12 ++ 4 files changed, 154 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.kt index 4ebaa5630c09..5c76e0838cc6 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.kt @@ -40,7 +40,8 @@ public class TextAttributeProps private constructor() { if (value == ReactConstants.UNSET.toFloat()) { Float.NaN } else { - if (allowFontScaling) toPixelFromSP(value) else toPixelFromDIP(value) + if (allowFontScaling) toPixelFromSP(value, maxFontSizeMultiplier) + else toPixelFromDIP(value) } } @@ -169,7 +170,7 @@ public class TextAttributeProps private constructor() { public var letterSpacing: Float get() { val letterSpacingPixels = - if (allowFontScaling) toPixelFromSP(letterSpacingInput) + if (allowFontScaling) toPixelFromSP(letterSpacingInput, maxFontSizeMultiplier) else toPixelFromDIP(letterSpacingInput) require(fontSize > 0) { "FontSize should be a positive value. Current value: $fontSize" } diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextAttributePropsTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextAttributePropsTest.kt index e656463b0f68..c7ea970d19be 100644 --- a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextAttributePropsTest.kt +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/views/text/TextAttributePropsTest.kt @@ -5,13 +5,18 @@ * LICENSE file in the root directory of this source tree. */ +@file:Suppress("DEPRECATION") // DisplayMetrics.scaledDensity, to simulate a system font scale + package com.facebook.react.views.text +import android.util.DisplayMetrics import android.view.Gravity import com.facebook.react.bridge.JavaOnlyMap +import com.facebook.react.common.mapbuffer.WritableMapBuffer import com.facebook.react.uimanager.DisplayMetricsHolder import com.facebook.react.uimanager.ReactStylesDiffMap import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.within import org.junit.After import org.junit.Before import org.junit.Test @@ -84,6 +89,124 @@ class TextAttributePropsTest { assertThat(textAlignment("end", isRTL = true)).isEqualTo(Gravity.LEFT) } + @Test + fun lineHeightIsCappedByMaxFontSizeMultiplier() { + setFontScale(2f) + + val textAttributes = + TextAttributeProps.fromReadableMap( + ReactStylesDiffMap( + JavaOnlyMap.of("fontSize", 14.0, "lineHeight", 20.0, "maxFontSizeMultiplier", 1.3) + ), + ) + + assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 1.3f, within(0.001f)) + } + + @Test + fun lineHeightIsCappedByMaxFontSizeMultiplierFromMapBuffer() { + setFontScale(2f) + + // Keys are iterated in ascending order, so lineHeight is set before maxFontSizeMultiplier. + val textAttributes = + TextAttributeProps.fromMapBuffer( + WritableMapBuffer() + .put(TextAttributeProps.TA_KEY_FONT_SIZE, 14.0) + .put(TextAttributeProps.TA_KEY_LINE_HEIGHT, 20.0) + .put(TextAttributeProps.TA_KEY_MAX_FONT_SIZE_MULTIPLIER, 1.3), + ) + + assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 1.3f, within(0.001f)) + } + + @Test + fun letterSpacingIsCappedByMaxFontSizeMultiplier() { + setFontScale(2f) + + val textAttributes = + TextAttributeProps.fromReadableMap( + ReactStylesDiffMap( + JavaOnlyMap.of("fontSize", 14.0, "letterSpacing", 2.0, "maxFontSizeMultiplier", 1.3) + ), + ) + + assertThat(textAttributes.fontSize).isEqualTo(37) // ceil(14 * 2 * 1.3) + assertThat(textAttributes.letterSpacing) + .isCloseTo(2f * DENSITY * 1.3f / textAttributes.fontSize, within(0.001f)) + } + + @Test + fun lineHeightScalesWithFontScaleWithoutMaxFontSizeMultiplier() { + setFontScale(2f) + + val textAttributes = + TextAttributeProps.fromReadableMap( + ReactStylesDiffMap(JavaOnlyMap.of("fontSize", 14.0, "lineHeight", 20.0)), + ) + + assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 2f, within(0.001f)) + } + + @Test + fun lineHeightIsNotCappedWhenFontScaleIsBelowMaxFontSizeMultiplier() { + setFontScale(1.2f) + + val textAttributes = + TextAttributeProps.fromReadableMap( + ReactStylesDiffMap( + JavaOnlyMap.of("fontSize", 14.0, "lineHeight", 20.0, "maxFontSizeMultiplier", 1.5) + ), + ) + + assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 1.2f, within(0.001f)) + } + + @Test + fun lineHeightIsNotCappedWhenMaxFontSizeMultiplierIsZero() { + setFontScale(2f) + + val textAttributes = + TextAttributeProps.fromReadableMap( + ReactStylesDiffMap( + JavaOnlyMap.of("fontSize", 14.0, "lineHeight", 20.0, "maxFontSizeMultiplier", 0.0) + ), + ) + + assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY * 2f, within(0.001f)) + } + + @Test + fun lineHeightIgnoresFontScaleWhenFontScalingIsDisabled() { + setFontScale(2f) + + val textAttributes = + TextAttributeProps.fromReadableMap( + ReactStylesDiffMap( + JavaOnlyMap.of( + "fontSize", + 14.0, + "lineHeight", + 20.0, + "maxFontSizeMultiplier", + 1.3, + "allowFontScaling", + false, + ) + ), + ) + + assertThat(textAttributes.lineHeight).isCloseTo(20f * DENSITY, within(0.001f)) + } + + private fun setFontScale(fontScale: Float) { + DisplayMetricsHolder.setScreenDisplayMetrics( + DisplayMetrics().apply { + density = DENSITY + scaledDensity = DENSITY * fontScale + }, + ) + } + private fun textAlignment(textAlign: String, isRTL: Boolean): Int { return TextAttributeProps.getTextAlignment( ReactStylesDiffMap(JavaOnlyMap.of("textAlign", textAlign)), @@ -91,4 +214,8 @@ class TextAttributePropsTest { Gravity.CENTER_HORIZONTAL, ) } + + private companion object { + const val DENSITY = 2f + } } diff --git a/packages/rn-tester/js/examples/Text/TextExample.android.js b/packages/rn-tester/js/examples/Text/TextExample.android.js index 39b1c94aa248..896cede7ce64 100644 --- a/packages/rn-tester/js/examples/Text/TextExample.android.js +++ b/packages/rn-tester/js/examples/Text/TextExample.android.js @@ -531,6 +531,18 @@ function MaxFontSizeMultiplierExample(props: {}): React.Node { Ignore inherited max (no max) + + lineHeight and letterSpacing are capped too (max 1x): these lines keep + the same spacing at any system font size + ); } diff --git a/packages/rn-tester/js/examples/Text/TextExample.ios.js b/packages/rn-tester/js/examples/Text/TextExample.ios.js index e367974e3da2..ff5ff29a097f 100644 --- a/packages/rn-tester/js/examples/Text/TextExample.ios.js +++ b/packages/rn-tester/js/examples/Text/TextExample.ios.js @@ -1269,6 +1269,18 @@ const examples = [ Ignore inherited max (no max) + + lineHeight and letterSpacing are capped too (max 1x): these lines + keep the same spacing at any system font size +