Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS] Implement missing showSoftInputOnFocus prop on TextInput on the new arch #35878

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

j-piasecki
Copy link
Contributor

Summary

showSoftInputOnFocus prop was not implemented on the new architecture TextInput component on iOS. I added the implementation based on how it's implemented on the old architecture:

if (showSoftInputOnFocus) {
// Resets to default keyboard.
self.backedTextInputView.inputView = nil;
// Without the call to reloadInputViews, the keyboard will not change until the textInput field (the first
// responder) loses and regains focus.
if (self.backedTextInputView.isFirstResponder) {
[self.backedTextInputView reloadInputViews];
}
} else {
// Hides keyboard, but keeps blinking cursor.
self.backedTextInputView.inputView = [UIView new];
}

Changelog

[IOS] [FIXED] - Added missing showSoftInputOnFocus on TextInput on the new architecture

Test Plan

Added the prop on the TextInput component and checked whether the keyboard opens.

@facebook-github-bot 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 Jan 18, 2023
@analysis-bot
Copy link

analysis-bot commented Jan 18, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 18,171,340 -4
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,540,480 +5
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 4c8e253
Branch: main

Copy link
Contributor

@philIip philIip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for working on this!

cc @sammy-SC

Comment on lines 625 to 626
auto const &currentTextInputProps = *std::static_pointer_cast<TextInputProps const>(_props);
[self _setShowSoftInputOnFocus:currentTextInputProps.traits.showSoftInputOnFocus];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we couple this logic with multiline? regardless, the initial props should go through the updateProps:oldProps pipeline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply, I must've missed the notification 😅. It's there because setting multiline causes the creation of a new underlying component (either RCTUITextView or RCTUITextField) and we need to apply the showSoftInputOnFocus prop again to the newly created view. Otherwise, it will be reset every time the multiline is set.

@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 Sep 27, 2023
Copy link

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Mar 25, 2024
Copy link

Fails
🚫

📋 Missing Changelog - Please add a Changelog to your PR description. See Changelog format

Generated by 🚫 dangerJS against f0eadd5

@j-piasecki
Copy link
Contributor Author

Not stale

@github-actions github-actions bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Mar 26, 2024
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.

None yet

4 participants