Fix TextInput vertical alignment issue when using lineHeight prop on iOS (Paper - old arch) #37465

fabOnReact · 2023-05-17T14:00:39Z

Summary:

Adding paragraphStyle.maximumLineHeight to a iOS UITextField displays the text under the UITextField (ios-screenshot-1, ios-screenshot-2, ios-screenshot-3). The issue reproduces on Storyboard App (iOS) using UITextField and paragraphStyle.maximumLineHeight. It is not caused by react-native.

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, react-native-screenshot-2)

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.

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:

set _UITextLayoutFragmentView to vertically align to the top. Required to correctly align _UITextLayoutFragmentView.
apply custom bounds with RCTUITextField textRectForBound to align _UITextLayoutFragmentView with the correct y coordinates and height
Adjust text baseline to be center aligned

fixes #28012 fixes #33986
Related #35741 #31112

Changelog:

[IOS] [FIXED] - Fix TextInput vertical alignment issue when using lineHeight prop on iOS (Paper - old arch)

Test Plan:

Extensive test included in the PR comments #37465 (comment)

Related Expensify/App#17767 (comment) Expensify/App#17767 (comment) More info type | task -- | -- task | Debug with Layout Inspector and verify height of the container of the TextInput. task | Compare coordinates (anchor point, x, y) of the different views (UITextLayoutCanvasView, UITextLayoutFragmentView).Find parameters that causes view to shift down by 50% (change the parameter and verify regression fixed. SOLUTION: The _UITextLayoutFragmentView.frame has Y coordinates > 0. The Y coordinates are relative to the parent views (UITextLayoutCanvas and RCTUITextField). task | Modify UITextLayoutCanvasView, UITextLayoutFragmentView frame position (UITextLayoutFramePosition). task | Read about UITextLayoutFragmentView (search) task | Try to change more values in paragraphStyles. Remove maximumLineHeight and verify if it is causing the issue. The issue seems to be caused by paragraphStyle.maxLineHeight. feature | Copy implementation RCTBaseTextShadowView layoutSubviewsWitContext task | Read Text implementation of baseline (github comment, postprocessAttributedText, layoutSubviewsWithContext, search, enumerate_attribute) task | Implement logic in RCTBaseTextInputShadowView layoutSubviewsWithContext task | Read implementation of RCTLayout and RCTLayoutContext task | Adapt implementation and use textContainer, textContainerOrigin and other APIS to align UITextLayoutFragmentView to position 0,0 (TextKit) feature | Copy implementation RCTBaseTextShadowView uiManagerWillPeformMounting task | Review the implementation in the main branch. Verify RCTBaseTextShadowViewEmbeddedShadowViewAttributeName task | Log in the console the number descendants.count task | Read RCTBaseTextInputManager uiManagerWillPerformMounting implementation (source) review | Review yesterday comment on discord and pages document (gist). feature | Modify y coordinates of a Dummy implementation of UITextField using NSTextContainer on a HelloWorld App (twitter example, gist#comment). task | Add UITextField and test it task | Change the UITextField attributedText maximumLineHeight and debug with layout inspector task | Use NSLayoutManager and NSTextContainer to change _UITextLayoutFragmentView coordinates frame.y. SOLUTION: textField.contentVerticalAlignment = UIControlContentVerticalAlignmentTop; feature | Try to align the UITextField top and apply existing solution task | Verify textField contentVerticalAlignment fixes regression.Change the textRectForBounds bounds to position _UITextLayoutFragmentView task | Add previous solution to react-native type task task Debug with Layout Inspector and verify height of the container of the TextInput. task "Compare coordinates (anchor point, x, y) of the different views (UITextLayoutCanvasView, UITextLayoutFragmentView). Find parameters that causes view to shift down by 50% (change the parameter and verify regression fixed. SOLUTION: The _UITextLayoutFragmentView.frame has Y coordinates > 0. The Y coordinates are relative to the parent views (UITextLayoutCanvas and RCTUITextField)." task Modify UITextLayoutCanvasView, UITextLayoutFragmentView frame position ([UITextLayoutFramePosition](https://www.google.com/search?q=UITextLayoutFragment+frame+position)). task Read about UITextLayoutFragmentView ([search](https://www.google.com/search?q=UITextLayoutCanvasView+UITextLayoutFragmentView%C2%A0maximumLineHeight%C2%A0&sxsrf=APwXEdd1MPGNECEcJTqTeOmRIfyXE6o1vA:1684209236160&ei=VP5iZLGjCebYseMPnv6S-A8&ved=0ahUKEwixnf6p-Pj-AhVmbGwGHR6_BP8Q4dUDCA8&uact=5&oq=UITextLayoutCanvasView+UITextLayoutFragmentView%C2%A0maximumLineHeight%C2%A0&gs_lcp=Cgxnd3Mtd2l6LXNlcnAQAzIECCMQJzoKCAAQRxDWBBCwA0oECEEYAFD3mAFYjaUBYKyoAWgCcAF4AIAB6QWIAckJkgEJMC4xLjEuNi0xmAEAoAEBoAECyAEIwAEB&sclient=gws-wiz-serp)) task Try to change more values in paragraphStyles. Remove [maximumLineHeight](https://developer.apple.com/documentation/uikit/nsparagraphstyle/1533343-maximumlineheight?language=objc) and verify if it is causing the issue. The issue seems to be caused by paragraphStyle.maxLineHeight. feature Copy implementation RCTBaseTextShadowView layoutSubviewsWitContext task Read Text implementation of baseline ([github comment](Expensify/App#17767 (comment)), [postprocessAttributedText](https://github.com/fabriziobertoglio1987/react-native/blob/243a148da49e778cc916fa607ed5b3668acab06f/packages/react-native/Libraries/Text/Text/RCTTextShadowView.m#L163), [layoutSubviewsWithContext](https://github.com/fabriziobertoglio1987/react-native/blob/243a148da49e778cc916fa607ed5b3668acab06f/packages/react-native/Libraries/Text/Text/RCTTextShadowView.m#L263), [search](https://github.com/search?q=repo:facebook/react-native+layoutSubviewsWithContext+&type=code), [enumerate_attribute](https://developer.apple.com/documentation/foundation/nsattributedstring/1412461-enumerateattribute?language=objc)) task Implement logic in RCTBaseTextInputShadowView [layoutSubviewsWithContext](https://github.com/facebook/react-native/blob/2b932c3820cfabef1590f6fcf8b95f65d9c21eb8/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputShadowView.m#L59) task Read implementation of RCTLayout and [RCTLayoutContext](https://github.com/facebook/react-native/blob/2b932c3820cfabef1590f6fcf8b95f65d9c21eb8/packages/react-native/React/Views/RCTLayout.h#L32-L36) task Adapt implementation and use textContainer, textContainerOrigin and other APIS to align UITextLayoutFragmentView to position 0,0 ([TextKit](https://developer.apple.com/documentation/uikit/textkit?language=objc)) feature Copy implementation RCTBaseTextShadowView uiManagerWillPeformMounting task Review the implementation in the main branch. Verify RCTBaseTextShadowViewEmbeddedShadowViewAttributeName task Log in the console the number descendants.count task Read RCTBaseTextInputManager uiManagerWillPerformMounting implementation ([source](https://github.com/facebook/react-native/blob/2b932c3820cfabef1590f6fcf8b95f65d9c21eb8/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputViewManager.m#L152-L157)) review Review yesterday comment on [discord](https://discord.com/channels/514829729862516747/514832108678807553/1108011289462181968) and pages document ([gist](https://gist.github.com/fabriziobertoglio1987/ebbd6292dacf49927afc670b5b422c0f)). feature Modify y coordinates of a Dummy implementation of UITextField using NSTextContainer on a HelloWorld App ([twitter example](https://twitter.com/LinguaBrowse/status/1658591561784127488?s=20), [gist#comment](https://gist.github.com/fabriziobertoglio1987/ebbd6292dacf49927afc670b5b422c0f?permalink_comment_id=4570667#gistcomment-4570667)). task Add UITextField and test it task Change the UITextField attributedText [maximumLineHeight](https://github.com/facebook/react-native/blob/a3102175c5dcccce48406fde4443f0e54c3a1299/packages/react-native/Libraries/Text/RCTTextAttributes.m#L174) and debug with layout inspector task Use [NSLayoutManager](https://developer.apple.com/documentation/uikit/nslayoutmanager) and [NSTextContainer](https://developer.apple.com/documentation/uikit/nstextcontainer?language=objc) to change _UITextLayoutFragmentView coordinates frame.y. SOLUTION: textField.contentVerticalAlignment = UIControlContentVerticalAlignmentTop; feature Try to align the UITextField top and apply existing solution task "Verify textField contentVerticalAlignment fixes regression. Change the textRectForBounds bounds to position _UITextLayoutFragmentView" task Add previous solution to react-native

type | task -- | -- issue | Text cut in edit mode when using paddingTop. editingRectForBounds does not apply _textContainerInset. task | Verify textField contentVerticalAlignment fixes regression.Change the textRectForBounds bounds to position _UITextLayoutFragmentView task | Debug using layout inspector task | Reimplement logic in editingRectForBounds. Check cached diff.

fabOnReact · 2023-05-17T14:03:56Z

before fix - increase/decrease padding top

to make the video more clear, we change the style paddingTop to both TextInput

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-11.at.11.25.24.mp4

after fix - increase/decrease padding top

We change the style paddingTop only for the first TextInput

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-11.at.11.18.19.mp4

before fix - increase/decrease border

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-11.at.10.53.38.mp4

after fix - increase/decrease border

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-11.at.11.21.25.mp4

fabOnReact · 2023-05-17T14:04:36Z

before the fix - text is not correctly aligned with different lineHeight, fontSize and height

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-12.at.11.56.58.mp4

after fix - text is correctly aligned with different lineHeight, fontSize and height

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-12.at.11.53.20.mp4

fabOnReact · 2023-05-17T14:05:18Z

Before - behaviour single line TextInput without lineHeight prop

2023-05-12.14-23-38.mp4

Before - behaviour single line TextInput with lineHeight prop

2023-05-12.14-27-24.mp4

Before - behaviour multi line TextInput without lineHeight prop

2023-05-12.14-30-27.mp4

Before - behaviour multi line TextInput with a lineHeight prop

2023-05-12.14-32-44.mp4

After - multi line TextInput with a lineHeight prop (first video)

2023-05-12.15-35-37.mp4

After - multi line TextInput with a lineHeight prop (second video)

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-12.at.15.50.24.mp4

After - multi line TextInput without a lineHeight prop

2023-05-12.14-59-35.mp4

fabOnReact · 2023-05-17T14:05:49Z

Before/After - Testing multiline on Fabric New Architecture (not yet available on Fabric)

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-12.at.16.22.49.mp4

Before/After - Testing singleline on Fabric New Architecture (not yet available on Fabric)

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-12.at.16.25.18.mp4

fabOnReact · 2023-05-17T14:06:23Z

After - Testing paddingTop on a single line TextInput

After - Testing borderWidth with single-line textinput

fabOnReact · 2023-05-17T14:07:40Z

Before - using fontSize multiplayer and lineHeight with TextInput

Test Case	fontSizeMultiplier setting
Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-15.at.20.55.15.mp4

After - using fontSize multiplayer and lineHeight with TextInput

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-15.at.23.30.26.mp4

Simulator.Screen.Recording.-.iPhone.14.Pro.Max.-.2023-05-15.at.23.34.50.mp4

analysis-bot · 2023-05-17T14:52:06Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,744,766	+206
android	hermes	armeabi-v7a	8,056,615	+427
android	hermes	x86	9,236,064	+704
android	hermes	x86_64	9,086,974	+799
android	jsc	arm64-v8a	9,307,443	+178
android	jsc	armeabi-v7a	8,496,678	+191
android	jsc	x86	9,369,665	+528
android	jsc	x86_64	9,624,680	+608

Base commit: 462c648
Branch: main

fabOnReact · 2023-05-25T06:28:34Z

Thanks a lot 🙏 @NandoMB

Issues Problem with TextInput lineHeight on iOS #28012 and TextInput with custom line height always breaks component #33986. They are fixed with the PR
Issue Problem with lineHeight in TextInput #31112 I replied in Problem with lineHeight in TextInput #31112 (comment)

fabOnReact

CLICK TO OPEN TESTS RESULTS

sammy-SC · 2023-06-21T09:29:07Z

@sammy-SC Disabling the debug flag did not show any warning. Could you please share the log of the CI step Facebook Internal - Linter? Thanks a lot. I wish you a good day.

Sorry for the slow reply, I was away for +2 weeks on vacation. I will just just fix the linter issues, they are only related to code formatting.

github-actions · 2023-07-05T12:54:18Z

This pull request was successfully merged by @fabriziobertoglio1987 in 35a1648.

^{When will my fix make it into a release? | Upcoming Releases}

sammy-SC · 2023-07-07T09:14:20Z

Hello @fabriziobertoglio1987

we will have to revert this change and evaluate next steps.

The change causes a slight difference on how text is rendered. I realise the change is small but it can lead to text's baselines being off, which we don't want.

facebook-github-bot · 2023-07-07T12:49:37Z

This pull request has been reverted by e075a3f.

The diff would trigger linter error internal in Facebook. I compared the commit and the original diff and I was able to identify the linter issue. I fixed the linter issue to avoid again failures when importing it. Comment facebook#37465 (comment) Imported in Facebook facebook@35a1648#diff-08824b4f7ca9c31bd2a7aeb8b1180bd55b2b834d99667861e14e59082d06c137R179-R190 Original Diff facebook@c4996fc#diff-08824b4f7ca9c31bd2a7aeb8b1180bd55b2b834d99667861e14e59082d06c137R170-R177

fabOnReact · 2023-07-11T12:43:27Z

Thanks, @sammy-SC, could you please share the source code for this example?

fabOnReact · 2023-07-12T07:14:38Z

Update 12/07

There is a difference in the baseline computation between RCTTextShadowView postprocessAttributedText and RCTTextAttributes effectiveParagraphStyle.

Given the following example:

<Text style={{lineHeight: 500, fontSize: 15, backgroundColor: 'red'}}>
  font size 15
  <Text style={{fontSize: 50, lineHeight: 500}}>font size 50</Text>
</Text>

The RCTTextShadowView implementation calculates the same baseline for the entire Text based on the highest lineHeight/fontSize
The RCTTextAttributes implementation calculates different baseline for each Text span, based on the Text span lineHeight and fontSize

fabOnReact · 2023-07-22T05:25:38Z

Thanks, @sammy-SC. I published PR #38359 to fix the regression from comment #37465 (comment).

