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

[modules-core] Add macOS support #26186

Merged
merged 5 commits into from
Jan 3, 2024
Merged

[modules-core] Add macOS support #26186

merged 5 commits into from
Jan 3, 2024

Conversation

tsapeta
Copy link
Member

@tsapeta tsapeta commented Jan 1, 2024

Why

Adds support for macOS platform to the expo-modules-core package. It's been separated from #22796.

How

  • Added appropriate aliases so we can refer to UIKit-specific classes on macOS as well. This is also what react-native-macos does, but we need to expose that to Swift.
  • Removed UIKit imports (replaced with Platform.h if needed).
  • Added custom implementation when necessary, e.g. app lifecycle notifications, finding the root view, getting the current view controller.

Test Plan

Tested in https://github.com/tsapeta/expo-macos-example as part of the bigger PR #22796

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Jan 1, 2024
@tsapeta tsapeta force-pushed the @tsapeta/macos/modules-core branch from 68de3da to 08825de Compare January 1, 2024 15:50
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jan 1, 2024
@tsapeta tsapeta marked this pull request as ready for review January 1, 2024 18:26
@@ -44,12 +43,32 @@ - (id)forwardingTargetForSelector:(SEL)selector

#if __has_include(<React-RCTAppDelegate/RCTAppDelegate.h>) || __has_include(<React_RCTAppDelegate/RCTAppDelegate.h>)

- (UIView *)findRootView:(UIApplication *)application
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't see any code to use the new method?

Copy link
Member Author

@tsapeta tsapeta Jan 2, 2024

Choose a reason for hiding this comment

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

Oops, looks like it remained here after resolving conflicts. You removed that method a while ago 😅
Edit: I removed it from here!

@@ -69,7 +88,7 @@ - (UIView *)createRootViewWithBridge:(RCTBridge *)bridge
moduleName:moduleName
initialProperties:initProps
fabricEnabled:enableFabric];
#if !TARGET_OS_TV
#if !TARGET_OS_TV && !TARGET_OS_OSX
rootView.backgroundColor = UIColor.systemBackgroundColor;
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure whether we should use NSColor.windowBackgroundColor for macos?

Copy link
Member Author

Choose a reason for hiding this comment

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

NSView doesn't have the backgroundColor property, but looks like we can change that on its layer. I'll change it.

@tsapeta tsapeta merged commit f233025 into main Jan 3, 2024
11 checks passed
@tsapeta tsapeta deleted the @tsapeta/macos/modules-core branch January 3, 2024 08:08
tsapeta added a commit that referenced this pull request Jan 10, 2024
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Jan 10, 2024
onizam95 pushed a commit to onizam95/expo-av-drm that referenced this pull request Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants