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

[HOLD facebook/react-native#37465] Fix TextInput vertical alignment issue when using lineHeight prop on iOS (Paper - old arch) #57

16 changes: 13 additions & 3 deletions packages/react-native/Libraries/Text/RCTTextAttributes.m
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,13 @@ - (NSParagraphStyle *)effectiveParagraphStyle

if (!isnan(_lineHeight)) {
CGFloat lineHeight = _lineHeight * self.effectiveFontSizeMultiplier;
paragraphStyle.minimumLineHeight = lineHeight;
paragraphStyle.maximumLineHeight = lineHeight;
isParagraphStyleUsed = YES;

// Text with lineHeight lower than font.lineHeight does not correctly vertically align.
if (lineHeight > self.effectiveFont.lineHeight) {
paragraphStyle.minimumLineHeight = lineHeight;
paragraphStyle.maximumLineHeight = lineHeight;
isParagraphStyleUsed = YES;
}
}

if (isParagraphStyleUsed) {
Expand Down Expand Up @@ -172,6 +176,12 @@ - (NSParagraphStyle *)effectiveParagraphStyle
NSParagraphStyle *paragraphStyle = [self effectiveParagraphStyle];
if (paragraphStyle) {
attributes[NSParagraphStyleAttributeName] = paragraphStyle;

// The baseline aligns the text vertically in the line height (_UITextLayoutFragmentView).
if (!isnan(paragraphStyle.maximumLineHeight) && paragraphStyle.maximumLineHeight >= font.lineHeight) {
CGFloat baseLineOffset = (paragraphStyle.maximumLineHeight - font.lineHeight) / 2.0;
attributes[NSBaselineOffsetAttributeName] = @(baseLineOffset);
}
}

// Decoration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic, assign, readonly) BOOL textWasPasted;
@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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,15 @@ - (void)paste:(id)sender
[super paste:sender];
}

- (void)setTextBorderInsetsAndFrame:(CGRect)bounds textBorderInsets:(UIEdgeInsets)textBorderInsets
{
_textBorderInsets = textBorderInsets;

// We apply `borderInsets` as the `RCTUITextView` layout offset.
self.frame = UIEdgeInsetsInsetRect(bounds, textBorderInsets);
[self setNeedsLayout];
}

// Turn off scroll animation to fix flaky scrolling.
// This is only necessary for iOS <= 13.
#if defined(__IPHONE_OS_VERSION_MAX_ALLOWED) && __IPHONE_OS_VERSION_MAX_ALLOWED < 140000
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,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
Expand All @@ -42,6 +44,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,27 @@ - (void)uiManagerWillPerformMounting

baseTextInputView.textAttributes = textAttributes;
baseTextInputView.reactBorderInsets = borderInsets;

// The CALayer _UITextLayoutFragmentView does not align correctly
// when adding paragraphStyle.maximumLineHeight to an iOS UITextField (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;

// Setting contentVerticalAlignment to UIControlContentVerticalAlignmentTop aligns
// _UITextLayoutFragmentView and UITextField on the same ordinate (y coordinate).
baseTextInputView.contentVerticalAlignment = UIControlContentVerticalAlignmentTop;

// Align vertically _UITextLayoutFragmentView in the center of the UITextField (TextInput).
CGFloat padding = (height - effectiveLineHeight) / 2.0;
baseTextInputView.fragmentViewContainerBounds = CGRectMake(0, padding, width, effectiveLineHeight);
}
} else {
baseTextInputView.contentVerticalAlignment = UIControlContentVerticalAlignmentCenter;
}
baseTextInputView.reactPaddingInsets = paddingInsets;

if (newAttributedText) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,22 @@ - (void)enforceTextAttributesIfNeeded
backedTextInputView.defaultTextAttributes = textAttributes;
}

// Fixes iOS alignment issue caused by adding paragraphStyle.maximumLineHeight to an iOS UITextField
// and 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;
Expand All @@ -87,9 +103,9 @@ - (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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ NS_ASSUME_NONNULL_BEGIN
@property (nonatomic, assign, readonly) BOOL textWasPasted;
@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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -157,7 +163,14 @@ - (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);
// The fragmentViewContainerBounds set the correct y coordinates for
Copy link

Choose a reason for hiding this comment

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

Minor, so not a blocker

Suggested change
UIEdgeInsets borderAndPaddingInsets = UIEdgeInsetsMake(_textContainerInset.top, leftPadding, _textContainerInset.bottom, rightPadding);
// The fragmentViewContainerBounds set the correct y coordinates for
UIEdgeInsets borderAndPaddingInsets = UIEdgeInsetsMake(_textContainerInset.top, leftPadding, _textContainerInset.bottom, rightPadding);
// The fragmentViewContainerBounds set the correct y coordinates for

Copy link
Author

@fabOnReact fabOnReact Jun 1, 2023

Choose a reason for hiding this comment

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

Thanks @cead22.

Adding a line-break before the comment

I believe you refer to

Can you capitalize all comments and add a line break before them (except for the one after the if which is in a new level of indentation)?

I added a line break before the comment with commit 3d12d11.

Github Website Issue

There was an issue with the GitHub (website), which would not update the PR Diff and commits. My remote branch fabriziobertoglio1987/text-input-vertical-alignment-expensify-2 included changes not available on Github Website PR.

CLICK TO OPEN VIDEO

2023-06-01.14-27-51.mp4

For this reason, I had to temporarily change the base branch.

Related Discussions: https://github.com/orgs/community/discussions/16351 isaacs/github#750 (comment)

I tested and reviewed the branch diff locally (git checkout text-input-vertical-alignment-expensify-2 && g diff ExpensifyRC1-0.72.0-alpha.0..), and the branch has no issues.

CLICK TO OPEN TESTS RESULTS

Screenshot 2023-06-01 at 2 47 34 PM

Sorry. I remain available. Thanks

// _UITextLayoutFragmentView to fix an iOS UITextField issue with lineHeight.
CGRect updatedBounds = self.fragmentViewContainerBounds.size.height > 0 ? self.fragmentViewContainerBounds : bounds;
return UIEdgeInsetsInsetRect([super textRectForBounds:updatedBounds], borderAndPaddingInsets);
}

- (CGRect)editingRectForBounds:(CGRect)bounds
Expand Down