Skip to content

Conversation

@IslamRustamov
Copy link
Contributor

@IslamRustamov IslamRustamov commented Nov 15, 2025

Summary:

This PR fixes this issue: #54542

When we assign inputAccessoryViewID prop to TextInput - setDefaultInputAccessoryView is being called on every prop check, which is calling reloadInputViews, which is causing keyboardDidShow event to be emitted again and again.

Changelog:

[IOS] [FIXED] - Moved if condition related to inputAccessoryViewID in a different place to reduce reloadInputViews calls

Test Plan:

Use the most basic listener to check how often the event is being fired:

  useEffect(() => {
    const showSubscription = Keyboard.addListener('keyboardDidShow', () => {
      console.log('Keyboard Shown');
    });
    return () => {
      showSubscription.remove();
    };
  }, []);

Would be appreciated if anyone hinted whether this change might cause the "input views were not reloaded at the right time" problem.

@meta-cla meta-cla 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 Nov 15, 2025
@facebook-github-bot 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 Nov 15, 2025
@IslamRustamov
Copy link
Contributor Author

IslamRustamov commented Nov 15, 2025

Maybe this even can be written like this:

...

  if (newTextInputProps.inputAccessoryViewID != oldTextInputProps.inputAccessoryViewID) {
    [self _setInputAccessoryViewID:RCTNSStringFromString(newTextInputProps.inputAccessoryViewID)];
  }
  
...
 
 - (void)_setInputAccessoryViewID:(NSString*)inputAccessoryViewID
{
    if (inputAccessoryViewID.length != 0) {
      _backedTextInputView.inputAccessoryViewID = inputAccessoryViewID;
      if (_backedTextInputView.isFirstResponder) {
        [_backedTextInputView reloadInputViews];
      }
    } else {
      _backedTextInputView.inputAccessoryViewID = NULL;
    }
}

upd: Also I believe that we should write:

if (_backedTextInputView.inputAccessoryViewID.length != 0) {

instead of

if (_backedTextInputView.inputAccessoryViewID) {

because RCTNSStringFromString will return "" in case of empty inputAccessoryViewID, so when we set inputAccessoryViewID and then remove it - those if cases will execute (because "" in Objective-C is true)

Correct me if I'm wrong

@yashhhh04
Copy link

🤖 Automated Code Review

📊 Pull Request Review Summary

Overview

This pull request addresses an issue related to excessive emissions of the keyboardDidShow event in the iOS implementation of React Native's TextInput. The change optimizes the prop update logic for inputAccessoryViewID, aiming to reduce unnecessary calls to reloadInputViews when updating properties.

🎯 Key Findings

  • Total Issues Found: 0
  • Critical: 0
  • High: 0
  • Medium: 0
  • Low: 0

🔍 Detailed Issues

🔴 Critical Issues

No critical issues found.

🟠 High Priority Issues

No high priority issues found.

🟡 Medium Priority Issues

No medium priority issues found.

🟢 Low Priority Issues

No low priority issues found.

✅ Positive Aspects

  • The change effectively addresses the problem of excessive event emissions by altering the condition under which reloadInputViews is called.
  • The additional comments provide context for the changes, improving maintainability and clarity for future developers.
  • The test plan is straightforward and relevant for validating the changes made in this PR.

📝 Overall Assessment

This pull request provides a clear and effective solution to the issue of excessive keyboardDidShow event emissions. The adjustments enhance performance by reducing the number of unnecessary calls to reloadInputViews. The code is readable, and best practices have been followed. No additional documentation or tests appear to be required at this time. Overall, this is a well-implemented fix that adheres to the expected standards. Great job!

✅ No issues found! Code looks good.


Generated by PR Review Agent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants