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

[menu-bar] Migrate MenuBar to Expo Modules #133

Merged
merged 9 commits into from
Jan 18, 2024

Conversation

alanjhughes
Copy link
Contributor

Why

Expo modules is now supported on MacOS

How

Migrate MenuBar to use Expo Modules

Test Plan

Running dev and release side by side. Everything seems to work as expected.

@gabrieldonadel
Copy link
Member

Hey @alanjhughes, thanks for opening this PR and migrating the MenuBar module.

I've been testing this locally and I'm getting this error when trying to setEnvVars. I'm not sure if we can use the speed syntax with requireNativeModule

Screen.Recording.2024-01-11.at.22.39.40.mov

@alanjhughes
Copy link
Contributor Author

alanjhughes commented Jan 12, 2024

@gabrieldonadel Should have realised that when I had to explicitly add the constants. Those objects mustn't be iterable. Updated

Copy link
Member

@gabrieldonadel gabrieldonadel left a comment

Choose a reason for hiding this comment

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

Nice work, just one more thing and we should be good to go

apps/menu-bar/modules/menu-bar/ios/MenuBarExceptions.swift Outdated Show resolved Hide resolved
@tsapeta tsapeta force-pushed the @tsapeta/install-expo-modules branch 2 times, most recently from 247da92 to 19d0d03 Compare January 18, 2024 10:15
Base automatically changed from @tsapeta/install-expo-modules to main January 18, 2024 13:52
@alanjhughes alanjhughes force-pushed the @alanhughes/migrate-menu-bar-to-expo-modules branch from f11ad6f to f979c69 Compare January 18, 2024 14:52
Copy link
Member

@gabrieldonadel gabrieldonadel left a comment

Choose a reason for hiding this comment

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

LGTM

CHANGELOG.md Outdated Show resolved Hide resolved
@alanjhughes alanjhughes merged commit ef63040 into main Jan 18, 2024
1 check passed
@alanjhughes alanjhughes deleted the @alanhughes/migrate-menu-bar-to-expo-modules branch January 18, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants