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

[macOS] Fix build on fabric #3154

Merged
merged 9 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions apple/RNGestureHandlerButton.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@
#import "RNGestureHandler.h"

#if TARGET_OS_OSX

#ifdef __cplusplus
#include <react/renderer/core/LayoutMetrics.h>
#endif

@protocol RCTComponentViewProtocol;

@interface RNGestureHandlerButton : NSControl
#else
@interface RNGestureHandlerButton : UIControl
Expand All @@ -19,4 +26,13 @@
@property (nonatomic, assign) UIEdgeInsets hitTestEdgeInsets;
@property (nonatomic) BOOL userEnabled;

#if TARGET_OS_OSX
#ifdef __cplusplus
- (void)mountChildComponentView:(RNGHUIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index;
- (void)unmountChildComponentView:(RNGHUIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index;
- (void)updateLayoutMetrics:(const facebook::react::LayoutMetrics &)layoutMetrics
oldLayoutMetrics:(const facebook::react::LayoutMetrics &)oldLayoutMetrics;
#endif
Copy link
Member

Choose a reason for hiding this comment

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

I actually have no idea whether it's safe to do (have method declarations behind a compile flag but not the implementation.

Why do it this way instead of just changing files where it was included to be Objective-C++?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was already change into Obj-C++. The thing is, changing it to Obj-C++ is not enough and without this flag you'll get compilation errors. We talked about it with @piaskowyk and this was the solution that he came up with. Let us know if you see better one 😅

Copy link
Member

Choose a reason for hiding this comment

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

I meant that everything that includes RNGestureHandlerButton.h should also be changed to Objective-C++ file not only RNGestureHandlerButton.m

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, that makes sense.

Done in 9ebcbff

Copy link
Member

Choose a reason for hiding this comment

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

Checks for __cplusplus shouldn't be necessary in that case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, done in d8d1772

#endif

@end
60 changes: 60 additions & 0 deletions apple/RNGestureHandlerButton.m → apple/RNGestureHandlerButton.mm
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@

#if !TARGET_OS_OSX
#import <UIKit/UIKit.h>
#else
#import <React/RCTUIKit.h>
#endif

#import <React/RCTConversions.h>
#import <React/RCTFabricComponentsPlugins.h>

/**
* Gesture Handler Button components overrides standard mechanism used by RN
* to determine touch target, which normally would reurn the UIView that is placed
Expand Down Expand Up @@ -82,4 +87,59 @@ - (RNGHUIView *)hitTest:(CGPoint)point withEvent:(UIEvent *)event
}
#endif

#if TARGET_OS_OSX
- (void)mountChildComponentView:(RNGHUIView *)childComponentView index:(NSInteger)index
{
if (childComponentView.superview != nil) {
return;
}

if (index < [[self subviews] count]) {
// Get the view currently at your desired index
NSView *existingView = [[self subviews] objectAtIndex:index];

// Now use this to insert your new view above the existing one
[self addSubview:childComponentView positioned:NSWindowAbove relativeTo:existingView];
} else {
// if the index is out of bounds, add the new subview at the end
[self addSubview:childComponentView];
}
}

- (void)unmountChildComponentView:(RNGHUIView *)childComponentView index:(NSInteger)index
{
[childComponentView removeFromSuperview];
}

- (void)updateLayoutMetrics:(const facebook::react::LayoutMetrics &)layoutMetrics
oldLayoutMetrics:(const facebook::react::LayoutMetrics &)oldLayoutMetrics
{
bool forceUpdate = oldLayoutMetrics == facebook::react::EmptyLayoutMetrics;

if (forceUpdate || (layoutMetrics.frame != oldLayoutMetrics.frame)) {
CGRect frame = RCTCGRectFromRect(layoutMetrics.frame);

if (!std::isfinite(frame.origin.x) || !std::isfinite(frame.origin.y) || !std::isfinite(frame.size.width) ||
!std::isfinite(frame.size.height)) {
// CALayer will crash if we pass NaN or Inf values.
// It's unclear how to detect this case on cross-platform manner holistically, so we have to do it on the mounting
// layer as well. NaN/Inf is a kinda valid result of some math operations. Even if we can (and should) detect (and
// report early) incorrect (NaN and Inf) values which come from JavaScript side, we sometimes cannot backtrace the
// sources of a calculation that produced an incorrect/useless result.
RCTLogWarn(
@"-[UIView(ComponentViewProtocol) updateLayoutMetrics:oldLayoutMetrics:]: Received invalid layout metrics (%@) for a view (%@).",
NSStringFromCGRect(frame),
self);
} else {
self.frame = frame;
self.bounds = CGRect{CGPointZero, frame.size};
}
}

if (forceUpdate || (layoutMetrics.displayType != oldLayoutMetrics.displayType)) {
self.hidden = layoutMetrics.displayType == facebook::react::DisplayType::None;
}
}
#endif

@end
4 changes: 4 additions & 0 deletions apple/RNGestureHandlerButtonComponentView.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#ifdef RCT_NEW_ARCH_ENABLED

#if !TARGET_OS_OSX
#import <UIKit/UIKit.h>
#else
#import <React/RCTUIKit.h>
#endif

#import <React/RCTViewComponentView.h>

Expand Down
11 changes: 8 additions & 3 deletions apple/RNGestureHandlerButtonComponentView.mm
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ @implementation RNGestureHandlerButtonComponentView {
RNGestureHandlerButton *_buttonView;
}

+ (BOOL)shouldBeRecycled
{
return NO;
}
j-piasecki marked this conversation as resolved.
Show resolved Hide resolved

// Needed because of this: https://github.com/facebook/react-native/pull/37274
+ (void)load
{
Expand All @@ -40,12 +45,12 @@ - (instancetype)initWithFrame:(CGRect)frame
return self;
}

- (void)mountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
- (void)mountChildComponentView:(RNGHUIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
{
[_buttonView mountChildComponentView:childComponentView index:index];
}

- (void)unmountChildComponentView:(UIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
- (void)unmountChildComponentView:(RNGHUIView<RCTComponentViewProtocol> *)childComponentView index:(NSInteger)index
{
[_buttonView unmountChildComponentView:childComponentView index:index];
}
Expand Down Expand Up @@ -97,7 +102,7 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
const auto &newProps = *std::static_pointer_cast<const RNGestureHandlerButtonProps>(props);

_buttonView.userEnabled = newProps.enabled;
#if !TARGET_OS_TV
#if !TARGET_OS_TV && !TARGET_OS_OSX
_buttonView.exclusiveTouch = newProps.exclusive;
#endif
_buttonView.hitTestEdgeInsets = UIEdgeInsetsMake(
Expand Down
Loading