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

Fixed iOS 10 compat on the master branch #1107

Closed
wants to merge 1 commit into from

Conversation

EddyVerbruggen
Copy link

In case merging the iOS10 branch takes a long time, or in case it's not compatible with cloud build tools like PhoneGap Build (which may not use the required Xcode version (8) of said branch), please consider this clean and easy fix for iOS 10 support.

Yes it still uses the old API that was deprecated since iOS 10, but it will be available in iOS for some time as Apple is always purposefully slow removing deprecated API's.

This fixes issues like #1096, #1034 and #1101 and I've also applied this patch to the Telerik fork in order to support the Telerik Platform (also a cloud build tool).

@shi11
Copy link

shi11 commented Oct 5, 2016

this worked for me.

@cmosboss
Copy link

cmosboss commented Oct 6, 2016

I am confused, does this work on the master branch or the iOS branch? I am getting the popup asking for permission, after that no notifications come up. I have tried using the ios 10 fix and this fix together and independently and nothing will work on ios 10. Any ideas?

@EddyVerbruggen
Copy link
Author

Master. Perhaps you're using it wrong? Plz share your code..

@cmosboss
Copy link

cmosboss commented Oct 6, 2016

So currently my process is to create an event that starts right away to request permission

       $cordovaLocalNotification.schedule({
          id: "1234",
          at: new Date().getMinutes(),
          text: "Click notification!!",
          title: "Test notification!",
        }).then(function () {
          console.log("Requesting permission for alert!!!");
        })

This makes ios and android ask for permission to access local notifications. After this I have a button that fires an event for 1 minute away from the current time.

    var alarmTime = new Date();
    alarmTime.setMinutes(alarmTime.getMinutes() + 1);
      $cordovaLocalNotification.schedule({
        id: "1",
        at: alarmTime,
        text: "Second Test Notification",
        title: "Test notification 2r",
      }).then(function () {
        console.log("The notification has been set");
      })

Note this works perfectly on android, after 1 minute I get my local notification, I have also tried making the 'id' a math.rand followed by .toString to check if the issue was ID and the issue persists.

@jmcrthrs
Copy link

jmcrthrs commented Oct 6, 2016

@zocheyado The notification will only show in iOS10 if your application is running in the background. If the application is in the foreground, no notification will appear.

#1084 (comment)

Starting in iOS 10, you can now actually display the notification when the app is in the foreground by using the appropriate arguments in the completion handler in the UNUserNotificationCenterDelegate

https://developer.apple.com/reference/usernotifications/unusernotificationcenterdelegate/1649518-usernotificationcenter?language=objc

@cmosboss
Copy link

cmosboss commented Oct 6, 2016

I just tested putting the application in the background and same issue. @jmcrthrs have you got it working on ios 10 ? If so did you use this fix or the ios 10 branch or neither.

@shi11
Copy link

shi11 commented Oct 6, 2016

interesting to know that this is for master. I manually added it to the ios10 branch. @zocheyado - try changing id to an int. I also didn't use the "at" parameter.

@cmosboss
Copy link

cmosboss commented Oct 6, 2016

@shi11 How then did you set your time? every? Is there a "every minute" option for testing? I will try adding ios10 branch, this fix again, and changing the ID to an int

@shi11
Copy link

shi11 commented Oct 6, 2016

@zocheyado it just fires when it's scheduled - defaults to now.

@cmosboss
Copy link

cmosboss commented Oct 6, 2016

@shi11 Correct me if I am wrong, but defaulting to "now" would no longer work because the notification will only fire in the background yes? I just tested on ios10 + this fix with id set to an int no luck using at, every or the default :/

@shi11
Copy link

shi11 commented Oct 6, 2016

@zocheyado hmm sorry, don't know what else might be causing it. I don't think "now" and whether it happens in the background are related. You've seen this thread: #1096

@ccj242
Copy link

ccj242 commented Oct 13, 2016

@EddyVerbruggen you're awesome! Thanks a bunch. So far so good!

@grahamj
Copy link

grahamj commented Oct 23, 2016

Works, thank you so much!

+1 👍 🐑

@FrankMN
Copy link

FrankMN commented Oct 24, 2016

Works!

@shivam5x
Copy link

shivam5x commented Oct 25, 2016

Hi,
Please provide us command to install - cordova plugin add .

Thanks

@cmosboss
Copy link

I still have not gotten this to work, anyone who could post how they install the repo + how they execute a local notification that actually works on ios 10 would be amazing.

@trembaczm
Copy link

trembaczm commented Oct 25, 2016

I signed up for github just to say I also have the same issue as @zocheyado. I've been waiting a few weeks now hoping that there would be a fix released for IOS 10. For Android everything schedules perfectly - uses the default noise, increments the number of alerts etc.

For IOS9 it schedules an alert with no noise and does not increment the number of alerts as more come in. Possibly my coding error, but I haven't wanted to spend time on it because IOS10 doesn't work.

For IOS10 it asks me if I'd like to allow notifications and even after allowing them (and making sure they are on in the settings) I still never receive any alerts when having the app in OR out of the foreground. It IS running in the background.

FYI -- I manually added the code fix to the files changed.

Any help would be greatly appreciated! Thank you.

@cmosboss
Copy link

To clarify I am having the exact same experiance as @WindierTuna

@EddyVerbruggen
Copy link
Author

EddyVerbruggen commented Oct 25, 2016

@zocheyado and @WindierTuna (welcome on GitHub!) I see a lot of 👍's here for the fix I provided so you may want to give this fork a try until my PR (or any other fix) is merged.

@ccj242
Copy link

ccj242 commented Oct 25, 2016

Yes @EddyVerbruggen's fork is working for me.

@cmosboss
Copy link

@EddyVerbruggen Do you happen to have an example of something incredibly basic so that we can remove the variable of our code? As in "Local notification in 1 minute". Etc so that we can test the fork without wondering if it is our code. Also I assume Cordova plugin add https://github.com/EddyVerbruggen/cordova-plugin-local-notifications

@EddyVerbruggen
Copy link
Author

@zocheyado check the first example here, and that command is correct (remove any old one first of course: cordova plugin ls and then cordova plugin remove id-of-the-plugin-to-remove).

@cmosboss
Copy link

cmosboss commented Oct 25, 2016

@EddyVerbruggen I actually have it working!!! Only thing left is badge number...any idea how to fix that? I assumed it would be tied to the alerts or do I have to set that manually somehow?

Edit it is in the list of options

var options = {
  id:         String,  // A unique id of the notification, best to use a numeric value
  at:         Date,    // This expects a date object
  text:       String,  // The message
  title:      String,  // The title of the message
  every:      String,  // 'minute', 'hour', 'day', 'week', 'month', 'year'
  badge:      Number,  // Displays a numerical badge
  sound:      String,  // The sound to be played (null means no sound)
  json:       String,  // Data to be passed through the notification to your app
  autoClear:  Boolean, // The notification is canceled when the user clicks it
  ongoing:    Boolean, // Prevent clearing the notification (Android only)
};

All bow to the honorable @EddyVerbruggen

@akkineniramana
Copy link

@zocheyado Could you tell me how it got worked for you, what changes you have made to your code to make it working. We also got allow notifications popup in IOS after that no notifications coming.

@akkineniramana
Copy link

@zocheyado We have removed old plugin and added new plugin using below command.
cordova plugin add https://github.com/Telerik-Verified-Plugins/LocalNotification

@trembaczm
Copy link

So it just started coming through! I haven't touched it! I was sitting on the settings screen talking to a coworker and it just randomly came through after like 7 minutes of nothing. Now it's been coming every minute like I typed. This is awesome and not good at the same time, but oh well this is good progress.

@zocheyado Any idea what options are required to be there? thanks so much for the help

@WilliamALiang
Copy link

@zocheyado On the ios10.0.2 work?

@akkineniramana
Copy link

@zocheyado @EddyVerbruggen I removed old plugin added new plugin, now only on simulator notifications are coming. But no notifications are coming on iphone device. Any idea why notifications are not coming on device

@TahirAhmadov
Copy link

This worked for me.

@trembaczm
Copy link

So I've traced my issue to actually be a couple of things. I needed this branch . I needed either an 'at' or an 'every' option. My application must be in the background in order for the notification to schedule (adding a time delay allowed me to hit the home button before it executed and see that it actually is working). My main problem now is that my app is apparently not polling the server when it is in the background and so no notifications are coming unless I go into the app and refresh then leave.

However, the app not running in the background is a completely different problem than this is. Thank you everyone for the help with this plugin!

@EddyVerbruggen
Copy link
Author

@WindierTuna Yeah if you need your app to "do something" when a notification comes in and your app isn't running or launched because of the notification then you'll need to switch to push notifications which can be configured to give your app a limited amount of time to perform operations when a push message arrives.

@trembaczm
Copy link

@EddyVerbruggen There is no notification coming in. This app actually reaches out to a server every 2 minutes to gather a new JSON object. If the new object has more in it than the last one then it schedules a notification to tell you there is new stuff. My issue is that on IOS (android is fine) when you push the home button it makes it so that my app has so network connectivity and then every 2 minutes it fails to reach out to the server. All I need to do is get my app to always be running in the foreground or background. However, I just assumed it would do this automatically like android does and I was wrong. Also, we are too far along in the process to change it so that the server sends notifications although I understand that is ideal.

@rwillett
Copy link
Collaborator

Eddy,

Just to let you know we used your fork on our Jambuster app, no changes to our code whatsoever and it works very well. Form start to finish 10 mins.

Thanks

Rob

@rwillett
Copy link
Collaborator

rwillett commented Nov 2, 2016

Something fundamental has changed in IOS 10 with how local notifications work. We're unsure if its IOS10 or this plugin.

I'll use examples.

Our app monitors a users GPS position when driving and alerts them if they drive into London's Congestion Zone. When the user crosses into the congestion zone, we set up three reminders for later in the evening to alert them to pay. These reminders are created using local notifications. We always keep the app in the foreground for testing.

IOS 8
We can see the local notifications being created and are in the Notification Drawer. Just to be clear, this is with the app running in the foreground.

IOS10

If we run the same code on IOS 10, we can see the same GPS trigger being set, we set the same local notification up but NO notification is received unless we put the app in the background in the gap between us creating the local notification and the trigger for the local notification firing. For testing purposes we have moved the local notification schedule to 30 secs after the GPS trigger.

This is on the same code base with the same test.

So is the issue with the way IOS 10 now handles local notifications OR is the issue with this plugin?

It would be good to know.

Also if this plugin has been abandoned then we need to look at what to do.

Rob

UPDATED

Now we understand that the notification framework for IOS 10 has changed and that the standard behavior is for notifications to NOT be displayed unless the app is the background. So this matches what we have seen in our testing.

This is a major change from Apple. We've also seen that @katzer has an IOS10 branch on the go, but that seems to be ONLY for IOS10, at time of writing 40% of iPhones are on IOS 9.

The ability to create 'permanent' notifications in the drawer, not transient ones when the app is in the foreground is rather important to us. Enough that we will contribute to a fund to get this working for IOS 9 and IOS 10 as before :)

It would be good to get a statement on what @katzer is planning so we can plan as well.

Thanks

Rob

@akkineniramana
Copy link

akkineniramana commented Nov 4, 2016

@WindierTuna @zocheyado @EddyVerbruggen

In Controller we are calling function every 30 seconds, in that we are making ajax call to get response and we want to display response message as notifications. We are using below code to send notifications in controller, but notifications are coming in simulator but not coming on phone. formattedwmsString variable will have response message. If I put below code with static text in app.js in ionicready function then in phone after opening app and pressing home button, notifications starts coming on phone. Could you please helm me where I'm doing wrong?

cordova.plugins.notification.local.schedule({
id: 1,
text: formattedwmsString,
title : 'WMS'
});

@rwillett
Copy link
Collaborator

rwillett commented Nov 4, 2016

@akkineniramana

You can only display the notifications in the drawer when the app is in the background.

You also don't have the at: option, I believe that this is needed, we had/have an issue with

at:  now 

so we schedule the notifications for at least five seconds in the future.

Here's our code lifted directly from our master source. We know this works as we've tested it to death.

        var d          = new Date();
        var now        = d.getTime();
        var year       = d.getFullYear();
        var month      = d.getMonth();
        var day        = d.getDate();

        var reminder0  = new Date(now + 60 * 1000);
        var reminder1  = new Date(year , month , day , 19 , 30 , 0);
        var reminder2  = new Date(year , month , day , 20 , 30 , 0);
        var reminder3  = new Date(year , month , day , 21 , 0 , 0);

        var notification = [ { id: 1 , // We hard code in these values so we can easily check, update or delete them
                               sound: "" , // Turn the sound off
                               at: reminder0 ,
                               data: { type: "CongestionChargeNotification" } ,
                               text: "You've crossed into the Congestion Zone and might need to pay. You'll get more reminders later today."
                             } ,
                             { id: 2 , // We hard code in these values so we can easily check, update or delete them
                               sound: "" , // Turn the sound off
                               at: reminder1 ,
                               data: { type: "CongestionChargeNotification" } ,
                               text: "Reminder 1 - Don't forget to pay the Congestion Charge before midnight."
                             } ,
                             { id: 3 , // We hard code in these values so we can easily check, update or delete them
                               sound: "" , // Turn the sound off
                               at: reminder2 ,
                               data: { type: "CongestionChargeNotification" } ,
                               text: "Reminder 2 - Don't forget to pay the Congestion Charge before midnight."
                             } ,
                             { id:  4 , // We hard code in these values so we can easily check, update or delete them
                               sound: "" , // Turn the sound off
                               at: reminder3 ,
                               data: { type: "CongestionChargeNotification" } ,
                               text: "Final Reminder - Don't forget to pay the Congestion Charge before midnight."
                             }];

        cordova.plugins.notification.local.schedule(notification);

