nandorojo / zeego

Menus for React (Native) done right.

https://zeego.dev

MIT License

1.45k stars 42 forks source link

fix: build error on android #27

Closed alantoa closed 1 year ago

alantoa commented 1 year ago

Why

fix some errors on Android:

got this error when I build the example project on Android because react-native-ios-context-menu uses the default android/gradle.properties file, so we need to override it.

SubContent not working on Android, because it uses old Content to pick children.

How

use expo-build-properties plugin to fix it.
use SubContext to replace the pickChildren method params with Context on Android.

Test Plan

remove the expo/android folder and then run expo run:android -d Pixel_5_API_33 or other devices.

Preview

Before

After

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
zeego	❌ Failed (Inspect)		Nov 17, 2022 at 3:08PM (UTC)
zeego-docs	❌ Failed (Inspect)		Nov 17, 2022 at 3:08PM (UTC)

nandorojo commented 1 year ago

Interesting…should we send an issue + PR to react native ios context menu then?

nandorojo commented 1 year ago

Also, naive question, but does removing this dependency on Android inside of react-native.config.js not fix this?

alantoa commented 1 year ago

hey @nandorojo, thanks for the comments.

use react-native.config.js absolutely can fix the react-native-ios-context-menu issue on Android, but I just noticed the @react-native-menu/menu also has a build issue, it's related to the Gradle version, so I created a PR for fix this.

so I think using the expo-build-properties plugin is necessary for now utils the PR merged.

alantoa commented 1 year ago

hey @nandorojo, @react-native-menu/menu has merged and released my PR, so I just:

removed expo-build-properties
upgraded @react-native-menu/menu version to v0.7.2
add react-native-ios-context-menu to react-native config

now is pretty working!

nandorojo commented 1 year ago

@alantoa is any change required for Zeego here?

alantoa commented 1 year ago

hey, @nandorojo no, this PR just includes bug fixes on Android.

fobos531 commented 1 year ago

@nandorojo The android build error part of the issue might be fixed by this https://github.com/dominicstop/react-native-ios-context-menu/pull/59#issuecomment-1405133828

alantoa commented 1 year ago

@nandorojo could we merge this PR? it's safe just fixed some android some issue, I'm already using this patch for a long time. let me know if need I to do any updates! thanks!