Skip to content

Commit

Permalink
Fix maxFontSizeMultiplier prop on Text and TextInput components…
Browse files Browse the repository at this point in the history
… in new architecture (#47614)

Summary:
The `maxFontSizeMultiplier` prop for `Text` and `TextInput` was not handled in Fabric / New Architecture as documented in #47499.

bypass-github-export-checks

[GENERAL] [FIXED] - Fix `maxFontSizeMultiplier` prop on `Text` and `TextInput` components in Fabric / New Architecture

Pull Request resolved: #47614

Test Plan:
I have not added any automated tests for this change but try to do so if requested. I have however added examples to RN Tester for both the Text and TextInput components, as well as compared the behaviour with Paper / Old Architecture. Both on version 0.76.

Noticed now I didn't do exactly the same steps in both videos, oops! Be aware that reapplying changes made in the Settings are currently half-broken on the new architecture, thus I'm restarting the app on Android and iOS. But this issue is unrelated to my changes. I've tested on main branch and it has the same issue.

Here are comparison videos between Paper and Fabric on iOS *after* I've made my fix.

| Paper  | Fabric |
| ------------- | ------------- |
| <video src="https://github.com/user-attachments/assets/f4fd009f-aa6d-41ab-92fa-8dcf1e351ba1" /> | <video src="https://github.com/user-attachments/assets/fda42cc6-34c2-42a7-a6e2-028e7c866075" /> |

| Paper  | Fabric |
| ------------- | ------------- |
| <video src="https://github.com/user-attachments/assets/59b59f7b-25d2-4b5b-a8e2-d2054cc6390b" /> | <video src="https://github.com/user-attachments/assets/72068566-8f2a-4463-874c-45a6f5b63b0d" /> |

Reviewed By: Abbondanzo

Differential Revision: D65953019

Pulled By: cipolleschi

fbshipit-source-id: 90c3c7e236229e9ad9bd346941fafe4af8a9d9fc
  • Loading branch information
RickardZrinski authored and robhogan committed Feb 11, 2025
1 parent 877e82c commit 8baa858
Show file tree
Hide file tree
Showing 11 changed files with 274 additions and 5 deletions.
2 changes: 2 additions & 0 deletions packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -7443,6 +7443,7 @@ public class com/facebook/react/views/text/TextAttributeProps {
public static final field TA_KEY_LETTER_SPACING S
public static final field TA_KEY_LINE_BREAK_STRATEGY S
public static final field TA_KEY_LINE_HEIGHT S
public static final field TA_KEY_MAX_FONT_SIZE_MULTIPLIER S
public static final field TA_KEY_OPACITY S
public static final field TA_KEY_ROLE S
public static final field TA_KEY_TEXT_DECORATION_COLOR S
Expand Down Expand Up @@ -7475,6 +7476,7 @@ public class com/facebook/react/views/text/TextAttributeProps {
protected field mLetterSpacingInput F
protected field mLineHeight F
protected field mLineHeightInput F
protected field mMaxFontSizeMultiplier F
protected field mNumberOfLines I
protected field mOpacity F
protected field mRole Lcom/facebook/react/uimanager/ReactAccessibilityDelegate$Role;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public class TextAttributeProps {
public static final short TA_KEY_LINE_BREAK_STRATEGY = 25;
public static final short TA_KEY_ROLE = 26;
public static final short TA_KEY_TEXT_TRANSFORM = 27;
public static final short TA_KEY_MAX_FONT_SIZE_MULTIPLIER = 29;

public static final int UNSET = -1;

Expand All @@ -81,6 +82,7 @@ public class TextAttributeProps {
protected float mLineHeight = Float.NaN;
protected boolean mIsColorSet = false;
protected boolean mAllowFontScaling = true;
protected float mMaxFontSizeMultiplier = Float.NaN;
protected int mColor;
protected boolean mIsBackgroundColorSet = false;
protected int mBackgroundColor;
Expand Down Expand Up @@ -227,6 +229,9 @@ public static TextAttributeProps fromMapBuffer(MapBuffer props) {
case TA_KEY_TEXT_TRANSFORM:
result.setTextTransform(entry.getStringValue());
break;
case TA_KEY_MAX_FONT_SIZE_MULTIPLIER:
result.setMaxFontSizeMultiplier((float) entry.getDoubleValue());
break;
}
}

Expand All @@ -243,6 +248,8 @@ public static TextAttributeProps fromReadableMap(ReactStylesDiffMap props) {
result.setLineHeight(getFloatProp(props, ViewProps.LINE_HEIGHT, ReactConstants.UNSET));
result.setLetterSpacing(getFloatProp(props, ViewProps.LETTER_SPACING, Float.NaN));
result.setAllowFontScaling(getBooleanProp(props, ViewProps.ALLOW_FONT_SCALING, true));
result.setMaxFontSizeMultiplier(
getFloatProp(props, ViewProps.MAX_FONT_SIZE_MULTIPLIER, Float.NaN));
result.setFontSize(getFloatProp(props, ViewProps.FONT_SIZE, ReactConstants.UNSET));
result.setColor(props.hasKey(ViewProps.COLOR) ? props.getInt(ViewProps.COLOR, 0) : null);
result.setColor(
Expand Down Expand Up @@ -411,7 +418,14 @@ private void setAllowFontScaling(boolean allowFontScaling) {
mAllowFontScaling = allowFontScaling;
setFontSize(mFontSizeInput);
setLineHeight(mLineHeightInput);
setLetterSpacing(mLetterSpacingInput);
}
}

private void setMaxFontSizeMultiplier(float maxFontSizeMultiplier) {
if (maxFontSizeMultiplier != mMaxFontSizeMultiplier) {
mMaxFontSizeMultiplier = maxFontSizeMultiplier;
setFontSize(mFontSizeInput);
setLineHeight(mLineHeightInput);
}
}

Expand All @@ -420,7 +434,7 @@ private void setFontSize(float fontSize) {
if (fontSize != ReactConstants.UNSET) {
fontSize =
mAllowFontScaling
? (float) Math.ceil(PixelUtil.toPixelFromSP(fontSize))
? (float) Math.ceil(PixelUtil.toPixelFromSP(fontSize, mMaxFontSizeMultiplier))
: (float) Math.ceil(PixelUtil.toPixelFromDIP(fontSize));
}
mFontSize = (int) fontSize;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ void TextAttributes::apply(TextAttributes textAttributes) {
allowFontScaling = textAttributes.allowFontScaling.has_value()
? textAttributes.allowFontScaling
: allowFontScaling;
maxFontSizeMultiplier = !std::isnan(textAttributes.maxFontSizeMultiplier)
? textAttributes.maxFontSizeMultiplier
: maxFontSizeMultiplier;
dynamicTypeRamp = textAttributes.dynamicTypeRamp.has_value()
? textAttributes.dynamicTypeRamp
: dynamicTypeRamp;
Expand Down Expand Up @@ -168,6 +171,7 @@ bool TextAttributes::operator==(const TextAttributes& rhs) const {
rhs.accessibilityRole,
rhs.role,
rhs.textTransform) &&
floatEquality(maxFontSizeMultiplier, rhs.maxFontSizeMultiplier) &&
floatEquality(opacity, rhs.opacity) &&
floatEquality(fontSize, rhs.fontSize) &&
floatEquality(fontSizeMultiplier, rhs.fontSizeMultiplier) &&
Expand Down Expand Up @@ -211,6 +215,8 @@ SharedDebugStringConvertibleList TextAttributes::getDebugProps() const {
debugStringConvertibleItem("fontStyle", fontStyle),
debugStringConvertibleItem("fontVariant", fontVariant),
debugStringConvertibleItem("allowFontScaling", allowFontScaling),
debugStringConvertibleItem(
"maxFontSizeMultiplier", maxFontSizeMultiplier),
debugStringConvertibleItem("dynamicTypeRamp", dynamicTypeRamp),
debugStringConvertibleItem("letterSpacing", letterSpacing),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class TextAttributes : public DebugStringConvertible {
std::optional<FontStyle> fontStyle{};
std::optional<FontVariant> fontVariant{};
std::optional<bool> allowFontScaling{};
Float maxFontSizeMultiplier{std::numeric_limits<Float>::quiet_NaN()};
std::optional<DynamicTypeRamp> dynamicTypeRamp{};
Float letterSpacing{std::numeric_limits<Float>::quiet_NaN()};
std::optional<TextTransform> textTransform{};
Expand Down Expand Up @@ -117,6 +118,7 @@ struct hash<facebook::react::TextAttributes> {
textAttributes.opacity,
textAttributes.fontFamily,
textAttributes.fontSize,
textAttributes.maxFontSizeMultiplier,
textAttributes.fontSizeMultiplier,
textAttributes.fontWeight,
textAttributes.fontStyle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,7 @@ constexpr static MapBuffer::Key TA_KEY_LINE_BREAK_STRATEGY = 25;
constexpr static MapBuffer::Key TA_KEY_ROLE = 26;
constexpr static MapBuffer::Key TA_KEY_TEXT_TRANSFORM = 27;
constexpr static MapBuffer::Key TA_KEY_ALIGNMENT_VERTICAL = 28;
constexpr static MapBuffer::Key TA_KEY_MAX_FONT_SIZE_MULTIPLIER = 29;

// constants for ParagraphAttributes serialization
constexpr static MapBuffer::Key PA_KEY_MAX_NUMBER_OF_LINES = 0;
Expand Down Expand Up @@ -1004,6 +1005,10 @@ inline MapBuffer toMapBuffer(const TextAttributes& textAttributes) {
builder.putBool(
TA_KEY_ALLOW_FONT_SCALING, *textAttributes.allowFontScaling);
}
if (!std::isnan(textAttributes.maxFontSizeMultiplier)) {
builder.putDouble(
TA_KEY_MAX_FONT_SIZE_MULTIPLIER, textAttributes.maxFontSizeMultiplier);
}
if (!std::isnan(textAttributes.letterSpacing)) {
builder.putDouble(TA_KEY_LETTER_SPACING, textAttributes.letterSpacing);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ static TextAttributes convertRawProp(
"allowFontScaling",
sourceTextAttributes.allowFontScaling,
defaultTextAttributes.allowFontScaling);
textAttributes.maxFontSizeMultiplier = convertRawProp(
context,
rawProps,
"maxFontSizeMultiplier",
sourceTextAttributes.maxFontSizeMultiplier,
defaultTextAttributes.maxFontSizeMultiplier);
textAttributes.dynamicTypeRamp = convertRawProp(
context,
rawProps,
Expand Down Expand Up @@ -266,6 +272,12 @@ void BaseTextProps::setProp(
defaults, value, textAttributes, fontVariant, "fontVariant");
REBUILD_FIELD_SWITCH_CASE(
defaults, value, textAttributes, allowFontScaling, "allowFontScaling");
REBUILD_FIELD_SWITCH_CASE(
defaults,
value,
textAttributes,
maxFontSizeMultiplier,
"maxFontSizeMultiplier");
REBUILD_FIELD_SWITCH_CASE(
defaults, value, textAttributes, letterSpacing, "letterSpacing");
REBUILD_FIELD_SWITCH_CASE(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,17 +135,19 @@ inline static CGFloat RCTBaseSizeForDynamicTypeRamp(const DynamicTypeRamp &dynam
inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(const TextAttributes &textAttributes)
{
if (textAttributes.allowFontScaling.value_or(true)) {
CGFloat fontSizeMultiplier = !isnan(textAttributes.fontSizeMultiplier) ? textAttributes.fontSizeMultiplier : 1.0;
if (textAttributes.dynamicTypeRamp.has_value()) {
DynamicTypeRamp dynamicTypeRamp = textAttributes.dynamicTypeRamp.value();
UIFontMetrics *fontMetrics =
[UIFontMetrics metricsForTextStyle:RCTUIFontTextStyleForDynamicTypeRamp(dynamicTypeRamp)];
// Using a specific font size reduces rounding errors from -scaledValueForValue:
CGFloat requestedSize =
isnan(textAttributes.fontSize) ? RCTBaseSizeForDynamicTypeRamp(dynamicTypeRamp) : textAttributes.fontSize;
return [fontMetrics scaledValueForValue:requestedSize] / requestedSize;
} else {
return textAttributes.fontSizeMultiplier;
fontSizeMultiplier = [fontMetrics scaledValueForValue:requestedSize] / requestedSize;
}
CGFloat maxFontSizeMultiplier =
!isnan(textAttributes.maxFontSizeMultiplier) ? textAttributes.maxFontSizeMultiplier : 0.0;
return maxFontSizeMultiplier >= 1.0 ? fminf(maxFontSizeMultiplier, fontSizeMultiplier) : fontSizeMultiplier;
} else {
return 1.0;
}
Expand Down
38 changes: 38 additions & 0 deletions packages/rn-tester/js/examples/Text/TextExample.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,37 @@ function AllowFontScalingExample(props: {}): React.Node {
);
}

function MaxFontSizeMultiplierExample(props: {}): React.Node {
return (
<View testID={'max-font-size-multiplier'}>
<Text>
When allowFontScaling is enabled, you can use the maxFontSizeMultiplier
prop to set an upper limit on how much the font size will be scaled.
</Text>
<Text
allowFontScaling={true}
maxFontSizeMultiplier={1}
style={{marginTop: 10}}>
This text will not scale up (max 1x)
</Text>
<Text allowFontScaling={true} maxFontSizeMultiplier={1.5}>
This text will scale up (max 1.5x)
</Text>
<Text allowFontScaling={true} maxFontSizeMultiplier={1}>
<Text>Inherit max (max 1x)</Text>
</Text>
<Text allowFontScaling={true} maxFontSizeMultiplier={1}>
<Text maxFontSizeMultiplier={1.5}>
Override inherited max (max 1.5x)
</Text>
</Text>
<Text allowFontScaling={true} maxFontSizeMultiplier={1}>
<Text maxFontSizeMultiplier={0}>Ignore inherited max (no max)</Text>
</Text>
</View>
);
}

function NumberOfLinesExample(props: {}): React.Node {
return (
<>
Expand Down Expand Up @@ -1370,6 +1401,13 @@ const examples = [
return <AllowFontScalingExample />;
},
},
{
title: 'maxFontSizeMultiplier attribute',
name: 'maxFontSizeMultiplier',
render(): React.Node {
return <MaxFontSizeMultiplierExample />;
},
},
{
title: 'selectable attribute',
name: 'selectable',
Expand Down
80 changes: 80 additions & 0 deletions packages/rn-tester/js/examples/Text/TextExample.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -1127,6 +1127,62 @@ const examples = [
);
},
},
{
title: 'maxFontSizeMultiplier attribute',
name: 'maxFontSizeMultiplier',
render(): React.Node {
return (
<View testID={'max-font-size-multiplier'}>
<Text>
When allowFontScaling is enabled, you can use the
maxFontSizeMultiplier prop to set an upper limit on how much the
font size will be scaled.
</Text>
<Text
allowFontScaling={true}
maxFontSizeMultiplier={1}
style={{marginTop: 10}}>
This text will not scale up (max 1x)
</Text>
<Text allowFontScaling={true} maxFontSizeMultiplier={1.5}>
This text will scale up (max 1.5x)
</Text>
<Text allowFontScaling={true} maxFontSizeMultiplier={1}>
<Text>Inherit max (max 1x)</Text>
</Text>
<Text allowFontScaling={true} maxFontSizeMultiplier={1}>
<Text maxFontSizeMultiplier={1.5}>
Override inherited max (max 1.5x)
</Text>
</Text>
<Text allowFontScaling={true} maxFontSizeMultiplier={1}>
<Text maxFontSizeMultiplier={0}>Ignore inherited max (no max)</Text>
</Text>
<Text
allowFontScaling={true}
style={{fontSize: 22}}
dynamicTypeRamp="title2">
This text will scale with 'title2' dynamic type ramp (no max)
</Text>
<Text
allowFontScaling={true}
style={{fontSize: 22}}
dynamicTypeRamp="title2"
maxFontSizeMultiplier={1.2}>
This text will scale with 'title2' dynamic type ramp (max 1.2x)
</Text>
<Text
allowFontScaling={true}
style={{fontSize: 22}}
dynamicTypeRamp="title2"
maxFontSizeMultiplier={1}>
This text uses 'title2' dynamic type ramp but will not scale up (max
1x)
</Text>
</View>
);
},
},
{
title: 'Inline views',
render: (): React.Node => <TextInlineView.Basic />,
Expand Down Expand Up @@ -1392,17 +1448,41 @@ const examples = [
<Text style={{fontSize: 20}} dynamicTypeRamp="title3">
Title 3
</Text>
<Text style={{fontSize: 17}} dynamicTypeRamp="headline">
Headline
</Text>
<Text style={{fontSize: 17}} dynamicTypeRamp="body">
Body
</Text>
<Text style={{fontSize: 16}} dynamicTypeRamp="callout">
Callout
</Text>
<Text style={{fontSize: 15}} dynamicTypeRamp="subheadline">
Subheadline
</Text>
<Text style={{fontSize: 13}} dynamicTypeRamp="footnote">
Footnote
</Text>
<Text style={{fontSize: 12}} dynamicTypeRamp="caption1">
Caption
</Text>
<Text style={{fontSize: 11}} dynamicTypeRamp="caption2">
Caption 2
</Text>
</View>
<View style={boxStyle}>
<Text style={boldStyle}>Without `dynamicTypeRamp`:</Text>
<Text style={{fontSize: 34}}>Large Title</Text>
<Text style={{fontSize: 28}}>Title</Text>
<Text style={{fontSize: 22}}>Title 2</Text>
<Text style={{fontSize: 20}}>Title 3</Text>
<Text style={{fontSize: 17}}>Headline</Text>
<Text style={{fontSize: 17}}>Body</Text>
<Text style={{fontSize: 16}}>Callout</Text>
<Text style={{fontSize: 15}}>Subheadline</Text>
<Text style={{fontSize: 13}}>Footnote</Text>
<Text style={{fontSize: 12}}>Caption</Text>
<Text style={{fontSize: 11}}>Caption 2</Text>
</View>
</View>
);
Expand Down
Loading

0 comments on commit 8baa858

Please sign in to comment.