Skip to content

Commit

Permalink
Fix Multiline TextInput with a fixed height scrolls to the bottom whe…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
fabOnReact authored and Titozzz committed Jun 18, 2024
1 parent d9e4278 commit e210c7c
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ - (void)setSelectedTextRange:(UITextRange *)selectedTextRange notifyDelegate:(BO
[super setSelectedTextRange:selectedTextRange];
}

// After restoring the previous cursor position, we manually trigger the scroll to the new cursor position (PR 38679).
- (void)scrollRangeToVisible:(NSRange)range
{
[super scrollRangeToVisible:range];
}

- (void)paste:(id)sender
{
_textWasPasted = YES;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,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)scrollRangeToVisible:(NSRange)selectedTextRange;

// 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 @@ -201,6 +201,11 @@ - (void)setSelectedTextRange:(UITextRange *)selectedTextRange notifyDelegate:(BO
[super setSelectedTextRange:selectedTextRange];
}

- (void)scrollRangeToVisible:(NSRange)range
{
// Singleline TextInput does not require scrolling after calling setSelectedTextRange (PR 38679).
}

- (void)paste:(id)sender
{
_textWasPasted = YES;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,9 @@ - (void)_setAttributedString:(NSAttributedString *)attributedString
UITextRange *selectedRange = _backedTextInputView.selectedTextRange;
NSInteger oldTextLength = _backedTextInputView.attributedText.string.length;
_backedTextInputView.attributedText = attributedString;
// Updating the UITextView attributedText, for example changing the lineHeight, the color or adding
// a new paragraph with \n, causes the cursor to move to the end of the Text and scroll.
// This is fixed by restoring the cursor position and scrolling to that position (iOS issue 652653).
if (selectedRange.empty) {
// Maintaining a cursor position relative to the end of the old text.
NSInteger offsetStart = [_backedTextInputView offsetFromPosition:_backedTextInputView.beginningOfDocument
Expand All @@ -604,6 +607,7 @@ - (void)_setAttributedString:(NSAttributedString *)attributedString
offset:newOffset];
[_backedTextInputView setSelectedTextRange:[_backedTextInputView textRangeFromPosition:position toPosition:position]
notifyDelegate:YES];
[_backedTextInputView scrollRangeToVisible:NSMakeRange(offsetStart, 0)];
}
[self _restoreTextSelection];
_lastStringStateWasUpdatedWith = attributedString;
Expand Down

0 comments on commit e210c7c

Please sign in to comment.