…iOS without changing Text baseline (Paper - old arch) (#38359) Summary: This PR fixes visual regression introduced with #37465 (comment) 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 PR implements the logic from RCTTextShadowView [#postprocessAttributedText](https://github.com/facebook/react-native/blob/9ab27e8895d6934e72ebdc601d169578ab9628f1/packages/react-native/Libraries/Text/Text/RCTTextShadowView.m#L165-L167) in RCTBaseTextInpuShadowView [#uiManagerWillPerformMounting](https://github.com/facebook/react-native/blob/4c944540f732c6055d447ecaf37d5c8f3eec1bc4/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputShadowView.m#L130-L192). [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" [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 fixes #28012 fixes #33986 Related #35741 #31112 ## Changelog: [IOS] [FIXED] - Fix TextInput vertical alignment issue when using lineHeight prop on iOS without changing Text baseline (Paper - old arch) Pull Request resolved: #38359 Test Plan: Extensive test included in the PR comments #37465 (comment) and Expensify/App#17767 (comment) Reviewed By: cipolleschi Differential Revision: D52325261 Pulled By: dmytrorykun fbshipit-source-id: d072a598bfaafbbffc41005b1fda1795cf3d8ab9

https://github.com/facebook/react-native/pull/37465/files#diff-38333c03094e568d775af03727306a9e737fec7434e31f2f6e849f9663e48052R134-R139

fabOnReact added 17 commits May 11, 2023 23:21

basic working functionality

9ae1c50

draft solution to fix alignment fontSize higher than lineHeight

d382cda

remove not required changes

d23d86e

fix regression with border and padding

c40e549

minor changes

4d1331a

Merge branch 'main' into cursor-getting-cut

2c5b47a

line height examples

590915b

re-introduce text-input examples

20cd89b

minor changes to example

025354f

rename variable to fragmentViewContainerBounds

0ff8dc5

clean up the diff

cdbf426

minor changes

17c16bd

reset Vertical alignment to center

a5a5e66

remove example

d4772ca

Merge branch 'main' into cursor-getting-cut

070bdb7

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 May 17, 2023

fabOnReact added 2 commits May 17, 2023 22:22

adding comments

acca018

more comments

85ce3bf

fabOnReact mentioned this pull request May 17, 2023

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [HOLD for payment 2024-06-06] [HOLD #37374][$1000] Text in some languages gets cut off at the bottom in composer on ios Expensify/App#17767

Closed

6 tasks

fabOnReact added 2 commits May 18, 2023 14:41

Merge branch 'main' into cursor-getting-cut

6481272

Avoid changing contentVerticalAlignment in textRectForBounds

3854ec1

minor change

4832e89

changes from code review

7a9a38b

fabOnReact commented May 30, 2023

View reviewed changes

fabOnReact added 3 commits May 30, 2023 11:19

minor improvement

c04bc43

Merge branch 'main' into cursor-getting-cut

90bfee9

adding a line break before comment

206951e

github-actions bot added the Merged This PR has been merged. label Jul 5, 2023

facebook-github-bot closed this in 35a1648 Jul 5, 2023

facebook-github-bot added the Reverted label Jul 7, 2023

fabOnReact mentioned this pull request Jul 15, 2023

Fix TextInput vertical alignment issue when using lineHeight prop on iOS without changing Text baseline (Paper - old arch) #38359

Closed

fabOnReact mentioned this pull request Jan 18, 2024

Line height is distributed unevenly when lineHeight <= fontSize #29507

Open

fabOnReact mentioned this pull request May 19, 2024

Fix TextInput vertical alignment issue when using lineHeight prop on iOS without changing Text baseline (Fabric - new arch) Expensify/react-native-live-markdown#350

Merged

fabOnReact added a commit to fabOnReact/react-native that referenced this pull request May 20, 2024

reintroduce check facebook#37465

176b1a6

https://github.com/facebook/react-native/pull/37465/files#diff-38333c03094e568d775af03727306a9e737fec7434e31f2f6e849f9663e48052R134-R139

fabOnReact mentioned this pull request May 20, 2024

Avoid setting a lineHeight lower of the fontSize lineHeight with Text and TextInput #44614

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix TextInput vertical alignment issue when using lineHeight prop on iOS (Paper - old arch) #37465

Fix TextInput vertical alignment issue when using lineHeight prop on iOS (Paper - old arch) #37465

fabOnReact commented May 17, 2023 •

edited

Loading

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

analysis-bot commented May 17, 2023 •

edited

Loading

fabOnReact commented May 25, 2023

fabOnReact left a comment

sammy-SC commented Jun 21, 2023 •

edited

Loading

github-actions bot commented Jul 5, 2023

sammy-SC commented Jul 7, 2023

facebook-github-bot commented Jul 7, 2023

fabOnReact commented Jul 11, 2023

fabOnReact commented Jul 12, 2023 •

edited

Loading

fabOnReact commented Jul 22, 2023

Fix TextInput vertical alignment issue when using lineHeight prop on iOS (Paper - old arch) #37465

Fix TextInput vertical alignment issue when using lineHeight prop on iOS (Paper - old arch) #37465

Conversation

fabOnReact commented May 17, 2023 • edited Loading

Summary:

Changelog:

Test Plan:

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

fabOnReact commented May 17, 2023

analysis-bot commented May 17, 2023 • edited Loading

fabOnReact commented May 25, 2023

fabOnReact left a comment

Choose a reason for hiding this comment

sammy-SC commented Jun 21, 2023 • edited Loading

github-actions bot commented Jul 5, 2023

sammy-SC commented Jul 7, 2023

facebook-github-bot commented Jul 7, 2023

fabOnReact commented Jul 11, 2023

fabOnReact commented Jul 12, 2023 • edited Loading

Update 12/07

fabOnReact commented Jul 22, 2023

fabOnReact commented May 17, 2023 •

edited

Loading

analysis-bot commented May 17, 2023 •

edited

Loading

sammy-SC commented Jun 21, 2023 •

edited

Loading

fabOnReact commented Jul 12, 2023 •

edited

Loading