Fix Multiline TextInput with a fixed height scrolls to the bottom when prepending new lines to the text #38679

fabOnReact · 2023-07-29T03:53:15Z

Summary:

Please re-state the problem that we are trying to solve in this issue.

Multiline TextInput with a fixed height will scroll to the bottom of the screen when prepending new lines to the text.

What is the root cause of that problem?

The issue is caused by iOS UITextView:

The cursor moves to the end of the text when prepending new lines.
Moving the cursor to the end of the text triggers the scroll to the bottom.

The behavior was reproduced on an iOS App (without react-native).
The example included below implements a Component RCTUITextView based on UITextView, which modifies the UITextView attributedText with the textViewDidChange callback (source code available in this comment).

Adding a new line on top of the UITextView on iOS results in:
Issue 1) The cursor moves to the end of TextInput text
Issue 2) The TextInput scrolls to the bottom

Reproducing the issue on an iOS App without react-native

2023-06-17.18-16-16.mp4

Issue 1) is already fixed in react-native, which restores the previous cursor position (on Fabric with _setAttributedString) after changing the text.
Issue 2) needs to be fixed in react-native.

What changes do you think we should make in order to solve the problem?

Setting the correct TextInput scroll position after re-setting the cursor.

Changelog:

[IOS] [FIXED] - Fix Multiline TextInput with a fixed height scrolls to the bottom when changing AttributedText

Test Plan:

Fabric (reproduces on controlled/not controlled TextInput example):

Before	After
Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-07-29.at.11.57.04.mp4	Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-07-29.at.11.55.11.mp4

Paper (reproduces only on controlled TextInput example):

function TextInputExample() {
  const [text, setText] = React.useState('');

  return (
    <View style={{marginTop: 200}}>
      <TextInput
        style={{height: 50, backgroundColor: 'white'}}
        multiline={true}
        value={text}
        onChangeText={text => {
          setText(text);
        }}
      />
    </View>
  );
}

Before	After
Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-07-29.at.13.05.48.mp4	Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-07-29.at.13.09.48.mp4

analysis-bot · 2023-07-29T04:18:00Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	17,885,642	-1
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	21,240,040	+1
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 475a156
Branch: main

NickGerleman · 2023-08-16T22:49:00Z

Would this also prevent scroll when adding to the end of the list?

fabOnReact · 2023-08-17T11:54:54Z

Thanks for the code review. I moved the PR to draft. I'm working on a solution for this issue.

fabOnReact · 2023-08-19T01:37:40Z

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2023-08-19.at.09.35.06.mp4

This reverts commit 00bcc51.

fabOnReact · 2023-08-20T03:09:59Z

Thanks for the code-review @NickGerleman. I updated the PR and I included a video test in this comment.

NickGerleman · 2023-08-21T23:01:33Z

...ges/react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm

+  BOOL previousScrollEnabled = _backedTextInputView.scrollEnabled;
+  NSInteger previousOffsetStart = [_backedTextInputView offsetFromPosition:_backedTextInputView.beginningOfDocument
+                                                        toPosition:selectedRange.start];
+  if (previousScrollEnabled && previousOffsetStart > 0 && attributedString.string.length >= previousOffsetStart) {
+    NSString *lastChar = [attributedString.string substringWithRange:NSMakeRange(previousOffsetStart - 1, 1)];
+    NSInteger previousOffsetFromEnd = oldTextLength - previousOffsetStart;
+    if (previousOffsetFromEnd > 0 && [lastChar isEqual:@"\n"]) {
+      _backedTextInputView.scrollEnabled = NO;
+    }
+  }


Could you explain this heuristic? Is it specific to adding newlines?

Thanks @NickGerleman

Could you explain this heuristic?

https://github.com/facebook/react-native/pull/38679/files#diff-65bdfd8888173755da0fd38dae8c70ba1561c11ec453e690195a2f327fa6b62bR603

Scrolling is temporarily disabled when prepending a new line character to the attributed string:

the cursor is not at the end of the string (previousOffsetFromEnd == 0)

the last typed character (lastChar) is a new line character

Is it specific to adding newlines?

Yes. It is specific to adding new lines.

It is iOS issue 652653, previously addressed with commit Maintain cursor position when the text changes in the text input.

Adding a new line changes the text spans and paragraph alignment. The old attributed string may have three spans with different paragraph alignments. The new attributed string could have one span with paragraph alignment 4 (see the test case below for a breakdown).

Changing the AttributedText (text) attributes (alignment) triggers a mutation of the attributed text (_textOf)

Mutating the AttributedString causes the cursor to move to the end of the string

Moving the cursor to the end of the string causes the TextView to scroll to the end

https://developer.apple.com/forums/thread/652653
https://stackoverflow.com/questions/34914948/how-to-stop-cursor-changing-position-when-i-setattributedtext-in-uitextview-dele

Reproducing the issue on an iOS App without react-native

2023-06-17.18-16-16.mp4

Sourcecode available at Expensify/App#19507 (comment).

Details of the changes in the AttributedString attributes (paragraph alignment) when adding new line

Before one of the AttributedString spans has NSParagraphStyle Alignment 0. The string text is "1\n\n2".

CLICK TO OPEN ATTRIBUTED STRING DETAILS

Printing description of oldAttributedString->attributes: Length 5 (8 blocks, 3 used, block 0 is at 0) 2 0x600002e70230 { NSBackgroundColor = "UIExtendedGrayColorSpace 0 0"; NSColor = "UIExtendedGrayColorSpace 0 1"; NSFont = "<UICTFont: 0x130457dc0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 14.00pt"; NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n 28L,\n 56L,\n 84L,\n 112L,\n 140L,\n 168L,\n 196L,\n 224L,\n 252L,\n 280L,\n 308L,\n 336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint ''"; } 1 0x600002f9d5e0 { NSBackgroundColor = "UIExtendedGrayColorSpace 0 0"; NSColor = "UIExtendedGrayColorSpace 0 1"; NSFont = "<UICTFont: 0x130457dc0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 14.00pt"; NSParagraphStyle = "Alignment 0, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n 28L,\n 56L,\n 84L,\n 112L,\n 140L,\n 168L,\n 196L,\n 224L,\n 252L,\n 280L,\n 308L,\n 336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint ''"; } 2 0x600002e70230 { NSBackgroundColor = "UIExtendedGrayColorSpace 0 0"; NSColor = "UIExtendedGrayColorSpace 0 1"; NSFont = "<UICTFont: 0x130457dc0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 14.00pt"; NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n 28L,\n 56L,\n 84L,\n 112L,\n 140L,\n 168L,\n 196L,\n 224L,\n 252L,\n 280L,\n 308L,\n 336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint ''"; }

After The AttributedString has only one span. The span has NSParagraphStyle Alignment 4. The string text is "1\n\n2".

CLICK TO OPEN ATTRIBUTED STRING DETAILS

Printing description of attributedString->attributes: Length 5 (2 blocks, 1 used, block 0 is at 0) 5 0x600002e70230 { NSBackgroundColor = "UIExtendedGrayColorSpace 0 0"; NSColor = "UIExtendedGrayColorSpace 0 1"; NSFont = "<UICTFont: 0x130457dc0> font-family: \".SFUI-Regular\"; font-weight: normal; font-style: normal; font-size: 14.00pt"; NSParagraphStyle = "Alignment 4, LineSpacing 0, ParagraphSpacing 0, ParagraphSpacingBefore 0, HeadIndent 0, TailIndent 0, FirstLineHeadIndent 0, LineHeight 0/0, LineHeightMultiple 0, LineBreakMode 0, Tabs (\n 28L,\n 56L,\n 84L,\n 112L,\n 140L,\n 168L,\n 196L,\n 224L,\n 252L,\n 280L,\n 308L,\n 336L\n), DefaultTabInterval 0, Blocks (\n), Lists (\n), BaseWritingDirection 0, HyphenationFactor 0, TighteningForTruncation NO, HeaderLevel 0 LineBreakStrategy 0 PresentationIntents (\n) ListIntentOrdinal 0 CodeBlockIntentLanguageHint ''"; }

I included a video of the two examples at this link.

facebook-github-bot · 2023-08-25T04:57:45Z

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

fabOnReact · 2023-08-31T07:53:46Z

Thanks @NickGerleman. Could you tell me what is the lint warning? I can fix it and push again the branch so it is easier to import.

NickGerleman · 2023-09-03T16:38:47Z

It's a formatting issue that I can fix before landing, but it is waiting on a reviewer of the imported PR. The best expert for iOS TextInput has been out of office.

Don’t depend on external variables that can break this.

fabOnReact · 2024-02-28T03:05:31Z

Thanks @NickGerleman. I replied to your code-review and further improved the solution as by Meta requirement.
#38679 (comment)
#38679 (comment)

NickGerleman · 2024-02-28T19:46:41Z

Thanks @NickGerleman. I replied to your code-review and further improved the solution as by Meta requirement. #38679 (comment) #38679 (comment)

I will take another look at this soon (still need to write the accompanying internal tests first).

NickGerleman · 2024-03-14T18:23:10Z

Thanks @NickGerleman. I replied to your code-review and further improved the solution as by Meta requirement. #38679 (comment) #38679 (comment)

I will take another look at this soon (still need to write the accompanying internal tests first).

Finally cleared some other commitments, so writing those tests now.

facebook-github-bot · 2024-03-28T21:36:36Z

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2024-03-28T23:10:45Z

@NickGerleman merged this pull request in f6badca.

…n prepending new lines to the text (#38679) Summary: ### Please re-state the problem that we are trying to solve in this issue. Multiline TextInput with a fixed height will scroll to the bottom of the screen when prepending new lines to the text. ### What is the root cause of that problem? The issue is caused by iOS UITextView: - The cursor moves to the end of the text when prepending new lines. - Moving the cursor to the end of the text triggers the scroll to the bottom. The behavior was reproduced on an iOS App (without react-native). The example included below implements a Component RCTUITextView based on UITextView, which modifies the UITextView attributedText with the textViewDidChange callback (source code available in this [comment](Expensify/App#19507 (comment))). Adding a new line on top of the UITextView on iOS results in: Issue 1) The cursor moves to the end of TextInput text Issue 2) The TextInput scrolls to the bottom <details><summary>Reproducing the issue on an iOS App without react-native</summary> <p> <video src="https://user-images.githubusercontent.com/24992535/246601549-99f480f3-ce80-4678-9378-f71c8aa67e17.mp4" width="900" /> </p> </details> Issue 1) is already fixed in react-native, which restores the previous cursor position (on Fabric with [_setAttributedString](https://github.com/fabriziobertoglio1987/react-native/blob/71e7bbbc2cf21abacf7009e300f5bba737e20d17/packages/react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm#L600-L610)) after changing the text. Issue 2) needs to be fixed in react-native. ### What changes do you think we should make in order to solve the problem? Setting the correct TextInput scroll position after re-setting the cursor. ## Changelog: [IOS] [FIXED] - Fix Multiline TextInput with a fixed height scrolls to the bottom when changing AttributedText Pull Request resolved: #38679 Test Plan: Fabric (reproduces on controlled/not controlled TextInput example): | Before | After | | ----------- | ----------- | | <video src="https://github.com/facebook/react-native/assets/24992535/e06b31fe-407d-4897-b608-73e0cc0f224a" width="350" /> | <video src="https://github.com/facebook/react-native/assets/24992535/fa2eaa31-c616-43c5-9596-f84e7b70d80a" width="350" /> | Paper (reproduces only on controlled TextInput example): ```javascript function TextInputExample() { const [text, setText] = React.useState(''); return ( <View style={{marginTop: 200}}> <TextInput style={{height: 50, backgroundColor: 'white'}} multiline={true} value={text} onChangeText={text => { setText(text); }} /> </View> ); } ``` | Before | After | | ----------- | ----------- | | <video src="https://github.com/facebook/react-native/assets/24992535/6cb1f2de-717e-4dce-be0a-644f6a051c08" width="350" /> | <video src="https://github.com/facebook/react-native/assets/24992535/dee6edb6-76c6-48b0-b78f-99626235d30e" width="350" /> | Reviewed By: sammy-SC, cipolleschi Differential Revision: D48674090 Pulled By: NickGerleman fbshipit-source-id: 349e7b0910e314ec94b45b68c38571fed41ef117

fix input scroll up

8658471

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 Jul 29, 2023

fabOnReact changed the title ~~Fix input field scrolls up when tap enter in 1st line of text~~ Fix iOS TextInput scrolls up when tap on 1st line of text Jul 29, 2023

fabOnReact changed the title ~~Fix iOS TextInput scrolls up when tap on 1st line of text~~ Fix Multiline TextInput with a fixed height scrolls to the bottom when prepending new lines to the text Jul 29, 2023

fabOnReact mentioned this pull request Jul 29, 2023

Fix Multiline TextInput with a fixed height scrolls to the bottom when prepending new lines to the text Expensify/react-native#60

Merged

Merge branch 'main' into fix-input-field-scrolls-up

3ac73aa

fabOnReact marked this pull request as ready for review July 29, 2023 07:33

fabOnReact mentioned this pull request Jul 29, 2023

[Ready for Payment][$1000] iOS - Chat - "Edit comment" input field scrolls up when tap enter in 1st line of text Expensify/App#19507

Closed

6 tasks

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jul 29, 2023

fabOnReact marked this pull request as draft August 17, 2023 11:48

fabOnReact added 5 commits August 17, 2023 20:05

adding conditional checks before disabling scroll

64b64e4

minor changes

323cd2d

Merge branch 'main' into fix-input-field-scrolls-up

ce0c4d8

refactor logic

56a2859

adding conditional checks

39d0351

fabOnReact added 5 commits August 19, 2023 10:41

minor changes

0a29729

minor changes

d996eef

change type previousScrollEnabled to BOOL

abbd60e

example

00bcc51

Revert "example"

3318f3e

This reverts commit 00bcc51.

fabOnReact marked this pull request as ready for review August 20, 2023 03:10

NickGerleman reviewed Aug 21, 2023

View reviewed changes

fabOnReact added 12 commits February 16, 2024 19:34

scroll to the previous position after re-storing the cursor

2e13d47

using scrollRangeToVisible instead of scrollEnabled=false

e3dfac0

remove scrollEnabled = NO

22b7747

Merge branch 'main' into fix-input-field-scrolls-up

65e9946

add log message to RCTUITextField

6340d60

use self.selectedRange instead of calculating new range

a44d620

adding explanatory comments

358706c

don’t use instance variable, instead use the offsetStart variable.

38a7bf0

Don’t depend on external variables that can break this.

updating comments

b2580e3

updating comments

55d11bf

Merge branch 'main' into fix-input-field-scrolls-up

48b2207

updating comments

aff06a4

fabOnReact marked this pull request as ready for review February 19, 2024 03:17

facebook-github-bot closed this in f6badca Mar 28, 2024

facebook-github-bot added the Merged This PR has been merged. label Mar 28, 2024

fabOnReact mentioned this pull request Jun 5, 2024

upgrade react-native-live-markdown to 0.1.76 Expensify/App#42454

Merged

50 tasks

This was referenced Jun 28, 2024

[CI] Move iOS Executors to M1 #45213

Closed

[Hermes] Bump Hermes #45220

Merged

cipolleschi mentioned this pull request Jul 22, 2024

[RN][0.74]Fix XROS build #45580

Merged

cipolleschi mentioned this pull request Jul 31, 2024

[RN][Hermes] Fore Hermes versions for slices through env var #45840

Merged

dominictb mentioned this pull request Aug 30, 2024

[HOLD for payment 2024-12-17] [HOLD for facebook/react-native#46411[$250] iOS - Chat - Composer not auto scrolled to the bottom when editing message with 10 line breaks Expensify/App#48122

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Multiline TextInput with a fixed height scrolls to the bottom when prepending new lines to the text #38679

Fix Multiline TextInput with a fixed height scrolls to the bottom when prepending new lines to the text #38679

fabOnReact commented Jul 29, 2023 •

edited

Loading

analysis-bot commented Jul 29, 2023 •

edited

Loading

NickGerleman commented Aug 16, 2023

fabOnReact commented Aug 17, 2023

fabOnReact commented Aug 19, 2023

fabOnReact commented Aug 20, 2023

NickGerleman Aug 21, 2023

fabOnReact Aug 23, 2023

facebook-github-bot commented Aug 25, 2023

fabOnReact commented Aug 31, 2023

NickGerleman commented Sep 3, 2023

fabOnReact commented Feb 28, 2024

NickGerleman commented Feb 28, 2024

NickGerleman commented Mar 14, 2024

facebook-github-bot commented Mar 28, 2024

facebook-github-bot commented Mar 28, 2024

Fix Multiline TextInput with a fixed height scrolls to the bottom when prepending new lines to the text #38679

Fix Multiline TextInput with a fixed height scrolls to the bottom when prepending new lines to the text #38679

Conversation

fabOnReact commented Jul 29, 2023 • edited Loading

Summary:

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

Changelog:

Test Plan:

analysis-bot commented Jul 29, 2023 • edited Loading

NickGerleman commented Aug 16, 2023

fabOnReact commented Aug 17, 2023

fabOnReact commented Aug 19, 2023

fabOnReact commented Aug 20, 2023

NickGerleman Aug 21, 2023

Choose a reason for hiding this comment

fabOnReact Aug 23, 2023

Choose a reason for hiding this comment

Details of the changes in the AttributedString attributes (paragraph alignment) when adding new line

facebook-github-bot commented Aug 25, 2023

fabOnReact commented Aug 31, 2023

NickGerleman commented Sep 3, 2023

fabOnReact commented Feb 28, 2024

NickGerleman commented Feb 28, 2024

NickGerleman commented Mar 14, 2024

facebook-github-bot commented Mar 28, 2024

facebook-github-bot commented Mar 28, 2024

fabOnReact commented Jul 29, 2023 •

edited

Loading

analysis-bot commented Jul 29, 2023 •

edited

Loading