Plugin.LocalNotification icon indicating copy to clipboard operation
Plugin.LocalNotification copied to clipboard

Various updates

Open mphill opened this issue 2 years ago • 7 comments

What does this PR do?

  1. Added iOS 15 options for time sensitive notifications and relevance score (sorting notifications)
  2. Fixed issue with references to System.Drawing causing crash on iOS when using images from byte[]
  3. Added thread identifier to allow you to group notifications on iOS - perhaps this can be called from a WithGroup(string) method on the builder
  4. Added “data only” notifications for iOS. A use case for this allows you to mark a notification as handled so it will not present. This would be useful if you scheduled a reminder with a reminder 20 minutes after that reminder if no action was taken. But you don't want to show that notification if action was taken.

For more details on iOS 15 notification changes, see https://onesignal.com/blog/ios-notification-changes-updates-from-apples-wwdc-21/

Why are we doing this? Any context or related work?

This addresses:

https://github.com/thudugala/Plugin.LocalNotification/issues/223 https://github.com/thudugala/Plugin.LocalNotification/pull/224

mphill avatar Mar 15 '22 21:03 mphill

@mphill thank you fro the PR. please look into my comments

thudugala avatar Mar 17 '22 04:03 thudugala

I don't see any comments, did you mean commits?

Looks like you mostly bypassed this PR and implemented the changes directly.

Also, summaryArguments will possibly be removed in iOS 16

https://developer.apple.com/documentation/usernotifications/unnotificationcontent/2963115-summaryargument

mphill avatar Mar 17 '22 11:03 mphill

https://github.com/thudugala/Plugin.LocalNotification/commit/1770ab5f3cc60a79e68c102962300b30089bc767#diff-29601a30f9f82e603c2c76063fa0fd32a45c9df7798ba636d6db94583937a2df

I'm not sure this is the best approach. Essentially what I was trying to add is the ability to conditionally show a message. This would be useful for daily reminders or tasks that you may have already completed and don't need a notification for.

I my case with the code I added in this PR I was able to handle this:

            Plugin.LocalNotification.NotificationCenter.Current.NotificationReceiving = (e) => {

                var payload = new NotificationPayload(e.ReturningData);

                var logs = Locator.Current.GetService<ILogRepository>();

                if (logs.IsTimeOfDayCompleted(payload.TimeOfDay))
                {
                    Debug.Write($"Handled {e.NotificationId} {e.Title}");
                    return Task.FromResult(new NotificationEventReceivingArgs()
                    {
                        Handled = true
                    });
                }


                Debug.Write($"Not Handled {e.NotificationId} {e.Title}");
                return Task.FromResult(new NotificationEventReceivingArgs()
                {
                    Handled = false
                });
            };

The code base was not using NotificationReceiving, I just added support:

                if(NotificationCenter.Current.NotificationReceiving != null)
                {
                    var result = await NotificationCenter.Current.NotificationReceiving.Invoke(notificationRequest);

                    if (result.Handled)
                    {
                        requestHandled = true;
                        presentationOptions = UNNotificationPresentationOptions.None;
                    }
                }

Perhaps adding a method like:

Func<NotificationRequest, bool> IsHandled { get; set; } is another solution

mphill avatar Mar 17 '22 11:03 mphill

I don't see any comments, did you mean commits?

@mphill Can you see my PR review comments?

thudugala avatar Mar 17 '22 19:03 thudugala

NotificationReceiving is implemented in iOS, please look at NotificationServiceExtension.cs

thudugala avatar Mar 17 '22 19:03 thudugala

NotificationReceiving is implemented in iOS, please look at NotificationServiceExtension.cs

The implmentation for local and remote notification appear to be different, it looks like the extension class is only for remote notifications:

https://developer.apple.com/documentation/usernotifications/unnotificationserviceextension

I did not see this trigger at all while I was testing which is why I added:

                if(NotificationCenter.Current.NotificationReceiving != null)
                {
                    var result = await NotificationCenter.Current.NotificationReceiving.Invoke(notificationRequest);

                    if (result.Handled)
                    {
                        requestHandled = true;
                        presentationOptions = UNNotificationPresentationOptions.None;
                    }
                }

Which does fire when a local notification is triggered

I don't see any content in the conversation tab for the PR, I didn't see any comments.

mphill avatar Mar 17 '22 21:03 mphill

Here is some addition documentation on modifying notification content:

https://developer.apple.com/documentation/usernotifications/modifying_content_in_newly_delivered_notifications

It appears you can only modify remote notification, all of the properties on a local notifications only have getters. The extension applies only to remote notification (I believe)

Here is a stackoverflow of someone having the same issue:

https://stackoverflow.com/questions/45033911/unable-to-update-local-scheduled-notification-content

They ended up deleting the notification and creating a new one to emulate dynamic content changes. I don't think that's a good approach. With local notifications I think it's ok you can't modify the content when presenting. What would the use case even be?

I think Func<NotificationRequest, bool> IsHandled { get; set; } might be a good way to go on this personally.

mphill avatar Mar 18 '22 13:03 mphill