feat(iOS): add all supported ReturnKeyTypes

[elencho]

Summary:

Related issue: https://github.com/facebook/react-native/issues/43243 As documentation stated for the ReturnKeyType prop on the input there are different options, like "next", "go" and etc. They are actually supported on the iOS side but we can't use it in our react native app, because of the hardcoded version of DoneButton on existing code: So, I decided to add support for types which were in documentation

Changelog:

[IOS] [ADDED]: ReturnKeyTypes

Test Plan:

ran yarn jest react-native-codegen and yarn jest react-native, both successfully:

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	19,544,176	-15,551
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	22,900,330	-12,606
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: c13790ff1d11ef95ad04f7d7dc87f09a9cbf7025 Branch: main

[facebook-github-bot]

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

[cipolleschi]

@elencho gentle reminder that there are some changes requested.

[elencho]

@elencho gentle reminder that there are some changes requested.

Sorry I just saw your comment, will fix in few days and get back to you. Thanks for your feedback!!

[elencho]

NOTE: I removed Default value, because it was causing a bug: when returnKey was undefined from js it was still showing "Default"

[cipolleschi]

NOTE: I removed Default value, because it was causing a bug: when returnKey was undefined from js it was still showing "Default"

Why you think it is a bug? I feel like it is the right behavior to fall back to Default if nothing is specified, no?

[elencho]

NOTE: I removed Default value, because it was causing a bug: when returnKey was undefined from js it was still showing "Default"

Why you think it is a bug? I feel like it is the right behavior to fall back to Default if nothing is specified, no?

Okay i will revert

[facebook-github-bot]

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

[cipolleschi]

/rebase - this comment automatically rebase on top of main

[elencho]

/rebase - this comment automatically rebase on top of main

is something wrong with the tests? that is blocking the merge

[cipolleschi]

@elencho No, nothing wrong. I have to run the internal linter (which is different from the OSS one, unfortunately) and ask for a review internally. I'm still catching up from AppJS and the ReactConf, that's why I have been a little slow, these days.

[facebook-github-bot]

@cipolleschi merged this pull request in facebook/react-native@ed9978b8de307d11def9190a461fc18b881800e0.

[github-actions[bot]]

This pull request was successfully merged by @elencho in ed9978b8de307d11def9190a461fc18b881800e0.

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

[kirillzyusko]

@elencho @cipolleschi so, I tested [email protected] and... Is there a way to hide this inputAccessoryView? I always have it right now (I specified returnKeyType="none" but it still present):

And presence of two toolbars looks really weird (if I'm using KeyboardToolbar from react-native-keyboard-toolbar):

For me it looks like a breaking change and it prevents me to migrate to 0.75 version 😐

[kirillzyusko]

I have a feeling like it had to be removed https://github.com/facebook/react-native/pull/43362#issuecomment-2101581007

Because right now when you didn't specify returnTypeKey (i. e. it's undefined) the toolbar always shows "Default" and this button doesn't do anything (i. e. I press, but nothing happens).

I have a strong feeling that it's a regression 😅 Correct me if I'm wrong 😅

[elencho]

Hello @kirillzyusko , thank you for feedback. Just tested and you are right, I would love to contribute again here. Can I investigate this and come back with fixed solution?

[kirillzyusko]

Can I investigate this and come back with fixed solution?

Yes, please 🙏

I also would like to get @cipolleschi opinion on that - whether it should be considered as a release blocker or not and what is the best way to resolve it here.

The straightforward approach is to remove UIReturnKeyDefault from the Set. Or if undefined or null is coming from JS, then do not substitute the value with UIReturnKeyDefault by default and allow that value to be nil, but it may affect other places of the framework (just afraid to get NullPointerException somewhere in the code 😅).

Anyway, it's just suggestions 🙃

[cipolleschi]

Hi There, sorry for the issue!

I also would like to get @cipolleschi opinion on that - whether it should be considered as a release blocker or not and what is the best way to resolve it here.

We cannot stop the release, which is happening tomorrow, at this point, especially for this that has not been reported by anyone else. However, we are planning todo 75.1 next week, so this fix can go in there.

[kirillzyusko]

We cannot stop the release, which is happening tomorrow, at this point, especially for this that has not been reported by anyone else.

@cipolleschi got it!

However, we are planning todo 75.1 next week, so this fix can go in there.

That would be amazing to include this fix in 0.75.1 👍 Do you think that removing UIReturnKeyDefault from a set can be included in 0.75.1? Or do you want to have a different fix?

[elencho]

@kirillzyusko I will try to include it, but if there wont be another fix I will remove that

[cipolleschi]

@elencho just to understand, do you plan to look into this issue?

[elencho]

@elencho just to understand, do you plan to look into this issue?

Yes, I will contribute tomorrow

[elencho]

New PR link @cipolleschi @kirillzyusko