Skip to content

Commit 9f58406

Browse files
authored
fix(macOS): reconcile coordinate systems for mouse events (#2403)
## Summary: On iOS, [-hitTest:withEvent:](https://developer.apple.com/documentation/uikit/uiview/hittest(_:with:)?changes=la_3_2_7_2_8&language=objc) works with the coordinate system of the view that is being hit tested. On the other hand, on macOS, [-hitTest:](https://developer.apple.com/documentation/appkit/nsview/hittest(_:)?language=objc) works with the coordinate system of the *superview*. React Native macOS currently appears to mix these two coordinate systems, which can lead to unexpected behaviors, one of which that I have personally observed is `mouseDown:` events being delievered to a view that does not pass OS-level hit testing (e.g. the hit point is completely outside of the view that was allegedly hit, even if accounting for not clipping to bounds and any possible descendant view larger than the hit view). To enforce consistent behavior, we will use the macOS style coordinate system for APIs that follow the macOS signature, and the iOS style coordinate system for APIs that follow the iOS signature, or are obviously React Native APIs. Several APIs were scrubbed for calls and implementation. The following APIs shall work with superview's coordinate space: * `- hitTest:` The following APIs shall work with the view's (i.e. self) coordinate space: * `RCTUIViewHitTestWithEvent` * `- reactTagAtMouseLocationFromEvent:` * `- reactTagAtPoint:` * `- hitTest:withEvent:` * `- betterHitTest:withEvent:` * `- pointInside:withEvent:` With the proper coordinate system usage, we no longer need new RN components inheriting from NSView to behave in special manners. Updating documentation to reflect that. ## Test Plan: Manual Testing: * Downstream scenario around misrouted mouseDown: events was verified fixed * Sanity tested mouse clicks -- everything seems to work as expected
1 parent 85ddc46 commit 9f58406

File tree

7 files changed

+27
-54
lines changed

7 files changed

+27
-54
lines changed

docs/WritingNativeComponents.md

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,25 +12,5 @@ internal class FixedVisualEffectView: NSVisualEffectView {
1212
override var isFlipped: Bool {
1313
return true
1414
}
15-
16-
/// This subclass is necessary due to differences in hitTest()'s implementation between iOS and macOS
17-
/// On iOS / UIKit, hitTest(_ point:, with event:) takes a point in the receiver's local coordinate system.
18-
/// On macOS / AppKit, hitTest(_ point) takes a point in the reciever's superviews' coordinate system.
19-
/// RCTView assumes the iOS implementation, so it has an override of hitTest(_ point). Let's copy the
20-
/// implementatation to our native component, so that clicks for subviews of type RCTView are handled properly.
21-
/// Another solution would be to add an RCTView subview that covers the full bounds of our native view
22-
open override func hitTest(_ point: NSPoint) -> NSView? {
23-
var pointForHitTest = point
24-
for subview in subviews {
25-
if let subview = subview as? RCTView {
26-
pointForHitTest = subview.convert(point, from: superview)
27-
}
28-
let result = subview.hitTest(pointForHitTest)
29-
if (result != nil) {
30-
return result
31-
}
32-
}
33-
return nil
34-
}
3515
}
3616
```

packages/react-native/React/Base/RCTTouchHandler.m

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,9 @@ - (void)_recordNewTouches:(NSSet *)touches
128128
#else // [macOS
129129
// -[NSView hitTest:] takes coordinates in a view's superview coordinate system.
130130
// The assumption here is that a RCTUIView/RCTSurfaceView will always have a superview.
131-
CGPoint touchLocation = [self.view.superview convertPoint:touch.locationInWindow fromView:nil];
132-
NSView *targetView = [self.view hitTest:touchLocation];
131+
NSView *superview = [[self view] superview];
132+
const CGPoint touchLocationInSuperview = [superview convertPoint:touch.locationInWindow fromView:nil];
133+
NSView *targetView = [self.view hitTest:touchLocationInSuperview];
133134
// Don't record clicks on scrollbars.
134135
if ([targetView isKindOfClass:[NSScroller class]]) {
135136
continue;
@@ -148,8 +149,7 @@ - (void)_recordNewTouches:(NSSet *)touches
148149
} else {
149150
_shouldSendMouseUpOnSystemBehalf = NO;
150151
}
151-
touchLocation = [targetView convertPoint:touchLocation fromView:self.view.superview];
152-
152+
153153
while (targetView) {
154154
BOOL isUserInteractionEnabled = NO;
155155
if ([((RCTUIView*)targetView) respondsToSelector:@selector(isUserInteractionEnabled)]) { // [macOS]
@@ -161,7 +161,8 @@ - (void)_recordNewTouches:(NSSet *)touches
161161
targetView = targetView.superview;
162162
}
163163

164-
NSNumber *reactTag = [targetView reactTagAtPoint:touchLocation];
164+
const CGPoint touchLocationInSelf = [targetView convertPoint:touchLocationInSuperview fromView:self.view.superview];
165+
NSNumber *reactTag = [targetView reactTagAtPoint:touchLocationInSelf];
165166
BOOL isUserInteractionEnabled = NO;
166167
if ([((RCTUIView*)targetView) respondsToSelector:@selector(isUserInteractionEnabled)]) { // [macOS]
167168
isUserInteractionEnabled = ((RCTUIView*)targetView).isUserInteractionEnabled; // [macOS]

packages/react-native/React/Base/RCTUIKit.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,10 @@ CGPathRef UIBezierPathCreateCGPathRef(UIBezierPath *path);
484484

485485
NS_INLINE RCTPlatformView *RCTUIViewHitTestWithEvent(RCTPlatformView *view, CGPoint point, __unused UIEvent *__nullable event)
486486
{
487-
return [view hitTest:point];
487+
// [macOS IMPORTANT -- point is in local coordinate space, but OSX expects super coordinate space for hitTest:
488+
NSView *superview = [view superview];
489+
NSPoint pointInSuperview = superview != nil ? [view convertPoint:point toView:superview] : point;
490+
return [view hitTest:pointInSuperview];
488491
}
489492

490493
BOOL RCTUIViewSetClipsToBounds(RCTPlatformView *view);

packages/react-native/React/Base/macOS/RCTUIKit.m

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,10 @@ - (void)setTransform:(CGAffineTransform)transform
454454

455455
- (NSView *)hitTest:(NSPoint)point
456456
{
457-
return [self hitTest:NSPointToCGPoint(point) withEvent:nil];
457+
// IMPORTANT point is passed in super coordinates by OSX, but expected to be passed in local coordinates
458+
NSView *superview = [self superview];
459+
NSPoint pointInSelf = superview != nil ? [self convertPoint:point fromView:superview] : point;
460+
return [self hitTest:pointInSelf withEvent:nil];
458461
}
459462

460463
- (BOOL)wantsUpdateLayer
@@ -505,7 +508,11 @@ - (BOOL)becomeFirstResponder
505508

506509
- (NSView *)hitTest:(CGPoint)point withEvent:(__unused UIEvent *)event
507510
{
508-
return self.userInteractionEnabled ? [super hitTest:NSPointFromCGPoint(point)] : nil;
511+
// [macOS
512+
// IMPORTANT point is expected to be passed in local coordinates, but OSX expects point to be super
513+
NSView *superview = [self superview];
514+
NSPoint pointInSuperview = superview != nil ? [self convertPoint:point toView:superview] : point;
515+
return self.userInteractionEnabled ? [super hitTest:pointInSuperview] : nil;
509516
}
510517

511518
- (BOOL)pointInside:(CGPoint)point withEvent:(__unused UIEvent *)event

packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -680,17 +680,7 @@ - (RCTUIView *)betterHitTest:(CGPoint)point withEvent:(UIEvent *)event // [macOS
680680
}
681681

682682
for (RCTUIView *subview in [self.subviews reverseObjectEnumerator]) { // [macOS]
683-
#if !TARGET_OS_OSX // [macOS]
684683
RCTUIView *hitView = [subview hitTest:[subview convertPoint:point fromView:self] withEvent:event]; // [macOS]
685-
#else // [macOS
686-
// Native macOS views require the point to be in the super view coordinate space for hit testing.
687-
CGPoint hitTestPoint = point;
688-
// Fabric components use the target view coordinate space for hit testing
689-
if ([subview isKindOfClass:[RCTViewComponentView class]]) {
690-
hitTestPoint = [subview convertPoint:point fromView:self];
691-
}
692-
RCTUIView *hitView = [subview hitTest:hitTestPoint withEvent:event]; // [macOS]
693-
#endif // macOS]
694684
if (hitView) {
695685
return hitView;
696686
}
@@ -1533,4 +1523,4 @@ - (NSString *)componentViewName_DO_NOT_USE_THIS_IS_BROKEN
15331523
Class<RCTComponentViewProtocol> RCTViewCls(void)
15341524
{
15351525
return RCTViewComponentView.class;
1536-
}
1526+
}

packages/react-native/React/Views/RCTView.m

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -251,18 +251,7 @@ - (RCTPlatformView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event // [macOS
251251
// of the hit view will return YES from -pointInside:withEvent:). See:
252252
// - https://developer.apple.com/library/ios/qa/qa2013/qa1812.html
253253
for (RCTUIView *subview in [sortedSubviews reverseObjectEnumerator]) { // [macOS]
254-
CGPoint pointForHitTest = CGPointZero; // [macOS
255-
#if !TARGET_OS_OSX // [macOS]
256-
pointForHitTest = [subview convertPoint:point fromView:self];
257-
#else // [macOS
258-
// Paper and Fabric components use the target view coordinate space for hit testing
259-
if ([subview isKindOfClass:[RCTView class]] || [subview respondsToSelector:@selector(updateProps:oldProps:)]) {
260-
pointForHitTest = [subview convertPoint:point fromView:self];
261-
} else {
262-
// Native macOS views require the point to be in the super view coordinate space for hit testing.
263-
pointForHitTest = point;
264-
}
265-
#endif // macOS]
254+
CGPoint pointForHitTest = [subview convertPoint:point fromView:self];
266255
hitSubview = RCTUIViewHitTestWithEvent(subview, pointForHitTest, event); // macOS]
267256
if (hitSubview != nil) {
268257
break;
@@ -1653,4 +1642,4 @@ - (void)keyUp:(NSEvent *)event {
16531642
}
16541643
#endif // macOS]
16551644

1656-
@end
1645+
@end

packages/react-native/React/Views/UIView+React.m

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,10 +573,13 @@ - (NSString *)react_recursiveDescription
573573

574574
// [macOS
575575
#pragma mark - Hit testing
576-
#if TARGET_OS_OSX
576+
#if TARGET_OS_OSX
577+
// IMPORTANT -- point is in local coordinate space, unlike the hitTest: version from OSX which is in super's coordinate space
577578
- (RCTPlatformView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event
578579
{
579-
return [self hitTest:point];
580+
NSView *superview = [self superview];
581+
NSPoint pointInSuper = superview != nil ? [self convertPoint:point toView:superview] : point;
582+
return [self hitTest:pointInSuper];
580583
}
581584
#endif // macOS]
582585

0 commit comments

Comments
 (0)