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

Fix onSubmitEditing firing on delete/tab #442

Merged
merged 2 commits into from
Jun 17, 2020

Conversation

tido64
Copy link
Member

@tido64 tido64 commented Jun 10, 2020

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

onSubmitEditing is fired on delete/tab keys because we check -[RCTBaseTextInputView textInputShouldReturn], which also sends a submit event, too early.

Resolves #429

Changelog

[macOS] [Fixed] - Fix onSubmitEditing firing on delete/tab

Test Plan

  1. Launch RNTester and go to TextInput screen
  2. Filter on "submit"
  3. Type something in the multiline TextInput instance and try deleting/tabbing.
Microsoft Reviewers: Open in CodeFlow

`-[RCTBaseTextInputView textInputShouldReturn]` also sends a submit
event.
@tido64 tido64 requested a review from tom-un as a code owner June 10, 2020 11:05
@@ -353,7 +353,7 @@ - (BOOL)textView:(NSTextView *)textView doCommandBySelector:(SEL)commandSelector
BOOL commandHandled = NO;
id<RCTBackedTextInputDelegate> textInputDelegate = [_backedTextInputView textInputDelegate];
// enter/return
if (textInputDelegate.textInputShouldReturn && (commandSelector == @selector(insertNewline:) || commandSelector == @selector(insertNewlineIgnoringFieldEditor:))) {
if ((commandSelector == @selector(insertNewline:) || commandSelector == @selector(insertNewlineIgnoringFieldEditor:)) && textInputDelegate.textInputShouldReturn) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@tom-un, @shergin, @TheSavior: I don't have the full context on why textInputShouldReturn also sends a submit event. Is this something we should fix upstream to avoid future surprises?

Copy link
Member Author

@tido64 tido64 Jun 10, 2020

Choose a reason for hiding this comment

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

Also, the current behaviour is to always submit when pressing Enter. Is that the correct behaviour? How would one normally input a newline in a multiline TextInput? I had assumed that one would press Enter for newline, and submit with Cmd+Enter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it to submit only on ⌘+Enter.

@tom-un tom-un merged commit 8481328 into microsoft:master Jun 17, 2020
@tido64 tido64 deleted the fix-onSubmitEditing branch June 17, 2020 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OnSubmitEditing fires at wrong time on TextInput
2 participants