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

fix: Improve text line height calculation to properly align text on iOS #46884

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 40 additions & 2 deletions packages/react-native/Libraries/Text/Text/RCTTextView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#import <React/RCTUtils.h>
#import <React/UIView+React.h>
#import <react/featureflags/ReactNativeFeatureFlags.h>

#import <React/RCTTextShadowView.h>

Expand Down Expand Up @@ -98,6 +99,36 @@ - (void)setTextStorage:(NSTextStorage *)textStorage
[self setNeedsDisplay];
}

- (CGPoint)calculateDrawingPointWithTextStorage:(NSTextStorage *)textStorage
contentFrame:(CGRect)contentFrame {
UIFont *font = [textStorage attribute:NSFontAttributeName atIndex:0 effectiveRange:NULL];
if (!font) {
font = [UIFont systemFontOfSize:14];
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

I couldn’t find systemFontSize used anywhere else in the repo, and the default font size of 14 has been hardcoded. Also, since systemFontSize is not available on tvOS, I’m not sure if it would be a suitable replacement. Let me know your thoughts on whether we should stick with the hardcoded value for consistency or if there’s a preferred approach.

}

NSParagraphStyle *paragraphStyle = [textStorage attribute:NSParagraphStyleAttributeName atIndex:0 effectiveRange:NULL];

CGFloat lineHeight = font.lineHeight;
if (paragraphStyle && paragraphStyle.minimumLineHeight > 0) {
lineHeight = paragraphStyle.minimumLineHeight;
}

CGFloat ascent = font.ascender;
CGFloat descent = fabs(font.descender);
CGFloat textHeight = ascent + descent;

CGFloat verticalOffset = 0;
if (textHeight > lineHeight) {
CGFloat difference = textHeight - lineHeight;
verticalOffset = difference / 2.0;
} else if (textHeight < lineHeight) {
CGFloat difference = lineHeight - textHeight;
verticalOffset = -(difference / 2.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 think comment explaining the calculations might be useful

Copy link
Author

Choose a reason for hiding this comment

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

Sure, added comments


return CGPointMake(contentFrame.origin.x, contentFrame.origin.y + verticalOffset);
}

- (void)drawRect:(CGRect)rect
{
[super drawRect:rect];
Expand All @@ -118,8 +149,15 @@ - (void)drawRect:(CGRect)rect
#endif

NSRange glyphRange = [layoutManager glyphRangeForTextContainer:textContainer];
[layoutManager drawBackgroundForGlyphRange:glyphRange atPoint:_contentFrame.origin];
[layoutManager drawGlyphsForGlyphRange:glyphRange atPoint:_contentFrame.origin];

if (ReactNativeFeatureFlags::enableLineHeightCentering()) {
CGPoint drawingPoint = [self calculateDrawingPointWithTextStorage:_textStorage contentFrame:_contentFrame];
[layoutManager drawBackgroundForGlyphRange:glyphRange atPoint:drawingPoint];
[layoutManager drawGlyphsForGlyphRange:glyphRange atPoint:drawingPoint];
} else {
[layoutManager drawBackgroundForGlyphRange:glyphRange atPoint:_contentFrame.origin];
[layoutManager drawGlyphsForGlyphRange:glyphRange atPoint:_contentFrame.origin];
}

__block UIBezierPath *highlightPath = nil;
NSRange characterRange = [layoutManager characterRangeForGlyphRange:glyphRange actualGlyphRange:NULL];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#import "RCTParagraphComponentAccessibilityProvider.h"

#import <MobileCoreServices/UTCoreTypes.h>
#import <react/featureflags/ReactNativeFeatureFlags.h>
#import <react/renderer/components/text/ParagraphComponentDescriptor.h>
#import <react/renderer/components/text/ParagraphProps.h>
#import <react/renderer/components/text/ParagraphState.h>
Expand Down Expand Up @@ -326,6 +327,38 @@ @implementation RCTParagraphTextView {
CAShapeLayer *_highlightLayer;
}

- (CGRect)calculateCenteredFrameWithAttributedText:(NSAttributedString *)attributedText
frame:(CGRect)frame {
UIFont *font = [attributedText attribute:NSFontAttributeName atIndex:0 effectiveRange:NULL];
if (!font) {
font = [UIFont systemFontOfSize:14];
}

NSParagraphStyle *paragraphStyle = [attributedText attribute:NSParagraphStyleAttributeName atIndex:0 effectiveRange:NULL];
CGFloat lineHeight = font.lineHeight;

if (paragraphStyle && paragraphStyle.minimumLineHeight > 0) {
lineHeight = paragraphStyle.minimumLineHeight;
}

CGFloat ascent = font.ascender;
CGFloat descent = fabs(font.descender);
CGFloat textHeight = ascent + descent;

CGFloat verticalOffset = 0;
if (textHeight > lineHeight) {
CGFloat difference = textHeight - lineHeight;
verticalOffset = difference / 2.0;
} else if (textHeight < lineHeight) {
CGFloat difference = lineHeight - textHeight;
verticalOffset = -(difference / 2.0);
}

frame.origin.y += verticalOffset;

return frame;
}

- (void)drawRect:(CGRect)rect
{
if (!_state) {
Expand All @@ -343,6 +376,11 @@ - (void)drawRect:(CGRect)rect

CGRect frame = RCTCGRectFromRect(_layoutMetrics.getContentFrame());

if (ReactNativeFeatureFlags::enableLineHeightCentering()) {
NSAttributedString *attributedText = RCTNSAttributedStringFromAttributedString(_state->getData().attributedString);
frame = [self calculateCenteredFrameWithAttributedText:attributedText frame:frame];
}

[nativeTextLayoutManager drawAttributedString:_state->getData().attributedString
paragraphAttributes:_paragraphAttributes
frame:frame
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<7d80322a6a37083c5e52e6914de49ce2>>
* @generated SignedSource<<ac76caa581b2c41bc95ed973bbd27b99>>
*/

/**
Expand Down Expand Up @@ -58,12 +58,6 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun enableAlignItemsBaselineOnFabricIOS(): Boolean = accessor.enableAlignItemsBaselineOnFabricIOS()

/**
* When enabled, custom line height calculation will be centered from top to bottom.
*/
@JvmStatic
public fun enableAndroidLineHeightCentering(): Boolean = accessor.enableAndroidLineHeightCentering()

/**
* Feature flag to enable the new bridgeless architecture. Note: Enabling this will force enable the following flags: `useTurboModules` & `enableFabricRenderer.
*/
Expand Down Expand Up @@ -130,6 +124,12 @@ public object ReactNativeFeatureFlags {
@JvmStatic
public fun enableLayoutAnimationsOnIOS(): Boolean = accessor.enableLayoutAnimationsOnIOS()

/**
* When enabled, custom line height calculation will be centered from top to bottom.
*/
@JvmStatic
public fun enableLineHeightCentering(): Boolean = accessor.enableLineHeightCentering()

/**
* Enables the reporting of long tasks through `PerformanceObserver`. Only works if the event loop is enabled.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<761d3e7b100a4f5ee6f8bda71f84918b>>
* @generated SignedSource<<ee55d752b258637159b99ba75e217395>>
*/

/**
Expand All @@ -25,7 +25,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
private var batchRenderingUpdatesInEventLoopCache: Boolean? = null
private var completeReactInstanceCreationOnBgThreadOnAndroidCache: Boolean? = null
private var enableAlignItemsBaselineOnFabricIOSCache: Boolean? = null
private var enableAndroidLineHeightCenteringCache: Boolean? = null
private var enableBridgelessArchitectureCache: Boolean? = null
private var enableCleanTextInputYogaNodeCache: Boolean? = null
private var enableDeletionOfUnmountedViewsCache: Boolean? = null
Expand All @@ -37,6 +36,7 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
private var enableGranularShadowTreeStateReconciliationCache: Boolean? = null
private var enableIOSViewClipToPaddingBoxCache: Boolean? = null
private var enableLayoutAnimationsOnIOSCache: Boolean? = null
private var enableLineHeightCenteringCache: Boolean? = null
private var enableLongTaskAPICache: Boolean? = null
private var enableMicrotasksCache: Boolean? = null
private var enablePreciseSchedulingForPremountItemsOnAndroidCache: Boolean? = null
Expand Down Expand Up @@ -114,15 +114,6 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

override fun enableAndroidLineHeightCentering(): Boolean {
var cached = enableAndroidLineHeightCenteringCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.enableAndroidLineHeightCentering()
enableAndroidLineHeightCenteringCache = cached
}
return cached
}

override fun enableBridgelessArchitecture(): Boolean {
var cached = enableBridgelessArchitectureCache
if (cached == null) {
Expand Down Expand Up @@ -222,6 +213,15 @@ public class ReactNativeFeatureFlagsCxxAccessor : ReactNativeFeatureFlagsAccesso
return cached
}

override fun enableLineHeightCentering(): Boolean {
var cached = enableLineHeightCenteringCache
if (cached == null) {
cached = ReactNativeFeatureFlagsCxxInterop.enableLineHeightCentering()
enableLineHeightCenteringCache = cached
}
return cached
}

override fun enableLongTaskAPI(): Boolean {
var cached = enableLongTaskAPICache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<1ed46e0bf712406c1bf6159e1bca3c15>>
* @generated SignedSource<<eb10b9718add54a31ee41200c8b48076>>
*/

/**
Expand Down Expand Up @@ -38,8 +38,6 @@ public object ReactNativeFeatureFlagsCxxInterop {

@DoNotStrip @JvmStatic public external fun enableAlignItemsBaselineOnFabricIOS(): Boolean

@DoNotStrip @JvmStatic public external fun enableAndroidLineHeightCentering(): Boolean

@DoNotStrip @JvmStatic public external fun enableBridgelessArchitecture(): Boolean

@DoNotStrip @JvmStatic public external fun enableCleanTextInputYogaNode(): Boolean
Expand All @@ -62,6 +60,8 @@ public object ReactNativeFeatureFlagsCxxInterop {

@DoNotStrip @JvmStatic public external fun enableLayoutAnimationsOnIOS(): Boolean

@DoNotStrip @JvmStatic public external fun enableLineHeightCentering(): Boolean

@DoNotStrip @JvmStatic public external fun enableLongTaskAPI(): Boolean

@DoNotStrip @JvmStatic public external fun enableMicrotasks(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<19cf402242ebd8b3a08dfb7c755b801b>>
* @generated SignedSource<<3c8abaac37fc7644f7ace96919390074>>
*/

/**
Expand Down Expand Up @@ -33,8 +33,6 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun enableAlignItemsBaselineOnFabricIOS(): Boolean = true

override fun enableAndroidLineHeightCentering(): Boolean = false

override fun enableBridgelessArchitecture(): Boolean = false

override fun enableCleanTextInputYogaNode(): Boolean = false
Expand All @@ -57,6 +55,8 @@ public open class ReactNativeFeatureFlagsDefaults : ReactNativeFeatureFlagsProvi

override fun enableLayoutAnimationsOnIOS(): Boolean = true

override fun enableLineHeightCentering(): Boolean = false

override fun enableLongTaskAPI(): Boolean = false

override fun enableMicrotasks(): Boolean = false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<a307a1f9b5064547ce32f9519bbf27c4>>
* @generated SignedSource<<f726bd566cebccbd10dac4ec6d2b974d>>
*/

/**
Expand All @@ -29,7 +29,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private var batchRenderingUpdatesInEventLoopCache: Boolean? = null
private var completeReactInstanceCreationOnBgThreadOnAndroidCache: Boolean? = null
private var enableAlignItemsBaselineOnFabricIOSCache: Boolean? = null
private var enableAndroidLineHeightCenteringCache: Boolean? = null
private var enableBridgelessArchitectureCache: Boolean? = null
private var enableCleanTextInputYogaNodeCache: Boolean? = null
private var enableDeletionOfUnmountedViewsCache: Boolean? = null
Expand All @@ -41,6 +40,7 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
private var enableGranularShadowTreeStateReconciliationCache: Boolean? = null
private var enableIOSViewClipToPaddingBoxCache: Boolean? = null
private var enableLayoutAnimationsOnIOSCache: Boolean? = null
private var enableLineHeightCenteringCache: Boolean? = null
private var enableLongTaskAPICache: Boolean? = null
private var enableMicrotasksCache: Boolean? = null
private var enablePreciseSchedulingForPremountItemsOnAndroidCache: Boolean? = null
Expand Down Expand Up @@ -123,16 +123,6 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

override fun enableAndroidLineHeightCentering(): Boolean {
var cached = enableAndroidLineHeightCenteringCache
if (cached == null) {
cached = currentProvider.enableAndroidLineHeightCentering()
accessedFeatureFlags.add("enableAndroidLineHeightCentering")
enableAndroidLineHeightCenteringCache = cached
}
return cached
}

override fun enableBridgelessArchitecture(): Boolean {
var cached = enableBridgelessArchitectureCache
if (cached == null) {
Expand Down Expand Up @@ -243,6 +233,16 @@ public class ReactNativeFeatureFlagsLocalAccessor : ReactNativeFeatureFlagsAcces
return cached
}

override fun enableLineHeightCentering(): Boolean {
var cached = enableLineHeightCenteringCache
if (cached == null) {
cached = currentProvider.enableLineHeightCentering()
accessedFeatureFlags.add("enableLineHeightCentering")
enableLineHeightCenteringCache = cached
}
return cached
}

override fun enableLongTaskAPI(): Boolean {
var cached = enableLongTaskAPICache
if (cached == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<bda9e94a5fe61a16e7d001ea3acfed0c>>
* @generated SignedSource<<73688196634e93c5b6ea63181e13f738>>
*/

/**
Expand Down Expand Up @@ -33,8 +33,6 @@ public interface ReactNativeFeatureFlagsProvider {

@DoNotStrip public fun enableAlignItemsBaselineOnFabricIOS(): Boolean

@DoNotStrip public fun enableAndroidLineHeightCentering(): Boolean

@DoNotStrip public fun enableBridgelessArchitecture(): Boolean

@DoNotStrip public fun enableCleanTextInputYogaNode(): Boolean
Expand All @@ -57,6 +55,8 @@ public interface ReactNativeFeatureFlagsProvider {

@DoNotStrip public fun enableLayoutAnimationsOnIOS(): Boolean

@DoNotStrip public fun enableLineHeightCentering(): Boolean

@DoNotStrip public fun enableLongTaskAPI(): Boolean

@DoNotStrip public fun enableMicrotasks(): Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public class CustomLineHeightSpan(height: Float) : LineHeightSpan, ReactSpan {
v: Int,
fm: FontMetricsInt,
) {
if (ReactNativeFeatureFlags.enableAndroidLineHeightCentering()) chooseCenteredHeight(fm)
if (ReactNativeFeatureFlags.enableLineHeightCentering()) chooseCenteredHeight(fm)
else chooseOriginalHeight(fm)
}
}
Loading