-
Notifications
You must be signed in to change notification settings - Fork 994
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
Fix for new cordova paths #213 #439
base: master
Are you sure you want to change the base?
Conversation
Cordova 7.0+ changed the android paths structure, this should fix that problem and also maintain compatibility with older versions
Works for me. |
@mburger81 this plugin seems to be abandoned :/ i gave up and used the other firebase plugin (https://github.com/arnesson/cordova-plugin-firebase), which i gave up too when i saw that it needs the local notifications plugin that doesn't support ios 9, so i gave up on firebase and went to onesignal that's really simple to setup to push notifications for cordova. |
@Grohden you could fork the plugin and release it on npm resolving the issue and make the plugin open to onayone which would like to use it? For us the problem using cordova-plugin-firebase is not, not supporting ios9, today only about 5% are less then ios10. But this plugin is still only in beta. We had used before onesignal, but we removed it for this reason: Using onesignal put's another cloud service in the middle. onesignal makes a push on firebase, so you have this iter "our server -> onesignal server -> firebase server -> device" a lot of point of failures in my opinion :) |
@mburger81 I'm aware of that detail, but from my point of view My only concern about onesignal is the company future, but that doesn't seems to be uncertain. About keeping a version of the plugin: i don't know Objective-C or Swift, neither do i own an apple device (including a mac), so can't help in that part. I would help with Android and JS part in my spare time, which will be almost nonexistent in the next month when i get back to university, so i can't maintain any projects for the next year.. but i still can help anyone's fork |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please get this fix merged;
I would love to help but I'm not a part of the team. I ended up just doing my locally. It seems that @fechanique is not interested in this project anymore. |
@doejon i've merged the fix (there were a lot of changes on the file since i fixed), but i don't use this plugin anymore and i could not test, i would appreciate if anyone can test it and confirm if it's still working. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need reverse strings (or !isNewFolderStructures) and replace "platforms/android/app" on "platforms/android/app/"
var gServicesWritePath = isNewFolderStructures ? "platforms/android/" : "platforms/android/app" ;
var stringsPath = (isNewFolderStructures ? "platforms/android" : "platforms/android/app/src/main") + "/res/values/strings.xml"
@Grohden All right, I'm sorry |
This should fix the new cordova folder structures ( #213 )