Skip to content
This repository has been archived by the owner on Sep 4, 2020. It is now read-only.

Remove unnecessary didRegisterUserNotificationSettings implementation, which did break cordova-local-notifications-plugin as a side effect #215

Merged
merged 2 commits into from
Oct 10, 2015

Conversation

EdMcBane
Copy link
Contributor

Problem is due to conflicting implementations of didRegisterUserNotificationSettings that prevents cordova-local-notifications-plugin from being notified of successful calls to registerUserNotificationSettings.

If implementing didRegisterUserNotificationSettings does indeed become necessary, consider using some common notification mechanism such as the one at https://github.com/katzer/cordova-common-registerusernotificationsettings to avoid conflicts with other plugins.

Fixes #183 and katzer/cordova-plugin-local-notifications#695

Conflicting implementation of didRegisterUserNotificationSettings
prevents cordova-local-notifications-plugin from being notified of success.
emoving it fixes the problem as it was redundant (simply calls
registerForRemoteNotifications, which is called anyway from PushPlugin:init
right after registerUserNotificationSettings).

Fixes phonegap/issues/183 and
katzer/cordova-plugin-local-notifications/issues/695
@lylepratt
Copy link

+1 for merging this! Having this same problem now that we are forced to use the new phonegap-plugin-push plugin.

@n9986
Copy link

n9986 commented Oct 10, 2015

Agreed. +1 to merge this ASAP.

macdonst added a commit that referenced this pull request Oct 10, 2015
Remove unnecessary didRegisterUserNotificationSettings implementation, which did break cordova-local-notifications-plugin as a side effect
@macdonst macdonst merged commit 57a49ea into phonegap:master Oct 10, 2015
@macdonst
Copy link
Member

Much thanks to @EdMcBane

@lylepratt
Copy link

Thank you for the quick merge!

@macdonst
Copy link
Member

That's what open source is all about.

@katzer
Copy link

katzer commented Dec 2, 2015

👍

@lock
Copy link

lock bot commented Jun 3, 2018

This thread has been automatically locked.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants