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 handling of keyDown/keyUp events by TextInput #1345

Merged
merged 2 commits into from
Sep 7, 2022

Conversation

christophpurrer
Copy link

@christophpurrer christophpurrer commented Aug 10, 2022

Please select one of the following

  • 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

This extends the ability to intercept keyDown and keyUp events to TextInput.
We need this for the ability to insert newlines when holding shift in chat, along with support arrow up/down from the search input.

Question:

Will the change here cause a conflict with #1044 ?

Changelog

[macOS] [Added] - Fix handling of keyDown/keyUp events by TextInput

Test Plan

I have updated the demo for the last test plan

Screen.Recording.2022-08-26.at.3.23.05.PM.mov

Original Test Plan

Using the added rn-tester sample ... capturing keyEvents via keyCastr.app

Before

before.mov

After

after.mov

This extends the ability to intercept `keyDown` and `keyUp` events to `TextInput`.
We need this for the ability to insert newlines when holding shift in chat, along with support arrow up/down from the search input.
@Saadnajmi
Copy link
Collaborator

Saadnajmi commented Sep 7, 2022

One thing I'm a little confused on:

For both singleLine and multiLine TextInput, the doCommandBySelector method should only be invoked if we call [super keyDown] inside the NSTextField right? If that's the case, then we shouldn't need to check for validKeysDown inside doCommandBySelector at all (because we only call [super keyDown] if textInputShouldHandleKeyEvent returns NO, AKA, it's not in validKeysDown. So then why did you still have to do special handling for the "blurOnEscape" feature?

I tried to confirm my theory (calling [super keyDown] eventually calls doCommandBySelector) by setting up a bunch of breakpoints, and commenting out the custom handling, but it seems we do indeed need it...

Copy link
Collaborator

@Saadnajmi Saadnajmi left a comment

Choose a reason for hiding this comment

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

Despite my confusion, change seems sound

@Saadnajmi Saadnajmi merged commit e9791d2 into microsoft:main Sep 7, 2022
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
This extends the ability to intercept `keyDown` and `keyUp` events to `TextInput`.
We need this for the ability to insert newlines when holding shift in chat, along with support arrow up/down from the search input.

Co-authored-by: Scott Kyle <[email protected]>
Co-authored-by: Saad Najmi <[email protected]>
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Mar 10, 2023
This extends the ability to intercept `keyDown` and `keyUp` events to `TextInput`.
We need this for the ability to insert newlines when holding shift in chat, along with support arrow up/down from the search input.

Co-authored-by: Scott Kyle <[email protected]>
Co-authored-by: Saad Najmi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants