flutter_local_notifications icon indicating copy to clipboard operation
flutter_local_notifications copied to clipboard

[flutter_local_notifications] `AndroidFlutterLocalNotificationsPlugin` Consume selected notification

Open KirillBorodin opened this issue 1 year ago • 4 comments

Sets an extra to Android intent which indicates that selected notification has been consumed.

The feature is a fix of the issue: #1926

KirillBorodin avatar Jan 16 '24 14:01 KirillBorodin

LGTM BUMP!

giridat avatar Jan 20 '24 06:01 giridat

Not sure if you sure this but a similar had already been raised at https://github.com/MaikuB/flutter_local_notifications/pull/2150 though your one is conveys what it does more clearly. With that in mind, I believe what I wrote https://github.com/MaikuB/flutter_local_notifications/pull/2150#issuecomment-1890351060 applies here too. Let me know what you think about changing how your PR works to match what I mentioned there

MaikuB avatar Jan 22 '24 09:01 MaikuB

Not sure if you sure this but a similar had already been raised at #2150 though your one is conveys what it does more clearly. With that in mind, I believe what I wrote #2150 (comment) applies here too. Let me know what you think about changing how your PR works to match what I mentioned there

Hi @MaikuB , I believe your comment may not directly apply to this PR. We have designed the functionality to be opt-in, requiring an explicit call to consume to actually consume the notification.

If this method does not meet your expectations, we propose an alternative: instead of omitting the notification when getNotificationAppLaunchDetails is called, we plan to still return it but include a consumed flag within the notification details returned by getNotificationAppLaunchDetails. This allows the caller to decide whether to redirect the user to the screen indicated by the notification or to ignore it, recognizing that it has been marked as consumed. This modification aims to maintain consistent behavior across Android, iOS, and macOS platforms.

I understand that a similar concern was raised in issue #2150. However, this PR seeks to more clearly define the implementation. Reflecting on your insights shared in #2150 (comment), could you please provide your feedback on adjusting this PR to reflect those suggestions?

bitsydarel avatar Feb 20 '24 14:02 bitsydarel

@bitsydarel it applies to this PR as well as they both try to solve the same problem. What I'm referring to would also allow for an opt-in and the "cause" of this issue is developers have had assumptions before upon calling the getNotificationAppLaunchDetails(), a subsequent call to getNotificationAppLaunchDetails() wouldn't result in the same notification being returned with the assumption that the plugin has consumed the notification. Part of this might be because the FCM plugin does this.

As there have been these expectations before, what I'm proposing is something similar to what you're saying but it's not based on updating the response and it also retains the opt-in behaviour. If you look at what I wrote in https://github.com/MaikuB/flutter_local_notifications/pull/2150#issuecomment-1890351060, I had described the proposal already. This was about updating the getNotificationAppLaunchDetails() to take an additional argument so that the plugin it would return and consume the notification in one go. This would be up to developers to opt-in and reduces the burden on developers using the API as there only be one API call to make compared the current approach in the PR that involves two separate API calls. If an app opted into this behaviour then wouldn't also be the need to check a consumed flag in the response as calling getNotificationAppLaunchDetails() again could just return null. Hope that is clearer now

MaikuB avatar Feb 22 '24 10:02 MaikuB