This sets up four reminders for a user to pay the London Congestion Zone for 19:30. 20:30 and 21:00. We use an array to schedule them, as we had issues with scheduling them one at a time. I recall that the advice was to schedule even one notification in an array. However that could be wrong.

The data: option is used so we can delete specific notifications rather than all notifications from the app.

This code works as we spent all yesterday checking exactly this on Android and IOS on the ios10 branch by driving in and out of London. The code was written some months ago though.

Rob

@akkineniramana
Copy link

thanks @WindierTuna Its working now

@rschramm
Copy link

rschramm commented Nov 4, 2016

My app needs to schedule recurring notifications at custom intervals, such as every 15 minutes. Because iOS 9 and earlier did not support this, we ended up having to schedule each one manually and we were/are concerned about reaching the max limit of 64 that I understand is also a limitation of iOS.

Its my understanding that iOS 10 now supports these kind of custom intervals where as previous versions of iOS did not. Is this correct?

Does or will this plugin support the ability to create recurring notifications with custom intervals? If so, when will that be available?

Also, has anyone else faced this issue and have any thoughts on the best way to work around it if and until it can work natively through iOS and this plugin...

Thank you.

keithdmoore added a commit to keithdmoore/cordova-plugin-local-notifications that referenced this pull request Nov 5, 2016
@Nuajan
Copy link

Nuajan commented Nov 8, 2016

I agree with @rwillett about to get a statement on what @katzer is planning.

Seems to be clear that notifications will not come up when app is in foreground but what about the events? Trigger or Click events are not been fired at all with current API used.

Is there any planning of changing to new API? any timeline?

Thanks to all.

@stoconnor
Copy link

Out of interest has anyone verified this fix on iOS 10.2? I updated yesterday to Xcode 8.2 and localnotifications not coming in. Setting notification as follows:

        $cordovaLocalNotification.schedule({
            id: "1234",
            title: "Notification",
            text: "Text.....",
            at: myTime
        }).then(function () {
            console.log("The notification has been set");
        });

It's worth nothing I've only recently added the plugin, works perfectly on android. I manually changed the filed in Xcode to match this PR.

@trembaczm
Copy link

@stoconnor. I believe we determined that the notifications don't display if your app is in the foreground, but if you delay the notification and then put your app in the background it will work. Just read through the comments above and try the various things. We had it pretty nailed down.

@stoconnor
Copy link

@WindierTuna, I delay all my notifications and test them in background mode. Tried most of the above comments which lead me to believe there may be some change in iOS 10.2 that has stopped this functionality again.

I'll try removing the plugin and adding the iOS branch to my app see if this make a difference but I had manually changed the files in Xcode and built it from Xcode.

hasandogu added a commit to hasandogu/cordova-plugin-local-notifications that referenced this pull request Feb 7, 2017
@mauricioprzv
Copy link

run that cordova plugin add https://github.com/Telerik-Verified-Plugins/LocalNotification
Its here the documentation http://plugins.telerik.com/cordova/plugin/localnotification
THATS WORKS

@LufoX11
Copy link

LufoX11 commented Apr 21, 2017

@mauricioprzv Yes, that works with the app in the background, like the original one.

@jd0048
Copy link

jd0048 commented May 25, 2017

Background local notification not working any updates?

@rwillett
Copy link
Collaborator

@jd0048

If you have a problem, raise it properly with a proper explanation of what you think should happen and what you have done. Your comment here had no information in it.

@jd0048
Copy link

jd0048 commented May 26, 2017

@rwillett i have issue is something like when my app is running in foreground local notification works perfect but when my app go to background local notification is not working

this description is fine or you need some more details?

@rwillett
Copy link
Collaborator

@jd0048

Raise a separate issue as you are hijacking anther thread. Fill in the template correctly with a proper explanation. I still don't understand what isn't working as we schedule notifications all the time, put the app into the background and they work.

You have not said anything about versions, code, OS, hardware, errors, what you expect to happen. A single line saying "something doesn't work" is meaningless. The template is there for a reason, use it.

@GitToTheHub
Copy link
Collaborator

One question:
Why is that little change not merged after one year??? .. What's the problem?

@katzer
Copy link
Owner

katzer commented Oct 30, 2017

finally fixed :D

@katzer katzer closed this Oct 30, 2017
@GitToTheHub
Copy link
Collaborator

Yeah :D

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

Successfully merging this pull request may close these issues.