Only forward click events to NSTextview

[shwanton]

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

Summary

#1346 added the NSTextView api to RCTTextView. With this change, NSTextView is the first responder and RCTTextView apis are ignored.

We still want to use RCTTextView to customize the context menu (on right mouse down), handle accessibility, manage selection etc. We do want the NSTextView to become first responder only on left mouse click.

This change will forward left mouse down clicks to the NSTextView & make it first responder only when selectable.

This change also fixes a bug where a RightMouseDown is not paired with a subsequent RightMouseUp and the RCTTouchHandler gets out of sync. This usually happens when a right mouse down triggers a context menu in a text input. Because text inputs do not fire a MouseUp, the MouseDown events are not cleaned up & the next MouseDown will assert an error. If you click into the selectable text & then click out of the window, the next time you click into the view the RCTTouchHandler will error.

To fix this, we always manually send the cancelTouchWithEvent when we see a RightMouseDown in our textview.

Once this PR lands, we can add support for customizing the context menu,

Changelog

[macOS] [Fixed] - Fix RCTTextView events forwarding to NSTextView & handles right mouse down correctly
[macOS] [Fixed] - Fix RCTTouchHandler events getting out of sync when matching MouseUp events are not registered

Test Plan

Before - Right/Left mouse events called on NSTextView (Also shows the RCTTouchHandler bug!)

CleanShot.2022-11-17.at.09.25.09.mp4

After - Right/Left mouse events called on RCTTextView & Forwarded to NSTextView

CleanShot.2022-11-17.at.09.19.47.mp4

Before manually cancelling event on right mouse down

Right click on text input -> click on text view -> touch events out of sync

CleanShot.2022-11-17.at.08.30.40-converted-trimmed.mp4

After manually cancelling event on right mouse down

Right click on text input -> click on text view -> touch events in sync!

CleanShot.2022-11-17.at.08.31.37-converted.2.mp4

[Saadnajmi]

For context, is this an existing fix that you're porting, or a new PR?

[shwanton]

For context, is this an existing fix that you're porting, or a new PR?

This is multiple PR's from our Messenger fork that has been in production for awhile now. Our implementation differs in that we have a custom application that manages the event loop & sends add/remove messages to the RCTTouchHandler.

We don't want to upstream that api since it required subclassing the application. This version leaves the event handling in the TextView.

[mischreiber]

Possibly a minor thing, but I noticed that three-finger "tap to define" doesn't seem to be forwarded to the NSTextView? I haven't done enough digging yet to figure out which API is responsible for that behavior -- it seems to trigger the regular mouseDown: API, but I haven't spotted the override point for it.

[shwanton]

Possibly a minor thing, but I noticed that three-finger "tap to define" doesn't seem to be forwarded to the NSTextView? I haven't done enough digging yet to figure out which API is responsible for that behavior -- it seems to trigger the regular mouseDown: API, but I haven't spotted the override point for it.

Interesting, I'll need to investigate what we do on Messenger for "tap to define".

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: b4dd208
Branch: main

[shwanton]

@mischreiber It looks like the three-finger "tap to define" actually fires a quick look event.

The first event is a mouse down, the second is the three-finger tap. Notice how it doesn't trigger the mouseDown on TextView.

CleanShot.2022-12-21.at.15.11.49-converted.mp4

If we want to override this action, we can implement a custom quick look action.
https://stackoverflow.com/questions/20507457/how-can-i-override-the-3-finger-tap-behavior-in-a-nstextview

[Saadnajmi]

To give an update, we talked about it internally and do like this change. Once merge conflicts are resolved, we'd be happy to look at it again!