-
Notifications
You must be signed in to change notification settings - Fork 1
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
remove background push and fix push response handling #15
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,22 +51,10 @@ public static INotifoMobilePush UseFirebasePluginEventsProvider(this INotifoMobi | |
/// <param name="notifo">The <see cref="INotifoMobilePush"/> instance.</param> | ||
/// <param name="data">The notification data dictionary.</param> | ||
/// <returns>A <see cref="Task"/> representing the result of the asynchronous operation.</returns> | ||
public static async Task DidReceiveMessageAsync(this INotifoMobilePush notifo, NSDictionary data) | ||
public static void DidReceiveMessageAsync(this INotifoMobilePush notifo, NSDictionary data) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should not be called "Async", if it is not async. |
||
{ | ||
notifo.RaiseDebug(Strings.ReceivedNotification, null, data); | ||
|
||
static bool ContainsPullRefreshRequest(NSDictionary data) | ||
{ | ||
var aps = data?.ObjectForKey(new NSString(Constants.ApsKey)) as NSDictionary; | ||
|
||
return aps?.ContainsKey(new NSString(Constants.ContentAvailableKey)) == true; | ||
} | ||
|
||
if (ContainsPullRefreshRequest(data)) | ||
{ | ||
await notifo.DidReceivePullRefreshRequestAsync(); | ||
} | ||
|
||
|
||
FirebasePushNotificationManager.DidReceiveMessage(data); | ||
} | ||
|
||
|
@@ -99,7 +87,7 @@ public static void RemoteNotificationRegistrationFailed(this INotifoMobilePush n | |
/// <param name="completionHandler">The action to execute when you have finished processing the user's response.</param> | ||
public static void DidReceiveNotificationResponse(this INotifoMobilePush notifo, UNUserNotificationCenter center, UNNotificationResponse response, Action completionHandler) | ||
{ | ||
notifo.DidReceiveNotificationResponse(center, response, completionHandler); | ||
notifo.DidReceiveNotificationResponse(response); | ||
|
||
if (CrossFirebasePushNotification.Current is IUNUserNotificationCenterDelegate notificationDelegate) | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,14 +48,16 @@ public async Task DidReceiveNotificationRequestAsync(UNNotificationRequest reque | |
// 3. Enrich with images, which need to be downloaded. | ||
await EnrichImagesAsync(content, notification); | ||
|
||
await HandlePendingNotificationsAsync(); | ||
|
||
// 4. Custom enrichment code (could be potentially be expensive). | ||
EnrichWithCustomCode(content, notification); | ||
} | ||
|
||
/// <inheritdoc /> | ||
public async Task DidReceivePullRefreshRequestAsync(PullRefreshOptions? options = null) | ||
public async Task HandlePendingNotificationsAsync() | ||
{ | ||
options ??= new PullRefreshOptions(); | ||
var options = new PullRefreshOptions(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes the full options useless. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah that's right. Could be removed completely but what do you think in general of removing the pullRefresh? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In general I tried not to have some non-configurable values, but it is weird, that these options are configured over the method. So we should just add a method to INotifoPush like
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have introduced this change in the base branch |
||
|
||
// iOS does not maintain a queue of undelivered notifications, therefore we have to query here. | ||
var notifications = await GetPendingNotificationsAsync(options.Take, options.Period, default); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow the formatting is really fucked up here with tabs and spaces.