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

cancel local notification on android #2122

Closed
aminerol opened this issue Aug 23, 2021 · 9 comments
Closed

cancel local notification on android #2122

aminerol opened this issue Aug 23, 2021 · 9 comments

Comments

@aminerol
Copy link

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch react-native-push-notification@8.0.0 for the project I'm working on.

due to deprecation of cancelLocalNotifications in favor of cancelLocalNotification, the function in component/index.android.js needs to be renamed as well to cancelLocalNotification

Here is the diff that solved my problem:

diff --git a/node_modules/react-native-push-notification/component/index.android.js b/node_modules/react-native-push-notification/component/index.android.js
index 3496c04..0a609cb 100644
--- a/node_modules/react-native-push-notification/component/index.android.js
+++ b/node_modules/react-native-push-notification/component/index.android.js
@@ -36,8 +36,8 @@ NotificationsComponent.prototype.unsubscribeFromTopic = function(topic) {
 	RNPushNotification.unsubscribeFromTopic(topic);
 };
 
-NotificationsComponent.prototype.cancelLocalNotifications = function(details) {
-	RNPushNotification.cancelLocalNotifications(details);
+NotificationsComponent.prototype.cancelLocalNotification = function(details) {
+	RNPushNotification.cancelLocalNotification(details);
 };
 
 NotificationsComponent.prototype.clearLocalNotification = function(details, tag) {

This issue body was partially generated by patch-package.

@dimadeveatii
Copy link
Contributor

I can confirm this is a valid issue.
On Android, cancelLocalNotification method doesn't work.

@Dallas62 can we get a hotfix released soon? We're using this library in production and this issue prevents us from making a release.

@Dallas62
Copy link
Collaborator

cancelLocalNotification(s) are working the same way, expect for the warning.
The fix provided is the same as suggested, just removed extra steps.
If you have issues, please describe the problem with steps.

For the unsubscribeTopic, the fix will come later, the warning is not blocking.

@dimadeveatii
Copy link
Contributor

@Dallas62 thanks for a quick reaction.

When calling PushNotification.cancelLocalNotification method it gets to this line:

return this.callNative('cancelLocalNotification', [notificationId]);

Now, in component/index.android.js there is no such method as cancelLocalNotification, only cancelLocalNotifications. As a result, this line gets executed, and nothing happens:

Fix:
There should be defined method cancelLocalNotification in component/index.android.js file.

@dimadeveatii
Copy link
Contributor

Cancelling local notifications in android is completely broken in v8.0.0, regardless that you call the deprecated cancelLocalNotifications or the new cancelLocalNotification methods.

@Dallas62
Copy link
Collaborator

Dallas62 commented Aug 24, 2021 via email

@dimadeveatii dimadeveatii mentioned this issue Aug 24, 2021
@dimadeveatii
Copy link
Contributor

Thanks @Dallas62 !
When would we be able to see it released on npm?

@Dallas62
Copy link
Collaborator

Tonight if the PR is ready.

@dimadeveatii
Copy link
Contributor

The PR changes are very straightforward, please see it here: https://github.com/zo0r/react-native-push-notification/pull/2125/files

@Dallas62
Copy link
Collaborator

Just release the patch.
Thanks for this catch & PR!

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

No branches or pull requests

3 participants