zetavg / react-native-system-notification

Android system notifications for React Native. Supports push notifications with GCM integrated.
https://www.npmjs.com/package/react-native-system-notification
244 stars 102 forks source link

Support RN 0.29+: use getCurrentActivity instead of passing a reference to the activity #58

Open kmjennison opened 7 years ago

kmjennison commented 7 years ago

React Native 0.29 moves getPackages() into MainApplication.java. To support 0.29, we should extend ReactContextBaseJavaModule and use getCurrentActivity() to get a reference when needed rather than requiring one at construction time.

See: https://github.com/facebook/react-native/commit/49f20f41546e3ba8e7fe43c84c4c701684d0434d#commitcomment-17965418

andy9775 commented 7 years ago

Did you even manage to get it working?

I tried running this package on RN 0.30.0 and it tells me that getPackages cannot be overriden because the method does not exist in the parent class. Further, if I add new NotificationPackage(this) to my MainApplication.java file it throws a wrong number of arguments error (NotificationPackage constructor is not expecting an argument).

kmjennison commented 7 years ago

@andy9775 No, I haven't had time to work on this yet. We're still running an older version of RN until our dependencies work with RN >0.28.

Note that you won't be able to reference the activity in MainApplication.java with this, because MainApplication is not an activity like ReactActivity.java was. This library has to change, not just the way you're integrating it into your app.

Here's an example of a PR from another React Native library that fixed this problem: https://github.com/yamill/react-native-orientation/pull/85/files

To use this library now, you can run an older version of RN, or if you have the time, submit a PR for this library.

andy9775 commented 7 years ago

Ok thanks for the link.