MaikuB / flutter_local_notifications

A Flutter plugin for displaying local notifications on Android, iOS, macOS and Linux
2.46k stars 1.4k forks source link

Support for custom notification actions #17

Closed MaikuB closed 2 years ago

MaikuB commented 6 years ago

The plugin should provide the ability to specify custom notification actions. However, this will depends on the Flutter engine being able to support headless-Dart code as per https://github.com/flutter/flutter/issues/6192 and https://github.com/flutter/flutter/issues/3671

It appears Android support is there but will wait to see on if the engine can support it for iOS applications, and for Flutter to provide abstractions to access the functionality. Without the support being added in, then this won't work for scenarios like when the application has been terminated as the logic associated with the action would've been defined in Dart.

Update with remaining work (IMO)

Note that last two perhaps could be omitted given market share and those require using deprecated APIs

Edit:

mk-dev-1 commented 2 years ago

Thank you so much. I will give it a go with your changes and report back. It will take a couple of days for sure though.

AdamBridges commented 2 years ago

@AdamBridges Thank you for looking at it, although I can't see that this should resolve the issue as the error log clearly points to the line before...

@mk-dev-1 Yeah, I wasn't certain. My logic was that because your logic insists that the NotificationResponse will never be null, it's somehow being insisted in the Android getNotificationAppLaunchDetails() method and tries to return a NotificationResponse, but since the notificationResponseType parameter cannot return a Null value (whereas id, payload, actionId, and input can), it decides to crash.

UPDATE: I was just testing and was able to reproduce the issue with a Pixel 2 Emulator running API 31: Terminate app; launch app manually; send app to background; and then use a notification action that calls getNotificationAppLaunchDetails(). The provided PR seems to fix the issue. @MaikuB

mk-dev-1 commented 2 years ago

I can confirm I no longer see any crashes with the latest changes. Thank you once again for your great work!

MaikuB commented 2 years ago

@mk-dev-1 this is now incorporated in the 10.0.0-dev.19 prerelease. Not sure how you referenced the fix in your app but the branch I had mentioned will be deleted soon. As you're using the pre-release and from what I recall, have been using the plugin for quite a while, are you able to share your experience in using the pre-release? I assume you've used it to make use of notification actions. Trying to figure out when to promote it to a stable release though the issue you found would've been one that's existed for quite a while now...

noinskit commented 2 years ago

Hi, I'm finally giving this a try, works great so far! Until now I was using my own fork that handled notification actions in my app.

I already have one question: Is it possible to reinitialize notification categories on iOS? They're now part of DarwinInitializationSettings passed to initialize. My problem is that in some languages user's gender tends to impact the notification action title. Or is it safe to call initialize multiple times and expect the last configuration to be used?

mk-dev-1 commented 2 years ago

@mk-dev-1 this is now incorporated in the 10.0.0-dev.19 prerelease. Not sure how you referenced the fix in your app but the branch I had mentioned will be deleted soon. As you're using the pre-release and from what I recall, have been using the plugin for quite a while, are you able to share your experience in using the pre-release? I assume you've used it to make use of notification actions. Trying to figure out when to promote it to a stable release though the issue you found would've been one that's existed for quite a while now...

After testing it and using it in the beta channel of my app, I have been using the 10.0 prereleases in production for quite a while without any major issues or negative feedback. I don't see any reason to not promote it to a stable release ;-)

MaikuB commented 2 years ago

@noinskit you'd need to try to confirm but the underlying native Apple API call is one that sets the categories so would think it should work. This might call for a platform-specific API exposed via the plugin so you don't have to call initalize() several times. Happy to take a PR for it though I'm thinking to make this a stable release soon given what @mk-dev-1 has said. Curious though if you managed to get around to testing iOS @AdamBridges?

noinskit commented 2 years ago

@MaikuB a new platform-specific API for (re)initializing iOS categories sounds reasonable to me. I agree that it should not block 1.0.0.

AdamBridges commented 2 years ago

@noinskit you'd need to try to confirm but the underlying native Apple API call is one that sets the categories so would think it should work. This might call for a platform-specific API exposed via the plugin so you don't have to call initalize() several times. Happy to take a PR for it though I'm thinking to make this a stable release soon given what @mk-dev-1 has said. Curious though if you managed to get around to testing iOS @AdamBridges?

Soon. Probably sometime in September so I'll follow up then.

MaikuB commented 2 years ago

Soon. Probably sometime in September so I'll follow up then.

Ok thanks. I'll probably move it to stable before then. Was looking to see if others could give feedback before then but hopefully this has given enough time for feedback...

MaikuB commented 2 years ago

Has anyone run into issue #1694? I was going to do a stable release but then saw this reported. It looks like a pub cache issue than a plugin issue though

mk-dev-1 commented 2 years ago

Has anyone run into issue #1694? I was going to do a stable release but then saw this reported. It looks like a pub cache issue than a plugin issue though

I believe you are right. Can't say I have seen this issue...

MaikuB commented 2 years ago

Forgot to close this out since 10.0 was moved to stable and there's been more updates since then so will do so now. Thanks all for the contribution :)