Skip to content
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

iOS: Add a maxFontSizeMultiplier prop to <Text> and <TextInput> #20915

Closed

Conversation

rigdern
Copy link
Contributor

@rigdern rigdern commented Aug 30, 2018

Motivation

Whenever a user changes the system font size to its maximum allowable setting, React Native apps that allow font scaling can become unusable because the text gets too big. Experimenting with a native app like iMessage on iOS, the font size used for non-body text (e.g. header, navigational elements) is capped while the body text (e.g. text in the message bubbles) is allowed to grow.

This PR introduces a new prop on <Text> and <TextInput> called maxFontSizeMultiplier. This enables devs to set the maximum allowed text scale factor on a Text/TextInput. The default is 0 which means no limit.

Another PR will add this feature to Android.

Test Plan

I created a test app which utilizes all categories of values of maxFontSizeMultiplier:

  • undefined: inherit from parent
  • 0: no limit
  • 1, 1.2: fixed limits

I tried this with Text, TextInput with value, and TextInput with children. For Text, I also verified that nesting works properly (if a child Text doesn't specify maxFontSizeMultiplier, it inherits it from its parent).

Lastly, we've been using a version of this in Skype for several months.

Release Notes

[GENERAL] [ENHANCEMENT] [Text/TextInput] - Added maxFontSizeMultiplier prop to prevent some text from getting unusably large as user increases OS's font scale setting (iOS)

Adam Comella
Microsoft Corp.

**Motivation**

Whenever a user changes the system font size to its maximum allowable setting, React Native apps that allow font scaling can become unusable because the text gets too big. Experimenting with a native app like iMessage on iOS, the font size used for non-body text (e.g. header, navigational elements) is capped while the body text (e.g. text in the message bubbles) is allowed to grow.

This PR introduces a new prop on `<Text>` and `<TextInput>` called `maxContentSizeMultiplier`. This enables devs to set the maximum allowed text scale factor on a Text/TextInput. The default is 0 which means no limit.

Another PR will add this feature to Android.

**Test Plan**

I created a test app which utilizes all categories of values of `maxContentSizeMultiplier`:
  - `undefined`: inherit from parent
  - `0`: no limit
  - `1`, `1.2`: fixed limits

I tried this with `Text`, `TextInput` with `value`, and `TextInput` with children. For `Text`, I also verified that nesting works properly (if a child `Text` doesn't specify `maxContentSizeMultiplier`, it inherits it from its parent).

Lastly, we've been using a version of this in Skype for several months.
@rigdern rigdern requested a review from shergin as a code owner August 30, 2018 01:27
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 30, 2018
@react-native-bot react-native-bot added Platform: iOS iOS applications. Component: TextInput Related to the TextInput component. Core Team labels Aug 30, 2018
@janicduplessis
Copy link
Contributor

@rigdern I think it would be better to call it maxFontSizeMultiplier for consistency with fontSizeMultiplier and allowFontScaling.

Copy link
Contributor

@shergin shergin left a comment

Choose a reason for hiding this comment

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

Thanks! ❤️

@@ -14,6 +14,9 @@
NSString *const RCTTextAttributesIsHighlightedAttributeName = @"RCTTextAttributesIsHighlightedAttributeName";
NSString *const RCTTextAttributesTagAttributeName = @"RCTTextAttributesTagAttributeName";

// Setting the default to 0 indicates that there is no max.
static CGFloat defaultMaxContentSizeMultiplier = 0.0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would inline that.

@@ -24,7 +24,6 @@ @interface RCTTextViewManager () <RCTUIManagerObserver>
@implementation RCTTextViewManager
{
NSHashTable<RCTTextShadowView *> *_shadowViews;
CGFloat _fontSizeMultiplier;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@shergin
Copy link
Contributor

shergin commented Sep 3, 2018

@rigdern Can/should we rename this to maxFontSizeMultiplier though?

@rigdern rigdern changed the title iOS: Add a maxContentSizeMultiplier prop to <Text> and <TextInput> iOS: Add a maxFontSizeMultiplier prop to <Text> and <TextInput> Sep 4, 2018
@rigdern
Copy link
Contributor Author

rigdern commented Sep 4, 2018

@shergin, @janicduplessis I renamed the prop to maxFontSizeMultiplier as suggested.

Thanks for the feedback.

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Sep 4, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

@rigdern merged commit 01d5eff into facebook:master.


Once this commit is added to a release, you will see the corresponding version tag below the description at 01d5eff. If the commit has a single master tag, it is not yet part of a release.

@facebook facebook locked as resolved and limited conversation to collaborators Sep 5, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Sep 5, 2018
gengjiawen pushed a commit to gengjiawen/react-native that referenced this pull request Sep 14, 2018
…book#20915)

Summary:
**Motivation**

Whenever a user changes the system font size to its maximum allowable setting, React Native apps that allow font scaling can become unusable because the text gets too big. Experimenting with a native app like iMessage on iOS, the font size used for non-body text (e.g. header, navigational elements) is capped while the body text (e.g. text in the message bubbles) is allowed to grow.

This PR introduces a new prop on `<Text>` and `<TextInput>` called `maxFontSizeMultiplier`. This enables devs to set the maximum allowed text scale factor on a Text/TextInput. The default is 0 which means no limit.

Another PR will add this feature to Android.

**Test Plan**

I created a test app which utilizes all categories of values of `maxFontSizeMultiplier`:
  - `undefined`: inherit from parent
  - `0`: no limit
  - `1`, `1.2`: fixed limits

I tried this with `Text`, `TextInput` with `value`, and `TextInput` with children. For `Text`, I also verified that nesting works properly (if a child `Text` doesn't specify `maxFontSizeMultiplier`, it inherits it from its parent).

Lastly, we've been using a version of this in Skype for several months.

**Release Notes**

[GENERAL] [ENHANCEMENT] [Text/TextInput] - Added maxFontSizeMultiplier prop to prevent some text from getting unusably large as user increases OS's font scale setting (iOS)

Adam Comella
Microsoft Corp.
Pull Request resolved: facebook#20915

Differential Revision: D9646739

Pulled By: shergin

fbshipit-source-id: c823f59c1e342c22d6297b88b2cb11c5a1f10310
guiccbr pushed a commit to guiccbr/react-native that referenced this pull request Jan 18, 2019
…book#20915)

Summary:
**Motivation**

Whenever a user changes the system font size to its maximum allowable setting, React Native apps that allow font scaling can become unusable because the text gets too big. Experimenting with a native app like iMessage on iOS, the font size used for non-body text (e.g. header, navigational elements) is capped while the body text (e.g. text in the message bubbles) is allowed to grow.

This PR introduces a new prop on `<Text>` and `<TextInput>` called `maxFontSizeMultiplier`. This enables devs to set the maximum allowed text scale factor on a Text/TextInput. The default is 0 which means no limit.

Another PR will add this feature to Android.

**Test Plan**

I created a test app which utilizes all categories of values of `maxFontSizeMultiplier`:
  - `undefined`: inherit from parent
  - `0`: no limit
  - `1`, `1.2`: fixed limits

I tried this with `Text`, `TextInput` with `value`, and `TextInput` with children. For `Text`, I also verified that nesting works properly (if a child `Text` doesn't specify `maxFontSizeMultiplier`, it inherits it from its parent).

Lastly, we've been using a version of this in Skype for several months.

**Release Notes**

[GENERAL] [ENHANCEMENT] [Text/TextInput] - Added maxFontSizeMultiplier prop to prevent some text from getting unusably large as user increases OS's font scale setting (iOS)

Adam Comella
Microsoft Corp.
Pull Request resolved: facebook#20915

Differential Revision: D9646739

Pulled By: shergin

fbshipit-source-id: c823f59c1e342c22d6297b88b2cb11c5a1f10310
guiccbr pushed a commit to guiccbr/react-native that referenced this pull request Jan 21, 2019
…book#20915)

Summary:
**Motivation**

Whenever a user changes the system font size to its maximum allowable setting, React Native apps that allow font scaling can become unusable because the text gets too big. Experimenting with a native app like iMessage on iOS, the font size used for non-body text (e.g. header, navigational elements) is capped while the body text (e.g. text in the message bubbles) is allowed to grow.

This PR introduces a new prop on `<Text>` and `<TextInput>` called `maxFontSizeMultiplier`. This enables devs to set the maximum allowed text scale factor on a Text/TextInput. The default is 0 which means no limit.

Another PR will add this feature to Android.

**Test Plan**

I created a test app which utilizes all categories of values of `maxFontSizeMultiplier`:
  - `undefined`: inherit from parent
  - `0`: no limit
  - `1`, `1.2`: fixed limits

I tried this with `Text`, `TextInput` with `value`, and `TextInput` with children. For `Text`, I also verified that nesting works properly (if a child `Text` doesn't specify `maxFontSizeMultiplier`, it inherits it from its parent).

Lastly, we've been using a version of this in Skype for several months.

**Release Notes**

[GENERAL] [ENHANCEMENT] [Text/TextInput] - Added maxFontSizeMultiplier prop to prevent some text from getting unusably large as user increases OS's font scale setting (iOS)

Adam Comella
Microsoft Corp.
Pull Request resolved: facebook#20915

Differential Revision: D9646739

Pulled By: shergin

fbshipit-source-id: c823f59c1e342c22d6297b88b2cb11c5a1f10310
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
…book#20915)

Summary:
**Motivation**

Whenever a user changes the system font size to its maximum allowable setting, React Native apps that allow font scaling can become unusable because the text gets too big. Experimenting with a native app like iMessage on iOS, the font size used for non-body text (e.g. header, navigational elements) is capped while the body text (e.g. text in the message bubbles) is allowed to grow.

This PR introduces a new prop on `<Text>` and `<TextInput>` called `maxFontSizeMultiplier`. This enables devs to set the maximum allowed text scale factor on a Text/TextInput. The default is 0 which means no limit.

Another PR will add this feature to Android.

**Test Plan**

I created a test app which utilizes all categories of values of `maxFontSizeMultiplier`:
  - `undefined`: inherit from parent
  - `0`: no limit
  - `1`, `1.2`: fixed limits

I tried this with `Text`, `TextInput` with `value`, and `TextInput` with children. For `Text`, I also verified that nesting works properly (if a child `Text` doesn't specify `maxFontSizeMultiplier`, it inherits it from its parent).

Lastly, we've been using a version of this in Skype for several months.

**Release Notes**

[GENERAL] [ENHANCEMENT] [Text/TextInput] - Added maxFontSizeMultiplier prop to prevent some text from getting unusably large as user increases OS's font scale setting (iOS)

Adam Comella
Microsoft Corp.
Pull Request resolved: facebook#20915

Differential Revision: D9646739

Pulled By: shergin

fbshipit-source-id: c823f59c1e342c22d6297b88b2cb11c5a1f10310
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: TextInput Related to the TextInput component. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants