fix: onChangeText should be called only once if changed via js in a multiline textinput by deepanshushuklad11 · Pull Request #45401 · facebook/react-native

deepanshushuklad11 · 2024-07-12T07:31:32Z

Summary:

Issue was onChangeText was called 5-6 times if maxLength was set in a multiline component and TextInput Value was changed via state update.
`if (_maxLength) {
NSInteger allowedLength = MAX(
_maxLength.integerValue - (NSInteger)backedTextInputView.attributedText.string.length + (NSInteger)range.length,
0);

if (text.length > allowedLength) {
  // If we typed/pasted more than one character, limit the text inputted.
  if (text.length > 1) {
    if (allowedLength > 0) {
      // make sure unicode characters that are longer than 16 bits (such as emojis) are not cut off
      NSRange cutOffCharacterRange = [text rangeOfComposedCharacterSequenceAtIndex:allowedLength - 1];
      if (cutOffCharacterRange.location + cutOffCharacterRange.length > allowedLength) {
        // the character at the length limit takes more than 16bits, truncation should end at the character before
        allowedLength = cutOffCharacterRange.location;
      }
    }
    // Truncate the input string so the result is exactly maxLength
    NSString *limitedString = [text substringToIndex:allowedLength];
    NSMutableAttributedString *newAttributedText = [backedTextInputView.attributedText mutableCopy];
    // Apply text attributes if original input view doesn't have text.
    if (backedTextInputView.attributedText.length == 0) {
      newAttributedText = [[NSMutableAttributedString alloc]
          initWithString:[self.textAttributes applyTextAttributesToText:limitedString]
              attributes:self.textAttributes.effectiveTextAttributes];
    } else {
      [newAttributedText replaceCharactersInRange:range withString:limitedString];
    }
    backedTextInputView.attributedText = newAttributedText;
    _predictedText = newAttributedText.string;

    // Collapse selection at end of insert to match normal paste behavior.
    UITextPosition *insertEnd = [backedTextInputView positionFromPosition:backedTextInputView.beginningOfDocument
                                                                   offset:(range.location + allowedLength)];
    [backedTextInputView setSelectedTextRange:[backedTextInputView textRangeFromPosition:insertEnd
                                                                              toPosition:insertEnd]
                               notifyDelegate:YES];

    [self textInputDidChange];
  }

  return nil; // Rejecting the change.
}}`

This is the original code snippet.
It was happening because of wrong check of maxLength with text length if (text.length > allowedLength) this should be
(text.length > _maxLength.integerValue) and if (allowedLength <= 0) we should not change the string and fire textInputDidChange

Changelog:

[IOS] [FIXED] : Fixing maxLength check which was firing onChange multiple times

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Test Plan:

Tested in Ios
Ran yarn test

analysis-bot · 2024-07-12T08:03:43Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	21,268,015	+136
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	24,465,006	+15
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: d68a177
Branch: main

deepanshushuklad11 · 2024-07-15T10:05:12Z

@cortinico @cipolleschi Any update on this

cipolleschi

Hi @deepanshushuklad11, thanks for the fix and sorry for te late reply. I missed the PR on friday.

I see that this is the fix only for the Old Arch. Can you verify whether the problem exists also in the New Arch and fix it there as well?
The component where I'd start looking into is RCTTextInputComponentView.

Thank you so much!

deepanshushuklad11 · 2024-07-15T14:20:03Z

Hi @deepanshushuklad11, thanks for the fix and sorry for te late reply. I missed the PR on friday.

I see that this is the fix only for the Old Arch. Can you verify whether the problem exists also in the New Arch and fix it there as well? The component where I'd start looking into is RCTTextInputComponentView.

Thank you so much!

@cipolleschi Its not happening on new arch

deepanshushuklad11 · 2024-07-17T05:51:11Z

@cipolleschi Anything else required from my end?

facebook-github-bot · 2024-07-18T12:58:53Z

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

cipolleschi · 2024-07-18T12:59:38Z

Sorry for the delay! I imported the PR, let's see if the internal CI is happy about it!

deepanshushuklad11 · 2024-07-23T17:05:22Z

