From 35a1648d0c93ef2db1de6b8dbe7fc1d997c24039 Mon Sep 17 00:00:00 2001 From: Fabrizio Bertoglio Date: Wed, 5 Jul 2023 05:50:20 -0700 Subject: [PATCH] Fix TextInput vertical alignment issue when using lineHeight prop on iOS (Paper - old arch) (#37465) Summary: Adding paragraphStyle.maximumLineHeight to a iOS UITextField displays the text under the UITextField ([ios-screenshot-1][1], [ios-screenshot-2][2], [ios-screenshot-3][3]). The issue reproduces on Storyboard App (iOS) using UITextField and paragraphStyle.maximumLineHeight. It is not caused by react-native. [1]: https://user-images.githubusercontent.com/24992535/238834159-566f7eef-ea2d-4fd4-a519-099b0a12046c.png "ios-screenshot-1" [2]: https://user-images.githubusercontent.com/24992535/238834184-feb454a9-6504-4832-aec8-989f1d027861.png "ios-screenshot-2" [3]: https://user-images.githubusercontent.com/24992535/238834283-cf572f94-a641-4790-92bf-bbe43afb1443.png "ios-screenshot-3" The issue is caused by a private class _UITextLayoutFragmentView (a CALayer children of UITextField), which assumes the wrong position when setting the lineHeight. _UITextLayoutFragmentView frame's y coordinates are set to 30, instead of 0 ([react-native-screenshot-1][4], [react-native-screenshot-2][5]) - The _UITextLayoutFragmentView layer does not correctly position - The issue is caused by adding paragraphStyle.maximumLineHeight to UITextField.attributedText - The parent UITextField bounds do not correctly position child _UITextLayoutFragmentView. The issue causes the below text Sdfsd to display under the TextInput. [4]: https://github.com/Expensify/App/assets/24992535/06726b45-7e35-4003-9fcc-50c8d0dff0f6 [5]: https://github.com/Expensify/App/assets/24992535/d9745d29-8863-4170-bcc3-e78fa7e550d2 I was able to fix the issue and correctly align the private layout view _UITextLayoutFragmentView using the public API RCTUITextField textRectForBound, which allows modifying the UITextField frame and inset. The solution consists in the following steps, applied only to UITextField with lineHeight prop: 1) set _UITextLayoutFragmentView to vertically align to the top. Required to correctly align _UITextLayoutFragmentView. 2) apply custom bounds with RCTUITextField textRectForBound to align _UITextLayoutFragmentView with the correct y coordinates and height 2) Adjust text baseline to be center aligned fixes https://github.com/facebook/react-native/issues/28012 ## Changelog: [IOS] [FIXED] - Fix TextInput vertical alignment issue when using lineHeight prop on iOS (Paper - old arch) Pull Request resolved: https://github.com/facebook/react-native/pull/37465 Test Plan: Extensive test included in the PR comments https://github.com/facebook/react-native/pull/37465#issuecomment-1551459879 Reviewed By: NickGerleman Differential Revision: D46086661 Pulled By: sammy-SC fbshipit-source-id: faece0940b153f3525ddcfae9417e943c957a5bf --- .../Libraries/Text/RCTTextAttributes.m | 14 ++++++++++--- .../Text/TextInput/Multiline/RCTUITextView.h | 3 +++ .../Text/TextInput/Multiline/RCTUITextView.m | 8 +++++++ .../RCTBackedTextInputViewProtocol.h | 3 +++ .../TextInput/RCTBaseTextInputShadowView.m | 18 ++++++++++++++++ .../Text/TextInput/RCTBaseTextInputView.h | 2 ++ .../Text/TextInput/RCTBaseTextInputView.m | 21 ++++++++++++++++--- .../TextInput/Singleline/RCTUITextField.h | 2 ++ .../TextInput/Singleline/RCTUITextField.m | 19 ++++++++++++++++- 9 files changed, 83 insertions(+), 7 deletions(-) diff --git a/packages/react-native/Libraries/Text/RCTTextAttributes.m b/packages/react-native/Libraries/Text/RCTTextAttributes.m index c8323388ce684b..c2ca6407a9778f 100644 --- a/packages/react-native/Libraries/Text/RCTTextAttributes.m +++ b/packages/react-native/Libraries/Text/RCTTextAttributes.m @@ -130,9 +130,12 @@ - (NSParagraphStyle *)effectiveParagraphStyle if (!isnan(_lineHeight)) { CGFloat lineHeight = _lineHeight * self.effectiveFontSizeMultiplier; - paragraphStyle.minimumLineHeight = lineHeight; - paragraphStyle.maximumLineHeight = lineHeight; - isParagraphStyleUsed = YES; + // text with lineHeight lower then font.lineHeight does not correctly vertically align + if (lineHeight > self.effectiveFont.lineHeight) { + paragraphStyle.minimumLineHeight = lineHeight; + paragraphStyle.maximumLineHeight = lineHeight; + isParagraphStyleUsed = YES; + } } if (isParagraphStyleUsed) { @@ -172,6 +175,11 @@ - (NSParagraphStyle *)effectiveParagraphStyle NSParagraphStyle *paragraphStyle = [self effectiveParagraphStyle]; if (paragraphStyle) { attributes[NSParagraphStyleAttributeName] = paragraphStyle; + // The baseline aligns the text vertically in the line height + if (!isnan(paragraphStyle.maximumLineHeight) && paragraphStyle.maximumLineHeight >= font.lineHeight) { + CGFloat baseLineOffset = (paragraphStyle.maximumLineHeight - font.lineHeight) / 2.0; + attributes[NSBaselineOffsetAttributeName] = @(baseLineOffset); + } } // Decoration diff --git a/packages/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.h b/packages/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.h index 205f9943262add..b02d7964f76a5d 100644 --- a/packages/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.h +++ b/packages/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.h @@ -27,6 +27,9 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, assign, readonly) BOOL dictationRecognizing; @property (nonatomic, copy, nullable) NSString *placeholder; @property (nonatomic, strong, nullable) UIColor *placeholderColor; +@property (nonatomic, assign) CGRect fragmentViewContainerBounds; +@property (nonatomic, assign) UIEdgeInsets textBorderInsets; +@property (nonatomic, assign) UIControlContentVerticalAlignment contentVerticalAlignment; @property (nonatomic, assign) CGFloat preferredMaxLayoutWidth; diff --git a/packages/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.m b/packages/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.m index ce8a77ec6c1275..27565331b98ad3 100644 --- a/packages/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.m +++ b/packages/react-native/Libraries/Text/TextInput/Multiline/RCTUITextView.m @@ -169,6 +169,14 @@ - (void)paste:(id)sender [super paste:sender]; } +- (void)setTextBorderInsetsAndFrame:(CGRect)bounds textBorderInsets:(UIEdgeInsets)textBorderInsets +{ + _textBorderInsets = textBorderInsets; + // We apply `borderInsets` as `RCTUITextView` layout offset. + self.frame = UIEdgeInsetsInsetRect(bounds, textBorderInsets); + [self setNeedsLayout]; +} + // Turn off scroll animation to fix flaky scrolling. // This is only necessary for iOS <= 14. #if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && __IPHONE_OS_VERSION_MAX_ALLOWED < 140000 diff --git a/packages/react-native/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h b/packages/react-native/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h index a8719ecd4d0165..55b8946c0f664b 100644 --- a/packages/react-native/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h +++ b/packages/react-native/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h @@ -35,6 +35,8 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, assign, readonly) CGFloat zoomScale; @property (nonatomic, assign, readonly) CGPoint contentOffset; @property (nonatomic, assign, readonly) UIEdgeInsets contentInset; +@property (nonatomic, assign) CGRect fragmentViewContainerBounds; +@property (nonatomic, assign) UIControlContentVerticalAlignment contentVerticalAlignment; // This protocol disallows direct access to `selectedTextRange` property because // unwise usage of it can break the `delegate` behavior. So, we always have to @@ -43,6 +45,7 @@ NS_ASSUME_NONNULL_BEGIN // If the change was a result of user actions (like typing or touches), we MUST notify the delegate. - (void)setSelectedTextRange:(nullable UITextRange *)selectedTextRange NS_UNAVAILABLE; - (void)setSelectedTextRange:(nullable UITextRange *)selectedTextRange notifyDelegate:(BOOL)notifyDelegate; +- (void)setTextBorderInsetsAndFrame:(CGRect)bounds textBorderInsets:(UIEdgeInsets)textBorderInsets; // This protocol disallows direct access to `text` property because // unwise usage of it can break the `attributeText` behavior. diff --git a/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputShadowView.m b/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputShadowView.m index 04d2446f86d9b3..396e41e44a64bf 100644 --- a/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputShadowView.m +++ b/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputShadowView.m @@ -177,6 +177,24 @@ - (void)uiManagerWillPerformMounting baseTextInputView.textAttributes = textAttributes; baseTextInputView.reactBorderInsets = borderInsets; + + // Fixes iOS issue caused by adding paragraphStyle.maximumLineHeight to an iOS UITextField. + // The CALayer _UITextLayoutFragmentView does not align correctly (see issue #28012). + if (!isnan(textAttributes.lineHeight) && !isnan(textAttributes.effectiveFont.lineHeight)) { + CGFloat effectiveLineHeight = textAttributes.lineHeight * textAttributes.effectiveFontSizeMultiplier; + CGFloat fontLineHeight = textAttributes.effectiveFont.lineHeight; + if (effectiveLineHeight >= fontLineHeight * 2.0) { + CGFloat height = self.layoutMetrics.frame.size.height; + CGFloat width = self.layoutMetrics.frame.size.width; + // sets the same origin.y coordinates for _UITextLayoutFragmentView and UITextField frame + baseTextInputView.contentVerticalAlignment = UIControlContentVerticalAlignmentTop; + // vertically center aligns the _UITextLayoutFragmentView in the parent UITextField + CGFloat padding = (height - effectiveLineHeight) / 2.0; + baseTextInputView.fragmentViewContainerBounds = CGRectMake(0, padding, width, effectiveLineHeight); + } + } else { + baseTextInputView.contentVerticalAlignment = UIControlContentVerticalAlignmentCenter; + } baseTextInputView.reactPaddingInsets = paddingInsets; if (newAttributedText) { diff --git a/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.h b/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.h index 209947de9b4aaa..7258c36e1d8b8f 100644 --- a/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.h +++ b/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.h @@ -31,6 +31,8 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, strong, nullable) RCTTextAttributes *textAttributes; @property (nonatomic, assign) UIEdgeInsets reactPaddingInsets; @property (nonatomic, assign) UIEdgeInsets reactBorderInsets; +@property (nonatomic, assign) CGRect fragmentViewContainerBounds; +@property (nonatomic, assign) UIControlContentVerticalAlignment contentVerticalAlignment; @property (nonatomic, copy, nullable) RCTDirectEventBlock onContentSizeChange; @property (nonatomic, copy, nullable) RCTDirectEventBlock onSelectionChange; diff --git a/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.m b/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.m index cb293db64411b4..ca672bea307265 100644 --- a/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.m +++ b/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.m @@ -76,6 +76,22 @@ - (void)enforceTextAttributesIfNeeded backedTextInputView.defaultTextAttributes = textAttributes; } +// Fixes iOS alignment issue caused by adding paragraphStyle.maximumLineHeight to an iOS UITextField +// vertically aligns _UITextLayoutFragmentView with the parent view UITextField +- (void)setContentVerticalAlignment:(UIControlContentVerticalAlignment)contentVerticalAlignment +{ + _contentVerticalAlignment = contentVerticalAlignment; + self.backedTextInputView.contentVerticalAlignment = contentVerticalAlignment; +} + +// Custom bounds used to control vertical position of CALayer _UITextLayoutFragmentView +// _UITextLayoutFragmentView is the CALayer of UITextField +- (void)setFragmentViewContainerBounds:(CGRect)fragmentViewContainerBounds +{ + _fragmentViewContainerBounds = fragmentViewContainerBounds; + self.backedTextInputView.fragmentViewContainerBounds = fragmentViewContainerBounds; +} + - (void)setReactPaddingInsets:(UIEdgeInsets)reactPaddingInsets { _reactPaddingInsets = reactPaddingInsets; @@ -87,9 +103,8 @@ - (void)setReactPaddingInsets:(UIEdgeInsets)reactPaddingInsets - (void)setReactBorderInsets:(UIEdgeInsets)reactBorderInsets { _reactBorderInsets = reactBorderInsets; - // We apply `borderInsets` as `backedTextInputView` layout offset. - self.backedTextInputView.frame = UIEdgeInsetsInsetRect(self.bounds, reactBorderInsets); - [self setNeedsLayout]; + // Borders are added using insets (UITextField textRectForBound, UITextView setFrame) + [self.backedTextInputView setTextBorderInsetsAndFrame:self.bounds textBorderInsets:reactBorderInsets]; } - (NSAttributedString *)attributedText diff --git a/packages/react-native/Libraries/Text/TextInput/Singleline/RCTUITextField.h b/packages/react-native/Libraries/Text/TextInput/Singleline/RCTUITextField.h index 91f8eb087acf87..b581589f1dfcb1 100644 --- a/packages/react-native/Libraries/Text/TextInput/Singleline/RCTUITextField.h +++ b/packages/react-native/Libraries/Text/TextInput/Singleline/RCTUITextField.h @@ -27,6 +27,8 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, assign, readonly) BOOL dictationRecognizing; @property (nonatomic, strong, nullable) UIColor *placeholderColor; @property (nonatomic, assign) UIEdgeInsets textContainerInset; +@property (nonatomic, assign) CGRect fragmentViewContainerBounds; +@property (nonatomic, assign) UIEdgeInsets textBorderInsets; @property (nonatomic, assign, getter=isEditable) BOOL editable; @property (nonatomic, getter=isScrollEnabled) BOOL scrollEnabled; @property (nonatomic, strong, nullable) NSString *inputAccessoryViewID; diff --git a/packages/react-native/Libraries/Text/TextInput/Singleline/RCTUITextField.m b/packages/react-native/Libraries/Text/TextInput/Singleline/RCTUITextField.m index 4d0afd97ae682a..1d46459e0b8400 100644 --- a/packages/react-native/Libraries/Text/TextInput/Singleline/RCTUITextField.m +++ b/packages/react-native/Libraries/Text/TextInput/Singleline/RCTUITextField.m @@ -55,6 +55,12 @@ - (void)setTextContainerInset:(UIEdgeInsets)textContainerInset [self setNeedsLayout]; } +- (void)setTextBorderInsetsAndFrame:(CGRect)bounds textBorderInsets:(UIEdgeInsets)textBorderInsets +{ + _textBorderInsets = textBorderInsets; + [self setNeedsLayout]; +} + - (void)setPlaceholder:(NSString *)placeholder { [super setPlaceholder:placeholder]; @@ -170,7 +176,18 @@ - (CGRect)caretRectForPosition:(UITextPosition *)position - (CGRect)textRectForBounds:(CGRect)bounds { - return UIEdgeInsetsInsetRect([super textRectForBounds:bounds], _textContainerInset); + // Text is vertically aligned to the center + CGFloat leftPadding = _textContainerInset.left + _textBorderInsets.left; + CGFloat rightPadding = _textContainerInset.right + _textBorderInsets.right; + UIEdgeInsets borderAndPaddingInsets = + UIEdgeInsetsMake(_textContainerInset.top, leftPadding, _textContainerInset.bottom, rightPadding); + if (self.fragmentViewContainerBounds.size.height > 0) { + // apply custom bounds to fix iOS UITextField issue with lineHeight + // sets the correct y coordinates for _UITextLayoutFragmentView + return UIEdgeInsetsInsetRect([super textRectForBounds:self.fragmentViewContainerBounds], borderAndPaddingInsets); + } else { + return UIEdgeInsetsInsetRect([super textRectForBounds:bounds], borderAndPaddingInsets); + } } - (CGRect)editingRectForBounds:(CGRect)bounds