-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Allow multiline TextInputs be submittable without blurring #33653
Conversation
Hi @tonyd33! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Base commit: fbe72cf |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Base commit: fbe72cf |
@makovkastar has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
55ad402
to
716cbfd
Compare
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
returnKeyAction(convertRawProp(context, rawProps, | ||
"returnKeyAction", | ||
sourceProps.returnKeyAction, | ||
{})), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we not pass "blurOnSubmit" here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're no longer using "blurOnSubmit" in native code as it's being replaced by "returnKeyAction". This conversion is done in TextInput.js
const RawValue &value, | ||
ReturnKeyAction &result) { | ||
auto string = (std::string)value; | ||
if (string == "default") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where do we set "default"
? Is this set somewhere when the component doesn't pass the prop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it should always be set to one of "blur"
, "blurAndSubmit"
or "newline"
. This is removed now
@makovkastar has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Posting some of @yungsters's feedback on this PR: After thinking about this more, I don't think "return key" is the right name for this prop. The button on mobile keyboards is not necessarily a "return key" like it is on web and desktop platforms. Neither "return key" nor "submit key" are great because both imply one of the behaviors:
That said, I think it would still be better to use terminology that already exists in the interface. So… maybe submitBehavior or submitAction instead of returnKeyAction? |
Yeah. All of returnKeyAction, submitBehavior, and submitAction face a similar problem but the latter two probably address it better than returnKeyAction. This has been changed to submitBehavior. |
@makovkastar has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@tonyd33 any update? this pr sloved my problem , use multiline and onSubmitEditing same time and when press send don't track newline. |
@makovkastar - we'd love to get this merged - is there anything else you need from @tonyd33 on this? |
This pull request was successfully merged by Tony Du in 1e3cb91. When will my fix make it into a release? | Upcoming Releases |
…rop as deprecated in typescript declaration file of TextInput (#45588) Summary: Hi, I just found out that #33653 adds a new prop in `TextInput` that enables multiline `TextInput` be able to submit without blurring. It does that by adding a new prop called `submitBehavior` which accepts `'submit' | 'blurAndSubmit' | 'newline'`: https://github.com/facebook/react-native/blob/700b403e06fdcbcde2a4ade9570eb572431487ea/packages/react-native/Libraries/Components/TextInput/TextInput.js#L195 https://github.com/facebook/react-native/blob/700b403e06fdcbcde2a4ade9570eb572431487ea/packages/react-native/Libraries/Components/TextInput/TextInput.js#L910-L928 It also marks `blurOnSubmit` prop as deprecated since it can now be handled from `submitBehavior`: https://github.com/facebook/react-native/blob/700b403e06fdcbcde2a4ade9570eb572431487ea/packages/react-native/Libraries/Components/TextInput/TextInput.js#L896-L908 However, that PR doesn't update `TextInput.d.ts` file which results Typescript to complain that the type doesn't exist: <img width="760" alt="text_input_error" src="https://github.com/user-attachments/assets/2235cb36-1e4e-4ec9-a8b0-c09728a3336f"> So this PR adds and updates the types in declaration file to support them in Typescript <img width="520" alt="fixed" src="https://github.com/user-attachments/assets/a7a3a0c4-9f3e-4644-bfac-ae60ac21d0f7"> ## Changelog: [GENERAL] [FIXED] - add missing `submitBehavior` prop and mark `blurOnSubmit` prop as deprecated in Typescript declaration file of `TextInput` Pull Request resolved: #45588 Test Plan: Before: <img width="295" alt="before" src="https://github.com/user-attachments/assets/90ed0cd1-c127-4667-bf72-6b5317ea4dd6"> After: <img width="589" alt="after1" src="https://github.com/user-attachments/assets/826002a4-45dc-4f97-882d-7622238ac766"> <img width="833" alt="after2" src="https://github.com/user-attachments/assets/467eeecd-4b0b-4740-ac78-253e3c7aa901"> Reviewed By: christophpurrer Differential Revision: D60107516 Pulled By: dmytrorykun fbshipit-source-id: ce79e41aefc1ef39dc1d44179405cf6a8d5e12de
Summary
For multiline TextInputs, it's possible to send the submit event when pressing the return key only with
blurOnSubmit
. However, there's currently no way to do so without blurring the input and dismissing the keyboard. This problem is apparent when we want to use a TextInput to span multiple lines but still have it be submittable (but not blurrable), like one might want for a TODO list.Changelog
[General] [Added] - Add
returnKeyAction
prop toTextInput
component[General] [Deprecated] - Remove usages of
blurOnSubmit
in native code and convertblurOnSubmit
toreturnKeyAction
in the JavaScript conversion layerTest Plan
Verified old usages of combinations of
blurOnSubmit
andmultiline
matched previous behavior and that the newreturnKeyAction
prop behaves as expected.With the changes, the TODO list example from before now looks like this: