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

fix(push-notifications): use id and tag for canceling active notification #1041

Merged
merged 11 commits into from
Jun 16, 2022

Conversation

theproducer
Copy link
Contributor

Fixes an issue where it was not possible to delete individual notifications on Android.

closes: #740

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

Since getDeliveredNotifications now returns the notification tag, the types should be edited to reflect that, the tag should be optional and documented that it's only returned for Android push notifications (getDeliveredNotifications also returns local notifications, but they don't usually have a tag)

*
* Only available on Android (from push notifications).
*
* @since 1.0.9
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @since 1.0.9
* @since 4.0.0

This will be available on version 4.0.0 of the plugin

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

Added a comment.

Also, you have to re run npm run build to update the README

Integer id = notif.getInteger("id");
ids.add(id);
String tag = notif.getString("tag");
Integer id = notif.getInteger("id", 0);
Copy link
Member

Choose a reason for hiding this comment

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

The id is not optional, so you shouldn't need the default value.
Have you seen it being null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really, I removed it.

@theproducer theproducer merged commit fa710a6 into capacitor-4 Jun 16, 2022
@theproducer theproducer deleted the push-notifications-delete-active branch June 16, 2022 16:30
hmaretic24 pushed a commit to hmaretic24/capacitor-plugins that referenced this pull request Jan 20, 2023
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.

None yet

4 participants