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

Conversation

@merrygobyebye
Copy link
Contributor

@merrygobyebye merrygobyebye commented Oct 8, 2018

Add compatibility with katzer/cordova-plugin-local-notifications 0.9.0-beta.3

Description

  • Add code to save previous UNUserNotificationCenterDelegate and call it for all non-push notifications
  • Similar changes have been made to katzer/cordova-plugin-local-notifications in this pull request

Related Issue

#2266

Motivation and Context

This solves the problem of this plugin clobbering any previously assigned delegate to the UNUserNotificationCenter. As @wezzle mentioned, "this is something all plugins working with the notification center should do as there can only be one delegate set on the notification center."

How Has This Been Tested?

  1. Create a sample application with both cordova-plugin-local-notifications and phonegap-plugin-push installed
  2. Schedule a local notification for several seconds in the future, background the app, and observe that it shows up in the notification center
  3. Send a push notification while the app is in the foreground and observe that the on('notification') callback is executed
  4. Send a push notification while the app is in the background and observe that it appears in the notification center

Environment:
[email protected]
Cordova [email protected]
iPhone X [email protected]

Screenshots (if appropriate):

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@merrygobyebye merrygobyebye changed the title Issue-2266: Add compatibility with katzer/cordova-plugin-local-notifications 0.9.0-beta.3 Issue #2266: Add compatibility with katzer/cordova-plugin-local-notifications 0.9.0-beta.3 Oct 8, 2018
@merrygobyebye merrygobyebye force-pushed the issue-2226-katzer-cordova-plugin-local-notifications-compatibility branch from d14be79 to 34b48c7 Compare October 8, 2018 13:48
@merrygobyebye merrygobyebye force-pushed the issue-2226-katzer-cordova-plugin-local-notifications-compatibility branch from 34b48c7 to ab1410f Compare October 8, 2018 14:28
@dpalou
Copy link

dpalou commented Feb 13, 2019

Thanks to this patch I'm able to receive 'notification' callbacks when the app receives a push notification in foreground. With the master branch the callback is never called and the notification isn't displayed.

It would be great if this fix could be integrated so we don't have to use a fork :)

@martin-richter-uk
Copy link

martin-richter-uk commented Feb 27, 2019

Do I need to install only plugin from this pull request? Or * katzer/cordova-plugin-local-notifications in this pull request is also required?

It seems that, in my case, it does not fix the issue when the app is completely killed (not running in the background).

@gaspachoandalus
Copy link

Did anyone merged this PR in its own fork ?

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.

4 participants