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

clearAll notifications on Android if count zero when in background #2521

Merged
merged 1 commit into from
Aug 14, 2018

Conversation

chriscant
Copy link
Contributor

Description

Update FCMService notification handler for Android when in background so count==0 initially clears all existing notifications, closely matching iOS functionality. Note that iOS only clears notifications if the count has gone down eg from 1 to 0. Notifications are not cleared if count not given.

Update payload documentation with note to explain behaviour on Android and iOS.
Please change or move the note if required.

Related Issue

#2520

Motivation and Context

If notification(s) are no longer relevant then they should be removed, even if app not running.
This can be signalled by setting badge count to zero.

This was not happening in Android.

Note that after all current notifications have been cleared (a) a new notification will be created if a message or title is given and (b) the notification event handlers are called if the app is running.

How Has This Been Tested?

Tested by sending notifications with different payloads to Android and iOS devices.

Types of changes

The change could be viewed as one or all of these types:

  • 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.

if (badgeCount == 0) {
NotificationManager mNotificationManager = (NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE);
mNotificationManager.cancelAll();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chriscant okay, I haven't had a chance to test yet but what I see looks good. Let me confirm my thinking.

if count >= 0 then set the badge number, which is the current behaviour
if count == 0 then clear all the notifications which is new behaviour
if count is not provided in the push payload then extractBadgeCount returns -1 and neither of the above two statements are true so we get the existing behaviour.

So the only time the behaviour of the plugin changes is if someone explicitly sets the count/badge to 0 in the push payload? Note, that seems to be the best way forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's my understanding, yes.

Note that if a title or message is also supplied then a new notification is created, as per current behaviour.

And the event handler is still called, as per current behaviour.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet that's what I was hoping for 🤞 I'll be able to test soon.

@macdonst macdonst merged commit 2b4748c into phonegap:master Aug 14, 2018
@macdonst
Copy link
Member

@chriscant thanks for the PR. I finally got a chance to test and merge.

@macdonst macdonst mentioned this pull request Aug 14, 2018
9 tasks
@lock
Copy link

lock bot commented Sep 13, 2018

This thread has been automatically locked.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 13, 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.

2 participants