Sorry for the delay! I imported the PR, let's see if the internal CI is happy about it!

Hey @cipolleschi any update ?

cipolleschi · 2024-07-30T09:30:56Z

Hi @deepanshushuklad11, sorry for the delay, I have been quite busy the past week. I'll try to get to it this week.

facebook-github-bot · 2024-07-31T11:11:33Z

@cipolleschi merged this pull request in d88dd14.

react-native-bot · 2024-07-31T11:11:36Z

This pull request was successfully merged by @deepanshushuklad11 in d88dd14

^{When will my fix make it into a release? | How to file a pick request?}

Summary: Fixes #47563 It seems like a regression from #45401, where it was aimed to fix `onChangeText` being called multiple times when changing the text programmatically in an input with the `multiline` prop set as true. This PR reverts that partially, as the `maxLength` check is not being evaluated correctly before setting the text. Not reverting it completely as when removing the second part of the fix, the `onChangeText` gets called multiple times again. ## Changelog: [IOS] [FIXED] - Fixing TextInput `maxLength` not working in old arch Pull Request resolved: #48126 Test Plan: The issue could be reproduced in the rn-tester. See my videos with the before and after the fix. <details> <summary>Before:</summary> https://github.com/user-attachments/assets/86fd67eb-fc14-469a-a5f8-8e83b49f857c </details> <details> <summary>After:</summary> https://github.com/user-attachments/assets/368383b1-c1bd-4e0b-ac44-c78022462fa0 </details> Reviewed By: cortinico Differential Revision: D67025182 Pulled By: cipolleschi fbshipit-source-id: 720c400eef362618106ae434aef421c7529214fe

fix: onchange called only once if changed via js

c6b3b3f

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jul 12, 2024

cipolleschi requested changes Jul 15, 2024

View reviewed changes

deepanshushuklad11 requested a review from cipolleschi July 15, 2024 15:14

cipolleschi approved these changes Jul 18, 2024

View reviewed changes

facebook-github-bot closed this in d88dd14 Jul 31, 2024

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

mateoguzmana mentioned this pull request Dec 5, 2024

[iOS] Paper: TextInput maxLength is not working in old arch #48126

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: onChangeText should be called only once if changed via js in a multiline textinput#45401

fix: onChangeText should be called only once if changed via js in a multiline textinput#45401
deepanshushuklad11 wants to merge 1 commit intofacebook:mainfrom
deepanshushuklad11:fix/onchange_called_multipletimes

deepanshushuklad11 commented Jul 12, 2024 •

edited

Loading

Uh oh!

analysis-bot commented Jul 12, 2024

Uh oh!

deepanshushuklad11 commented Jul 15, 2024

Uh oh!

cipolleschi left a comment

Uh oh!

deepanshushuklad11 commented Jul 15, 2024

Uh oh!

deepanshushuklad11 commented Jul 17, 2024

Uh oh!

facebook-github-bot commented Jul 18, 2024

Uh oh!

cipolleschi commented Jul 18, 2024

Uh oh!

deepanshushuklad11 commented Jul 23, 2024

Uh oh!

cipolleschi commented Jul 30, 2024

Uh oh!

facebook-github-bot commented Jul 31, 2024

Uh oh!

react-native-bot commented Jul 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

deepanshushuklad11 commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Changelog:

Test Plan:

Uh oh!

analysis-bot commented Jul 12, 2024

Uh oh!

deepanshushuklad11 commented Jul 15, 2024

Uh oh!

cipolleschi left a comment

Choose a reason for hiding this comment

Uh oh!

deepanshushuklad11 commented Jul 15, 2024

Uh oh!

deepanshushuklad11 commented Jul 17, 2024

Uh oh!

facebook-github-bot commented Jul 18, 2024

Uh oh!

cipolleschi commented Jul 18, 2024

Uh oh!

deepanshushuklad11 commented Jul 23, 2024

Uh oh!

cipolleschi commented Jul 30, 2024

Uh oh!

facebook-github-bot commented Jul 31, 2024

Uh oh!

react-native-bot commented Jul 31, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

deepanshushuklad11 commented Jul 12, 2024 •

edited

Loading